[Freeciv-Dev] Re: (PR#12191) make all drawing queued

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

<URL: http://bugs.freeciv.org/Ticket/Display.html?id=12191 >

Here is an updated full queueing patch.  Although this touches a lot of 
code, it's probably ready to go in.  I've done a fair bit of playing 
with it and believe it to be less buggy than the current code, at the least.

- All queue functions are made static.

- All refresh functions, plus update_map_canvas_visible and 
update_city_descriptions, are made wrappers for the queue functions. 
(Maybe these should be renamed?)

- The flush/dirty system still isn't changed.

- Queue function calls in packhand.c are changed back to refresh calls. 
  Unnecessary calls are simply removed from handle_tile_info.

-jason


? diff
Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.187
diff -u -r1.187 mapview_common.c
--- client/mapview_common.c     12 Feb 2005 05:02:55 -0000      1.187
+++ client/mapview_common.c     12 Feb 2005 06:43:07 -0000
@@ -60,28 +60,28 @@
 static void dirty_overview(void);
 static void flush_dirty_overview(void);
 
+enum update_type {
+  /* Masks */
+  UPDATE_NONE = 0,
+  UPDATE_CITY_DESCRIPTIONS = 1,
+  UPDATE_MAP_CANVAS_VISIBLE = 2
+};
+static void queue_mapview_update(enum update_type update);
+static void queue_mapview_tile_update(struct tile *ptile);
+static void queue_mapview_unit_update(struct unit *punit, struct tile *ptile);
+static void queue_mapview_city_update(struct city *pcity, struct tile *ptile,
+                                     bool full_refresh);
+static void unqueue_mapview_updates(bool write_to_screen);
+
 /**************************************************************************
  Refreshes a single tile on the map canvas.
 **************************************************************************/
 void refresh_tile_mapcanvas(struct tile *ptile, bool write_to_screen)
 {
-  int canvas_x, canvas_y;
-
-  if (tile_to_canvas_pos(&canvas_x, &canvas_y, ptile)) {
-    const int W = NORMAL_TILE_WIDTH, H = NORMAL_TILE_HEIGHT;
-
-    /* We draw an extra NORMAL_TILE_XXX / 2 on each side because with
-     * corner and edge sprites this much extra may need to be drawn.  This
-     * is only applicable when the underlying tile changes however.  There
-     * should probably be a refresh_unit_mapcanvas/refresh_city_mapcanvas
-     * functions that handle updates for those items more elegantly. */
-    update_map_canvas(canvas_x - W / 2, canvas_y - H / 2, 2 * W, 2 * H);
-
-    if (write_to_screen) {
-      flush_dirty();
-    }
+  queue_mapview_tile_update(ptile);
+  if (write_to_screen) {
+    unqueue_mapview_updates(TRUE);
   }
-  overview_update_tile(ptile);
 }
 
 /**************************************************************************
@@ -90,35 +90,10 @@
 void refresh_unit_mapcanvas(struct unit *punit, struct tile *ptile,
                            bool write_to_screen)
 {
-  if (unit_type_flag(punit->type, F_CITIES)) {
-    /* For settlers we (often) have to update the whole citymap area because
-     * of the 't' overlays or the citymap outlines.  The above check could
-     * be more rigorous so that no update is done unless it's needed...
-     *
-     * HACK: The addition below accounts for grid lines that may actually
-     * be on a tile outside of the city radius.  This is similar to what's
-     * done in refresh_tile_mapcanvas. */
-    int width = get_citydlg_canvas_width() + NORMAL_TILE_WIDTH;
-    int height = get_citydlg_canvas_height() + NORMAL_TILE_HEIGHT;
-    int canvas_x, canvas_y;
-
-    tile_to_canvas_pos(&canvas_x, &canvas_y, ptile);
-    update_map_canvas(canvas_x - (width - NORMAL_TILE_WIDTH) / 2,
-                     canvas_y - (height - NORMAL_TILE_HEIGHT) / 2,
-                     width, height);
-  } else {
-    int canvas_x, canvas_y;
-
-    if (tile_to_canvas_pos(&canvas_x, &canvas_y, ptile)) {
-      canvas_y += NORMAL_TILE_HEIGHT - UNIT_TILE_HEIGHT;
-      update_map_canvas(canvas_x, canvas_y,
-                       UNIT_TILE_WIDTH, UNIT_TILE_HEIGHT);
-    } 
-  }
+  queue_mapview_unit_update(punit, ptile);
   if (write_to_screen) {
-    flush_dirty();
+    unqueue_mapview_updates(TRUE);
   }
-  overview_update_tile(ptile);
 }
 
 /**************************************************************************
@@ -130,37 +105,11 @@
 void refresh_city_mapcanvas(struct city *pcity, struct tile *ptile,
                            bool full_refresh, bool write_to_screen)
 {
-  if (full_refresh && (draw_map_grid || draw_borders)) {
-    /* We have to make sure we update any workers on the map grid, then
-     * redraw the city descriptions on top of them.  So we calculate the
-     * rectangle covered by the city's map, and update that.  Then we
-     * queue up a city description redraw for later.
-     *
-     * HACK: The addition below accounts for grid lines that may actually
-     * be on a tile outside of the city radius.  This is similar to what's
-     * done in refresh_tile_mapcanvas. */
-    int canvas_x, canvas_y;
-    int width = get_citydlg_canvas_width() + NORMAL_TILE_WIDTH;
-    int height = get_citydlg_canvas_height() + NORMAL_TILE_HEIGHT;
-
-    (void) tile_to_canvas_pos(&canvas_x, &canvas_y, pcity->tile);
-
-    update_map_canvas(canvas_x - (width - NORMAL_TILE_WIDTH) / 2,
-                     canvas_y - (height - NORMAL_TILE_HEIGHT) / 2,
-                     width, height);
-  } else {
-    int canvas_x, canvas_y;
-
-    if (tile_to_canvas_pos(&canvas_x, &canvas_y, ptile)) {
-      canvas_y += NORMAL_TILE_HEIGHT - UNIT_TILE_HEIGHT;
-      update_map_canvas(canvas_x, canvas_y,
-                       UNIT_TILE_WIDTH, UNIT_TILE_HEIGHT);
-    }
-  }
+  queue_mapview_city_update(pcity, ptile,
+                           full_refresh && (draw_map_grid || draw_borders));
   if (write_to_screen) {
-    flush_dirty();
+    unqueue_mapview_updates(TRUE);
   }
