I want to separate gui_coord from koord.

[neroden]

I want to make a separate type for screen coordinates and switch the GUI over to using it.

This has inherent type-safety benefits, and readability benefits, as screen coordinates and map coordinates will become different.  

It also has other benefits:
- gui_coord will be simpler than koord
- gui_coord can have functions added to it to simplify the gui code.
- The integer conversion problems of gui_coord and koord will be *separate* and can be dealt with *separately* rather than in one huge mess.
- The size of the coordinates for gui_coord and koord can be changed independently (gui_coord might get larger for people with really large screens, koord is unlikely to get larger unless people have really huge maps, which are too slow to play right now).
- It might allow for the map rotation code to be simplified.

It has one downside:
- It's going to be a *large* patch due to replacing koord with gui_coord in most of the gui directory (apart from where it's referring to the map, obviously).

I would really like to make this change, as it would make the code a lot cleaner.  

Would it be accepted?  

And is there anything else I should do while I'm doing this?  (For instance, I have to change every declaration of "zeichnen" -- I could translate it to "display", or "draw".  I'm also changing practically every line with set_groesse or get_groesse in it -- I could change those to set_size and get_size.)

[prissi]

The problem is, that current dialoge were not made with larger screens (and fonts) in mind. When renovating the screen coordinate system, It would make sense that dialogues would use a font relative system of displaying. (This might not affect your patch, but should kept in mind.)

zeichnen->display => why not, for dings_t it is already display. That is one easy search and replace.

About what functions you are thinking? The mapping of to_display_coords( koord3d pos, koord offset_in_tile ) ?

[neroden]

The problem is, that current dialoge were not made with larger screens (and fonts) in mind. When renovating the screen coordinate system, It would make sense that dialogues would use a font relative system of displaying. (This might not affect your patch, but should kept in mind.)

This should not affect my patch, but I will keep it in mind.  :-)

zeichnen->display => why not, for dings_t it is already display. That is one easy search and replace.

About what functions you are thinking? The mapping of to_display_coords( koord3d pos, koord offset_in_tile ) ?

Yes, the mapping from koord<->gui_coord could be made into a method.  I was thinking also that some of the clipping and some of the proportional layout code might turn out to have common computations, though I can't tell whether that is the case yet.

[neroden]

It's a very large patch.  It applies cleanly against standard as of now... and it works.  No behavior change.

http://files.[ simutrans [dot] us (site down, do not visit) ]/files/get/L-WDKQeChX/gui-coord.zip

(EDIT: New version of the patch adapted to the latest changes in standard as of this evening:
http://files.[ simutrans [dot] us (site down, do not visit) ]/files/get/tVyygS_YgG/gui-coord-2.diff )

This was the painful part.  Well, actually, merging it to experimental will be painful too.  :-)  I hope it can go in relatively quickly because keeping it up to date with changes in standard will be painful...

Anyway, after this is done I'm going to try to do the further cleanup I was talking about, but that should be less painful.  And this should be an improvement in readability in itself.

The karte_to_screen and screen_to_karte routines had to have their interface rewritten, since they're now converting between two different data types.  They're still inline.

EDIT: To be clear on what's going on here, koord is now supposed to be *only* for map coords, which have the 45 degree rotation in the 'iso view' (well, with a couple of weird exceptions like the case where it's used for map coords *and* different data is overloaded in it when x = -1), and gui_coord is used for all pixel coords and similar things which aren't rotated 45 degrees.  I'm going to try to clean up the conversions further after this but this helped disentangle things a *lot*.

Zeichnen was also renamed "display", as proposed.

[prissi]

Some remarks:
gui_coord is not a good choice, since it is rather drawing coordinates, draw_koord. (To indicate close relationship to koord, but this can be done by search and replace easily ... ). As such they should not reside in gui/ but maybe in dataobj.cc; also all routines in simding.cc (and also other dings) simview.cc and all other drawing stuff should use those coordinates then too.

That the offset is not an sint16 might be a problem with some scrollbars. Did you test this with very large savegames?

[neroden]

Some remarks:
gui_coord is not a good choice, since it is rather drawing coordinates, draw_koord. (To indicate close relationship to koord, but this can be done by search and replace easily ... ). As such they should not reside in gui/ but maybe in dataobj.cc;

Those are easy enough changes to make.  I'd still rather have it be draw_coord, in the spirit of translating to English -- eventually koord should be something like map_coord too -- but I could leave the k if you really want me to.

also all routines in simding.cc (and also other dings) simview.cc and all other drawing stuff should use those coordinates then too.

That's the right thing to do, but it's going to make the patch even *bigger* and harder to maintain.... I could do it in a second pass or I could just go ahead and make the patch enormous.

(EDIT: oddly most of these routines only use koord for map coordinates, and use 'plain' sint16 for the display coordinates. So there's nothing much to do to make them not use koord for drawing coordinates.  I'll try to clean that up later...)

That the offset is not an sint16 might be a problem with some scrollbars. Did you test this with very large savegames?

The offset is not an sint16?  Did I make an accidental change here?.... I thought it was still an sint16...  EDIT: it still seems to be an sint16?...

EDIT: Here is the version of the patch with draw_coord (not gui_coord) and the relocation of the new files to dataobj/ ; it's also updated to patch the current version:  http://files.[ simutrans [dot] us (site down, do not visit) ]/files/get/Xkit2XDRIn/gui-coord-3.diff

[prissi]

Anyway, this have to wait since after 26, since I will be on holyday and have not enough time to integrate or test any such large patch. Not to mention that on my netbook building simutrans takes two hours.

[TurfIt]

Anybody have a copy of this patch?

[neroden]

I do, TurfIt.  But it has to be rebuilt due to changes.

[prissi]

In the mean time, all new code uses KOORD_VAL for size for X and Y variables; since this is the code which is passed to display_xxx routines.

[Dwachs]

something closely related is now in r6911