[Freeciv-Dev] Re: (PR#2858)

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

Rafa³ Bursig via RT wrote:
> This patch fix some problems with redrawing GUI widgets in SDLClient.
> 
> All widgets are part of map and must be redraw after redraw of map,
> lastest unification of draw code make this redraw imposible inside gui code.
> 
> This function replace "show_city_descriptions()" in
> "update_map_canvas_visible()" and all clients use it like wraper to
> "show_city_descriptions()" funct.

The problem is that some graphics aren't buffered when they're drawn - 
they are drawn directly onto the screen.  In the case of city 
descriptions, this is enforced in the common code by the order of the 
drawing.  (Other graphics are drawn directly as well, but only in the GUI.)

As long as this is the case, there will be problems.  We can introduce 
workarounds for them on a case-by-case basis, but the workarounds 
generally have disadvantages.  The SDL client has the biggest problem 
since the map widgets, even with a redraw_map_widgets() function, will 
flicker every time they're drawn.  This is quite noticable.

Another workaround is that show_city_descriptions is called for every 
expose event (in clients for which this makes sense).

I'm now convinced that the solution is to say all drawing done by the 
client-common code should be buffered.  We leave the decision of how to 
do the buffering up to the GUI.

Thus in update_map_canvas_visible, the current order is:

   update_map_canvas(..., TRUE); // writes to display
   show_city_descriptions(); // also writes to display

whereas it should be

   update_map_canvas(..., FALSE); //doesn't write to display
   show_city_descriptions(); // doesn't write to display
   flush_mapcanvas_full(); // writes to display

This works out well for everyone.  It cuts down on flicker and speeds up 
the drawing process in normal cases.  For the SDL client, it means you 
can simply draw the map widgets on top of the canvas when it is flushed. 
  Eventually, you'll want to introduce a second layer of buffering to 
avoid the widgets flickering every time this happens.

The attached patch accomplishes this in the simplest way.  It does not 
introduce a flush_mapcanvas_full, but just flushes the canvas using the 
existing function (which often will give off-screen coordinates).  It 
would be possible to add a write_to_screen parameter to 
show_city_descriptions() and show_city_desc(), but this is probably 
overkill (although it does make changes like this easier in the future).

It is tested under all clients other than MUI.

Although a rather short patch, this is a non-trivial design change.  Are 
there any disadvantages of having the city descriptions be buffered?

jason

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.24
diff -u -r1.24 mapview_common.c
--- client/mapview_common.c     2003/01/17 20:20:56     1.24
+++ client/mapview_common.c     2003/01/23 01:28:42
@@ -575,13 +575,15 @@
 
     width = height = map_tile_width + map_tile_height;
     update_map_canvas(map_view_x0, map_view_y0 - map_tile_width, width,
-                     height, TRUE);
+                     height, FALSE);
   } else {
     update_map_canvas(map_view_x0, map_view_y0, map_tile_width,
-                     map_tile_height, TRUE);
+                     map_tile_height, FALSE);
   }
 
   show_city_descriptions();
+
+  flush_mapcanvas(0, 0, map_win_width, map_win_height);
 }
 
 /**************************************************************************
Index: client/gui-gtk/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-gtk/mapview.c,v
retrieving revision 1.149
diff -u -r1.149 mapview.c
--- client/gui-gtk/mapview.c    2003/01/17 20:20:56     1.149
+++ client/gui-gtk/mapview.c    2003/01/23 01:28:43
@@ -771,7 +771,6 @@
        gdk_draw_pixmap( map_canvas->window, civ_gc, map_canvas_store,
                ev->area.x, ev->area.y, ev->area.x, ev->area.y,
                ev->area.width, ev->area.height );
-       show_city_descriptions();
       }
     }
     refresh_overview_canvas();
@@ -975,14 +974,14 @@
   }
   w2 = gdk_string_width(prod_fontset, buffer2);
 
-  gtk_draw_shadowed_string(map_canvas->window, main_fontset,
+  gtk_draw_shadowed_string(map_canvas_store, main_fontset,
                           toplevel->style->black_gc,
                           toplevel->style->white_gc,
                           canvas_x - (w + w2) / 2,
                           canvas_y + ascent,
                           buffer);
   gdk_gc_set_foreground(civ_gc, colors_standard[color]);
-  gtk_draw_shadowed_string(map_canvas->window, prod_fontset,
+  gtk_draw_shadowed_string(map_canvas_store, prod_fontset,
                           toplevel->style->black_gc,
                           civ_gc,
                           canvas_x - (w + w2) / 2 + w,
@@ -997,7 +996,7 @@
     get_city_mapview_production(pcity, buffer, sizeof(buffer));
 
     gdk_string_extents(prod_fontset, buffer, NULL, NULL, &w, &ascent, NULL);
-    gtk_draw_shadowed_string(map_canvas->window, prod_fontset,
+    gtk_draw_shadowed_string(map_canvas_store, prod_fontset,
                             toplevel->style->black_gc,
                             toplevel->style->white_gc, canvas_x - w / 2,
                             canvas_y + ascent + 3, buffer);
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.37
diff -u -r1.37 mapview.c
--- client/gui-gtk-2.0/mapview.c        2003/01/17 20:20:56     1.37
+++ client/gui-gtk-2.0/mapview.c        2003/01/23 01:28:44
@@ -792,7 +792,6 @@
       gdk_draw_pixmap(map_canvas->window, civ_gc, map_canvas_store,
                ev->area.x, ev->area.y, ev->area.x, ev->area.y,
                ev->area.width, ev->area.height);
-      show_city_descriptions();
       cleared = FALSE;
     } else {
       if (!cleared) {
@@ -1028,7 +1027,7 @@
       rect2.width = 0;
     }
 
-    gtk_draw_shadowed_string(map_canvas->window,
+    gtk_draw_shadowed_string(map_canvas_store,
                             toplevel->style->black_gc,
                             toplevel->style->white_gc,
                             canvas_x - (rect.width + rect2.width) / 2,
@@ -1038,7 +1037,7 @@
       pango_layout_set_font_description(layout, city_productions_font);
       pango_layout_set_text(layout, buffer2, -1);
       gdk_gc_set_foreground(civ_gc, colors_standard[color]);
-      gtk_draw_shadowed_string(map_canvas->window,
+      gtk_draw_shadowed_string(map_canvas_store,
                               toplevel->style->black_gc,
                               civ_gc,
                               canvas_x - (rect.width + rect2.width) / 2
@@ -1058,7 +1057,7 @@
     pango_layout_set_text(layout, buffer, -1);
 
     pango_layout_get_pixel_extents(layout, &rect, NULL);
-    gtk_draw_shadowed_string(map_canvas->window,
+    gtk_draw_shadowed_string(map_canvas_store,
                             toplevel->style->black_gc,
                             toplevel->style->white_gc,
                             canvas_x - rect.width / 2,
Index: client/gui-mui/mapclass.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-mui/mapclass.c,v
retrieving revision 1.90
diff -u -r1.90 mapclass.c
--- client/gui-mui/mapclass.c   2003/01/18 18:58:26     1.90
+++ client/gui-mui/mapclass.c   2003/01/23 01:28:46
@@ -455,6 +455,7 @@
 
   APTR cliphandle;
 
+  /* FIXME: this needs to draw to the backing store, not to the display. */
 
   if (!(new_font = _font(o))) return;
 