-  overview_update_tile(ptile);
 }
 
 /**************************************************************************
@@ -548,7 +497,9 @@
                        update_x1 - update_x0, common_y1 - common_y0);
     }
   } else {
-    update_map_canvas_visible();
+    dirty_all();
+    update_map_canvas(0, 0, mapview_canvas.store_width,
+                     mapview_canvas.store_height);
   }
 
   map_center = get_center_tile_mapcanvas();
@@ -1757,9 +1708,7 @@
 **************************************************************************/
 void update_map_canvas_visible(void)
 {
-  dirty_all();
-  update_map_canvas(0, 0, mapview_canvas.store_width,
-                   mapview_canvas.store_height);
+  queue_mapview_update(UPDATE_MAP_CANVAS_VISIBLE);
 }
 
 /* The maximum city description width and height.  This gives the dimensions
@@ -1967,7 +1916,7 @@
       refresh_unit_mapcanvas(punit1, punit1->tile, FALSE);
     }
 
-    flush_dirty();
+    unqueue_mapview_updates(TRUE);
     gui_flush();
 
     usleep_since_timer_start(anim_timer, 10000);
@@ -1977,6 +1926,7 @@
       && tile_to_canvas_pos(&canvas_x, &canvas_y,
                           losing_unit->tile)) {
     refresh_unit_mapcanvas(losing_unit, losing_unit->tile, FALSE);
+    unqueue_mapview_updates(FALSE);
     canvas_copy(mapview_canvas.tmp_store, mapview_canvas.store,
                canvas_x, canvas_y, canvas_x, canvas_y,
                NORMAL_TILE_WIDTH, NORMAL_TILE_HEIGHT);
@@ -2278,6 +2228,20 @@
 }
 
 /**************************************************************************
+  Appends a tile to the list.  The list is allocated if NULL.  The (new)
+  list is returned.
+**************************************************************************/
+static struct tile_list *append_tile_to_list(struct tile *ptile,
+                                            struct tile_list *plist)
+{
+  if (!plist) {
+    plist = tile_list_new();
+  }
+  tile_list_append(plist, ptile);
+  return plist;
+}
+
+/**************************************************************************
   Queue this tile to be refreshed.  The refresh will be done some time
   soon thereafter, and grouped with other needed refreshes.
 
@@ -2286,10 +2250,7 @@
 **************************************************************************/
 void queue_mapview_tile_update(struct tile *ptile)
 {
-  if (!tile_updates) {
-    tile_updates = tile_list_new();
-  }
-  tile_list_prepend(tile_updates, ptile);
+  tile_updates = append_tile_to_list(ptile, tile_updates);
   queue_add_callback();
 }
 
