[Freeciv-Dev] Re: update for isometric scrolling problem (PR#1222)

[Jason Short <vze2zq63@xxxxxxxxxxx>]

Vasco Alexandre Da Silva Costa wrote:

> On Sat, 19 Jan 2002, Ross W. Wetmore wrote:
<snip>


> > Are you suggesting the entire GUI needs to deal with things exclusively
> > on a pixel basis, for example, topleft needs to have pixel granularity,
> > or what specifically is the (TM) feature that you are looking for here?
> Yes. That is what i was talking about. The topleft should have pixel
> granularity. We must deal with things on a pixel basis.
Changing the code to match this is a non-trivial issue, though.  The 
problem is that the topleft corner is dealt with on a per-gui basis.  So 
a first step IMO is to unify these constructs and force every GUI to 
adapt to the norm.
One alternative is to change each gui independently, starting with GTK. 
 But this is a step in the wrong direction IMO; it just fragments the 
GUI code even more.
Another alternative is to go ahead and create the mapview struct and put 
only the offset data into it now.  The GUI will continue to use its own 
storage for the rest of the mapview data.  This will be very confusing.

> The map canvas has a variable size and this size varies in pixels. 
> Why are we pretending it is a well behaved rectangle that is always a
> multiple of the tile size? This is the root of many bugs and oddities in
> the map view.
It's a bit more complicated than I think you realize.  The current code has

  #define EXTRA_BOTTOM_ROW (is_isometric ? 6 : 1)

which you might think can be removed entirely if we deal only in pixel 
coordinates.  Not so.  In the corecleanups Ross changes this to
/* If we have isometric view we need to be able to scroll a little extra
 *  down. The places that need to be adjusted are the same as above.
 * Note that if the native and view axes are not aligned, there needs
 *  to be a little extra to allow for the bounding box corner to fall
 *  outside rather than just touch the edge.
 * For FLAT-EARTH maps, a little more is needed to get into the corners.
 *
 * This is now a border width split evenly around the mapview with
 *  odd tiles at the BOTTOM/RIGHT (i.e. where scrollbars sit).
 * 2001-12-11 rww
 */
#define EXTRA_SIZED                                 \
  ((get_maptype(MAP_TYPE_ISO) != 0) != is_isometric \
    ? (get_maptype(MAP_TYPE_TORUS) == 0 ?           \
       (is_isometric ? 15 : 9) : 3) : 0)
#define EXTRA_BOTTOM_ROW    ((is_isometric ? 3 : 1) + EXTRA_SIZED )

which at first glance just looks confusing.  But what this is trying to 
do is separate EXTRA_BOTTOM_ROW (which now _does_ go away if we deal 
only in pixel coordinates) and EXTRA_SIZED, which is an additional fudge 
factor that hopes to handle the border cases for when isometric maps are 
used on non-wrapping flat-rectangular worlds, or vice versa.
If that doesn't make sense, try this: suppose we're using an overhead 
(flat) view GUI on an iso-rectangular non-wrapping map.  When we scroll 
to a border we have a problem:
__________
|    \   | <- off-world, black tiles
|REAL \  |
|WORLD \ |
|_______\|

how do we know when to stop the mapview from scrolling right?  At first 
glance one might think that we should force all 4 corners to be on-map 
(real) positions, but this makes it very difficult to see the borders of 
the map (only one border tile can be shown at a time).  An easy solution 
is to just force the center tile to be real, but this doesn't let us 
line things up when the mapview and the map are coaligned.  The current 
code, which Ross has extended above, checks to see if the mapview and 
map are coaligned, and if they aren't adds on some extra values to 
EXTRA_BOTTOM_ROW.  This isn't an unreasonable solution, but it still has 
several problems (1. It forces us to hardcode topology checks into the 
GUI.  2. It won't scale well, as the values are chosen to work well on a 
standard-sized mapview window).
This is a very mixed GUI and topological issue, and is pretty tricky to 
begin with.  Trying to deal with it in pixel coordinates will be 
especially tricky.

> We must treat the problem in this fashion: the "map_canvas" is in pixel
> coordinates, and the "map" is in tile coordinates.
>
> The coordinates must be translated between these units by a set of
> functions. The code should use this set of functions everywhere instead of
> doing this ad-hoc like it is doing right now.
The above problem aside, I don't think it will be that difficult.  The 
conversions are currently mostly done in a set of well-defined functions 
 in mapview_common, so most changes will just need to be done there.  A 
few other places will need to be cleaned up.
But, right now mapview_common is crippled because its code cannot 
directly manipulate mapview data.  We have to rely on the calling code 
to pass in pointers to the data, manipulate them, and then update after 
we return.  This is less than ideal.

jason