[Freeciv-Dev] Re: (PR#4648) how to do wrapping in map_to_canvas_pos?

["rwetmore@xxxxxxxxxxxx" <rwetmore@xxxxxxxxxxxx>]

You understand what the is_isometric parameter does. You just need to
understand why you need to do this in the general case. You see the
problems when you don't, but haven't quite put all the pieces in the
proper perspective yet it seems.

Note, that doing things in the order currently done in the genralized
code means you do the steps you outline below as the pre-unnormalize
stage. The full is_isometric fix is just an addon clause. Thus one can
structure the routines in a nested way so as to call the simpler case
below when one doesn't need to worry about the target normalized set
and the full entry point when one does.

Or one can recognize that the general case just adds an extra if-clause
that makes no real difference in performance and will always work in
any situation.

map_distance gives you a distance range "in the wrap or native coordinate
system". As you say, this does not need to consider isometric views and
is why the pre-unnormalize step is always so useful. The fact that it
does it wrt the center of the view rather than an origin that makes all
coordinates in the target normalized set positive offsets makes it less
suitable for most operations (e.g. for use in array lookups). In the
pre-unnormalize step this is just a matter of a simple origin shift
though, and using the one in the general unnormalize solves this in
advance rather than as an a posteriori fixup.

The problem that the final fixup solves is to provide a normal set that
is aligned with a different set of target axes than the native ones. In
particular, this set allows one to display more tiles in a rectangular
GUI window that is rotated from the native set. When one needs to do
this there is no simple way but to do the vector math.

But there a might be other transformations that are interesting for any
number of other uses - the one used in the generalized unnormalize is
designed specifically to align the normalized set to a rotated rectangular
grid when this is desired, i.e. when the is_iso argument says to do this.

Cheers,
RossW
=====

Jason Short wrote:
> rwetmore@xxxxxxxxxxxx wrote:
> 
>>I'm not sure what the problem with unnnormalize_map_pos() really is,
>>but let me try and relate all the normalize/unnormalize/map_distance
>>algorithms and how they reflect special case/optimizations of the
>>general case reflected by unnormalize_map_pos() as a primer.
> 
> 
> The core problem, I think, is that unnormalize_map_pos not only does the 
> unnormalization but does it into a set of specified choosing (e.g., iso 
> or non-iso) - which simply isn't worth the extra complexity.
> 
> If you get rid of the is_iso parameter things become much simpler:
> 
>    bool unnormalize_map_pos(int x0, int y0, int *x, int *y)
>    {
>      int nx, ny;
> 
>      map_to_native_pos(&nx, &ny, *x, *y);
> 
>      if (map_has_wrapx()) {
>        nx = x0 + WRAP(nx - x0, map.xsize);
>      }
>      if (map_has_wrapy()) {
>        ny = y0 + WRAP(ny - y0, map.ysize);
>      }
> 
>      native_to_map_pos(x, y, nx, ny);
> 
>      return is_real_map_pos(*x, *y);
>    }
> 
> the only drawback is that if you use the mapview origin as the (x0,y0) 
> point there are some tiles that will never be drawn for some topologies.
> 
> Originally I thought the solution was to provide the is_iso parameter. 
> This does indeed fix things, but it makes the math much harder.  (As a 
> side note, if we were ever to draw tiles multiple times this would 
> probably be the only/easiest way).  Instead I think the solution is to 
> base the unnormalize off of the center tile of the mapview.
> 
> That said, map_distance_vector is exactly the same operation as this 
> unnormalize - but the origin it is given (x0,y0) is taken as the center 
> position of the "un"normal representative set, and the value returned is 
> a vector from this position (*dx,*dy) rather than an absolute position. 
>   Conveniently enough, this makes its usage in map_to_canvas_pos simpler 
> than it would otherwise be.
> 
> jason