[Freeciv-Dev] (PR#1180) [PATCH] cleanup to canvas_pos_to_map_pos

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

Is there any chance of any of these variants of the patch making it in
to CVS?  If not, why not, and what needs to be done?

Attached (again) is my preferred variant.

jason

? foo
? get_data_dirs.diff
? tileset_list.diff
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.8
diff -u -r1.8 citydlg_common.c
--- client/citydlg_common.c     2002/07/18 19:20:54     1.8
+++ client/citydlg_common.c     2002/07/20 17:14:27
@@ -54,35 +54,39 @@
 **************************************************************************/
 void canvas_pos_to_city_pos(int canvas_x, int canvas_y, int *map_x, int *map_y)
 {
+  int orig_canvas_x = canvas_x, orig_canvas_y = canvas_y;
   if (is_isometric) {
-    *map_x = -2;
-    *map_y = 2;
+    /*
+     * The top-left corner is in the center of tile (-2, 2).  Aside from
+     * this, the math used here is identical to that used in
+     * canvas_pos_to_map_pos; see that function for a full explanation.
+     */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* Shift the tile right so the top corner of tile (-2,-2) is at (0,0). */
+    canvas_y += NORMAL_TILE_HEIGHT / 2;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /* Scale to a square. */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x + canvas_y > 3 * NORMAL_TILE_WIDTH / 2)
-      (*map_x)++;
-    if (canvas_x - canvas_y > NORMAL_TILE_WIDTH / 2)
-      (*map_y)--;
-    if (canvas_y - canvas_x > NORMAL_TILE_WIDTH / 2)
-      (*map_y)++;
+    /* Rotate by -pi/4. */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
+
+    /* Scale again. */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+
+    /* Add on the offset of the top-left corner (see above) to get the
+       final coordinates. */
+    *map_x -= 2;
+    *map_y += 2;
   } else {
     *map_x = canvas_x / NORMAL_TILE_WIDTH;
     *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
-  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)", canvas_x, 
canvas_y, *map_x, *map_y);
+  freelog(LOG_DEBUG, "canvas_pos_to_city_pos(pos=(%d,%d))=(%d,%d)",
+         orig_canvas_x, orig_canvas_y, *map_x, *map_y);
 }
 
 /**************************************************************************
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.12
diff -u -r1.12 mapview_common.c
--- client/mapview_common.c     2002/03/19 15:48:38     1.12
+++ client/mapview_common.c     2002/07/20 17:14:28
@@ -187,34 +187,65 @@
                           int map_view_topleft_map_y)
 {
   if (is_isometric) {
-    *map_x = map_view_topleft_map_x;
-    *map_y = map_view_topleft_map_y;
+    /* For another example of this math, see canvas_pos_to_city_pos(). */
 
-    /* first find an equivalent position on the left side of the screen. */
-    *map_x += canvas_x / NORMAL_TILE_WIDTH;
-    *map_y -= canvas_x / NORMAL_TILE_WIDTH;
-    canvas_x %= NORMAL_TILE_WIDTH;
+    /* If the tile width and height are not equal, then it will take very
+     * ugly math to figure anything out about it.  The easiest solution is
+     * just to scale the tile so that it's a perfect (diamond) square.  We
+     * are doing the following transformation to a tile:
+     *
+     *                                 XX  <- height=18, width=18
+     *   XX   <- height=3, width=6
+     * XXXXXX                        XXXXXX
+     *   XX   -----> becomes ----->
+     *                                 XX
+     *
+     * Note that all we really need to do is scale to the LCM of the width
+     * and height (in the above example, 6x6).  But it's easier just to scale
+     * to WIDTH*HEIGHT (18x18).
+     */
+    canvas_x *= NORMAL_TILE_HEIGHT;
+    canvas_y *= NORMAL_TILE_WIDTH;
 
-    /* Then move op to the top corner. */
-    *map_x += canvas_y / NORMAL_TILE_HEIGHT;
-    *map_y += canvas_y / NORMAL_TILE_HEIGHT;
-    canvas_y %= NORMAL_TILE_HEIGHT;
+    /*
+     * The "canvas coordinates" we're given are isometric coordinates.  So
+     * we next want to convert them back to "non-isometric" coordinates.  
+     * We'll keep the top-left corner of the canvas as (0,0) so that we 
+     * have an easy frame of reference.  The operation we're doing is thus
+     * just a -pi/4 rotation.   When applied to a square tile, it becomes:
+     *
+     *   1      1 4
+     *  234  ->  3
+     *   5      2 5
+     *
+     * This is the inverse of the rotation from map_pos_to_canvas_pos.
+     */
+    *map_x = canvas_x + canvas_y;
+    *map_y = canvas_y - canvas_x;
 
-    /* We are inside a rectangle, with 2 half tiles starting in the
-       corner, and two tiles further out. Draw a grid to see how this
-       works :). */
-    assert(NORMAL_TILE_WIDTH == 2 * NORMAL_TILE_HEIGHT);
-    canvas_y *= 2;             /* now we have a square. */
-    if (canvas_x > canvas_y) {
-      *map_y -= 1;
-    }
-    if (canvas_x + canvas_y > NORMAL_TILE_WIDTH) {
-      *map_x += 1;
-    }
+    /*
+     * Now the (x0, y0) tile originally had its top corner at (0,0) and
+     * its bottom corner at (0,NORMAL_MAP_HEIGHT).  These are positions 1
+     * and 5 on the diagram above.  We have transformed these two positions:
+     *   (0,0) -> (0,0) -> (0,0)
+     *   (0,H) -> (0,W*H) -> (W*H,W*H)
+     * The tile is now an cardinally-oriented rectangle with these as its
+     * opposite corners.  This makes it easy to tell what tile our
+     * coordinates belong to - we just need to scale the coordinates again.
+     *
+     * Unfortunately, integer division doesn't work so well for negative
+     * numbers; -5/2==-2 while we're looking for -3.  So this is a bit
+     * uglier than it would otherwise be.
+     */
+    *map_x = DIVIDE(*map_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(*map_y, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
   } else {                     /* is_isometric */
-    *map_x = map_view_topleft_map_x + canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = map_view_topleft_map_y + canvas_y / NORMAL_TILE_HEIGHT;
+    *map_x = canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
   }
+
+  *map_x += map_view_topleft_map_x;
+  *map_y += map_view_topleft_map_y;
 
   /*
    * If we are outside the map find the nearest tile, with distance as
Index: common/shared.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/shared.h,v
retrieving revision 1.95
diff -u -r1.95 shared.h
--- common/shared.h     2002/05/25 17:44:08     1.95
+++ common/shared.h     2002/07/20 17:14:30
@@ -52,6 +52,13 @@
 
 #define BOOL_VAL(x) ((x)!=0)
 
+/*
+ * DIVIDE() divides and rounds down, rather than just divides and
+ * rounds toward 0.
+ */
+#define DIVIDE(n, d) \
+    ((n) % (d) < 0 ? (n) / (d) - 1 : (n) / (d))
+
 /* Deletes bit no in val,
    moves all bits larger than no one down,
    and inserts a zero at the top. */