[Freeciv-Dev] (PR#2941) RFC: canvas_put_sprite

["Jason Short via RT" <rt@xxxxxxxxxxxxxx>]

The client cleanups have passed a crucial point.  All of the 
mapview-specific drawing code has been moved into mapview_common.

The next goal is to unify pixmap_put_tile() and pixmap_put_tile_iso(). 
These are the core tile drawing functions, and they work for both the 
mapview and the city dialogs.  Once they are unified (along with the 
bulk of city_dialog_update_map(), aka update_citydlg_canvas), then we 
will have a common base of code onto which Daniel, Rafal, and myself can 
make changes without having to make them in six very different places. 
In other words, then we can make some serious progress with Daniel's 
"super-secret" new rendering code, Rafal's ideas for making cool use of 
blending masks, and my vague idea of supporting civ3 tilesets.

The idea that the drawing code should be provided independently by each 
GUI does have some attractions.  But ultimately, since the drawing code 
is tied to so many other parts of the code (tilespec, tileset, etc.) 
doing it this way forms a huge mass of inertia against which it is 
difficult to introduce any new features.  Thue did it before by 
completely forking the code into iso and non-iso variants.  But to 
introduce further variants *without* exponential quantities of code 
means putting the pieces back together.

So at this point, we are hitting the lowest level of GUI functions.  A 
variety of these functions will be needed (one for each drawing 
operation).  To keep patches simple, I'd like to start with introducing 
them one at a time.  So at this point we need to make a naming decision.

Each function will have to do a drawing operation onto a canvas.  The 
canvas may be the map canvas, or it may be a city dialog canvas.  In 
theory, in future other canvases may also be used - although whether we 
want to design for extensibility or not is questionable.

So take the basic function that draws a portion of a sprite onto the map 
canvas.  There are three options that I can see for naming (others may 
be possible):

1.  Introduce two variants, citydlg_put_sprite() and 
mapcanvas_put_sprite() (or cityview_put_sprite() and 
mapview_put_sprite()).  Each just draws that portion of the sprite onto 
the proper buffer.  citydlg_put_sprite would, obviously, have to take an 
additional parameter (the struct city_dialog).  The problem here is that 
both functions will contain nearly identical code; in fact they may end 
up both being wrappers for pixmap_put_sprite.  And we would probably 
want to use a wrapper at the calling end to split the calls up - i.e. 
put_sprite() calls mapcanvas_put_sprite() which calls 
pixmap_put_sprite() - which is a bit excessive.

2.  Introduce a single function, gui_put_sprite(), that takes a struct 
city_dialog *.  Now this function will draw to the mapview if this 
pointer is NULL or draw to a city dialog window if such is specified. 
The problem here is that this is not extensible to additional canvases; 
for instance if Rafal wants to buffer the mapview separately in 
different situations he will find it very difficult.

3.  Introduce a single function, canvas_put_sprite(), that takes a void* 
(or a struct canvas *) as a parameter.  Now the function just draws the 
sprite onto the canvas that is specified.  In the case of the void*, 
this will probably just point to the pixmap/surface onto which the 
sprite will be drawn.  The problem here is that since the high-level 
mapview drawing routines are only designed for one mapview, we would 
need to introduce the hack that NULL refers to the "default" map canvas. 
  Later we can work to have all of these functions be passed (by the 
GUI) the parameter that specifies which map canvas.  (The city dialog 
functions will always be called by the GUI, so there is no trouble there 
for any of these 3 ideas.)  Using a struct canvas * instead means an 
additional level of indirection, but is slightly more type-safe (the 
NULL special case would still apply).  We might even be able to use a 
struct Sprite * that specifies the drawing target (again, with a NULL 
special-case).


For a long time I was planning to use idea #1 for this.  After some 
rethinking (and some discussions with Mike) I came up with #2.  Then 
while implementing it it occurred to me that idea #3 might be best.

I'd really like some input here.  For reference, here is an 
implementation of idea #2 that implements this only for the put_sprite 
case (remember that there will be a number of these functions).

The patch removes gui_map_put_black_tile_iso() (a rather over-specific 
function) and simply calls gui_put_sprite() instead.  Whatever idea we 
go with above, this small substitution seems like the simplest way to 
proceed.

jason

Index: client/citydlg_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/citydlg_common.h,v
retrieving revision 1.9
diff -u -r1.9 citydlg_common.h
--- client/citydlg_common.h     2003/01/29 05:00:04     1.9
+++ client/citydlg_common.h     2003/01/30 06:02:02
@@ -19,6 +19,7 @@
 #include "shared.h"            /* bool type */
 
 struct city;
+struct city_dialog;
 
 enum citizen_type {
   CITIZEN_ELVIS,
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.26
diff -u -r1.26 mapview_common.c
--- client/mapview_common.c     2003/01/29 22:57:45     1.26
+++ client/mapview_common.c     2003/01/30 06:02:02
@@ -450,9 +450,9 @@
                           width, height, height_unit,
                           draw);
     } else {
-      gui_map_put_black_tile_iso(canvas_x, canvas_y,
-                                offset_x, offset_y,
-                                width, height);
+      gui_put_sprite(NULL, canvas_x, canvas_y,
+                    sprites.black_tile,
+                    offset_x, offset_y, width, height);
     }
   }
 }
