[Freeciv-Dev] (PR#3924) Bugfix for map/canvas coordination functions

["a-l@xxxxxxx" <a-l@xxxxxxx>]

This patch fixes two bugs:
one in canvas_to_map_pos(), and one in map_to_canvas_pos().
Both are relevant only for non-isometric mode.

If this is not formalized already, I would propose that the
following should be a defined property of the two functions
in question:

That this verification procedure should hold true:
{
  map_to_canvas_pos(&x, &y, tile_x1, tile_y1);

  x += NORMAL_TILE_WIDTH / 2;
  y += NORMAL_TILE_HEIGHT / 2;

  canvas_to_map_pos(&tile_x2, &tile_y2, x, y);

  assert(tile_x1 == tile_x2);
  assert(tile_y1 == tile_y2);
}
For all tiles at all times.

We must add half a tile because of PR#3800, the bounding
draw box thing. Ie nobody should "fix" PR#3800.

It is currently only valid for the tiles that are
within the visible mapview canvas at any given time.

That makes precise map/canvas coordination impossible for, say, a
topology independent selection rectangle that is bigger than mapview.
You could possibly also run into problems when you try to make
efficient pixel-smooth scrolling by establishing a pixel buffer
outside the mapview (or so I imagine).

I don't think any current cvs code trips over these bugs, because none
of it concerns itself with out-of-sight canvas coordinates.


More technical description of the bugfixes for the interested:

Bug 1:  map_to_canvas_pos()

In this situation:

  When the wrapping axis (0, tile_y) is within the visible mapview,
  and not the leftmost column, ie mapview_canvas.map_x0 != 0.

The problem is:

  map_to_canvas_pos() only outputs meaningful canvas coordinates for
  those tiles that are currently within the mapview. All other tiles
  are assigned a canvas X coordinate of -1 * NORMAL_TILE_WIDTH.

However, when axis (0, tile_y) is out of sight, map_to_canvas_pos()
gives meaningful, consistent, canvas coordinates for all tiles,
sometimes negative.

Bug 2:  canvas_to_map_pos()

  The leftmost tile (mapview_canvas.map_x0) is defined by a sum of
  pixels that is twice the size of the sums that define other tiles.

Example of correct behaviour:

Say mapsize x is 80 and tile width is 30 pixels.
(0, y) is the leftmost tile in the visible mapview:

TILE      Valid X canvas
...
(79, y) = [-30, -1]
(0, y)  = [0, 29]
(1, y)  = [30, 59]
...

Before the bugfix, (0, y) was defined by [-29, 29] which is incorrect.


Arnstein

--- cvs-Apr-02/client/mapview_common.c  Thu Apr  3 15:38:37 2003
+++ bugfix-map-canvas/client/mapview_common.c   Thu Apr  3 15:41:04 2003
@@ -214,7 +214,7 @@
                                    + mapview_canvas.tile_width)) {
       *canvas_x = map_x + map.xsize - mapview_canvas.map_x0;
     } else {
-      *canvas_x = -1;
+      *canvas_x = map_x - mapview_canvas.map_x0;
     }
 
     *canvas_y = map_y - mapview_canvas.map_y0;
@@ -235,6 +235,8 @@
 **************************************************************************/
 bool canvas_to_map_pos(int *map_x, int *map_y, int canvas_x, int canvas_y)
 {
+  const int W = NORMAL_TILE_WIDTH, H = NORMAL_TILE_HEIGHT;
+
   if (is_isometric) {
     /* The basic operation here is a simple pi/4 rotation; however, we
      * have to first scale because the tiles have different width and
@@ -261,13 +263,13 @@
      *
      * For another example of this math, see canvas_pos_to_city_pos().
      */
-    const int W = NORMAL_TILE_WIDTH, H = NORMAL_TILE_HEIGHT;
-
     *map_x = DIVIDE(canvas_x * H + canvas_y * W, W * H);
     *map_y = DIVIDE(canvas_y * W - canvas_x * H, W * H);
   } else {                     /* is_isometric */
-    *map_x = canvas_x / NORMAL_TILE_WIDTH;
-    *map_y = canvas_y / NORMAL_TILE_HEIGHT;
+    *map_x = canvas_x / W;
+    *map_y = canvas_y / H;
+    if (canvas_x < 0 && canvas_x % W)   *map_x -= 1;
+    if (canvas_y < 0 && canvas_y % H)   *map_y -= 1;
   }
 
   *map_x += mapview_canvas.map_x0;