[Freeciv-Dev] Re: remove map_canvas_adjust_[xy] (PR#1129)

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

Since these are all gone from the corecleanups, I have no problems with
their removal. Since they are mostly misplaced (un)normalize_map_pos() 
(the canvas doesn't have any adjustable capability), this is the default
algorithm. 

In some cases they reflect the wrap (for x) or clip( for y) specific 
functions needed to handle the standard map. But as the GUI is rewritten, 
these should all get done generically, and any current fix will likely go 
away. With a modest amount of sanity testing to insure the intermediate 
world is not unusable, anything is suitable.

Note, get_canvas_xy() rescales to pixel coordinates, which these don't
do, right?

The proof is in the testing though.

Cheers
RossW
=====

At 05:39 PM 01/12/13 +0100, Raimar Falke wrote:
>On Thu, Dec 13, 2001 at 01:46:46AM -0800, jdorje@xxxxxxxxxxxxxxxxxxxxx wrote:
>> The functions map_canvas_adjust_x and map_canvas_adjust_y are 
>> GUI-specific functions listed in the interface specification (i.e. 
>> include in gui-stub), but not used anywhere outside of the GUI-specific 
>> code.
>> 
>> The functions are supposed to wrap X and Y coordinates, and then 
>> translate them to fit within the GUI window.  This means they are the 
>> functional equivalent of Ross's unnormalize_map_pos or my 
>> find_representative_map_pos, plus a subtraction of the window origin 
>> (I'm sure Ross would point out that they mix concepts badly :-).  The 

Of course, corecleanup10 will show you examples of how to do this right,
and cleanup most of the spaghetti code in these areas as well. With a
mildly sleazy hack to pass map_type via is_earth to the client, the 
standard 4 topologies in either standard or isometric view work fully, 
with unlimited scrolling in any direction on a torus world, and 
respectable centering functions that handle the 3 cases with a fixed
map dimension. Fixes are largely confined to the new mapview_common
and citydlg_common routines. (Still not extended to native isometric).

>> major problem is that the interface uses different functions for X and Y 
>> coordinates, and will therefore be worthless under any map that doesn't 
>> wrap along the X and Y axes.
>> 
>> Furthermore, these functions are used very little.  gui-gtk and gui-xaw 
>> don't even define them.  gui-beos has the stub functions in place, but 
>> no more.  gui-mui does _not_ have the functions defined, but tries to 
>> use them anyway in some #defined-out code.  gui-win32 does define the 
>> functions, and uses them in one place.

These functions exist only in the non-gtk flavours + stub, which I 
presume means they are in stock CVS since I never do more than propagate 
standard changes there. Aka they are completely irrelevant if the other
ports catchup.
 
>> I therefore see no problem with simply removing them.  This will take 
>> things one step further toward iso-rectangular maps.  In this patch I 
>> therefore remove the stub functions from gui-stub and gui-beos, change 
>> the pretend usage in gui-mui, remove the functions from gui-win32 and 
>> replace their usage with simply get_canvas_xy.  This last is the only 
>> questionable part of the patch AFAICT, but it looks to me like 
>> get_canvas_xy is the more appropriate function to use here anyway.  It 
>> definitely needs testing, though (since I can't even compile win32...).

get_canvas_xy is a wrapper for map_to canvas_pos() which currently
requires a GUI specific function to return the window size. These
values are totally unneeded for anything but the final GUI window
test condition, i.e. normalize_canvas_pos() or is_real_canvas_pos() 
since for a clipped canvas topology all you can do is test to see
if the final transformed coordinates are in bounds.

The values should be stored in a GUI map_context struct (MC) which if
referenced via map, and updated only on startup or resize are not
required as args to any function. Resetting the environment can be
done by set_GUIMapContext(map, mc), just like setting a graphical 
context.

Thus, to get back to the point, long term the function is 
map_to_canvas_pos() (but including the pixel scaling factor).

>Andreas? Ross?
>
>       Raimar
>
>-- 
> email: rf13@xxxxxxxxxxxxxxxxx
> "That's fundamental game play!  My main enemy is *ALWAYS* fighting 
>  a 4-front war.  I make sure of it!"
>    -- Tony Stuckey, freeciv-dev