[Freeciv-Dev] isometric tile calculation cleanup (PR#1093)

[jdorje@xxxxxxxxxxxxxxxxxxxxx]

This short patch cleans up the math used to find canvas positions for 
isometric tiles.  I've also added a lot of comments to this rather 
complicated (for the uninitiated :-) calculation.  I've also snuck in a 
slight change to canvas_pos_to_map_pos.
I claim the effects of the code should be identical under the change 
(although this is hard to verify since the old code was pretty dense).
Here's the background: when I first looked into having freeciv handle 
different topologies, this was one of the key places that needed to be 
changed.  Unfortunately (and bafflingly), just changing the wrapping 
code didn't work, and since I couldn't understand the rest of this 
function (at all), I was stuck.  Eventually I did more research and 
understood that this was the calculation the should actually be done - 
Thue's math is just a less mature version of it [1].  Then I just 
replaced the old code with the (seemingly) correct code, and found that 
everything worked. :-)
One thing I'd definitely like some feedback on is the comments.  They're 
pretty detailed, and intended to describe the process pretty thoroughly. 
 Do they accomplish that?
[1] Rather than following the translation from flat to isometric I 
describe, Thue first finds the iso_y coordinate then subtracts that off 
so that he's left with only the iso_x coordinate.  If anyone wants a 
more detailed explanation, I'll be happy to give it.
jason

