Subject: bug#10304: 24.0.92: display bug

From: Eli Zaretskii <[email-masked]>
Date: Sun, 13 Jan 2013 20:08:32 +0200

> Date: Tue, 08 Jan 2013 07:43:50 +0200
> From: Eli Zaretskii <[email-masked]>
> Cc: [email-masked], [email-masked]
> 
> I think I do understand.  My suspicion is that we somehow fail to
> realize that the screen estate formerly occupied by the image, and
> everything that follows it, needs to be cleared in its entirety.  On
> the display engine level, the image takes just one "line" (called
> "glyph row"), and perhaps we somehow don't realize that the height of
> that "line" is large, and all of that needs to be cleared, not just
> the number of text lines of "normal" height that will replace the
> image on display.
> 
> Thanks for the details, they confirm my suspicions.  I now need to
> find whodunit in the code...

Please apply the patch below and run with it for a while.  It makes
the output of trace-redisplay more voluminous, but I don't see how
else we could catch the problem.  When the problem happens again,
please post the results.

Here's my analysis of what is involved; perhaps it will help you read
the output and make some changes on the spot, if needed.

The clearing of portions of display that no longer display anything is
done as part of dispnew.c:update_window.  It does that as part of the
call to update_window_line, when the latter is passed a screen line
(a.k.a. "row") that should be empty on display.  Such empty lines have
a single glyph (an "invented" blank character with charpos equal to
point-max), whose sole purpose is to facilitate clearing of empty
lines.  These empty lines have the enabled_p flag set, which means
they should be displayed.  (update_window ignores lines whose
enabled_p flag is reset, because these do not correspond to any part
of the displayed text.)

So, for us to fail to clear these empty lines, I see several possible
reasons:

 . the logic in update_window somehow skips the loop that starts at
   line 3495 and which displays the empty lines past the end of the
   buffer; or

 . some redisplay optimization in xdisp.c decides that those parts of
   display do not need to be updated, and thus excludes them from the
   glyph matrix it constructs in w->desired_matrix, so that those
   lines are not updated by update_window; or

 . the function x_clear_end_of_line, which is called by
   update_window_line, or its terminal-specific back-end (e.g.,
   xterm.c:x_clear_frame_area), which implements the meat of that
   function, somehow fail to clear the correct portion of the display.

The patches below are designed to report enough info for us to be able
to tell which of the above hypotheses, if any, is true.

Btw, any idea when these problems started happening?  Is it an old
problem, or did it start to appear only recently?

Thanks.

Here are the patches:

--- src/dispnew.c~0	2013-01-07 14:13:25.000000000 +0200
+++ src/dispnew.c	2013-01-13 14:22:21.549690800 +0200
@@ -3473,6 +3473,11 @@
       while (row < end && !row->enabled_p)
 	++row;
 
+      TRACE ((stderr,
+	      "update_window: first enabled: %d, last: %d, no_scrolling: %d\n",
+	      row - desired_matrix->rows, end - desired_matrix->rows - 1,
+	      desired_matrix->no_scrolling_p));
+
       /* Try reusing part of the display by copying.  */
       if (row < end && !desired_matrix->no_scrolling_p)
 	{
@@ -3481,6 +3486,7 @@
 	    {
 	      /* All rows were found to be equal.  */
 	      paused_p = 0;
+	      TRACE ((stderr, "scrolling_window found all rows equal\n"));
 	      goto set_cursor;
 	    }
 	  else if (rc > 0)
@@ -3488,10 +3494,18 @@
 	      /* We've scrolled the display.  */
 	      force_p = 1;
 	      changed_p = 1;
+	      TRACE ((stderr, "scrolling_window scrolled the display\n"));
 	    }
 	}
 
       /* Update the rest of the lines.  */
+      if (!(row < end && (force_p || !input_pending)))
+	{
+	  TRACE ((stderr,
+		  "NOT updating rest of lines; row = %d end = %d fp = %d ip = %d\n",
+		  row - desired_matrix->rows, end - desired_matrix->rows - 1,
+		  force_p, input_pending));
+	}
       for (; row < end && (force_p || !input_pending); ++row)
 	/* scrolling_window resets the enabled_p flag of the rows it
 	   reuses from current_matrix.  */
@@ -3533,12 +3547,17 @@
 	       tempted to optimize redisplay based on lines displayed
 	       in the first redisplay.  */
 	    if (MATRIX_ROW_BOTTOM_Y (row) >= yb)
-	      for (i = vpos + 1; i < w->current_matrix->nrows - 1; ++i)
-		MATRIX_ROW (w->current_matrix, i)->enabled_p = 0;
+	      {
+		TRACE ((stderr, "update_window marks rows %d - %d invalid\n",
+			vpos + 1, w->current_matrix->nrows - 2));
+		for (i = vpos + 1; i < w->current_matrix->nrows - 1; ++i)
+		  MATRIX_ROW (w->current_matrix, i)->enabled_p = 0;
+	      }
 	  }
 
       /* Was display preempted?  */
       paused_p = row < end;
+      TRACE ((stderr, "update_window %spreempted\n", paused_p ? "" : "NOT "));
 
     set_cursor:

--- src/xdisp.c~0	2013-01-07 14:13:25.000000000 +0200
+++ src/xdisp.c	2013-01-13 16:14:15.697145600 +0200
@@ -25568,6 +25568,9 @@
   from_y = WINDOW_TO_FRAME_PIXEL_Y (w, max (min_y, output_cursor.y));
   to_y = WINDOW_TO_FRAME_PIXEL_Y (w, to_y);
 
+  TRACE ((stderr, "clear_frame_area: [%d - %d] [%d - %d]\n",
+	  from_x, to_x, from_y, to_y));
+
   /* Prevent inadvertently clearing to end of the X window.  */
   if (to_x > from_x && to_y > from_y)
     {