[Freeciv-Dev] Re: (PR#4004) A canvas_iterate macro for mapview

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

Jason Short wrote:
> <URL: http://rt.freeciv.org/Ticket/Display.html?id=4004 >
> 
>>[jdorje - Mon Apr 14 17:31:56 2003]:
>>
>>I would like to write a new macro, canvas_iterate, to the mapview code. 
> 
> Here's a preliminary patch.  It has just one user; not really enough for
> full debugging.

And here's a bigger better patch.

- Updated to CVS.
- Fix some bugs in the macro.
- Add more users.

Only the macro in mapview_common.h is really intended for review here. 
The other code is just examples of how it can be used to simplify and 
improve drawing code.  Which it certainly does!

I think this is about ready for inclusion.  Ross and Greg, please take a 
look at it.

jason

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.104
diff -u -r1.104 mapview_common.c
--- client/mapview_common.c     23 Apr 2004 16:43:13 -0000      1.104
+++ client/mapview_common.c     23 Apr 2004 17:42:23 -0000
@@ -56,13 +56,16 @@
 **************************************************************************/
 void refresh_tile_mapcanvas(int x, int y, bool write_to_screen)
 {
+  int canvas_x, canvas_y;
+
   assert(is_real_map_pos(x, y));
   if (!normalize_map_pos(&x, &y)) {
     return;
   }
 
-  if (tile_visible_mapcanvas(x, y)) {
-    update_map_canvas(x, y, 1, 1, FALSE);
+  if (map_to_canvas_pos(&canvas_x, &canvas_y, x, y)) {
+    canvas_y += NORMAL_TILE_HEIGHT - UNIT_TILE_HEIGHT;
+    update_map_canvas(canvas_x, canvas_y, UNIT_TILE_WIDTH, UNIT_TILE_HEIGHT);
 
     if (update_city_text_in_refresh_tile
        && (draw_city_names || draw_city_productions)) {
@@ -1261,132 +1264,43 @@
   x, y, width, and height are in map coordinates; they need not be
   normalized or even real.
 **************************************************************************/
-void update_map_canvas(int x, int y, int width, int height, 
-                      bool write_to_screen)
+void update_map_canvas(int canvas_x, int canvas_y, int width, int height)
 {
-  int canvas_start_x, canvas_start_y;
+  const int gui_x = mapview_canvas.gui_x0 + canvas_x;
+  const int gui_y = mapview_canvas.gui_y0 + canvas_y;
 
   freelog(LOG_DEBUG,
-         "update_map_canvas(pos=(%d,%d), size=(%d,%d), write_to_screen=%d)",
-         x, y, width, height, write_to_screen);
+         "update_map_canvas(pos=(%d,%d), size=(%d,%d))",
+         canvas_x, canvas_y, width, height);
 
   if (is_isometric) {
-    int x_itr, y_itr, i;
-
-    /* First refresh the tiles above the area to remove the old tiles'
-     * overlapping graphics. */
-    put_tile_iso(x - 1, y - 1, D_B_LR); /* top_left corner */
-
-    for (i = 0; i < height - 1; i++) { /* left side - last tile. */
-      put_tile_iso(x - 1, y + i, D_MB_LR);
-    }
-    put_tile_iso(x - 1, y + height - 1, D_TMB_R); /* last tile left side. */
-
-    for (i = 0; i < width - 1; i++) {
-      /* top side */
-      put_tile_iso(x + i, y - 1, D_MB_LR);
-    }
-    if (width > 1) {
-      /* last tile top side. */
-      put_tile_iso(x + width - 1, y - 1, D_TMB_L);
-    } else {
-      put_tile_iso(x + width - 1, y - 1, D_MB_L);
-    }
-
-    /* Now draw the tiles to be refreshed, from the top down to get the
-     * overlapping areas correct. */
-    for (x_itr = x; x_itr < x + width; x_itr++) {
-      for (y_itr = y; y_itr < y + height; y_itr++) {
-       put_tile_iso(x_itr, y_itr, D_FULL);
-      }
-    }
-
-    /* Then draw the tiles underneath to refresh the parts of them that
-     * overlap onto the area just drawn. */
-    put_tile_iso(x, y + height, D_TM_R);  /* bottom side */
-    for (i = 1; i < width; i++) {
-      int x1 = x + i;
-      int y1 = y + height;
-      put_tile_iso(x1, y1, D_TM_R);
-      put_tile_iso(x1, y1, D_T_L);
-    }
-
-    put_tile_iso(x + width, y, D_TM_L); /* right side */
-    for (i=1; i < height; i++) {
-      int x1 = x + width;
-      int y1 = y + i;
-      put_tile_iso(x1, y1, D_TM_L);
-      put_tile_iso(x1, y1, D_T_R);
-    }
-
-    put_tile_iso(x + width, y + height, D_T_LR); /* right-bottom corner */
-
+    gui_rect_iterate(gui_x, gui_y, width, height, map_x, map_y, draw) {
+      put_tile_iso(map_x, map_y, draw);
+    } gui_rect_iterate_end;
 
     /* Draw the goto lines on top of the whole thing. This is done last as
      * we want it completely on top. */
-    for (x_itr = x - 1; x_itr <= x + width; x_itr++) {
-      for (y_itr = y - 1; y_itr <= y + height; y_itr++) {
-       int x1 = x_itr;
-       int y1 = y_itr;
-       if (normalize_map_pos(&x1, &y1)) {
-         adjc_dir_iterate(x1, y1, x2, y2, dir) {
-           if (get_drawn(x1, y1, dir)) {
-             draw_segment(x1, y1, dir);
-           }
-         } adjc_dir_iterate_end;
-       }
+    gui_rect_iterate(gui_x, gui_y, width, height, x1, y1, draw) {
+      if (normalize_map_pos(&x1, &y1)) {
+       adjc_dir_iterate(x1, y1, x2, y2, dir) {
+         if (get_drawn(x1, y1, dir)) {
+           draw_segment(x1, y1, dir);
+         }
+       } adjc_dir_iterate_end;
       }
-    }
-
-
-    /* Lastly draw our changes to the screen. */
-    /* top left corner */
-    map_to_canvas_pos(&canvas_start_x, &canvas_start_y, x, y);
-
-    /* top left corner in isometric view */
-    canvas_start_x -= height * NORMAL_TILE_WIDTH / 2;
-
-    /* because of where get_canvas_xy() sets canvas_x */
-    canvas_start_x += NORMAL_TILE_WIDTH / 2;
-
-    /* And because units fill a little extra */
-    canvas_start_y += NORMAL_TILE_HEIGHT - UNIT_TILE_HEIGHT;
-
-    /* Here we draw a rectangle that includes the updated tiles.  This
-     * method can fail if the area wraps off one side of the screen and
-     * back to the other. */
-    dirty_rect(canvas_start_x, canvas_start_y,
-              (height + width) * NORMAL_TILE_WIDTH / 2,
-              (height + width) * NORMAL_TILE_HEIGHT / 2
-              + NORMAL_TILE_HEIGHT / 2);
+    } gui_rect_iterate_end;
   } else {
     /* not isometric */
-    int map_x, map_y;
-
-    for (map_y = y; map_y < y + height; map_y++) {
-      for (map_x = x; map_x < x + width; map_x++) {
-       /*
-        * We don't normalize until later because we want to draw
-        * black tiles for unreal positions.
-        */
-       put_tile(map_x, map_y);
-      }
-    }
-    /* Here we draw a rectangle that includes the updated tiles.  This
-     * method can fail if the area wraps off one side of the screen and
-     * back to the other. */
-    map_to_canvas_pos(&canvas_start_x, &canvas_start_y, x, y);
-    dirty_rect(canvas_start_x, canvas_start_y,
-              width * NORMAL_TILE_WIDTH,
-              height * NORMAL_TILE_HEIGHT);
+    gui_rect_iterate(gui_x, gui_y, width, height, map_x, map_y, draw) {
+      /*
+       * We don't normalize until later because we want to draw
+       * black tiles for unreal positions.
+       */
+      put_tile(map_x, map_y);
+    } gui_rect_iterate_end;
   }
 
-  if (write_to_screen) {
-    /* We never want a partial flush; that would leave the screen in an
-     * inconsistent state.  If the caller tells us to write_to_screen we
-     * simply flush everything immediately. */
-    flush_dirty();
-  }
+  dirty_rect(canvas_x, canvas_y, width, height);
 }
 
 /**************************************************************************
@@ -1394,8 +1308,6 @@
 **************************************************************************/
 void update_map_canvas_visible(void)
 {
-  int map_x0, map_y0;
-
   dirty_all();
 
   /* Clear the entire mapview.  This is necessary since if the mapview is
@@ -1407,22 +1319,7 @@
   canvas_put_rectangle(mapview_canvas.store, COLOR_STD_BLACK,
                       0, 0, mapview_canvas.width, mapview_canvas.height);
 
-  canvas_to_map_pos(&map_x0, &map_y0, 0, 0);
-  if (is_isometric) {
-    /* just find a big rectangle that includes the whole visible area. The
-       invisible tiles will not be drawn. */
-    int width, height;
-
-    width = mapview_canvas.tile_width + mapview_canvas.tile_height + 2;
-    height = width;
-    update_map_canvas(map_x0 - 1, map_y0 - mapview_canvas.tile_width - 1,
-                     width, height, FALSE);
-  } else {
-    update_map_canvas(map_x0, map_y0,
-                     mapview_canvas.tile_width + 1,
-                     mapview_canvas.tile_height + 1,
-                     FALSE);
-  }
+  update_map_canvas(0, 0, mapview_canvas.width, mapview_canvas.height);
 
   show_city_descriptions();
 }
@@ -1433,7 +1330,6 @@
 void show_city_descriptions(void)
 {
   int canvas_x, canvas_y;
-  int map_x0, map_y0;
 
   if (!draw_city_names && !draw_city_productions) {
     return;
@@ -1441,43 +1337,16 @@
 
   prepare_show_city_descriptions();
 
-  canvas_to_map_pos(&map_x0, &map_y0, 0, 0);
-  if (is_isometric) {
-    int w, h;
-
-    for (h = -1; h < mapview_canvas.tile_height * 2; h++) {
-      int x_base = map_x0 + h / 2 + (h != -1 ? h % 2 : 0);
-      int y_base = map_y0 + h / 2 + (h == -1 ? -1 : 0);
-
-      for (w = 0; w <= mapview_canvas.tile_width; w++) {
-       int x = x_base + w;
-       int y = y_base - w;
-       struct city *pcity;
-
-       if (normalize_map_pos(&x, &y)
-           && (pcity = map_get_city(x, y))) {
-         map_to_canvas_pos(&canvas_x, &canvas_y, x, y);
-         show_city_desc(pcity, canvas_x, canvas_y);
-       }
-      }
-    }
-  } else {                     /* is_isometric */
-    int x1, y1;
-
-    for (x1 = 0; x1 <= mapview_canvas.tile_width; x1++) {
-      for (y1 = 0; y1 <= mapview_canvas.tile_height; y1++) {
-       int x = map_x0 + x1;
-       int y = map_y0 + y1;
-       struct city *pcity;
-
-       if (normalize_map_pos(&x, &y)
-           && (pcity = map_get_city(x, y))) {
-         map_to_canvas_pos(&canvas_x, &canvas_y, x, y);
-         show_city_desc(pcity, canvas_x, canvas_y);
-       }
-      }
+  gui_rect_iterate(mapview_canvas.gui_x0, mapview_canvas.gui_y0,
+                  mapview_canvas.width, mapview_canvas.height, x, y, draw) {
+    struct city *pcity;
+
+    if (normalize_map_pos(&x, &y)
+       && (pcity = map_get_city(x, y))) {
+      map_to_canvas_pos(&canvas_x, &canvas_y, x, y);
+      show_city_desc(pcity, canvas_x, canvas_y);
     }
-  }
+  } gui_rect_iterate_end;
 }
 
 /****************************************************************************
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.58
diff -u -r1.58 mapview_common.h
--- client/mapview_common.h     23 Apr 2004 16:43:13 -0000      1.58
+++ client/mapview_common.h     23 Apr 2004 17:42:23 -0000
@@ -136,6 +136,63 @@
   UPDATE_MAP_CANVAS_VISIBLE = 2
 };
 
+#define gui_rect_iterate(gui_x0, gui_y0, width, height, map_x, map_y, draw) \
+{                                                                          \
+  int W = (is_isometric ? (NORMAL_TILE_WIDTH / 2) : NORMAL_TILE_WIDTH);        
    \
+  int H = (is_isometric ? (NORMAL_TILE_HEIGHT / 2) : NORMAL_TILE_HEIGHT);   \
+  int GRI_x0 = DIVIDE((gui_x0), W), GRI_y0 = DIVIDE((gui_y0), H);          \
+  int GRI_x1 = DIVIDE((gui_x0) + (width) + W - 1, W);                      \
+  int GRI_y1 = DIVIDE((gui_y0) + (height) + H - 1, H);                     \
+  int GRI_itr, GRI_x_itr, GRI_y_itr, map_x, map_y;                         \
+  enum draw_type draw;                                                     \
+  int count;                                                               \
+  assert((width) > 0 && (height) > 0);                                     \
+  if (is_isometric) {                                                      \
+    /* Extra half-tile of UNIT_TILE_HEIGHT. */                             \
+    GRI_y1++;                                                              \
+    /* Tiles to the left/above overlap with us. */                         \
+    GRI_x0--;                                                              \
+    GRI_y0--;                                                              \
+  }                                                                        \
+  count = (GRI_x1 - GRI_x0) * (GRI_y1 - GRI_y0);                           \
+  for (GRI_itr = 0; GRI_itr < count; GRI_itr++) {                          \
+    GRI_x_itr = GRI_x0 + (GRI_itr % (GRI_x1 - GRI_x0));                        
    \
+    GRI_y_itr = GRI_y0 + (GRI_itr / (GRI_x1 - GRI_x0));                        
    \
+    if (is_isometric) {                                                        
    \
+      if ((GRI_x_itr + GRI_y_itr) % 2 != 0) {                              \
+       continue;                                                           \
+      }                                                                        
    \
+      draw = 0;                                                                
    \
+      if (GRI_x_itr > GRI_x0) {                                                
    \
+       draw |= D_L;                                                        \
+      }                                                                        
    \
+      if (GRI_x_itr < GRI_x1 - 1) {                                        \
+       draw |= D_R;                                                        \
+      }                                                                        
    \
+      if (GRI_y_itr > GRI_y0) {                                                
    \
+       draw |= D_M;                                                        \
+      }                                                                        
    \
+      if (GRI_y_itr > GRI_y0 + 1) {                                        \
+       draw |= D_T;                                                        \
+      }                                                                        
    \
+      if (GRI_y_itr < GRI_y1 - 2) {                                        \
+       draw |= D_B;                                                        \
+      }                                                                        
    \
+      assert((draw & (D_L | D_R)) && (draw & (D_T | D_M | D_B)));          \
+      assert((draw & D_M) || !((draw & D_T) && (draw & D_B)));             \
+      map_x = (GRI_x_itr + GRI_y_itr) / 2;                                 \
+      map_y = (GRI_y_itr - GRI_x_itr) / 2;                                 \
+    } else {                                                               \
+      draw = D_FULL;                                                       \
+      map_x = GRI_x_itr;                                                   \
+      map_y = GRI_y_itr;                                                   \
+    }
+    
+
+#define gui_rect_iterate_end                                               \
+  }                                                                        \
+}
+
 void refresh_tile_mapcanvas(int x, int y, bool write_to_screen);
 enum color_std get_grid_color(int x1, int y1, int x2, int y2);
 
@@ -181,8 +238,7 @@
                    int canvas_x, int canvas_y,
                    enum draw_type draw, bool citymode);
 
-void update_map_canvas(int x, int y, int width, int height,
-                      bool write_to_screen);
+void update_map_canvas(int canvas_x, int canvas_y, int width, int height);
 void update_map_canvas_visible(void);
 
 void show_city_descriptions(void);
Index: client/packhand.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/packhand.c,v
retrieving revision 1.362
diff -u -r1.362 packhand.c
--- client/packhand.c   14 Apr 2004 17:18:36 -0000      1.362
+++ client/packhand.c   23 Apr 2004 17:42:25 -0000
@@ -579,10 +579,7 @@
   if ((draw_map_grid || draw_borders) && can_client_change_view()) {
     /* We have to make sure we update any workers on the map grid, then
      * redraw the city descriptions on top of them. */
-    update_map_canvas(pcity->x - CITY_MAP_SIZE / 2,
-                     pcity->y - CITY_MAP_SIZE / 2,
-                     CITY_MAP_SIZE, CITY_MAP_SIZE, FALSE);
-    queue_mapview_update(UPDATE_CITY_DESCRIPTIONS);
+    queue_mapview_update(UPDATE_MAP_CANVAS_VISIBLE);
   } else {
     refresh_tile_mapcanvas(pcity->x, pcity->y, FALSE);
   }