Index: client/gui-gtk/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.c,v
retrieving revision 1.153
diff -u -r1.153 citydlg.c
--- client/gui-gtk/citydlg.c    2003/01/29 05:00:04     1.153
+++ client/gui-gtk/citydlg.c    2003/01/30 06:02:04
@@ -58,8 +58,6 @@
 
 #include "citydlg.h"
 
-struct city_dialog;
-
 /* get 'struct dialog_list' and related functions: */
 #define SPECLIST_TAG dialog
 #define SPECLIST_TYPE struct city_dialog
@@ -380,6 +378,14 @@
        player has to close and reopen the dialog to fix this. */
     city_dialog_update_map(pdialog);
   } dialog_list_iterate_end;
+}
+
+/**************************************************************************
+  Return a pixmap for the dialog.
+**************************************************************************/
+GdkDrawable *get_citydlg_pixmap(struct city_dialog *pdialog)
+{
+  return pdialog->map_canvas_store;
 }
 
 /****************************************************************
Index: client/gui-gtk/citydlg.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/citydlg.h,v
retrieving revision 1.8
diff -u -r1.8 citydlg.h
--- client/gui-gtk/citydlg.h    2002/11/17 02:21:12     1.8
+++ client/gui-gtk/citydlg.h    2003/01/30 06:02:04
@@ -17,4 +17,6 @@
 
 void reset_city_dialogs(void);
 
+GdkDrawable *get_citydlg_pixmap(struct city_dialog *pdialog);
+
 #endif                         /* FC__CITYDLG_H */
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.152
diff -u -r1.152 mapview.c
--- client/gui-gtk/mapview.c    2003/01/29 22:57:45     1.152
+++ client/gui-gtk/mapview.c    2003/01/30 06:02:05
@@ -85,6 +85,11 @@
                                      int canvas_x, int canvas_y,
                                      int offset_x, int offset_y,
                                      int width, int height);
+static void pixmap_put_sprite(GdkDrawable *pixmap,
+                             int pixmap_x, int pixmap_y,
+                             struct Sprite *ssprite,
+                             int offset_x, int offset_y,
+                             int width, int height);
 
 /* the intro picture is held in this pixmap, which is scaled to
    the screen size */
@@ -903,15 +908,20 @@
 }
 
 /**************************************************************************
-  Draw some or all of a black tile onto the mapview canvas.
+  Draw some or all of a black tile onto either the mapview canvas or
+  city dialog canvas.
 **************************************************************************/
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height)
-{
-  pixmap_put_black_tile_iso(map_canvas_store, canvas_x, canvas_y,
-                           offset_x, offset_y,
-                           width, height);     
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height)
+{
+  pixmap_put_sprite(dialog ? get_citydlg_pixmap(dialog) : map_canvas_store,
+                   canvas_x, canvas_y,
+                   sprite,
+                   offset_x, offset_y,
+                   width, height);
 }
 
 /**************************************************************************
Index: client/gui-gtk-2.0/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/citydlg.c,v
retrieving revision 1.25
diff -u -r1.25 citydlg.c
--- client/gui-gtk-2.0/citydlg.c        2003/01/29 05:00:04     1.25
+++ client/gui-gtk-2.0/citydlg.c        2003/01/30 06:02:07
@@ -57,8 +57,6 @@
 
 #include "citydlg.h"
 
-struct city_dialog;
-
 /* get 'struct dialog_list' and related functions: */
 #define SPECLIST_TAG dialog
 #define SPECLIST_TYPE struct city_dialog
@@ -373,6 +371,14 @@
   } dialog_list_iterate_end;
 
   popdown_all_city_dialogs();
