[Freeciv-Dev] (PR#14077) incomplete cityname draw

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

> [chrisk - Wed Sep 21 12:55:13 2005]:
> 
> 
> CVS 21 SEPT 2005 GTK2 HEAD
> 
> If the name of a city displayed in the map, is very long, it is not always
> drawn complete at the moment you discover the city. I assume this is
> connected to the fact, that the tiles over which the name is supposed
to be
> drawn, are not known to the client.

No, it's not connected to that fact.

> In the attached PNG, when I moved along to the city, at first the
beginning
> and the end of the name was not drawn. Only after I middle clicked on the
> city, I could read the whole name.
> 
> The red rectangle shows (approximately) the area where the text
drawing was
> restricted to in the first place.

Summary: this problem is complex and difficult to reproduce, but I think
this patch should fix it.

Background:

The code tracks the maximum width and height a city description may
have.  Any time we need to update the city description, then, we have to
redraw the entire bounding box of the text from scratch.  This is only
possible if we know how big this bounding box is.  But we don't know how
big it is until after we do the drawing (there's no complex layout code
here, just an update_city_desc call).  Thus, the first time you receive
info about a city with a long name the first update generally won't draw
the whole thing.  After this update the bounding box is correctly known,
and any future updates will be done correctly.

I suspect there is some gross inefficiency here.  In many cases more
than one update is done at a time so the bug is concealed...however this
is inefficiency in the drawing code of course since you don't really
want to draw more than one update at a time.  Also it's inefficient
because one really long city name means a large bounding box, and this
is used for updates of *all* cities - thus a game with a single
long-city-name will have slower drawing even if that city isn't actually
being drawn.

The problem is simple but the fix isn't easy.  The issue is we can't
start over from drawing mid-way because we're inside an update loop. 
This patch works around that by re-calling update_city_desc to put a new
update into the queue.  This also means that unqueue_mapview_updates
must be changed so that new updates can be queued from inside the function.

-jason

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.244
diff -p -u -r1.244 mapview_common.c
--- client/mapview_common.c     29 Aug 2005 13:56:49 -0000      1.244
+++ client/mapview_common.c     23 Sep 2005 01:01:36 -0000
@@ -1480,6 +1480,7 @@ void show_city_descriptions(int canvas_x
 {
   const int dx = max_desc_width - tileset_tile_width(tileset), dy = 
max_desc_height;
   const int offset_y = tileset_citybar_offset_y(tileset);
+  int new_max_width = max_desc_width, new_max_height = max_desc_height;
 
   if (!draw_city_names && !draw_city_productions) {
     return;
@@ -1516,11 +1517,25 @@ void show_city_descriptions(int canvas_x
 
       show_city_desc(mapview.store, canvas_x, canvas_y,
                     pcity, &width, &height);
+      freelog(LOG_NORMAL, "Drawing %s.", pcity->name);
 
-      max_desc_width = MAX(width, max_desc_width);
-      max_desc_height = MAX(height, max_desc_height);
+      if (width > max_desc_width || height > max_desc_height) {
+       /* The update was incomplete!  We queue a new update.  Note that
+        * this is recursively queueing an update within a dequeuing of an
+        * update.  This is allowed specifically because of the code in
+        * unqueue_mapview_updates.  See that function for more. */
+       freelog(LOG_NORMAL, "Re-queuing %s.", pcity->name);
+       update_city_description(pcity);
+      }
+      new_max_width = MAX(width, new_max_width);
+      new_max_height = MAX(height, new_max_height);
     }
   } gui_rect_iterate_end;
+
+  /* We don't update the new max values until the end, so that the
+   * check above to see what cities need redrawing will be complete. */
+  max_desc_width = MAX(max_desc_width, new_max_width);
+  max_desc_height = MAX(max_desc_height, new_max_height);
 }
 
 /****************************************************************************
@@ -2025,6 +2040,7 @@ void unqueue_mapview_updates(bool write_
     {-(max_desc_width - W) / 2, H, max_desc_width, max_desc_height},
     {-(city_width - W) / 2, -(city_height - H) / 2, city_width, city_height}
   };
+  struct tile_list *my_tile_updates[TILE_UPDATE_COUNT];
 
   int i;
 
@@ -2037,6 +2053,14 @@ void unqueue_mapview_updates(bool write_
   freelog(LOG_DEBUG, "unqueue_mapview_update: needed_updates=%d",
          needed_updates);
 
+  /* This code "pops" the lists of tile updates off of the static array and
+   * stores them locally.  This allows further updates to be queued within
+   * the function itself (namely, within update_map_canvas). */
+  for (i = 0; i < TILE_UPDATE_COUNT; i++) {
+    my_tile_updates[i] = tile_updates[i];
+    tile_updates[i] = NULL;
+  }
+
   if (map_exists()) {
     if ((needed_updates & UPDATE_MAP_CANVAS_VISIBLE)
        || (needed_updates & UPDATE_CITY_DESCRIPTIONS)) {
@@ -2051,8 +2075,8 @@ void unqueue_mapview_updates(bool write_
       int i;
 
       for (i = 0; i < TILE_UPDATE_COUNT; i++) {
-       if (tile_updates[i]) {
-         tile_list_iterate(tile_updates[i], ptile) {
+       if (my_tile_updates[i]) {
+         tile_list_iterate(my_tile_updates[i], ptile) {
            int x0, y0, x1, y1;
 
            (void) tile_to_canvas_pos(&x0, &y0, ptile);
@@ -2084,8 +2108,9 @@ void unqueue_mapview_updates(bool write_
     }
   }
   for (i = 0; i < TILE_UPDATE_COUNT; i++) {
-    if (tile_updates[i]) {
-      tile_list_unlink_all(tile_updates[i]);
+    if (my_tile_updates[i]) {
+      tile_list_unlink_all(my_tile_updates[i]);
+      tile_list_free(my_tile_updates[i]);
     }
   }
   needed_updates = UPDATE_NONE;