[Freeciv-Dev] Re: (PR#2366) unification of tile_visible_mapcanvas

["rwetmore@xxxxxxxxxxxx via RT" <rt@xxxxxxxxxxxxxx>]

Don't use get_canvas_xy(). It is (or should be deprecated).

Instead take this opportunity to move to a standardized set of coordinate
transformation functions, i.e. use map_to_canvas_pos(...) which in any
event is more descriptive/less ambiguous than get_canvas_xy().

These functions should all (except for index) be of the form ...
  bool <orig>_to_<dest>_pos(&ret_u, &ret_v, <const args>)

They do the 2-D transformation steps required and return a real/unreal
status, i.e. unreal equals failure. In most cases, the return result
will also be properly normalized, but this should not be an absolute
condition, but part of each function's defined interface.

Cheers,
RossW
=====

At 04:18 PM 02/11/19 -0800, Jason Short via RT wrote:
>
>
>The attached patch moves tile_visible_mapcavanvas out of the GUI code 
>and into mapview_common.
>
>It is straightforward, except that I've slipped a topology fix in.  This 
>function is not gen-topology safe in the non-isometric case.  Here 
>Ross's and my fixes are the same: just remove the special-case handling 
>for the non-iso case.  But if two steps at once is too much, it can 
>easily be done without.  (Actually, I tried to do it the other way but 
>gave up in frustration when the code was simply too long to fit well 
>under FreeCiv's formatting guidelines.)
>
>For the observant: I just now notice gui-mui's support functions.  These 
>will come in handy!
>
>jason
>
>? client/gui-gtk/diff
>? client/gui-stub/stub-update.diff
>? client/gui-xaw/diff
>Index: client//mapview_common.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
>retrieving revision 1.15
>diff -u -r1.15 mapview_common.c
>--- client//mapview_common.c   2002/11/19 20:13:38     1.15
>+++ client//mapview_common.c   2002/11/20 00:12:19
>@@ -308,6 +308,16 @@
> }
>
> /**************************************************************************
>+  Return TRUE iff the given map position has a tile visible on the
>+  map canvas.
>+**************************************************************************/
>+bool tile_visible_mapcanvas(int map_x, int map_y)
>+{
>+  int dummy_x, dummy_y; /* well, it needs two pointers... */
>+  return get_canvas_xy(map_x, map_y, &dummy_x, &dummy_y);
>+}
>
>+/**************************************************************************
>  Update (only) the visible part of the map
> **************************************************************************/
> void update_map_canvas_visible(void)
>Index: client//mapview_common.h
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
>retrieving revision 1.10
>diff -u -r1.10 mapview_common.h
>--- client//mapview_common.h   2002/11/19 20:13:38     1.10
>+++ client//mapview_common.h   2002/11/20 00:12:19
>@@ -54,6 +54,8 @@
>                               int map_view_map_width,
>                               int map_view_map_height);
>
>+bool tile_visible_mapcanvas(int map_x, int map_y);
>
> void update_map_canvas_visible(void);
>                               
> struct city *find_city_near_tile(int x, int y);
>Index: client//gui-beos/mapview.cpp
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-beos/mapview.cpp,v
>retrieving revision 1.7
>diff -u -r1.7 mapview.cpp
>--- client//gui-beos/mapview.cpp       2002/11/14 09:14:52     1.7
>+++ client//gui-beos/mapview.cpp       2002/11/20 00:12:19
>@@ -11,14 +11,6 @@
> #include "Defs.hpp"
> #include "mapview.h"
>
>-bool tile_visible_mapcanvas(int x, int y)     // HOOK
>-{
>-      // @@@@ will probably require some direct accesses
>-      NOT_FINISHED( "tile_visible_mapcanvas(int" );
>-      return FALSE;
>-}
>
>
> bool tile_visible_and_not_on_border_mapcanvas(int x, int y)   // HOOK
> {
>       // @@@@ will probably require some direct accesses
>Index: client//gui-gtk/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
>retrieving revision 1.133
>diff -u -r1.133 mapview.c
>--- client//gui-gtk/mapview.c  2002/11/19 20:13:38     1.133
>+++ client//gui-gtk/mapview.c  2002/11/20 00:12:20
>@@ -515,23 +515,6 @@
>   gtk_pixmap_set(GTK_PIXMAP(government_label), gov_sprite->pixmap, NULL);
> }
>
>
>-/**************************************************************************
>-...
>-**************************************************************************/
>-bool tile_visible_mapcanvas(int x, int y)
>-{
>-  if (is_isometric) {
>-    int dummy_x, dummy_y; /* well, it needs two pointers... */
>-    return get_canvas_xy(x, y, &dummy_x, &dummy_y);
>-  } else {
>-    return (y>=map_view_y0 && y<map_view_y0+map_canvas_store_theight &&
>-          ((x>=map_view_x0 && x<map_view_x0+map_canvas_store_twidth) ||
>-           (x+map.xsize>=map_view_x0 && 
>-            x+map.xsize<map_view_x0+map_canvas_store_twidth)));
>-  }
>-}
>
> /**************************************************************************
> ...
> **************************************************************************/
>Index: client//gui-gtk-2.0/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapview.c,v
>retrieving revision 1.21
>diff -u -r1.21 mapview.c
>--- client//gui-gtk-2.0/mapview.c      2002/11/19 20:13:39     1.21
>+++ client//gui-gtk-2.0/mapview.c      2002/11/20 00:12:21
>@@ -517,23 +517,6 @@
>                           gov_sprite->pixmap, NULL);
> }
>
>
>-/**************************************************************************
>-...
>-**************************************************************************/
>-bool tile_visible_mapcanvas(int x, int y)
>-{
>-  if (is_isometric) {
>-    int dummy_x, dummy_y; /* well, it needs two pointers... */
>-    return get_canvas_xy(x, y, &dummy_x, &dummy_y);
>-  } else {
>-    return (y>=map_view_y0 && y<map_view_y0+map_canvas_store_theight &&
>-          ((x>=map_view_x0 && x<map_view_x0+map_canvas_store_twidth) ||
>-           (x+map.xsize>=map_view_x0 && 
>-            x+map.xsize<map_view_x0+map_canvas_store_twidth)));
>-  }
>-}
>
> /**************************************************************************
> ...
> **************************************************************************/
>Index: client//gui-mui/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapview.c,v
>retrieving revision 1.47
>diff -u -r1.47 mapview.c
>--- client//gui-mui/mapview.c  2002/11/19 20:13:39     1.47
>+++ client//gui-mui/mapview.c  2002/11/20 00:12:21
>@@ -365,27 +365,6 @@
> /**************************************************************************
>  GUI Independ (with new access functions)
> **************************************************************************/
>-bool tile_visible_mapcanvas(int x, int y)
>-{
>-  if (is_isometric) {
>-    int dummy_x, dummy_y; /* well, it needs two pointers... */
>-    return get_canvas_xy(x, y, &dummy_x, &dummy_y);
>-  } else {
>-    int map_view_x0 = get_map_x_start();
>-    int map_view_y0 = get_map_y_start();
>-    int map_canvas_store_twidth = get_map_x_visible();
>-    int map_canvas_store_theight = get_map_y_visible();
>
>-    return (y>=map_view_y0 && y<map_view_y0+map_canvas_store_theight &&
>-          ((x>=map_view_x0 && x<map_view_x0+map_canvas_store_twidth) ||
>-           (x+map.xsize>=map_view_x0 && 
>-            x+map.xsize<map_view_x0+map_canvas_store_twidth)));
>-  }
>-}
>
>-/**************************************************************************
>- GUI Independ (with new access functions)
>-**************************************************************************/
> bool tile_visible_and_not_on_border_mapcanvas(int x, int y)
> {
>   if (is_isometric) {
>Index: client//gui-stub/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-stub/mapview.c,v
>retrieving revision 1.17
>diff -u -r1.17 mapview.c
>--- client//gui-stub/mapview.c 2002/11/19 20:13:39     1.17
>+++ client//gui-stub/mapview.c 2002/11/20 00:12:21
>@@ -22,12 +22,6 @@
>   *map_view_pixel_height = canvas_height;
> }
>
>-bool tile_visible_mapcanvas(int x, int y)
>-{
>-      /* PORTME */
>-      return FALSE;
>-}
>
> bool tile_visible_and_not_on_border_mapcanvas(int x, int y)
> {
>       /* PORTME */
>Index: client//gui-win32/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
>retrieving revision 1.36
>diff -u -r1.36 mapview.c
>--- client//gui-win32/mapview.c        2002/11/19 20:13:39     1.36
>+++ client//gui-win32/mapview.c        2002/11/20 00:12:22
>@@ -454,28 +454,6 @@
>   return sprites.citizen[frame];
> }           
>
>
>-/**************************************************************************
>
>-**************************************************************************/
>-bool
>-tile_visible_mapcanvas(int x, int y)
>-{
>-  if (is_isometric)
>-    {
>-      int dummy_x, dummy_y;
>-      return get_canvas_xy(x,y,&dummy_x,&dummy_y);
>-    }
>-  else
>-    {
>-      return (y>=map_view_y && y<map_view_y+map_view_height &&
>-            ((x>=map_view_x && x<map_view_x+map_view_width) ||
>-             (x+map.xsize>=map_view_x &&
>-              x+map.xsize<map_view_x+map_view_width)));   
>-    }    
>-  
>-}
>
> /**************************************************************************
>
> **************************************************************************/
>Index: client//gui-xaw/mapview.c
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
>retrieving revision 1.106
>diff -u -r1.106 mapview.c
>--- client//gui-xaw/mapview.c  2002/11/19 20:13:39     1.106
>+++ client//gui-xaw/mapview.c  2002/11/20 00:12:23
>@@ -345,17 +345,6 @@
> /**************************************************************************
> ...
> **************************************************************************/
>-bool tile_visible_mapcanvas(int x, int y)
>-{
>-  return (y>=map_view_y0 && y<map_view_y0+map_canvas_store_theight &&
>-        ((x>=map_view_x0 && x<map_view_x0+map_canvas_store_twidth) ||
>-         (x+map.xsize>=map_view_x0 && 
>-          x+map.xsize<map_view_x0+map_canvas_store_twidth)));
>-}
>
>-/**************************************************************************
>-...
>-**************************************************************************/
> bool tile_visible_and_not_on_border_mapcanvas(int x, int y)
> {
>   return ((y>=map_view_y0+2 || (y >= map_view_y0 && map_view_y0 == 0))
>Index: client//include/mapview_g.h
>===================================================================
>RCS file: /home/freeciv/CVS/freeciv/client/include/mapview_g.h,v
>retrieving revision 1.23
>diff -u -r1.23 mapview_g.h
>--- client//include/mapview_g.h        2002/11/19 20:13:39     1.23
>+++ client//include/mapview_g.h        2002/11/20 00:12:23
>@@ -24,7 +24,6 @@
>                           int *map_view_pixel_width,
>                           int *map_view_pixel_height);
>
>-bool tile_visible_mapcanvas(int x, int y);
> bool tile_visible_and_not_on_border_mapcanvas(int x, int y);
>
> void update_info_label(void)
>