? diff
? old
? topology
Index: client/citydlg_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.c,v
retrieving revision 1.1
diff -u -r1.1 citydlg_common.c
--- client/citydlg_common.c     2001/12/05 04:15:50     1.1
+++ client/citydlg_common.c     2001/12/05 06:23:12
@@ -25,15 +25,16 @@
 void city_pos_to_canvas_pos(int city_x, int city_y, int *canvas_x, int 
*canvas_y)
 {
   if (is_isometric) {
-    int diff_xy;
-
-    /* The line at y=0 isometric has constant x+y=1(tiles) */
-    diff_xy = (city_x + city_y) - (1);
-    *canvas_y = diff_xy/2 * NORMAL_TILE_HEIGHT + (diff_xy%2) * 
(NORMAL_TILE_HEIGHT/2);
-
-    /* The line at x=0 isometric has constant x-y=-3(tiles) */
-    diff_xy = city_x - city_y;
-    *canvas_x = (diff_xy + 3) * NORMAL_TILE_WIDTH/2;
+    /*
+     * The top-left corner is in the center of tile (-2, 2).  However,
+     * we're looking for the top-left corner of the tile, so we subtract
+     * off half a tile in each direction.  For a more rigorous example,
+     * see map_pos_to_canvas_pos().
+     */
+    int iso_x = (city_x - city_y) - (-4);
+    int iso_y = (city_x + city_y) - (0);
+    *canvas_x = (iso_x-1) * NORMAL_TILE_WIDTH / 2;
+    *canvas_y = (iso_y-1) * NORMAL_TILE_HEIGHT / 2;
   } else {
     *canvas_x = city_x * NORMAL_TILE_WIDTH;
     *canvas_y = city_y * NORMAL_TILE_HEIGHT;
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.2
diff -u -r1.2 mapview_common.c
--- client/mapview_common.c     2001/11/27 20:11:28     1.2
+++ client/mapview_common.c     2001/12/05 06:23:12
@@ -94,45 +94,47 @@
                          int map_view_pixel_height)
 {
   if (is_isometric) {
-    /* 
-     * canvas_x,canvas_y is the top corner of the actual tile, not the
-     * pixmap.  This function also handles non-adjusted tile coords
-     * (like -1, -2) as if they were adjusted.  You might want to
-     * first take a look at the simpler city_get_canvas() for basic
-     * understanding. 
-     */
-    int diff_xy;
-    int diff_x, diff_y;
+    /* For a simpler example of this math, see city_pos_to_canvas_pos(). */
+    int iso_x, iso_y;
 
+    /*
+     * First we wrap the coordinates to hopefully be within the the
+     * GUI window.  This isn't perfect; notice that when the mapview
+     * approaches the size of the map some tiles won't be shown at all.
+     */
     map_x %= map.xsize;
     if (map_x < map_view_topleft_map_x) {
       map_x += map.xsize;
     }
-    diff_xy =
-       (map_x + map_y) - (map_view_topleft_map_x +
-                          map_view_topleft_map_y);
-    /* one diff_xy value defines a line parallel with the top of the
-       isometric view. */
-    *canvas_y =
-       diff_xy / 2 * NORMAL_TILE_HEIGHT +
-       (diff_xy % 2) * (NORMAL_TILE_HEIGHT / 2);
-
-    /* changing both x and y with the same amount doesn't change the
-       isometric x value. (draw a grid to see it!) */
-    map_x -= diff_xy / 2;
-    map_y -= diff_xy / 2;
-    diff_x = map_x - map_view_topleft_map_x;
-    diff_y = map_view_topleft_map_y - map_y;
-
-    *canvas_x = (diff_x - 1) * NORMAL_TILE_WIDTH
-       + (diff_x == diff_y ? NORMAL_TILE_WIDTH : NORMAL_TILE_WIDTH / 2)
-       /* tiles starting above the visible area */
-       + (diff_y > diff_x ? NORMAL_TILE_WIDTH : 0);
-
-    /* We now have the corner of the sprite. For drawing we move
-       it. */
-    *canvas_x -= NORMAL_TILE_WIDTH / 2;
 
+    /*
+     * Next we convert the flat GUI coordinates to isometric GUI
+     * coordinates.  We'll make tile (x0, y0) be the origin, and
+     * transform like this:
+     *                     3
+     * 123                2 6
+     * 456 -> becomes -> 1 5 9
+     * 789                4 8
+     *                     7
+     */
+    iso_x = (map_x - map_y) - (map_view_topleft_map_x - 
map_view_topleft_map_y);
+    iso_y = (map_x + map_y) - (map_view_topleft_map_x + 
map_view_topleft_map_y);
+
+    /*
+     * As the above picture shows, each isometric-coordinate unit
+     * corresponds to a half-tile on the canvas.  Since the (x0, y0) tile
+     * actually has its top corner (of the diamond-shaped tile) located
+     * right at the corner of the canvas, to find the top-left corner of
+     * the surrounding rectangle we must subtract off an additional
+     * half-tile in the X direction.
+     */
+    *canvas_x = (iso_x - 1) * NORMAL_TILE_WIDTH / 2;
+    *canvas_y = iso_y * NORMAL_TILE_HEIGHT / 2;
+
+    /*
+     * Finally we clip; checking to see if _any part_ of the tile
+     * is visible on the canvas.
+     */
     return (*canvas_x > -NORMAL_TILE_WIDTH)
        && *canvas_x < (map_view_pixel_width + NORMAL_TILE_WIDTH / 2)
        && (*canvas_y > -NORMAL_TILE_HEIGHT)
@@ -199,18 +201,16 @@
     if (canvas_x + canvas_y > NORMAL_TILE_WIDTH) {
       *map_x += 1;
     }
-
-    /* If we are outside the map find the nearest tile, with distance
-       as seen on the map. */
-    nearest_real_pos(map_x, map_y);
   } else {                     /* is_isometric */
-    *map_x =
-       map_adjust_x(map_view_topleft_map_x +
-                    canvas_x / NORMAL_TILE_WIDTH);
-    *map_y =
-       map_adjust_y(map_view_topleft_map_y +
-                    canvas_y / NORMAL_TILE_HEIGHT);
+    *map_x = map_view_topleft_map_x + canvas_x / NORMAL_TILE_WIDTH;
+    *map_y = map_view_topleft_map_y + canvas_y / NORMAL_TILE_HEIGHT;
   }
+
+  /*
+   * If we are outside the map find the nearest tile, with distance
+   * as seen on the map.
+   */
+  nearest_real_pos(map_x, map_y);
 }
 
 /**************************************************************************