[Freeciv-Dev] Re: (PR#3924) Bugfix for map/canvas coordination functions

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

rwetmore@xxxxxxxxxxxx wrote:
> a-l@xxxxxxx wrote:
> 
>>This patch fixes two bugs:
>>one in canvas_to_map_pos(), and one in map_to_canvas_pos().
>>Both are relevant only for non-isometric mode.
>>
>>If this is not formalized already, I would propose that the
>>following should be a defined property of the two functions
>>in question:
>>
>>That this verification procedure should hold true:
>>{
>>  map_to_canvas_pos(&x, &y, tile_x1, tile_y1);
>>
>>  x += NORMAL_TILE_WIDTH / 2;
>>  y += NORMAL_TILE_HEIGHT / 2;
>>
>>  canvas_to_map_pos(&tile_x2, &tile_y2, x, y);
>>
>>  assert(tile_x1 == tile_x2);
>>  assert(tile_y1 == tile_y2);
>>}
>>For all tiles at all times.
> 
> 
> If you define the tile origin as the upper left corner, then I
> think you should make sure that any value from [0,NORMAL_TILEWIDTH)
> and [0,NORMAL_TILE_HEIGHT) can be added to the initial coordinate
> transformation and the result will still be true.
> 
> If you define the center of the tile as the tile origin the range
> is [-NORMAL_TILE_WIDTH/2,NORMAL_TILE_WIDTH - NORMAL_TILE_WIDTH/2).

This is impossible because iso tiles are diamond-shaped, not (cardinally 
aligned) rectangles.

>>We must add half a tile because of PR#3800, the bounding
>>draw box thing. Ie nobody should "fix" PR#3800.
> 
> Fix PR#3800 by defining the tile origin precisely and making the
> transformation conform to the definition.

PR#3800 is not a bug.  In iso-view, the top-left corner of the bounding 
box of the tile (which is implicitly defined as the tile origin) is not 
a part of the tile itself.

> The way to think of this is that canvas coordinates have a "FLAT_EARTH"
> normal set (no wrapping), and are masked by a given GUI rectangle/shaped
> window. Real canvas coordinates are within the allowed window and unreal
> ones are outside it. Thus the transformation returns a true/false value
> equivalent to normalize_canvas_pos() in addition to an algebraically
> correct result for the transformation.

The "real" association is not entirely correct, though - we may (but 
currently do not) use "unreal" canvas coordinates for drawing city 
descriptions, for instance, since the text goes below the tile itself.

jason