[Freeciv-Dev] (PR#2567) get_citizen_pixmap is bad

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

[rfalke - Fri Dec 13 18:49:06 2002]:

> get_citizen_pixmap is bad because it assumes that the sprite has no
> mask. It also doesn't test this assumption.

The attached patch removes the use of get_citizen_pixmap for gui-gtk.

jason

Index: client/gui-gtk/gui_main.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/gui_main.c,v
retrieving revision 1.124
diff -u -r1.124 gui_main.c
--- client/gui-gtk/gui_main.c   2003/01/01 11:51:33     1.124
+++ client/gui-gtk/gui_main.c   2003/01/04 06:55:39
@@ -650,7 +650,8 @@
     gtk_signal_connect(GTK_OBJECT(ebox), "button_press_event",
                        GTK_SIGNAL_FUNC(taxrates_callback), GINT_TO_POINTER(i));
 
-    econ_label[i] = gtk_pixmap_new(get_citizen_pixmap(c, i, NULL), NULL);
+    econ_label[i] = gtk_pixmap_new(get_citizen_sprite(c, i, NULL)->pixmap,
+                                  NULL);
     gtk_pixmap_set_build_insensitive(GTK_PIXMAP(econ_label[i]), FALSE);
     gtk_container_add(GTK_CONTAINER(ebox), econ_label[i]);
   }
@@ -666,7 +667,8 @@
   gtk_pixmap_set_build_insensitive(GTK_PIXMAP(flake_label), FALSE);
 
   government_label
-    = gtk_pixmap_new(get_citizen_pixmap(CITIZEN_UNHAPPY, 0, NULL), NULL);
+    = gtk_pixmap_new(get_citizen_sprite(CITIZEN_UNHAPPY, 0, NULL)->pixmap,
+                    NULL);
   gtk_pixmap_set_build_insensitive(GTK_PIXMAP(government_label), FALSE);
 
   timeout_label = gtk_label_new("");
Index: client/gui-gtk/happiness.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/happiness.c,v
retrieving revision 1.11
diff -u -r1.11 happiness.c
--- client/gui-gtk/happiness.c  2002/12/15 22:43:47     1.11
+++ client/gui-gtk/happiness.c  2003/01/04 06:55:39
@@ -171,10 +171,9 @@
   get_city_citizen_types(pcity, index, citizens);
 
   for (i = 0; i < num_citizens; i++) {
-    gdk_draw_pixmap(happiness_pixmap, civ_gc,
-                   get_citizen_pixmap(citizens[i], i, pcity),
-                   0, 0, i * offset, 0, SMALL_TILE_WIDTH,
-                   SMALL_TILE_HEIGHT);
+    pixmap_put_sprite_full(happiness_pixmap,
+                          i * offset, 0,
+                          get_citizen_sprite(citizens[i], i, pcity));
   }
 
   return happiness_pixmap;
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.146
diff -u -r1.146 mapview.c
--- client/gui-gtk/mapview.c    2003/01/01 11:51:33     1.146
+++ client/gui-gtk/mapview.c    2003/01/04 06:55:39
@@ -355,17 +355,21 @@
                      game.player_ptr->government);
 
   d=0;
-  for(;d<(game.player_ptr->economic.luxury)/10;d++)
-    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]),
-                  get_citizen_pixmap(CITIZEN_ELVIS, d, NULL), NULL);
+  for (; d < game.player_ptr->economic.luxury /10; d++) {
+    struct Sprite *sprite = get_citizen_sprite(CITIZEN_ELVIS, d, NULL);
+    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]), sprite->pixmap, sprite->mask);
+  }
  
-  
for(;d<(game.player_ptr->economic.science+game.player_ptr->economic.luxury)/10;d++)
-    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]),
-                  get_citizen_pixmap(CITIZEN_SCIENTIST, d, NULL), NULL);
+  for (; d < (game.player_ptr->economic.science
+            + game.player_ptr->economic.luxury) / 10; d++) {
+    struct Sprite *sprite = get_citizen_sprite(CITIZEN_SCIENTIST, d, NULL);
+    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]), sprite->pixmap, sprite->mask);
+  }
  
-   for(;d<10;d++)
-    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]),
-                  get_citizen_pixmap(CITIZEN_TAXMAN, d, NULL), NULL);
+  for (; d < 10; d++) {
+    struct Sprite *sprite = get_citizen_sprite(CITIZEN_TAXMAN, d, NULL);
+    gtk_pixmap_set(GTK_PIXMAP(econ_label[d]), sprite->pixmap, sprite->mask);
+  }
  
   update_timeout_label();
 }