@@ -1466,8 +1467,10 @@
        }
       }
 
+#if 0 /* This is no longer needed. */
       if (!(msg->flags & MADF_DRAWUPDATE) || ((msg->flags & MADF_DRAWUPDATE) 
&& (data->update == 3)))
        show_city_descriptions();
+#endif
     }
 
     data->update = 0;
Index: client/gui-sdl/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-sdl/mapview.c,v
retrieving revision 1.9
diff -u -r1.9 mapview.c
--- client/gui-sdl/mapview.c    2003/01/19 11:39:47     1.9
+++ client/gui-sdl/mapview.c    2003/01/23 01:28:47
@@ -1790,12 +1790,6 @@
 #ifdef DRAW_TIMING
   struct timer *tttt=new_timer_start(TIMER_USER,TIMER_ACTIVE);
 #endif   
-  
-#if 0  
-  /* redraw city descriptions
-     should be draw here - under map witgets */
-  show_city_descriptions();
-#endif
 
   /* redraw minimap */
   refresh_ID_background(ID_MINI_MAP_WINDOW);
Index: client/gui-win32/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-win32/mapview.c,v
retrieving revision 1.51
diff -u -r1.51 mapview.c
--- client/gui-win32/mapview.c  2003/01/17 20:20:56     1.51
+++ client/gui-win32/mapview.c  2003/01/23 01:28:47
@@ -178,7 +178,6 @@
           mapstoredc, 0, 0, SRCCOPY);
     SelectObject(mapstoredc, old);
     DeleteDC(mapstoredc);
-    show_city_descriptions();
   }
 } 
 
@@ -711,9 +710,11 @@
   char buffer[500];
   int y_offset;
   HDC hdc;
+  HBITMAP old;
 
   /* TODO: hdc should be stored statically */
-  hdc = GetDC(map_window);
+  hdc = CreateCompatibleDC(NULL);
+  old = SelectObject(hdc, mapstorebitmap);
   SetBkMode(hdc,TRANSPARENT);
 
   y_offset = canvas_y + NORMAL_TILE_HEIGHT;
@@ -763,7 +764,8 @@
     DrawText(hdc, buffer, strlen(buffer), &rc, DT_NOCLIP | DT_CENTER);
   }
 
-  ReleaseDC(map_window, hdc);
+  SelectObject(hdc, old);
+  DeleteDC(hdc);
 }
 
 /**************************************************************************
Index: client/gui-xaw/mapview.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/gui-xaw/mapview.c,v
retrieving revision 1.118
diff -u -r1.118 mapview.c
--- client/gui-xaw/mapview.c    2003/01/17 20:20:57     1.118
+++ client/gui-xaw/mapview.c    2003/01/23 01:28:48
@@ -571,7 +571,6 @@
                event->xexpose.x, event->xexpose.y,
                event->xexpose.width, event->xexpose.height,
                event->xexpose.x, event->xexpose.y);
-      show_city_descriptions();
     }
   }
   refresh_overview_canvas();
@@ -661,16 +660,15 @@
                                 enum color_std shadow,
                                 int x, int y, const char *string)
 {
-  Window wndw = XtWindow(map_canvas);
   size_t len = strlen(string);
 
   y += font->ascent;
 
   XSetForeground(display, font_gc, colors_standard[shadow]);
-  XDrawString(display, wndw, font_gc, x + 1, y + 1, string, len);
+  XDrawString(display, map_canvas_store, font_gc, x + 1, y + 1, string, len);
 
   XSetForeground(display, font_gc, colors_standard[foreground]);
-  XDrawString(display, wndw, font_gc, x, y, string, len);
+  XDrawString(display, map_canvas_store, font_gc, x, y, string, len);
 }
 
 /**************************************************************************