[Freeciv-Dev] Re: [PATCH] cleanup to canvas_pos_to_map_pos (PR#1183)

[Jason Short <vze2zq63@xxxxxxxxxxx>]

A new patch is attached.  The only real difference is that I've added a 
DIVIDE macro to common/shared.h.
See the previous e-mails for a fuller explanation, or just read the 
comments in the patch.
jason

Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.3
diff -u -r1.3 citydlg_common.c
--- client/citydlg_common.c     2001/12/13 15:30:30     1.3
+++ client/citydlg_common.c     2002/01/01 04:07:34
@@ -50,33 +50,36 @@
 **************************************************************************/
 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).  The math used
+     * here is identical to that used in canvas_pos_to_map_pos; see that
+     * function for a full explanation.
+     */
+    int flat_canvas_x, flat_canvas_y;
 
-    /* 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;
+    /* First we scale it 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)++;
+    /* Then we transform it to make it flat. */
+    flat_canvas_x = canvas_x + canvas_y;
+    flat_canvas_y = canvas_y - canvas_x;
+
+    /* Now we've got a square, so it's a simple matter to find the
+       position. */
+    *map_x = -2 + DIVIDE(flat_canvas_x,
+                        NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = 2 + DIVIDE(flat_canvas_y,
+                       NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
   } 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.4
diff -u -r1.4 mapview_common.c
--- client/mapview_common.c     2001/12/21 16:53:10     1.4
+++ client/mapview_common.c     2002/01/01 04:07:35
@@ -184,34 +184,66 @@
                           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(). */
+    int flat_canvas_x, flat_canvas_y;
 
-    /* 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 look flat, but they're really
+     * isometric coordinates.  So we next want to convert them back to
+     * "flat" coordinates.  We'll keep the top-left corner of the canvas
+     * as (0,0) so that we have an easy frame of reference.  The
+     * transformation, which we're applying to a square tile, becomes:
+     *
+     *   1      1 4
+     *  234  ->  3
+     *   5      2 5
+     *
+     * This is (ignoring scale) the inverse of the transformation being
+     * done in map_pos_to_canvas_pos.
+     */
+    flat_canvas_x = canvas_x + canvas_y;
+    flat_canvas_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 a flat rectangle with these as its opposite corners.
+     * This makes it easy to tell what tile our coordinates belong to!
+     *
+     * 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(flat_canvas_x, NORMAL_TILE_WIDTH * NORMAL_TILE_HEIGHT);
+    *map_y = DIVIDE(flat_canvas_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.88
diff -u -r1.88 shared.h
--- common/shared.h     2001/12/21 11:17:43     1.88
+++ common/shared.h     2002/01/01 04:07:35
@@ -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. */