@@ -443,15 +447,6 @@
 }
 
 /**************************************************************************
-  Access wrapper for get_citizen_sprite.
-**************************************************************************/
-GdkPixmap *get_citizen_pixmap(enum citizen_type type, int citizen_index,
-                             struct city *pcity)
-{
-  return get_citizen_sprite(type, citizen_index, pcity)->pixmap;
-}
-
-/**************************************************************************
 ...
 **************************************************************************/
 void set_indicator_icons(int bulb, int sol, int flake, int gov)
@@ -1311,6 +1306,40 @@
 }
 
 /**************************************************************************
+  Place part of a (possibly masked) sprite on a pixmap.
+**************************************************************************/
+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)
+{
+  if (ssprite->mask) {
+    gdk_gc_set_clip_origin(civ_gc, pixmap_x, pixmap_y);
+    gdk_gc_set_clip_mask(civ_gc, ssprite->mask);
+  }
+
+  gdk_draw_pixmap(pixmap, civ_gc, ssprite->pixmap,
+                 offset_x, offset_y,
+                 pixmap_x + offset_x, pixmap_y + offset_y,
+                 MIN(width, MAX(0, ssprite->width - offset_x)),
+                 MIN(height, MAX(0, ssprite->height - offset_y)));
+
+  gdk_gc_set_clip_mask(civ_gc, NULL);
+}
+
+/**************************************************************************
+  Place a (possibly masked) sprite on a pixmap.
+**************************************************************************/
+void pixmap_put_sprite_full(GdkDrawable *pixmap,
+                           int pixmap_x, int pixmap_y,
+                           struct Sprite *ssprite)
+{
+  pixmap_put_sprite(pixmap, pixmap_x, pixmap_y, ssprite,
+                   0, 0, ssprite->width, ssprite->height);
+}
+
+/**************************************************************************
 Only used for isometric view.
 **************************************************************************/
 static void pixmap_put_overlay_tile_draw(GdkDrawable *pixmap,
@@ -1322,16 +1351,9 @@
 {
   if (!ssprite || !width || !height)
     return;
-
-  gdk_gc_set_clip_origin(civ_gc, canvas_x, canvas_y);
-  gdk_gc_set_clip_mask(civ_gc, ssprite->mask);
 
-  gdk_draw_pixmap(pixmap, civ_gc, ssprite->pixmap,
-                 offset_x, offset_y,
-                 canvas_x+offset_x, canvas_y+offset_y,
-                 MIN(width, MAX(0, ssprite->width-offset_x)),
-                 MIN(height, MAX(0, ssprite->height-offset_y)));
-  gdk_gc_set_clip_mask(civ_gc, NULL);
+  pixmap_put_sprite(pixmap, canvas_x, canvas_y, ssprite,
+                   offset_x, offset_y, width, height);
 
   /* I imagine this could be done more efficiently. Some pixels We first
      draw from the sprite, and then draw black afterwards. It would be much
Index: client/gui-gtk/mapview.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.h,v
retrieving revision 1.15
diff -u -r1.15 mapview.h
--- client/gui-gtk/mapview.h    2002/12/13 19:15:12     1.15
+++ client/gui-gtk/mapview.h    2003/01/04 06:55:39
@@ -27,8 +27,6 @@
 struct city;
 
 GdkPixmap *get_thumb_pixmap(int onoff);
-GdkPixmap *get_citizen_pixmap(enum citizen_type type, int citizen_index,
-                             struct city *pcity);
 
 gint overview_canvas_expose(GtkWidget *w, GdkEventExpose *ev);
 gint map_canvas_expose(GtkWidget *w, GdkEventExpose *ev);
@@ -46,6 +44,10 @@
                     int canvas_x, int canvas_y, int citymode);
 void pixmap_put_black_tile(GdkDrawable *pm,
                           int canvas_x, int canvas_y);
+
+void pixmap_put_sprite_full(GdkDrawable *pixmap,
+                           int pixmap_x, int pixmap_y,
+                           struct Sprite *ssprite);
 
 void scrollbar_jump_callback(GtkAdjustment *adj, gpointer hscrollbar);
 void update_map_canvas_scrollbars_size(void);