[Freeciv-Dev] Re: (PR#8627) best overview for iso-maps

["Marcelo Burda" <mburda@xxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8627 >

Le sam 08/05/2004 à 07:01, Jason Short a écrit :
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=8627 >
> 
> 1.  The patch needs to be updated for current cvs.
Harg.. you make const created var in do_in_XX_pos!, you can say it to
me!
this is a good idea to avoid changing it and think this change map ones,
this is ok! but this make created vars very stricts!
> 
> 2.  There's still a problem with !TF_WRAPX maps.  The problem now is 
> that if you click on one of the black zig-zag areas you are clicking on 
> a nonexistant tile.  What happens afterward is unspecified.
> 
> If you click on the zig-zag on the left, because of integer rounding you 
> end up as if you had clicked on the tile just to the right of where you 
> clicked.
> 
> If you click on the zig-zag on the right, theoretically the assertion in 
> overview_to_map_pos should trigger.  However I can't get it to trigger 
> under the gtk2 client.  I believe this is another bug: clicking on the 
> rightmost pixel of the overview in the gtk2 client simply ignores the 
> click.  (However I did manage to get the assertion in the gtk client.)
FIXED, basically all rewrited!
> 
> Solving this is either ugly or takes some with.  Either:
> 
> - Use nearest_real_map_pos instead of normalize_map_pos in 
> overview_to_map_pos.  This is ugly and bad.
no
> 
> - Have overview_to_map_pos return a boolean.  Callers must filter based 
> on this return value.  This is probably the best thing to do but takes 
> some work.
> 
NO
> - Clip the left and right half-tile of the overview.  That is, instead 
> of adding on an extra half-tile to the overview dimensions, and a for 
> all positions with (y & 1) == 1, subtract off a half-tile from the 
> overview dimensions, and for all positions with (y & 1) == 0.  This will 
> look pretty but makes some positions hard to click on.
NO
> 
> 3.  The overview dimensions aren't set properly.  You set
> 
>    HEIGHT = MAX(120 / xsize, 1)
>    WIDTH = 2 * HEIGHT
> 
> but the goal is that the width be as close to 120 as possible.  So with 
> an xsize of 60 you should have HEIGHT==1, WIDTH==2 but instead you have 
> HEIGHT==2, WIDTH==4.  Instead I think you should have
> 
>    HEIGHT = MAX(60 / xsize, 1)
>    WIDTH = 2 * HEIGHT
> ?
original math not use  MAX function (this make a + 1)! you can't compare
its easly your proposal are no good, this make micro overview !
> Of course instead of using 60 or 120 here we should really use the 
> actual panel dimensions to allow for a resizable overview.  But this 
> will take some changes to the GUI code.  (Or maybe this is done already? 
>   I forget.)
this is a good idea. but i was not implements it
> 
> jason
-- 
 . /  .     '    ,    .      (*)   '        `     '      `    .    
  |    ,  |   `     ,     .      ,   '  Marcelo Julián Burda      .
 /  '     \     `     \@_     '      .        '      `        '    
~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~

diff -ruN -Xdiff_ignore freeciv/client/mapview_common.c 
freeciv_/client/mapview_common.c
--- freeciv/client/mapview_common.c     2004-05-08 17:21:05.000000000 +0200
+++ freeciv_/client/mapview_common.c    2004-05-08 23:48:45.000000000 +0200
@@ -1994,23 +1994,24 @@
 **************************************************************************/
 static void center_tile_overviewcanvas(int map_x, int map_y)
 {
-  /* The overview coordinates are equivalent to native coordinates. */
-  map_to_native_pos(&map_x, &map_y, map_x, map_y);
+  /* The overview coordinates are equivalent to natural coordinates. */
+  do_in_ntl_pos(ntl_x, ntl_y, map_x, map_y) {
 
-  /* NOTE: this embeds the map wrapping in the overview code.  This is
-   * basically necessary for the overview to be efficiently
-   * updated. */
-  if (topo_has_flag(TF_WRAPX)) {
-    overview.map_x0 = FC_WRAP(map_x - map.xsize / 2, map.xsize);
-  } else {
-    overview.map_x0 = 0;
-  }
-  if (topo_has_flag(TF_WRAPY)) {
-    overview.map_y0 = FC_WRAP(map_y - map.ysize / 2, map.ysize);
-  } else {
-    overview.map_y0 = 0;
-  }
-  redraw_overview();
+    /* NOTE: this embeds the map wrapping in the overview code.  This is
+     * basically necessary for the overview to be efficiently
+     * updated. */
+      if (topo_has_flag(TF_WRAPX)) {
+       overview.map_x0 = FC_WRAP(ntl_x - NTL_WIDTH / 2, NTL_WIDTH);
+      } else {
+       overview.map_x0 = 0;
+      }
+      if (topo_has_flag(TF_WRAPY)) {
+       overview.map_y0 = FC_WRAP(ntl_y - NTL_HEIGHT / 2, NTL_HEIGHT);
+      } else {
+       overview.map_y0 = 0;
+      }
+      redraw_overview();
+  } do_in_ntl_pos_end;
 }
 
 /**************************************************************************
@@ -2019,23 +2020,22 @@
 void map_to_overview_pos(int *overview_x, int *overview_y,
                         int map_x, int map_y)
 {
-  int gui_x, gui_y;
-
   /* The map position may not be normal, for instance when the mapview
    * origin is not a normal position.
    *
    * NOTE: this embeds the map wrapping in the overview code. */
-  map_to_native_pos(&gui_x, &gui_y, map_x, map_y);
-  gui_x -= overview.map_x0;
-  gui_y -= overview.map_y0;
-  if (topo_has_flag(TF_WRAPX)) {
-    gui_x = FC_WRAP(gui_x, map.xsize);
-  }
-  if (topo_has_flag(TF_WRAPY)) {
-    gui_y = FC_WRAP(gui_y, map.ysize);
-  }
-  *overview_x = OVERVIEW_TILE_WIDTH * gui_x;
-  *overview_y = OVERVIEW_TILE_HEIGHT * gui_y;
+  do_in_ntl_pos(ntl_x, ntl_y, map_x, map_y) {
+    int gui_x = ntl_x - overview.map_x0,
+       gui_y = ntl_y - overview.map_y0;
+    if (topo_has_flag(TF_WRAPX)) {
+      gui_x = FC_WRAP(gui_x, map.xsize);
+    }
+    if (topo_has_flag(TF_WRAPY)) {
+      gui_y = FC_WRAP(gui_y, map.ysize);
+    }
+    *overview_x = OVERVIEW_TILE_WIDTH * gui_x;
+    *overview_y = OVERVIEW_TILE_HEIGHT * gui_y;
+  } do_in_ntl_pos_end;
 }
 
 /**************************************************************************
@@ -2044,10 +2044,22 @@
 void overview_to_map_pos(int *map_x, int *map_y,
                         int overview_x, int overview_y)
 {
-  int nat_x = overview_x / OVERVIEW_TILE_WIDTH + overview.map_x0;
-  int nat_y = overview_y / OVERVIEW_TILE_HEIGHT + overview.map_y0;
+    int ntl_x = overview_x / OVERVIEW_TILE_WIDTH,
+       ntl_y = overview_y / OVERVIEW_TILE_HEIGHT;
 
-  native_to_map_pos(map_x, map_y, nat_x, nat_y);
+  if(topo_has_flag(TF_ISO)) {
+    if( !topo_has_flag(TF_WRAPX)) {
+      if ( ntl_x >= map.xsize * 2) {
+       ntl_x -= 2;
+      } else  if (ntl_x < 1 && ((ntl_y & 1) != 0 ) ) {
+       ntl_x += 2;
+      }
+    }
+  }
+  
+  natural_to_map_pos(map_x, map_y,
+                    ntl_x + overview.map_x0, ntl_y + overview.map_y0);
+ 
   if (!normalize_map_pos(map_x, map_y)) {
     /* All positions on the overview should be valid. */
     assert(FALSE);
@@ -2061,39 +2073,43 @@
 static void get_mapview_corners(int x[4], int y[4])
 {
   int map_x0, map_y0;
+  int TILE_WIDTH;
 
   canvas_to_map_pos(&map_x0, &map_y0, 0, 0);
   map_to_overview_pos(&x[0], &y[0], map_x0, map_y0);
 
   /* Note: these calculations operate on overview coordinates as if they
    * are native. */
+  /* there is to correct it to natural */
+  TILE_WIDTH = (topo_has_flag(TF_ISO) ? 2 : 1 ) * OVERVIEW_TILE_WIDTH;
+
   if (is_isometric && !topo_has_flag(TF_ISO)) {
     /* We start with the west corner. */
 
     /* North */
-    x[1] = x[0] + OVERVIEW_TILE_WIDTH * mapview_canvas.tile_width;
+    x[1] = x[0] + TILE_WIDTH * mapview_canvas.tile_width;
     y[1] = y[0] - OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_width;
 
     /* East */
-    x[2] = x[1] + OVERVIEW_TILE_WIDTH * mapview_canvas.tile_height;
+    x[2] = x[1] + TILE_WIDTH * mapview_canvas.tile_height;
     y[2] = y[1] + OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_height;
 
     /* South */
-    x[3] = x[0] + OVERVIEW_TILE_WIDTH * mapview_canvas.tile_height;
+    x[3] = x[0] + TILE_WIDTH * mapview_canvas.tile_height;
     y[3] = y[0] + OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_height;
   } else if (!is_isometric && topo_has_flag(TF_ISO)) {
     /* We start with the west corner.  Note the X scale is smaller. */
 
     /* North */
-    x[1] = x[0] + OVERVIEW_TILE_WIDTH * mapview_canvas.tile_width / 2;
+    x[1] = x[0] + TILE_WIDTH * mapview_canvas.tile_width / 2;
     y[1] = y[0] + OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_width;
 
     /* East */
-    x[2] = x[1] - OVERVIEW_TILE_WIDTH * mapview_canvas.tile_height / 2;
+    x[2] = x[1] - TILE_WIDTH * mapview_canvas.tile_height / 2;
     y[2] = y[1] + OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_height;
 
     /* South */
-    x[3] = x[2] - OVERVIEW_TILE_WIDTH * mapview_canvas.tile_width / 2;
+    x[3] = x[2] - TILE_WIDTH * mapview_canvas.tile_width / 2;
     y[3] = y[2] - OVERVIEW_TILE_HEIGHT * mapview_canvas.tile_width;
   } else {
     /* We start with the northwest corner. */
@@ -2101,7 +2117,7 @@
     int screen_height = mapview_canvas.tile_height * (is_isometric ? 2 : 1);
 
     /* Northeast */
-    x[1] = x[0] + OVERVIEW_TILE_WIDTH * screen_width - 1;
+    x[1] = x[0] + TILE_WIDTH * screen_width - 1;
     y[1] = y[0];
 
     /* Southeast */
@@ -2133,18 +2149,30 @@
 **************************************************************************/
 void overview_update_tile(int map_x, int map_y)
 {
-  int base_x, base_y;
-
-  /* Base overview positions are just like native positions, but scaled to
+  /* Base overview positions are just like natural positions, but scaled to
    * the overview tile dimensions. */
-  map_to_native_pos(&base_x, &base_y, map_x, map_y);
-  base_x *= OVERVIEW_TILE_WIDTH;
-  base_y *= OVERVIEW_TILE_HEIGHT;
-
-  canvas_put_rectangle(overview.store,
-                      overview_tile_color(map_x, map_y), base_x, base_y,
-                      OVERVIEW_TILE_WIDTH, OVERVIEW_TILE_HEIGHT);
-  dirty_overview();
+    do_in_ntl_pos(ntl_x, ntl_y, map_x, map_y) {
+      int base_y =ntl_y * OVERVIEW_TILE_HEIGHT,
+         base_x =ntl_x * OVERVIEW_TILE_WIDTH,
+         TILE_WIDTH = OVERVIEW_TILE_WIDTH;
+
+      if (topo_has_flag(TF_ISO)) {
+       TILE_WIDTH *= 2;
+       if(topo_has_flag(TF_WRAPX) && base_x > overview.width - TILE_WIDTH) {
+         /* ups we need update half tile at 0 */
+         canvas_put_rectangle(overview.store, 
+                              overview_tile_color(map_x, map_y),
+                              base_x - overview.width, base_y,
+                              TILE_WIDTH , OVERVIEW_TILE_HEIGHT); 
+       }     
+      } 
+
+    canvas_put_rectangle(overview.store,
+                        overview_tile_color(map_x, map_y), base_x, base_y,
+                        TILE_WIDTH, OVERVIEW_TILE_HEIGHT);
+
+    dirty_overview();
+  } do_in_ntl_pos_end;
 }
 
 /**************************************************************************
@@ -2152,7 +2180,20 @@
 **************************************************************************/
 void set_overview_dimensions(int width, int height)
 {
+  /* Set the scale of the overview map.  Note, since only the width is
+   * used to calculate the overview scale you can end up with a really
+   * tall or short overview if your map is unusually sized. */
+
+  OVERVIEW_TILE_WIDTH = (120 / width) + 1;
+  OVERVIEW_TILE_HEIGHT = OVERVIEW_TILE_WIDTH;
   overview.width = OVERVIEW_TILE_WIDTH * width;
+  if(topo_has_flag(TF_ISO)) {
+    OVERVIEW_TILE_HEIGHT = MAX(120 / width, 1);
+    OVERVIEW_TILE_WIDTH =  OVERVIEW_TILE_HEIGHT;
+    overview.width = OVERVIEW_TILE_WIDTH * width *2 
+      + (!topo_has_flag(TF_WRAPX)
+      ? OVERVIEW_TILE_WIDTH : 0);
+  }
   overview.height = OVERVIEW_TILE_HEIGHT * height;
 
   if (overview.store) {
diff -ruN -Xdiff_ignore freeciv/client/packhand.c freeciv_/client/packhand.c
--- freeciv/client/packhand.c   2004-05-01 01:48:36.000000000 +0200
+++ freeciv_/client/packhand.c  2004-05-08 23:52:03.000000000 +0200
@@ -1310,12 +1310,6 @@
   map_allocate();
   init_client_goto();
 
-  /* Set the scale of the overview map.  Note, since only the width is
-   * used to calculate the overview scale you can end up with a really
-   * tall or short overview if your map is unusually sized. */
-  OVERVIEW_TILE_WIDTH = (120 / map.xsize) + 1;
-  OVERVIEW_TILE_HEIGHT = OVERVIEW_TILE_WIDTH;
-
   set_overview_dimensions(map.xsize, map.ysize);
 }
 
diff -ruN -Xdiff_ignore freeciv/common/map.h freeciv_/common/map.h
--- freeciv/common/map.h        2004-05-08 17:21:07.000000000 +0200
+++ freeciv_/common/map.h       2004-05-08 23:52:03.000000000 +0200
@@ -207,9 +207,22 @@
       *(pnat_x) = (2 * (map_x) - *(pnat_y) - (*(pnat_y) & 1)) / 2)          \
    : (*(pnat_x) = (map_x), *(pnat_y) = (map_y)))
 
+#define natural_to_map_pos(pmap_x, pmap_y, nat_x, nat_y)                    \
+  (topo_has_flag(TF_ISO)                                                    \
+   ? (*(pmap_x) = ((nat_y) + (nat_x)) / 2,                                  \
+      *(pmap_y) = (nat_y) - *(pmap_x) + map.xsize)                          \
+   : (*(pmap_x) = (nat_x), *(pmap_y) = (nat_y)))
+
+#define map_to_natural_pos(pnat_x, pnat_y, map_x, map_y)                    \
+  (topo_has_flag(TF_ISO)                                                    \
+   ? (*(pnat_y) = (map_x) + (map_y) - map.xsize,                            \
+      *(pnat_x) = 2 * (map_x) - *(pnat_y))                                  \
+   : (*(pnat_x) = (map_x), *(pnat_y) = (map_y)))
+
+
 /* Provide a block to convert from map to native coordinates.  This allows
  * you to use a native version of the map position within the block.  Note
- * that any changes to the native position won't affect the map position. */
+ * that native position are declare as const. */
 #define do_in_native_pos(nat_x, nat_y, map_x, map_y)                        \
 {                                                                           \
   int _nat_x, _nat_y;                                                       \
@@ -221,6 +234,27 @@
   }                                                                         \
 }
 
+/* Provide a block to convert from map to natural coordinates. This allows
+ * you to use a natural version of the map position within the block.
+ * natural version vars are const and can't be changed inside the block */
+
+#define do_in_ntl_pos(ntl_x, ntl_y, map_x, map_y)                        \
+{                                                                           \
+  int _ntl_x, _ntl_y;                                                       \
+  map_to_natural_pos(&_ntl_x, &_ntl_y, map_x, map_y);                      \
+  {                                                                         \
+    const int ntl_x = _ntl_x, ntl_y = _ntl_y;
+
+#define do_in_ntl_pos_end                                                \
+  }                                                                         \
+}
+
+#define NAT_WIDTH map.xsize
+#define NAT_HEIGHT map.ysize
+
+#define NTL_WIDTH (topo_has_flag(TF_ISO) ? 2 * map.xsize : map.xsize)
+#define NTL_HEIGHT map.ysize
+
 static inline int map_pos_to_index(int map_x, int map_y);
 
 /* index_to_map_pos(int *, int *, int) inverts map_pos_to_index */