+}
+
+/**************************************************************************
+  Return a pixmap for the dialog.
+**************************************************************************/
+GdkDrawable *get_citydlg_pixmap(struct city_dialog *pdialog)
+{
+  return pdialog->map_canvas_store;
 }
 
 /****************************************************************
Index: client/gui-gtk-2.0/citydlg.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/citydlg.h,v
retrieving revision 1.2
diff -u -r1.2 citydlg.h
--- client/gui-gtk-2.0/citydlg.h        2002/11/30 20:06:49     1.2
+++ client/gui-gtk-2.0/citydlg.h        2003/01/30 06:02:07
@@ -17,4 +17,6 @@
 
 void reset_city_dialogs(void);
 
+GdkDrawable *get_citydlg_pixmap(struct city_dialog *pdialog);
+
 #endif                         /* FC__CITYDLG_H */
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.40
diff -u -r1.40 mapview.c
--- client/gui-gtk-2.0/mapview.c        2003/01/29 22:57:45     1.40
+++ client/gui-gtk-2.0/mapview.c        2003/01/30 06:02:07
@@ -85,6 +85,11 @@
                                      int canvas_x, int canvas_y,
                                      int offset_x, int offset_y,
                                      int width, int height);
+static void pixmap_put_sprite(GdkDrawable *pixmap,
+                             int pixmap_x, int pixmap_y,
+                             struct Sprite *ssprite,
+                             int offset_x, int offset_y,
+                             int width, int height);
 
 /* the intro picture is held in this pixmap, which is scaled to
    the screen size */
@@ -932,15 +937,20 @@
 }
 
 /**************************************************************************
-  Draw some or all of a black tile onto the mapview canvas.
+  Draw some or all of a black tile onto either the mapview canvas or
+  city dialog canvas.
 **************************************************************************/
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height)
-{
-  pixmap_put_black_tile_iso(map_canvas_store, canvas_x, canvas_y,
-                           offset_x, offset_y,
-                           width, height);     
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height)
+{
+  pixmap_put_sprite(dialog ? get_citydlg_pixmap(dialog) : map_canvas_store,
+                   canvas_x, canvas_y,
+                   sprite,
+                   offset_x, offset_y,
+                   width, height);
 }
 
 /**************************************************************************
Index: client/gui-sdl/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-sdl/citydlg.c,v
retrieving revision 1.6
diff -u -r1.6 citydlg.c
--- client/gui-sdl/citydlg.c    2003/01/24 00:51:02     1.6
+++ client/gui-sdl/citydlg.c    2003/01/30 06:02:09
@@ -77,8 +77,6 @@
 #include "citydlg.h"
 
 #if 0
-/* get 'struct dialog_list' and related functions: */
-struct city_dialog;
 
 #define SPECLIST_TAG dialog
 #define SPECLIST_TYPE struct city_dialog
Index: client/gui-sdl/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-sdl/mapview.c,v
retrieving revision 1.14
diff -u -r1.14 mapview.c
--- client/gui-sdl/mapview.c    2003/01/30 02:00:24     1.14
+++ client/gui-sdl/mapview.c    2003/01/30 06:02:10
@@ -199,13 +199,16 @@
 }
 
 /**************************************************************************
-  Draw some or all of a black tile onto the mapview canvas.
+  Draw some or all of a sprite onto the mapview or citydialog canvas.
 **************************************************************************/
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height)
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height)
 {
-  blit_entire_src((SDL_Surface *)sprites.black_tile,
+  assert(dialog == NULL);
+  blit_entire_src((SDL_Surface *)sprite,
                  Main.screen, canvas_x, canvas_y);
 }
 
Index: client/gui-win32/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
retrieving revision 1.53
diff -u -r1.53 mapview.c
--- client/gui-win32/mapview.c  2003/01/29 22:57:46     1.53
+++ client/gui-win32/mapview.c  2003/01/30 06:02:11
@@ -1351,22 +1351,25 @@
 }
 
 /**************************************************************************
-  Draw some or all of a black tile onto the mapview canvas.
+  Draw some or all of a sprite onto the mapview/cityview canvas.
 **************************************************************************/
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height)
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height)
 {
   HDC hdc;
   HBITMAP old;
 
   /* FIXME: we don't want to have to recreate the hdc each time! */
+  assert(dialog == NULL);
   hdc = CreateCompatibleDC(NULL);
   old = SelectObject(hdc, mapstorebitmap);
 
-  pixmap_put_black_tile_iso(hdc, canvas_x, canvas_y,
-                           offset_x, offset_y,
-                           width, height);
+  pixmap_put_overlay_tile_draw(hdc, canvas_x, canvas_y,
+                              sprite, offset_x, offset_y,
+                              width, height, 0);
 
   SelectObject(hdc, old);
   DeleteDC(hdc);
Index: client/gui-xaw/citydlg.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/citydlg.c,v
retrieving revision 1.89
diff -u -r1.89 citydlg.c
--- client/gui-xaw/citydlg.c    2003/01/29 05:00:05     1.89
+++ client/gui-xaw/citydlg.c    2003/01/30 06:02:12
@@ -315,6 +315,14 @@
   }
 }
 
