Re: Recenter video

[Eli Zaretskii <eliz@gnu.org>]

> From: Lennart Borgman <lennart.borgman@gmail.com>
> Date: Wed, 23 Jun 2010 18:14:25 +0200
> Cc: deng@randomsample.de, emacs-devel@gnu.org
> 
> > Doesn't your patch cause try_scrolling to be called?  If so, that's a
> > no-no when the window_end_valid flag is nil.
> 
> 
> I don't think so, but since you suspect it I must ask you why you
> think it does that.

As far as I remember, the history of this was as follows:

You found that the clip_changed flag was the one that prevented
try_scrolling from being called, in this part of redisplay_window:

  /* Try to scroll by specified few lines.  */
  if ((scroll_conservatively
       || scroll_step
       || temp_scroll_step
       || NUMBERP (current_buffer->scroll_up_aggressively)
       || NUMBERP (current_buffer->scroll_down_aggressively))
      && !current_buffer->clip_changed
      && CHARPOS (startp) >= BEGV
      && CHARPOS (startp) <= ZV)
    {
      /* The function returns -1 if new fonts were loaded, 1 if
	 successful, 0 if not successful.  */
      int rc = try_scrolling (window, just_this_one_p,
			      scroll_conservatively,
			      scroll_step,
			      temp_scroll_step, last_line_misfit);

(When try_scrolling cannot be called, or when it is called and reports
a failure, Emacs falls back to recentering around point.)

The clip_changed flag was supposed to be reset by
reconsider_clip_changes, but it wasn't reset because of the condition
indicated below:

  if (b->clip_changed
	   && !NILP (w->window_end_valid)  <<<<<<<<<<<<<<<<<
	   && w->current_matrix->buffer == b
	   && w->current_matrix->zv == BUF_ZV (b)
	   && w->current_matrix->begv == BUF_BEGV (b))

Your patch tweaks the clip_changed flag so that it is now not set, and
the obstacle for calling try_scrolling is thereby removed.  But the
window_end_valid flag is still nil, because your patch does nothing to
change its value.  Calling try_scrolling when window_end_valid is nil
is not allowed, because try_scrolling reuses the current glyph matrix,
which is only guaranteed to be up to date when window_end_valid is
non-nil.

To check the correctness of my description above, please put a
breakpoint inside try_scrolling conditioned by w->window_end_valid
being equal to Qnil, then scroll by leaning on the down arrow, and see
if the breakpoint ever breaks.  If it does break, your patch violates
the redisplay protocol regarding reuse of the current matrix.