[Freeciv-Dev] (PR#6439) get_mapview_scroll_window for iso-maps

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

The current implementation of get_mapview_scroll_window only works for 
non-iso maps that wrap in the X direction only.


Well, it doesn't really work for that.  The non-iso-view implementation 
is almost correct:

   xmin = 0
   xmax = map.xsize
   xsize = tile_width

   ymin = 0
   ymax = map.ysize + 1
   ysize = tile_height

except that this doesn't allow scrolling to some legitimate positions on 
the X axis, nor does it allow convenient wrapping using the scroll bars. 
  If we were to change this to:

   xmin = -1
   xsize = tile_width
   xmax = map.xsize + xsize

then both issues would be fixed.


The iso-view implementation is even more problematic.  For one thing, 
there is the same wrapping issue as in non-iso view.  For another, the 
handling of EXTRA_BOTTOM_ROW is fallacious:

   ymin = 0
   ysize = tile_height
   ymax = map.ysize + 6

which is theoretically flawed.  The minimum Y value isn't at the 
top-left corner of the mapview (the origin off of which scroll positions 
are based), it's at the top-right corner.  This is an offset of 
tile_width.  Setting ymin to 0 just happens to get things right because 
it means the northern border of the map will run straight through the 
origin.  The value of tile_height is also wrong, since the slider moves 
through map/native coordinates rather than gui-aligned ones.  And the 
ymax value is wrong, since it also ignores the offset from the minimum - 
but this happens to work out decently since the ysize value is too small 
anyway.  The result is that you can see the whole map, but the number of 
unreal positions you're shown at the top and bottom of the map are 
different and may vary depending on the size of the map.

These values should become

   ymin = tile_width - 6
   ysize = xsize = tile_width + tile_height
   ymax = map.ysize + tile_width + 6

to fix these problems.  Note that this gives 6 extra tiles at top and 
bottom of the map.  It's probably better to make this value proportional 
to the size of the window; say MAX(tile_width,tile_height)/2 - imagine 
if the map window were only 6 tiles across.

The attached patch easy-scroll.diff makes these changes.  Notice how the 
borders are now symmetric in iso-view, and scrolling along the X axis 
can be done fully in both views.

-----

All of this is an introduction to the second patch, which makes all of 
the above corrections but works for all topology conditions.  It is 
intended to be self-documenting, so I won't say anything more about it here.

I can't guarantee it doesn't have bugs, since there are 16 different 
topology/GUI scenarios and the size of the mapview window plays a role 
in behavior as well.  But I do believe it's pretty good.

jason

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.66
diff -u -r1.66 mapview_common.c
--- client/mapview_common.c     2003/10/06 11:46:27     1.66
+++ client/mapview_common.c     2003/10/08 15:56:45
@@ -355,11 +355,24 @@
 void get_mapview_scroll_window(int *xmin, int *ymin, int *xmax, int *ymax,
                               int *xsize, int *ysize)
 {
-  *xmin = *ymin = 0;
-  *xmax = map.xsize;
-  *ymax = map.ysize + EXTRA_BOTTOM_ROW;
-  *xsize = mapview_canvas.tile_width;
-  *ysize = mapview_canvas.tile_height;
+  if (is_isometric) {
+    *xsize = mapview_canvas.tile_height + mapview_canvas.tile_width;
+    *ysize = *xsize;
+  } else {
+    *xsize = mapview_canvas.tile_width;
+    *ysize = mapview_canvas.tile_height;
+  }
+
+  *xmin = -1;
+  *xmax = map.xsize + *xsize;
+
+  if (is_isometric) {
+    *ymin = mapview_canvas.tile_width - 6;
+    *ymax = map.ysize + mapview_canvas.tile_width + 6;
+  } else {
+    *ymin = 0;
+    *ymax = map.ysize + 1;
+  }
 }
 
 /****************************************************************************

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.66
diff -u -r1.66 mapview_common.c
--- client/mapview_common.c     2003/10/06 11:46:27     1.66
+++ client/mapview_common.c     2003/10/08 15:42:45
@@ -349,17 +349,132 @@
 }
 
 /****************************************************************************
-  Return the range of values that the mapview origin can take, in scroll
-  positions.  Useful for scrollbars or when manually clipping the window.
+  Return the scroll dimensions of the clipping window for the mapview window..
+
+  Imagine the entire map in scroll coordinates.  It is a rectangle.  Now
+  imagine the mapview "window" sliding around through this rectangle.  How
+  far can it slide?  In most cases it has to be able to slide past the
+  ends of the map rectangle so that it's capable of reaching the whole
+  area.
+
+  This function gives constraints on how far the window is allowed to
+  slide.  xmin and ymin are the minimum values for the window origin.
+  xsize and ysize give the scroll dimensions of the mapview window.
+  xmax and ymax give the maximum values that the bottom/left ends of the
+  window may reach.  The constraints, therefore, are that:
+
+    get_mapview_scroll_pos(&scroll_x, &scroll_y);
+    xmin <= scroll_x < xmax - xsize
+    ymin <= scroll_y < ymax - ysize
+
+  This function should be used anywhere and everywhere that scrolling is
+  constrained.
+
+  Note that scroll coordinates, not map coordinates, are used.  Currently
+  these correspond to native coordinates.
 ****************************************************************************/
 void get_mapview_scroll_window(int *xmin, int *ymin, int *xmax, int *ymax,
                               int *xsize, int *ysize)
 {
-  *xmin = *ymin = 0;
+  /* There are a number of factors that must be taken into account in
+   * calculating these values:
+   *
+   * 1. Basic constraints: X should generally range from 0 to map.xsize;
+   * Y from 0 to map.ysize.
+   *
+   * 2. Non-aligned borders: if the borders don't line up (an iso-view client
+   * with a standard map, for instance) the minimum and maximum must be
+   * extended if the map doesn't wrap in that direction.  They are extended
+   * by an amount proportional to the size of the screen.
+   *
+   * 3. Compression: on an iso-map native coordinates are compressed 2x in
+   * the X direction.
+   *
+   * 4. Translation: the min and max values give a range for the origin.
+   * Since the base constraint is on the minimal value contained in the
+   * mapview, we have to translate the minimum and maximum to account for
+   * this.
+   *
+   * 5. Wrapping: if the map wraps in a given direction, no border adjustment
+   * or translation is needed.  Instead we have to make sure the range is
+   * large enough to get the full wrap.
+   */
+  *xmin = 0;
+  *ymin = 0;
   *xmax = map.xsize;
-  *ymax = map.ysize + EXTRA_BOTTOM_ROW;
-  *xsize = mapview_canvas.tile_width;
-  *ysize = mapview_canvas.tile_height;
+  *ymax = map.ysize;
+
+  if (topo_has_flag(TF_ISO) != is_isometric) {
+    /* The mapview window is aligned differently than the map.  In this
+     * case we need to give looser constraints because (if the map doesn't
+     * wrap) the edges will not line up well. */
+
+    /* These are the dimensions of the bounding box. */
+    *xsize = *ysize =
+      mapview_canvas.tile_width + mapview_canvas.tile_height;
+
+    if (is_isometric) {
+      /* Step 2: Calculate extra border distance. */
+      *xmin = *ymin = -(MAX(mapview_canvas.tile_width,
+                           mapview_canvas.tile_height) + 1) / 2;
+      *xmax -= *xmin;
+      *ymax -= *ymin;
+
+      /* Step 4: Translate the Y coordinate.  The mapview origin is at the
+       * top-left corner of the window, which is offset at +tile_width from
+       * the minimum Y value (at the top-right corner). */
+      *ymin += mapview_canvas.tile_width;
+      *ymax += mapview_canvas.tile_width;
+    } else {
+      /* Compression. */
+      *xsize = (*xsize + 1) / 2;
+
+      /* Step 2: calculate border adjustment. */
+      *ymin = -(MAX(mapview_canvas.tile_width,
+                   mapview_canvas.tile_height) + 1) / 2;
+      *xmin = (*ymin + 1) / 2; /* again compressed */
+      *xmax -= *xmin;
+      *ymax -= *ymin;
+
+      /* Step 4: translate the X coordinate.  The mapview origin is at the
+       * top-left corner of the window, which is offset at +tile_height/2
+       * from the minimum X value (at the bottom-left corner). */
+      *xmin += mapview_canvas.tile_height / 2;
+      *xmax += (mapview_canvas.tile_height + 1) / 2;
+    }
+  } else {
+    *xsize = mapview_canvas.tile_width;
+    *ysize = mapview_canvas.tile_height;
+
+    if (is_isometric) {
+      /* Compression: Each vertical half-tile corresponds to one native
+       * unit (a full horizontal tile corresponds to a native unit). */
+      *ysize = (mapview_canvas.height - 1) / (NORMAL_TILE_HEIGHT / 2) + 1;
+
+      /* Isometric fixes: the above calculations are in half-tiles; since we
+       * need to see full tiles we have to extend the range a bit.  This also
+       * corrects for the off-by-one error caused by the X compression of
+       * native coordinates. */
+      (*xmin)--;
+      (*xmax)++;
+      (*ymax) += 2;
+    }
+  }
+
+  /* Now override the above to satisfy wrapping constraints.  We allow the
+   * scrolling to cover the full range of the map, plus one unit in each
+   * direction (to allow scrolling with the scroll bars, for instance). */
+  if (topo_has_flag(TF_WRAPX)) {
+    *xmin = -1;
+    *xmax = map.xsize + *xsize;
+  }
+  if (topo_has_flag(TF_WRAPY)) {
+    *ymin = -1;
+    *ymax = map.ysize + *ysize;
+  }
+
+  freelog(LOG_DEBUG, "x: %d<-%d->%d; y: %d<-%d->%d",
+         *xmin, *xsize, *xmax, *ymin, *ymax, *ysize);
 }
 
 /****************************************************************************