[Freeciv-Dev] Re: (PR#3936) introducing native coordinates

[Jason Dorje Short <vze49r5w@xxxxxxxxxxx>]

Raimar Falke wrote:
> On Mon, Apr 14, 2003 at 12:20:47PM -0500, Jason Dorje Short wrote:
>
> > Raimar Falke wrote:
> >
> > > On Mon, Apr 14, 2003 at 11:02:28AM -0500, Jason Dorje Short wrote:
> > >
> > > > Raimar Falke wrote:
> > > >
> > > > > On Sun, Apr 13, 2003 at 05:37:59PM -0500, Jason Dorje Short wrote:
> > > > I'm not sure.  Certainly 
> > > > there are some additional (non-mapview-specific) ones in mapview.
> > >
> > > > Conversely, if we use map coordinates as standard there are just a few 
> > > > places that need to convert to native coordinates.  These include a few 
> > > > map.c backend functions (normalize_map_pos, map_distance_vector, 
> > > > rand_map_pos, map_to_index_pos, index_to_map_pos) as well as a very few 
> > > > pieces of user code (savegames, map debug output).
> > >
> > > So we both say that our changes are minimal.
> > Yep.  I'm not against the idea of using native coordinates as standard. 
> > I'm just not willing to track down all the places that need to be 
> > fixed for it, since I've already spent 2 years doing it the other way.
> >
> > > > > - mapview: is reduced to one file (mapview_common) AFAIK
> > > > Don't just lump in everything in mapview.c and mapview_common.c into 
> > > > "mapview" and write it off.  mapview does have some very specialized 
> > > > needs (like clipping the window), but also some generalized ones as well 
> > > > (like rectangle_iterate).
> > >
> > > You agree that all this should and can be done in mapview_common?!
> > >
> > > I also said in the past that the mapview_common change won't be easy.
> > But again you're lumping all this stuff together.  There is a discrete 
> > set of changes:
> > - clip_mapview_window - localized and ugly.  Does not relate to the 
> > current discussion.
> > - mapview scrolling - non-localized and ugly.  Does not relate to the 
> > current discussion.
> > - overview window: non-localized, but fairly low-impact.
> >
> > - unnormalize_map_pos: localized and complicated.  Does not relate to 
> > the current discussion.
> > - Other misc changes.  These generally do relate to the current 
> > discussion.  For instance under your system, all of the mapview 
> > iterations need to be done in map coordinates and need a manual 
> > conversion.  This is not localized, and cannot be written off as "part 
> > of that mapview change" since it relates directly to this discussion.
> I really can't say anything about mapview(_common). I don't know it at
> the detail level which is needed.
Then you'll have to trust me when I saw that rewriting it to use native 
coordinates as standard won't be much fun (nor was changing mapgen to 
use map coordinates as standard).
I've been thinking about how to setup some iterator macros to simplify 
the mapview iteration segments - this would lessen the impact of such 
code.  But additional changes will be needed - for instance 
mapview_x0/mapview_y0 need to be in _map_ coordinates, then in 
map_to_canvas_pos you first convert from the native position you're 
given into map coordinates, then do the rest of the math.
This brings up another issue, which is that the term "map" is used 
synonymously with "standard" coordinates throughout the code.  If map 
coordinates are not standard, we should rename this coordinate 
system...perhaps to "tile coordinates".  Then the nomenclature becomes:
  map == standard == whatever coordinate system is the default
  native = global-map-oriented coordinates
  tile = tile-oriented coordinates

> > > Let me create some code so that we can have some facts. Hopefully this
> > > week.
> > >
> > > Is there an implementation of your approach which is complete? For
> > > measure of speed and general impact this doesn't have to support
> > > mapview.
> > Excellent.  See the gen-topologies patch in freeciv-test.  It is 
> > out-of-date for current CVS (by about 2 weeks, I think - I have an 
> > updated patch locally).
> I'm not subscribed to freeciv-test.
There is nothing on the e-mail lists.  Just get the freeciv-test CVS 
code, compile and run.  You can also get the patch from 
freeciv-test-patches CVS.
> > You do need to support mapview.  Most of the mapview code can remain 
> > unchanged (note, clip_mapview_window and mapview scrolling do not work 
> > perfectly).  What you will need to fix is the fact that mapview deals 
> > exclusively in map coordinates, so you'll need to convert to/from native 
> > coordinates all over the place.
> > But you can drop all of the existing map<->native conversion operations, 
> > so this will approximately balance out.  This includes huge quantities 
> > of mapgen.c, some stuff in map.c, and some scattered other places.
> > Compared to mapgen and mapview, everything else is tiny.  So as a 
> > measure of "general impact" these two are very significant.
>
> We haven't touched the question of wrapping till now. I think that
> both of http://www.freeciv.org/~rfalke/grid_pictures/grid_iso_wrap.png
> and http://www.freeciv.org/~rfalke/grid_pictures/grid_iso_rot_wrap.png
> are possible.
Both are possible, yes.  One is iso (grid_iso_wrap) and one is non-iso 
with a bizarelly chosen normal set (grid_iso_rot_wrap).
Wrapping is very simple.  It is easiest to understand if you think in 
natural or native coordinates (and certainly easiest to code that way).
> grid_iso_wrap.png is rather easy to implement and requires almost no
> changes to mapgen.c (at least I think so).
Yes, wrapping has no effect on anything outside of a couple of topology 
functions, and mapview (as discussed before).
jason