[Freeciv-Dev] Re: (PR#8863) suckiness of "citymap" images on mapview

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://rt.freeciv.org/Ticket/Display.html?id=8863 >

Here's an updated patch.  gtk2, gtk, xaw, and win32 clients are updated. 
  I also changed the internal design a bit:

- There is an array holding the list of possible colors.

- COLOR_STD_LAST is used for transparency.  This is the default, of course.

- pcity->client.color is an index into this array.

- In toggle_city_color we just increment/wrap the color index.

The only ugliness is that the array is a static one in mapview_common.c, 
but the default color index (DEFAULT_CITY_COLOR == 0) is defined in 
city.h since it must be used in city.c.

I didn't add any new colors to the array, although it makes sense to do so.

In future patches I think (most of) key_city_workers can be moved into 
control.c, and city_workers_display (in packhand.c) can be removed.

jason


Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.122
diff -u -r1.122 mapview_common.c
--- client/mapview_common.c     4 Jun 2004 05:03:27 -0000       1.122
+++ client/mapview_common.c     8 Jun 2004 03:38:15 -0000
@@ -804,6 +804,44 @@
   }
 }
 
+/*
+ * pcity->client.color is an index into the city_colors array.  The default
+ * color index is DEFAULT_CITY_COLOR in city.h.  When toggle_city_color is
+ * called the index moves forward one position.  Each color in the array
+ * tells what color the citymap will be drawn on the mapview; COLOR_STD_LAST
+ * is a marker meaning nothing will be drawn (which of course is the
+ * default).
+ *
+ * This array can be added to without breaking anything elsewhere.
+ */
+static enum color_std city_colors[] = {
+  COLOR_STD_LAST,  /* Default color (see DEFAULT_CITY_COLOR in city.h). */
+  COLOR_STD_RED,
+  COLOR_STD_WHITE,
+  COLOR_STD_YELLOW,
+};
+#define NUM_CITY_COLORS ARRAY_SIZE(city_colors)
+
+
+/****************************************************************************
+  Toggle the city color.  This cycles through the possible colors for the
+  citymap as shown on the mapview.  These colors are listed in the
+  city_colors array; above.
+****************************************************************************/
+void toggle_city_color(struct city *pcity)
+{
+  int canvas_x, canvas_y;
+  int width = get_citydlg_canvas_width();
+  int height = get_citydlg_canvas_height();
+
+  pcity->client.color = (pcity->client.color + 1) % NUM_CITY_COLORS;
+
+  map_to_canvas_pos(&canvas_x, &canvas_y, pcity->x, pcity->y);
+  update_map_canvas(canvas_x - (width - NORMAL_TILE_WIDTH) / 2,
+                   canvas_y - (height - NORMAL_TILE_HEIGHT) / 2,
+                   width, height);
+}
+
 /****************************************************************************
   Return the vertices of the given edge of the tile.  This will return
   FALSE if the edge doesn't exist (or if it does exist but is part of an
@@ -1426,6 +1464,30 @@
     } gui_rect_iterate_end;
   }
 
+  /* Draw citymap overlays on top. */
+  gui_rect_iterate(gui_x0, gui_y0, width, height, map_x, map_y, draw) {
+    if (((draw & D_B) || (draw & D_M))
+       && normalize_map_pos(&map_x, &map_y)) {
+      struct city *pcity = find_city_near_tile(map_x, map_y);
+      int city_x, city_y, canvas_x2, canvas_y2;
+
+      if (pcity
+         && city_colors[pcity->client.color] != COLOR_STD_LAST
+         && map_to_city_map(&city_x, &city_y, pcity, map_x, map_y)
+         && map_to_canvas_pos(&canvas_x2, &canvas_y2, map_x, map_y)) {
+       enum city_tile_type worker = get_worker_city(pcity, city_x, city_y);
+
+       put_city_worker(mapview_canvas.store,
+                       city_colors[pcity->client.color], worker,
+                       canvas_x2, canvas_y2);
+       if (worker == C_TILE_WORKER) {
+         put_city_tile_output(pcity, city_x, city_y,
+                              mapview_canvas.store, canvas_x2, canvas_y2);
+       }
+      }
+    }
+  } gui_rect_iterate_end;
+
   show_city_descriptions(canvas_x, canvas_y, width, height);
 
   if (!full) {
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.65
diff -u -r1.65 mapview_common.h
--- client/mapview_common.h     22 May 2004 18:12:21 -0000      1.65
+++ client/mapview_common.h     8 Jun 2004 03:38:15 -0000
@@ -257,6 +257,7 @@
 void put_unit_city_overlays(struct unit *punit,
                            struct canvas *pcanvas,
                            int canvas_x, int canvas_y);
+void toggle_city_color(struct city *pcity);
 void put_red_frame_tile(struct canvas *pcanvas,
                        int canvas_x, int canvas_y);
 
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.376
diff -u -r1.376 packhand.c
--- client/packhand.c   6 Jun 2004 21:02:15 -0000       1.376
+++ client/packhand.c   8 Jun 2004 03:38:15 -0000
@@ -396,7 +396,8 @@
 
   if(!pcity) {
     city_is_new = TRUE;
-    pcity=fc_malloc(sizeof(struct city));
+    pcity = create_city_virtual(get_player(packet->owner),
+                               packet->x, packet->y, packet->name);
     pcity->id=packet->id;
     idex_register_city(pcity);
     update_descriptions = TRUE;
@@ -594,7 +595,6 @@
   }
 
   if (city_workers_display==pcity)  {
-    put_city_workers(pcity, -1);
     city_workers_display=NULL;
   }
 
Index: client/gui-gtk/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapctrl.c,v
retrieving revision 1.97
diff -u -r1.97 mapctrl.c
--- client/gui-gtk/mapctrl.c    23 Apr 2004 22:58:05 -0000      1.97
+++ client/gui-gtk/mapctrl.c    8 Jun 2004 03:38:15 -0000
@@ -411,8 +411,7 @@
   }
 
   /* Shade tiles on usage */
