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

[Raimar Falke <rf13@xxxxxxxxxxxxxxxxx>]

On Thu, Apr 24, 2003 at 09:00:59AM -0400, Ross Wetmore wrote:
> 
> Raimar Falke wrote:
> >On Wed, Apr 23, 2003 at 05:50:43PM -0500, Jason Dorje Short wrote:
> >
> >>Raimar Falke wrote:
> [...]
> >>Using "map coordinates" rather than "native coordinates" as standard is 
> >>fastest, since the vast, vast, vast majority of operations are done on 
> >>map coordinates.
> >
> >I disagree here. Every tile access needs the compact form. The same is
> >true for normalize_map_pos and is_normal_map_pos. But without testing
> >we won't find out.
> 
> Wrong on aeveral counts.

> Tile accesses are done in indexed coordinates - memory is linear.

Yes. But the indexed form can easily (and is derived in the Jason's
gen_topo patch) derived from the compact form:

+int map_pos_to_index(int map_x, int map_y)
+{
+  int nat_x, nat_y;
+
+  CHECK_MAP_POS(map_x, map_y);
+  map_to_native_pos(&nat_x, &nat_y, map_x, map_y);
+  return nat_x + nat_y * map.xsize;
+}

> All game logic is handled in standard/internal coordinates including
> things like the local iterators. The Freeciv foundations are based on
> a standard map grid and changing this is a lot of work to make things
> less efficient and much more complex in a number of cases.

Most of the freeciv code only copies coordinates around. Some code
create new coordinates (iterators, MAPSTEP) and some code access
tiles. The question is which part of the code has a bigger impact on
the runtime.

> >>>- which iso variants do we support (IMHO both)
> >>
> >>Hmm?  The "variants" you talk about in the file are just a different 
> >>numbering system; there's nothing fundamentally different between them.
> 
> Raimar is starting from scratch with his version and may take some time
> until he comes up to speed on what is relevant and what is not.
> 
> Meanwhile we should try to avoid too much wheel reinvention and try to
> keep the discussion on reviewing things relevant to the proposed patch,
> rather than always getting sidetracked too far into dead ends.

I have abandoned the idea to write my own implementation since it
would similar to Jason gen_topo patch. However this patch has some
problems:
 - missing reasons for certain decisions
 - missing documentation (I wrote this now)
 - some slightly wrong code (native_to_map_pos doesn't return y=0)
 - code (unnormalize_map_pos) that hard to understand. I would say too
 hard.

If these are addressed I'm for appling this patch.

> >>>- in which form is wrapping defined? Do we support multiple wrapping
> >>>forms (iso view form and non-iso view form)?
> >>
> >>I don't know what this means.  Wrapping is defined for both iso and 
> >>non-iso maps, and is identical in both when done in "native coordinates".
> >
> >Ok so the wrapping operations are defnied in the compact
> >form. Question: is wrapping defined in compact form the same as
> >wrapping defined in iso-view form? IMHO yes but I'm not sure.
> 
> The wrapping concept is the same no matter which coordinate you choose
> to use for the implementation. Some coordinate systems are more
> efficient at doing this, i.e. aligned with the wrap axes.

This is about
http://www.freeciv.org/~rfalke/grid_pictures/grid_iso_rot_wrap.png vs
http://www.freeciv.org/~rfalke/grid_pictures/grid_iso_wrap.png vs a
similar picture for
http://www.freeciv.org/~rfalke/grid_pictures/grid_compact.png.

> >>>- how can mapview code done in an easier way (IMHO with an
> >>>intermediate form)
> >>
> >>Again you are lumping a whole collection of unrelated operations into 
> >>one term.
> >
> >This is a bit terse.
> >
> >I'm trying to split unnormalize_map_pos into two operations. First the
> >wrapping and than the rotating and scaling to pixels. What is your
> >opinion?
> 
> Unnormalize_map_pos only does (un)wrapping.
> 
> Rotations and scaling are done in other code.

> I think you need to take another look ...

Quite possible.

        Raimar

-- 
 email: rf13@xxxxxxxxxxxxxxxxx
 "Just because you put a flag on the moon doesn't make it yours, it just
  puts a hole in the moon."