[Freeciv-Dev] Re: [PATCH] cleanup to canvas_pos_to_map_pos (PR#1183)

["Ross W. Wetmore" <rwetmore@xxxxxxxxxxxx>]

At 06:12 PM 02/01/01 -0500, Jason Short wrote:
>Ross W. Wetmore wrote:
>
>> At 10:39 PM 01/12/31 -0500, Jason Short wrote:
>> 
>>>Ross W. Wetmore wrote:
>
>[snip: off-by-one error in canvas_pos_to_[city|map]_pos]

> Your code is slightly 
>different because you add a "roundoff" value to the rotated_canvas_* 
>values before dividing, so you'll see the off-by-one error in a 
>different place.  When I try out the corecleanups, I see the off-by-one 
>in the upper right of the mapview and the lower left of the cityview.

No you don't. You see an accumulated slippage of the grid position, 
which means you still haven't done the legwork tests that show this :-).

Stick a worker in a tile, then approach the tile along the 4 orthogonal
directions clicking until the worker is picked up. Note where the tile
borderline is in each case. Now do the next tile, you will see that the
mouse grid and the pixmap grid aren't quite the same size, so each row
out is a little more skewed. It is only skewed along one diagonal/axis, 
too.

If the problem was just in a "different" place it would be near the 
center, i.e. within the distance of the rounding parameter, and not far 
off at the edges :-).

>Now, you say that each tile isn't quite 
>NORMAL_TILE_WIDTHxNORMAL_TILE_HEIGHT in size, and so we have to fudge 
>things with a roundoff factor to get things to work out. 

No, the roundoff (as I keep saying) is only to centre the mouseclicks
by shifting the centerpoint or borderlines where the integral value
changes. It is a local tile offset, just like the (2,-2) except that
it is of granularity NORMAL_TILE_WIDTH or whatever the divisor is, i.e.
smaller than a full tile shift. Look at the code.

The roundoff has nothing to do with one edge being out, a scaling issue, 
or your off-by-one divide problem.

> I'm pretty 
>sure this is incorrect as well. 

Well, I have to agree with this, you certainly don't have the roundoff 
picture right, or properly separated :-).

> But more technically, if this 
>were the case it would be necessary for the same effect to be present in 
>map_pos_to_canvas_pos.  These functions 
>(map_pos_to_canvas_pos/canvas_pos_to_map_pos) use algorithms that are 
>exact inverses, so any effect in one must be present in the other.  I 
>really think you're making things more complicated than they are here 
>(and perhaps confusing the off-by-one error for a scaling problem).
>
>jason

There is a scaling problem with the mapview as well. But it seems much more
severe with the citydlg, or at least it irritated me more there :-). The 
tile size is the key element of the scaling in both cases, which is why 
my hunch says there is something there. I just haven't gotten around to 
tracking down the full details. 

Note, mouseclicks are events that produce raw canvas coordinates. 
Painting tiles is done by converting from map coordinates to offsets
from the topleft position. They actually go through different codepaths
all the way, and need to be lined up. You can shift the tile grid by
changing the topleft point just like the roundoff does for mouseclicks.

If you think about the roundoff, you will understand that it can only
move its whole grid around on top of the citymap, and not change the
offset at just one tile (i.e. your edge tile).

Which is why without seeing the discontinuous jump of a full tile, I
don't think the off-by-one issue you had to fix is quite as "obvious" 
as you seem to think.

I never had an off-by-one problem, nor really did the original code :-).

Cheers,
RossW
=====