[Freeciv-Dev] (PR#8964) goto line bugs

["Jason Short" <jdorje@xxxxxxxxxxxxxxxxxxxxx>]

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

There are at least one and probably two bugs with the drawing of goto lines.

Assume we're using classic view.  The same problems _may_ occur in 
iso-view, but perhaps not since nearby tiles tend to get redrawn 
unnecessarily in this view.


1.  If a goto line is drawn from (x,y) to (x+1,y+1) a canvas line is put 
on top.  This covers parts of the (x,y) tile and the (x+1,y+1) tile. 
However it also covers some parts of the (x,y+1) and (x+1,y) tile.  So 
when this goto line is erased at least one of these tiles must be redrawn.

In undraw_segment() there actually is code to handle this, but it 
doesn't seem to work.  I don't know why only 2 directions are handled 
when it seems clear that 4 directions have this problem.

This bug is easy to reproduce in cvs.  Just launch trident and move a 
goto line around.  Look very closely and you can see the occasional 
single pixel of the goto line left in place.  This does not depend on 
PR#8945; it is probably an old bug.


2.  The second bug is very similar but harder to reproduce.  Again 
consider the diagonal goto line from (x,y) to (x+1,y+1).

Now suppose while this goto line is still "active", something happens at 
the (x,y+1) tile which contains part of the canvas line.  We redraw the 
entirety of this tile.  However the goto line is not a part of this tile 
so it is not redrawn; a single pixel of it gets overwritten.

This bug is much harder to reproduce; I haven't been able to do it.  I 
believe you'd have to have multiple computers (or multiple viewports) to 
do it, so that you could set up a goto line on one computer and then 
(without moving the mouse on that computer) have another computer move a 
unit into the tile.


I believe the attached patch fixes both problems.  Note that one place 
should use map_to_gui_vector() when it is written.

jason

Index: client/mapview_common.c
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.c,v
retrieving revision 1.124
diff -u -r1.124 mapview_common.c
--- client/mapview_common.c     12 Jun 2004 06:38:29 -0000      1.124
+++ client/mapview_common.c     12 Jun 2004 07:05:20 -0000
@@ -1430,8 +1430,14 @@
   }
 
   /* Draw the goto lines on top of the whole thing. This is done last as
-   * we want it completely on top. */
-  gui_rect_iterate(gui_x0, gui_y0, width, height, map_x, map_y, draw) {
+   * we want it completely on top.
+   *
+   * Note that a pixel right on the border of a tile may actually contain a
+   * goto line from an adjacent tile.  Thus we draw any extra goto lines
+   * from adjacent tiles (if they're close enough). */
+  gui_rect_iterate(gui_x0 - GOTO_WIDTH, gui_y0 - GOTO_WIDTH,
+                  width + 2 * GOTO_WIDTH, height + 2 * GOTO_WIDTH,
+                  map_x, map_y, draw) {
     if (((draw & D_B) || (draw & D_M))
        && normalize_map_pos(&map_x, &map_y)) {
       adjc_dir_iterate(map_x, map_y, adjc_x, adjc_y, dir) {
@@ -1733,7 +1739,7 @@
 **************************************************************************/
 void undraw_segment(int src_x, int src_y, int dir)
 {
-  int dest_x, dest_y;
+  int dest_x, dest_y, canvas_x, canvas_y, canvas_dx, canvas_dy;
 
   assert(!is_drawn_line(src_x, src_y, dir));
 
@@ -1741,26 +1747,23 @@
     assert(0);
   }
 
-  refresh_tile_mapcanvas(src_x, src_y, FALSE);
-  refresh_tile_mapcanvas(dest_x, dest_y, FALSE);
-
-  if (!is_isometric) {
-    if (NORMAL_TILE_WIDTH % 2 == 0 || NORMAL_TILE_HEIGHT % 2 == 0) {
-      if (dir == DIR8_NORTHEAST) {
-       /* Since the tile doesn't have a middle we draw an extra pixel
-        * on the adjacent tile when drawing in this direction. */
-       if (!MAPSTEP(dest_x, dest_y, src_x, src_y, DIR8_EAST)) {
-         assert(0);
-       }
-       refresh_tile_mapcanvas(dest_x, dest_y, FALSE);
-      } else if (dir == DIR8_SOUTHWEST) {      /* the same */
-       if (!MAPSTEP(dest_x, dest_y, src_x, src_y, DIR8_SOUTH)) {
-         assert(0);
-       }
-       refresh_tile_mapcanvas(dest_x, dest_y, FALSE);
-      }
-    }
+  /* Note that if source and dest tiles are not adjacent (because the
+   * mapview wraps around) this will not give the correct behavior.  This is
+   * consistent with the current design which fails when the size of the
+   * mapview approaches the size of the map. */
+  (void) map_to_canvas_pos(&canvas_x, &canvas_y, src_x, src_y);
+  if (is_isometric) {
+    canvas_dx = (DIR_DX[dir] - DIR_DY[dir]) * NORMAL_TILE_WIDTH / 2;
+    canvas_dy = (DIR_DX[dir] + DIR_DY[dir]) * NORMAL_TILE_HEIGHT / 2;
+  } else {
+    canvas_dx = NORMAL_TILE_WIDTH * DIR_DX[dir];
+    canvas_dy = NORMAL_TILE_HEIGHT * DIR_DY[dir];
   }
+
+  update_map_canvas(MIN(canvas_x, canvas_x + canvas_dx),
+                   MIN(canvas_y, canvas_y + canvas_dy),
+                   abs(canvas_dx) + NORMAL_TILE_WIDTH,
+                   abs(canvas_dy) + NORMAL_TILE_HEIGHT);
 }
 
 /****************************************************************************
Index: client/mapview_common.h
===================================================================
RCS file: /home/freeciv/CVS/freeciv/client/mapview_common.h,v
retrieving revision 1.66
diff -u -r1.66 mapview_common.h
--- client/mapview_common.h     10 Jun 2004 01:04:52 -0000      1.66
+++ client/mapview_common.h     12 Jun 2004 07:05:20 -0000
@@ -128,6 +128,7 @@
 #define D_FULL D_TMB_LR
 
 #define BORDER_WIDTH 2
+#define GOTO_WIDTH 2
 
 enum update_type {
   /* Masks */