-  city_workers_color = (city_workers_color % 3) + 1;
-  put_city_workers(pcity, city_workers_color);
+  toggle_city_color(pcity);
 }
 
 
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.222
diff -u -r1.222 mapview.c
--- client/gui-gtk/mapview.c    6 Jun 2004 06:00:08 -0000       1.222
+++ client/gui-gtk/mapview.c    8 Jun 2004 03:38:16 -0000
@@ -790,67 +790,37 @@
   }
 }
 
-/**************************************************************************
-...
-**************************************************************************/
-void put_city_workers(struct city *pcity, int color)
-{
-  int canvas_x, canvas_y;
-  static struct city *last_pcity=NULL;
-  struct canvas store = {map_canvas->window, NULL};
-
-  if (color==-1) {
-    if (pcity!=last_pcity)
-      city_workers_color = city_workers_color%3 + 1;
-    color=city_workers_color;
+/****************************************************************************
+  Draw a single tile of the citymap onto the mapview.  The tile is drawn
+  as the given color with the given worker on it.  The exact method of
+  drawing is left up to the GUI.
+****************************************************************************/
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y)
+{
+  if (worker == C_TILE_EMPTY) {
+    gdk_gc_set_stipple(fill_tile_gc, gray25);
+  } else if (worker == C_TILE_WORKER) {
+    gdk_gc_set_stipple(fill_tile_gc, gray50);
+  } else {
+    return;
   }
-  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
-
-  city_map_checked_iterate(pcity->x, pcity->y, i, j, x, y) {
-    enum city_tile_type worked = get_worker_city(pcity, i, j);
 
-    if (!map_to_canvas_pos(&canvas_x, &canvas_y, x, y)) {
-      continue;
-    }
+  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
 
-    /* stipple the area */
-    if (!is_city_center(i, j)) {
-      if (worked == C_TILE_EMPTY) {
-       gdk_gc_set_stipple(fill_tile_gc, gray25);
-      } else if (worked == C_TILE_WORKER) {
-       gdk_gc_set_stipple(fill_tile_gc, gray50);
-      } else
-       continue;
-
-      if (is_isometric) {
-       gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
-       gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
-       gdk_draw_pixmap(map_canvas->window, fill_tile_gc, map_canvas_store,
-                       canvas_x, canvas_y,
-                       canvas_x, canvas_y,
-                       NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_gc_set_clip_mask(fill_tile_gc, NULL);
-      } else {
-       gdk_draw_pixmap(map_canvas->window, civ_gc, map_canvas_store,
-                       canvas_x, canvas_y,
-                       canvas_x, canvas_y,
-                       NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-      }
-    }
+  if (is_isometric) {
+    gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
+    gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
+  }
 
-    /* draw tile output */
-    if (worked == C_TILE_WORKER) {
-      put_city_tile_output(pcity, i, j, &store, canvas_x, canvas_y);
-    }
-  } city_map_checked_iterate_end;
+  gdk_draw_rectangle(pcanvas->pixmap, fill_tile_gc, TRUE,
+                    canvas_x, canvas_y,
+                    NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
 
-  last_pcity=pcity;
+  if (is_isometric) {
+    gdk_gc_set_clip_mask(fill_tile_gc, NULL);
+  }
 }
 
 /**************************************************************************
Index: client/gui-gtk-2.0/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk-2.0/mapctrl.c,v
retrieving revision 1.39
diff -u -r1.39 mapctrl.c
--- client/gui-gtk-2.0/mapctrl.c        23 Apr 2004 22:58:05 -0000      1.39
+++ client/gui-gtk-2.0/mapctrl.c        8 Jun 2004 03:38:16 -0000
@@ -391,8 +391,7 @@
   }
 
   /* Shade tiles on usage */
-  city_workers_color = (city_workers_color % 3) + 1;
-  put_city_workers(pcity, city_workers_color);
+  toggle_city_color(pcity);
 }
 
 
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.128
diff -u -r1.128 mapview.c
--- client/gui-gtk-2.0/mapview.c        6 Jun 2004 06:00:08 -0000       1.128
+++ client/gui-gtk-2.0/mapview.c        8 Jun 2004 03:38:16 -0000
@@ -853,67 +853,37 @@
   }
 }
 
-/**************************************************************************
-...
-**************************************************************************/
-void put_city_workers(struct city *pcity, int color)
-{
-  int canvas_x, canvas_y;
-  static struct city *last_pcity=NULL;
-  struct canvas store = {.type = CANVAS_PIXMAP, .v.pixmap = 
map_canvas->window};
-
-  if (color==-1) {
-    if (pcity!=last_pcity)
-      city_workers_color = city_workers_color%3 + 1;
-    color=city_workers_color;
+/****************************************************************************
+  Draw a single tile of the citymap onto the mapview.  The tile is drawn
+  as the given color with the given worker on it.  The exact method of
+  drawing is left up to the GUI.
+****************************************************************************/
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y)
+{
+  if (worker == C_TILE_EMPTY) {
+    gdk_gc_set_stipple(fill_tile_gc, gray25);
+  } else if (worker == C_TILE_WORKER) {
+    gdk_gc_set_stipple(fill_tile_gc, gray50);
+  } else {
+    return;
   }
-  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
-
-  city_map_checked_iterate(pcity->x, pcity->y, i, j, x, y) {
-    enum city_tile_type worked = get_worker_city(pcity, i, j);
 
-    if (!map_to_canvas_pos(&canvas_x, &canvas_y, x, y)) {
-      continue;
-    }
+  gdk_gc_set_foreground(fill_tile_gc, colors_standard[color]);
 
-    /* stipple the area */
-    if (!is_city_center(i, j)) {
-      if (worked == C_TILE_EMPTY) {
-       gdk_gc_set_stipple(fill_tile_gc, gray25);
-      } else if (worked == C_TILE_WORKER) {
-       gdk_gc_set_stipple(fill_tile_gc, gray50);
-      } else
-       continue;
-
-      if (is_isometric) {
-       gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
-       gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
-       gdk_draw_drawable(map_canvas->window, fill_tile_gc, map_canvas_store,
-                         canvas_x, canvas_y,
-                         canvas_x, canvas_y,
-                         NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_gc_set_clip_mask(fill_tile_gc, NULL);
-      } else {
-       gdk_draw_drawable(map_canvas->window, civ_gc, map_canvas_store,
-                         canvas_x, canvas_y,
-                         canvas_x, canvas_y,
-                         NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-       gdk_draw_rectangle(map_canvas->window, fill_tile_gc, TRUE,
-                          canvas_x, canvas_y,
-                          NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-      }
-    }
+  if (is_isometric) {
+    gdk_gc_set_clip_origin(fill_tile_gc, canvas_x, canvas_y);
+    gdk_gc_set_clip_mask(fill_tile_gc, sprites.black_tile->mask);
+  }
 
-    /* draw tile output */
-    if (worked == C_TILE_WORKER) {
-      put_city_tile_output(pcity, i, j, &store, canvas_x, canvas_y);
-    }
-  } city_map_checked_iterate_end;
+  gdk_draw_rectangle(pcanvas->v.pixmap, fill_tile_gc, TRUE,
+                    canvas_x, canvas_y,
+                    NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
 
-  last_pcity=pcity;
+  if (is_isometric) {
+    gdk_gc_set_clip_mask(fill_tile_gc, NULL);
+  }
 }
 
 /**************************************************************************
Index: client/gui-win32/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
retrieving revision 1.119
diff -u -r1.119 mapview.c
--- client/gui-win32/mapview.c  6 Jun 2004 06:00:09 -0000       1.119
+++ client/gui-win32/mapview.c  8 Jun 2004 03:38:16 -0000
@@ -599,13 +599,16 @@
   }
 }
 
-/**************************************************************************
-
-**************************************************************************/
-void
-put_city_workers(struct city *pcity, int color)
+/****************************************************************************
+  Draw a single tile of the citymap onto the mapview.  The tile is drawn
+  as the given color with the given worker on it.  The exact method of
+  drawing is left up to the GUI.
+****************************************************************************/
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y)
 {
-       /* PORTME */
+  /* PORTME */
 }
 
 /**************************************************************************
Index: client/gui-xaw/mapctrl.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapctrl.c,v
retrieving revision 1.84
diff -u -r1.84 mapctrl.c
--- client/gui-xaw/mapctrl.c    23 Apr 2004 22:58:06 -0000      1.84
+++ client/gui-xaw/mapctrl.c    8 Jun 2004 03:38:16 -0000
@@ -275,8 +275,7 @@
   if(!pcity) return;
 
   /* Shade tiles on usage */
-  city_workers_color = (city_workers_color%3)+1;
-  put_city_workers(pcity, city_workers_color);
+  toggle_city_color(pcity);
 }
 
 /**************************************************************************
Index: client/gui-xaw/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
retrieving revision 1.174
diff -u -r1.174 mapview.c
--- client/gui-xaw/mapview.c    6 Jun 2004 06:00:09 -0000       1.174
+++ client/gui-xaw/mapview.c    8 Jun 2004 03:38:16 -0000
@@ -747,52 +747,35 @@
   }
 }
 
-
-/**************************************************************************
- Shade the tiles around a city to indicate the location of workers
-**************************************************************************/
-void put_city_workers(struct city *pcity, int color)
+/****************************************************************************
+  Draw a single tile of the citymap onto the mapview.  The tile is drawn
+  as the given color with the given worker on it.  The exact method of
+  drawing is left up to the GUI.
+****************************************************************************/
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y)
 {
-  int canvas_x, canvas_y;
-  static struct city *last_pcity = NULL;
-  struct canvas store = {XtWindow(map_canvas)};
-
-  if (color == -1) {
-    if (pcity != last_pcity)
-      city_workers_color = (city_workers_color%3)+1;
-    color = city_workers_color;
+  if (worker == C_TILE_EMPTY) {
+    XSetStipple(display, fill_tile_gc, gray25);
+  } else if (worker == C_TILE_WORKER) {
+    XSetStipple(display, fill_tile_gc, gray50);
+  } else {
+    return;
   }
 
   XSetForeground(display, fill_tile_gc, colors_standard[color]);
-  city_map_checked_iterate(pcity->x, pcity->y, i, j, x, y) {
-    enum city_tile_type worked = get_worker_city(pcity, i, j);
 
-    if (!map_to_canvas_pos(&canvas_x, &canvas_y, x, y)) {
-      continue;
-    }
-    if (!is_city_center(i, j)) {
-      if (worked == C_TILE_EMPTY) {
-       XSetStipple(display, fill_tile_gc, gray25);
-      } else if (worked == C_TILE_WORKER) {
-       XSetStipple(display, fill_tile_gc, gray50);
-      } else
-       continue;
-      XCopyArea(display, map_canvas_store, XtWindow(map_canvas), civ_gc,
-               canvas_x, canvas_y,
-               NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT, canvas_x, canvas_y);
-      XFillRectangle(display, XtWindow(map_canvas), fill_tile_gc,
-                    canvas_x, canvas_y,
-                    NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
-    }
-    if (worked == C_TILE_WORKER) {
-      put_city_tile_output(pcity, i, j, &store, canvas_x, canvas_y);
-    }
-  } city_map_checked_iterate_end;
-
-  last_pcity = pcity;
+#if 0
+  XCopyArea(display, map_canvas_store, XtWindow(map_canvas), civ_gc,
+           canvas_x, canvas_y,
+           NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT, canvas_x, canvas_y);
+#endif
+  XFillRectangle(display, pcanvas->pixmap, fill_tile_gc,
+                canvas_x, canvas_y,
+                NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
 }
 
-
 /**************************************************************************
 ...
 **************************************************************************/
Index: client/include/mapview_g.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/include/mapview_g.h,v
retrieving revision 1.52
diff -u -r1.52 mapview_g.h
--- client/include/mapview_g.h  17 May 2004 07:16:43 -0000      1.52
+++ client/include/mapview_g.h  8 Jun 2004 03:38:16 -0000
@@ -71,7 +71,9 @@
 void update_map_canvas_scrollbars_size(void);
 
 void put_cross_overlay_tile(int x,int y);
-void put_city_workers(struct city *pcity, int color);
+void put_city_worker(struct canvas *pcanvas,
+                    enum color_std color, enum city_tile_type worker,
+                    int canvas_x, int canvas_y);
 
 void draw_selection_rectangle(int canvas_x, int canvas_y, int w, int h);
 void tileset_changed(void);
Index: common/city.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.c,v
retrieving revision 1.216
diff -u -r1.216 city.c
--- common/city.c       4 Jun 2004 15:49:59 -0000       1.216
+++ common/city.c       8 Jun 2004 03:38:17 -0000
@@ -2719,6 +2719,10 @@
   pcity->tax_bonus = 100;
   pcity->science_bonus = 100;
 
+  pcity->client.occupied = FALSE;
+  pcity->client.happy = pcity->client.unhappy = FALSE;
+  pcity->client.color = DEFAULT_CITY_COLOR;
+
   unit_list_init(&pcity->units_supported);
   pcity->debug = FALSE;
 
Index: common/city.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/common/city.h,v
retrieving revision 1.146
diff -u -r1.146 city.h
--- common/city.h       4 Jun 2004 15:49:59 -0000       1.146
+++ common/city.h       8 Jun 2004 03:38:17 -0000
@@ -264,6 +264,10 @@
     /* Only used at the client (the serer is omniscient). */
     bool occupied;
     bool happy, unhappy;
+
+    /* The color is an index into the city_colors array in mapview_common */
+#define DEFAULT_CITY_COLOR 0
+    int color;
   } client;
 
   int steal;                 /* diplomats steal once; for spies, gets harder */