+/**************************************************************************
+  Return a pixmap for the dialog.
+**************************************************************************/
+Pixmap get_citydlg_pixmap(struct city_dialog *pdialog)
+{
+  return XtWindow(pdialog->map_canvas);
+}
+
 /****************************************************************
 ...
 *****************************************************************/
Index: client/gui-xaw/citydlg.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/citydlg.h,v
retrieving revision 1.6
diff -u -r1.6 citydlg.h
--- client/gui-xaw/citydlg.h    2000/03/21 18:02:13     1.6
+++ client/gui-xaw/citydlg.h    2003/01/30 06:02:12
@@ -23,5 +23,6 @@
 void citydlg_key_close(Widget w);
 void citydlg_msg_close(Widget w);
 
+Pixmap get_citydlg_pixmap(struct city_dialog *pdialog);
 
 #endif  /* FC__CITYDLG_H */
Index: client/gui-xaw/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
retrieving revision 1.119
diff -u -r1.119 mapview.c
--- client/gui-xaw/mapview.c    2003/01/29 22:57:46     1.119
+++ client/gui-xaw/mapview.c    2003/01/30 06:02:12
@@ -36,6 +36,7 @@
 #include "timing.h"
 #include "unit.h"
 
+#include "citydlg.h"
 #include "civclient.h"
 #include "climap.h"
 #include "climisc.h"
@@ -621,14 +622,45 @@
 }
 
 /**************************************************************************
-  Draw some or all of a black tile onto the mapview canvas.
+  Draw a single masked sprite to the pixmap.
 **************************************************************************/
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height)
+static void pixmap_put_sprite(Pixmap pixmap,
+                             int canvas_x, int canvas_y,
+                             struct Sprite *sprite,
+                             int offset_x, int offset_y,
+                             int width, int height)
+{
+  if (sprite->mask) {
+    XSetClipOrigin(display, civ_gc, canvas_x, canvas_y);
+    XSetClipMask(display, civ_gc, sprite->mask);
+  }
+
+  XCopyArea(display, sprite->pixmap, pixmap, 
+           civ_gc,
+           offset_x, offset_y,
+           width, height, 
+           canvas_x, canvas_y);
+
+  if (sprite->mask) {
+    XSetClipMask(display, civ_gc, None);
+  }
+}
+
+/**************************************************************************
+  Draw some or all of a black tile onto either the mapview canvas or
+  city dialog canvas.
+**************************************************************************/
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height)
 {
-  /* PORTME */
-  assert(0);
+  pixmap_put_sprite(dialog ? get_citydlg_pixmap(dialog) : map_canvas_store,
+                   canvas_x, canvas_y,
+                   sprite,
+                   offset_x, offset_y,
+                   width, height);
 }
 
 /**************************************************************************
@@ -990,15 +1022,9 @@
                                    struct Sprite *ssprite)
 {
   if (!ssprite) return;
-      
-  XSetClipOrigin(display, civ_gc, canvas_x, canvas_y);
-  XSetClipMask(display, civ_gc, ssprite->mask);
-      
-  XCopyArea(display, ssprite->pixmap, pixmap, 
-           civ_gc, 0, 0,
-           ssprite->width, ssprite->height, 
-           canvas_x, canvas_y);
-  XSetClipMask(display, civ_gc, None); 
+
+  pixmap_put_sprite(pixmap, canvas_x, canvas_y,
+                   ssprite, 0, 0, ssprite->width, ssprite->height);
 }
 
 /**************************************************************************
Index: client/include/mapview_g.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/include/mapview_g.h,v
retrieving revision 1.30
diff -u -r1.30 mapview_g.h
--- client/include/mapview_g.h  2003/01/29 22:57:46     1.30
+++ client/include/mapview_g.h  2003/01/30 06:02:13
@@ -13,9 +13,11 @@
 #ifndef FC__MAPVIEW_G_H
 #define FC__MAPVIEW_G_H
 
-#include "mapview_common.h"
 #include "shared.h"            /* bool type */
 
+#include "citydlg_common.h"
+#include "mapview_common.h"
+
 struct unit;
 struct city;
 
@@ -44,9 +46,11 @@
                          int offset_x, int offset_y, int offset_y_unit,
                          int width, int height, int height_unit,
                          enum draw_type draw);
-void gui_map_put_black_tile_iso(int canvas_x, int canvas_y,
-                               int offset_x, int offset_y,
-                               int width, int height);
+void gui_put_sprite(struct city_dialog *dialog,
+                   int canvas_x, int canvas_y,
+                   struct Sprite *sprite,
+                   int offset_x, int offset_y,
+                   int width, int height);
 void flush_mapcanvas(int canvas_x, int canvas_y,
                     int pixel_width, int pixel_height);