@@ -2297,12 +2258,13 @@
   Queue this unit to be refreshed.  The refresh will be done some time
   soon thereafter, and grouped with other needed refreshes.
 **************************************************************************/
-void queue_mapview_unit_update(struct unit *punit)
+void queue_mapview_unit_update(struct unit *punit, struct tile *ptile)
 {
-  if (!unit_updates) {
-    unit_updates = tile_list_new();
+  if (unit_type_flag(punit->type, F_CITIES)) {
+    city_updates = append_tile_to_list(ptile, city_updates);
+  } else {
+    unit_updates = append_tile_to_list(ptile, unit_updates);
   }
-  tile_list_prepend(unit_updates, punit->tile);
   queue_add_callback();
 }
 
@@ -2310,12 +2272,14 @@
   Queue this city to be refreshed.  The refresh will be done some time
   soon thereafter, and grouped with other needed refreshes.
 **************************************************************************/
-void queue_mapview_city_update(struct city *pcity)
+void queue_mapview_city_update(struct city *pcity, struct tile *ptile,
+                              bool full_refresh)
 {
-  if (!city_updates) {
-    city_updates = tile_list_new();
+  if (full_refresh) {
+    city_updates = append_tile_to_list(ptile, city_updates);
+  } else {
+    unit_updates = append_tile_to_list(ptile, unit_updates);
   }
-  tile_list_prepend(city_updates, pcity->tile);
   queue_add_callback();
 }
 
@@ -2330,7 +2294,9 @@
   if (map_exists()) {
     if ((needed_updates & UPDATE_MAP_CANVAS_VISIBLE)
        || (needed_updates & UPDATE_CITY_DESCRIPTIONS)) {
-      update_map_canvas_visible();
+      dirty_all();
+      update_map_canvas(0, 0, mapview_canvas.store_width,
+                       mapview_canvas.store_height);
     } else {
       int min_x = mapview_canvas.width, min_y = mapview_canvas.height;
       int max_x = 0, max_y = 0;
@@ -2379,13 +2345,17 @@
       }
 
       if (city_updates) {
+       int width = get_citydlg_canvas_width() + NORMAL_TILE_WIDTH;
+       int height = get_citydlg_canvas_height() + NORMAL_TILE_HEIGHT;
+
        tile_list_iterate(city_updates, ptile) {
          int x0, y0, x1, y1;
 
          if (tile_to_canvas_pos(&x0, &y0, ptile)) {
-           y0 += NORMAL_TILE_HEIGHT - UNIT_TILE_HEIGHT;
-           x1 = x0 + UNIT_TILE_WIDTH;
-           y1 = y0 + UNIT_TILE_HEIGHT;
+           x0 -= (width - NORMAL_TILE_WIDTH) / 2;
+           y0 -= (height - NORMAL_TILE_HEIGHT) / 2;
+           x1 = x0 + width;
+           y1 = y0 + height;
            min_x = MIN(min_x, x0);
            min_y = MIN(min_y, y0);
            max_x = MAX(max_x, x1);
@@ -2890,6 +2860,7 @@
     if (tile_size_changed) {
       update_map_canvas_visible();
       center_tile_overviewcanvas(get_center_tile_mapcanvas());
+      unqueue_mapview_updates(TRUE);
       redrawn = TRUE;
     }
 
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.91
diff -u -r1.91 mapview_common.h
--- client/mapview_common.h     12 Feb 2005 05:02:55 -0000      1.91
+++ client/mapview_common.h     12 Feb 2005 06:43:07 -0000
@@ -60,13 +60,6 @@
 #define BORDER_WIDTH 2
 #define GOTO_WIDTH 2
 
-enum update_type {
-  /* Masks */
-  UPDATE_NONE = 0,
-  UPDATE_CITY_DESCRIPTIONS = 1,
-  UPDATE_MAP_CANVAS_VISIBLE = 2
-};
-
 /*
  * Iterate over all map tiles that intersect with the given GUI rectangle.
  * The order of iteration is guaranteed to satisfy the painter's algorithm.
@@ -292,12 +285,6 @@
                                      size_t growth_buffer_len,
                                      enum color_std *grwoth_color);
 
-void queue_mapview_update(enum update_type update);
-void queue_mapview_tile_update(struct tile *ptile);
-void queue_mapview_unit_update(struct unit *punit);
-void queue_mapview_city_update(struct city *pcity);
-void unqueue_mapview_updates(bool write_to_screen);
-
 void map_to_overview_pos(int *overview_x, int *overview_y,
                         int map_x, int map_y);
 void overview_to_map_pos(int *map_x, int *map_y,
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.468
diff -u -r1.468 packhand.c
--- client/packhand.c   9 Feb 2005 17:15:17 -0000       1.468
+++ client/packhand.c   12 Feb 2005 06:43:08 -0000
@@ -775,7 +775,7 @@
   last_turn_gold_amount=game.player_ptr->economic.gold;
 #endif
 
-  queue_mapview_update(UPDATE_CITY_DESCRIPTIONS);
+  update_city_descriptions();
 
   if (sound_bell_at_new_turn &&
       (!game.player_ptr->ai.control || ai_manual_turn_done)) {
@@ -1952,28 +1952,9 @@
 
   /* refresh tiles */
   if (can_client_change_view()) {
-    /* the tile itself */
-    if (tile_changed || old_known!=ptile->known)
-      queue_mapview_tile_update(ptile);
-
-    /* if the terrain or the specials of the tile
-       have changed it affects the adjacent tiles */
-    if (tile_changed) {
-      adjc_iterate(ptile, tile1) {
-       if (tile_get_known(tile1) >= TILE_KNOWN_FOGGED)
-         queue_mapview_tile_update(tile1);
-      }
-      adjc_iterate_end;
-      return;
-    }
-
-    /* the "furry edges" on tiles adjacent to an TILE_UNKNOWN tile are
-       removed here */
-    if (old_known == TILE_UNKNOWN && packet->known >= TILE_KNOWN_FOGGED) {     
-      cardinal_adjc_iterate(ptile, tile1) {
-       if (tile_get_known(tile1) >= TILE_KNOWN_FOGGED)
-         queue_mapview_tile_update(tile1);
-      } cardinal_adjc_iterate_end;
+    /* the tile itself (including the necessary parts of adjacent tiles) */
+    if (tile_changed || old_known!=ptile->known) {
+      refresh_tile_mapcanvas(ptile, FALSE);
     }
   }