[Freeciv-Dev] Re: find_representative_map_pos (PR#1151)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

There is nothing technically wrong with this, but it sets up things it
doesn't use, and when the extensions that might follow actually do use 
them it probably won't be as coded here. At least I fervently hope so.

In particular this is the map window. The concept isn't well defined
probably is going to be used incorrectly and doesn't really do anything
here.

The (un)wrapping conditions are not generic, but only do some crude
form of what is needed for a standard WRAP_X map. There is no reason
not to at least code the new functionality correctly, rather than put
some hack in, then have to fix it up.

This adds nothing useful to the current codebase, or to furthering the 
longer issues, which need more context than this to be properly discussed.

I think this would be better handled by resolving the larger picture, 
then starting to put selected subsets of it in in a systemmatic manner
once everyone knows where it is going.

Cheers,
RossW
=====

At 10:21 PM 01/12/19 -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>The attached patch provides a new topology function, 
>find_representative_map_pos, that wraps a map position to be within a 
>given map "window".
>
>Its immediate use is in map_pos_to_canvas_pos(), where it can provide 
>the topology backend (wrapping) for the conversion from a map position 
>to a canvas position.  This allows the topology code to be separated 
>from the GUI code.
>
>Please test and review.
>
>jason
>? jason.gz
>? old
>? topology
>Index: client/mapview_common.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
>retrieving revision 1.3
>diff -u -r1.3 mapview_common.c
>--- client/mapview_common.c    2001/12/13 15:30:30     1.3
>+++ client/mapview_common.c    2001/12/20 06:15:27
>@@ -93,6 +93,7 @@
>                         int map_view_pixel_width,
>                         int map_view_pixel_height)
> {
>+  struct win_map_context win_mc;
>   if (is_isometric) {
>     /* For a simpler example of this math, see
>        city_pos_to_canvas_pos(). */
>@@ -100,20 +101,24 @@
> 
>     /*
>      * 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.
>+     * GUI window.  We leave the actual wrapping up to the
>+     * find_representative_map_pos function, but we must give it the
>+     * location of the GUI window.
>      */
>-    map_x %= map.xsize;
>-    if (map_x < map_view_topleft_map_x) {
>-      map_x += map.xsize;
>-    }
>+    win_mc.win_is_iso = 1;
>+    win_mc.x0 = map_view_topleft_map_x - 1;
>+    win_mc.y0 = map_view_topleft_map_y - 1;
>+    /* We must round up; plus we get an extra tile on all four sides. */
>+    win_mc.width = (map_view_pixel_width - 1) / (NORMAL_TILE_WIDTH/2) + 3;
>+    win_mc.height = (map_view_pixel_height - 1) / (NORMAL_TILE_HEIGHT/2)
+ 3;
> 
>+    find_representative_map_pos(&map_x, &map_y, &win_mc);
>+
>     /*
>      * 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
>@@ -147,31 +152,18 @@
>       && (*canvas_y > -NORMAL_TILE_HEIGHT)
>       && (*canvas_y < map_view_pixel_height);
>   } else {                    /* is_isometric */
>-    /* map_view_map_width is the width in tiles/map positions */
>-    int map_view_map_width =
>-      (map_view_pixel_width + NORMAL_TILE_WIDTH - 1) / NORMAL_TILE_WIDTH;
>-
>-    if (map_view_topleft_map_x + map_view_map_width <= map.xsize) {
>-      *canvas_x = map_x - map_view_topleft_map_x;
>-    } else if (map_x >= map_view_topleft_map_x) {
>-      *canvas_x = map_x - map_view_topleft_map_x;
>-    }
>-      else if (map_x <
>-             map_adjust_x(map_view_topleft_map_x +
>-                          map_view_map_width)) {*canvas_x =
>-        map_x + map.xsize - map_view_topleft_map_x;
>-    } else {
>-      *canvas_x = -1;
>-    }
>-
>-    *canvas_y = map_y - map_view_topleft_map_y;
>-
>-    *canvas_x *= NORMAL_TILE_WIDTH;
>-    *canvas_y *= NORMAL_TILE_HEIGHT;
>-
>-    return *canvas_x >= 0
>-      && *canvas_x < map_view_pixel_width
>-      && *canvas_y >= 0 && *canvas_y < map_view_pixel_height;
>+    win_mc.win_is_iso = 0;
>+    win_mc.x0 = map_view_topleft_map_x;
>+    win_mc.y0 = map_view_topleft_map_y;
>+    win_mc.width = (map_view_pixel_width - 1) / NORMAL_TILE_WIDTH + 1;
>+    win_mc.height = (map_view_pixel_height - 1) / NORMAL_TILE_HEIGHT + 1;
>+
>+    find_representative_map_pos(&map_x, &map_y, &win_mc);
>+
>+    *canvas_x = (map_x - map_view_topleft_map_x) * NORMAL_TILE_WIDTH;
>+    *canvas_y = (map_y - map_view_topleft_map_y) * NORMAL_TILE_HEIGHT;
>+    return *canvas_x >= 0 && *canvas_x < map_view_pixel_width
>+        && *canvas_y >= 0 && *canvas_y < map_view_pixel_height;
>   }
> }
> 
>Index: common/map.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/common/map.c,v
>retrieving revision 1.103
>diff -u -r1.103 map.c
>--- common/map.c       2001/12/13 19:13:16     1.103
>+++ common/map.c       2001/12/20 06:15:28
>@@ -1272,6 +1272,48 @@
>   return (0 <= *y && *y < map.ysize);
> }
> 
>+
>+/**************************************************************************
>+Wraps (map_x, map_y) to be within the specified "window" if possible.  Note
>+this "window" may or may not have anything to do with a GUI window; it's
>+just like a fixed area floating over the underlying map.
>+
>+Note, (map_x, map_y) need not be real to fit under the "window": the
>+window may float over unreal positions as well.  However, we will never
>+wrap unreal coordinates to try to fit them within the window (any code
>+which requires this functionality would most likely be flawed anyway).
>+**************************************************************************/
>+void find_representative_map_pos(int *map_x, int *map_y,
>+                              struct win_map_context *win_mc)
>+{
>+  /* We absolutely don't wrap unreal coordinates. */
>+  if (*map_y < 0 || *map_y >= map.ysize)
>+    return;
>+              
>+  /* The implementation if this function is incredibly tricky,
>+     although it doesn't seem like it would be.  The problem is
>+     that not only is (*map_x,*map_y) not necessarily normal,
>+     but neither is (win_mc->x0,win_mc->y0).  In this
>+     implementation I assume that (win_mc->x0,win_mc->y0) is at
>+     least _close_ to normal...which is the case right now.  Once
>+     we allow wrapping in multiple directions, nothing outside
>+     of linear algebra methods will be reasonable. */
>+#define WRAP(x, size) ((x) % (size) >= 0 ?   \
>+                     (x) % (size) :        \
>+                     (x) % (size) + (size))
>+      
>+  if (win_mc->win_is_iso) {
>+    /* This wrapping isn't complete, but it'll do for now. */
>+    *map_x = WRAP(*map_x, map.xsize);
>+    if (*map_x < win_mc->x0)
>+      (*map_x) += map.xsize;
>+  } else {
>+    (*map_x) -= win_mc->x0;
>+    *map_x = WRAP(*map_x, map.xsize);
>+    (*map_x) += win_mc->x0;
>+  }
>+}
>+
> /**************************************************************************
> Twiddle *x and *y to point the the nearest real tile, and ensure that the
> position is normalized.
>Index: common/map.h
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/common/map.h,v
>retrieving revision 1.108
>diff -u -r1.108 map.h
>--- common/map.h       2001/12/13 19:13:16     1.108
>+++ common/map.h       2001/12/20 06:15:29
>@@ -295,6 +295,13 @@
>    (y) == map.ysize-1 || (x) == map.xsize-1)
> 
> int normalize_map_pos(int *x, int *y);
>+struct win_map_context {
>+      int x0, y0;
>+      int width, height;
>+      int win_is_iso;
>+};
>+void find_representative_map_pos(int *map_x, int *map_y,
>+                              struct win_map_context *win_mc);
> void nearest_real_pos(int *x, int *y);
> void map_distance_vector(int *dx, int *dy, int x0, int y0, int x1, int y1);
> int map_num_tiles(void);
>