Subject: Re: bug#76186: 31.0.50; (recenter 0) sometimes does not recenter as expected

From: Eli Zaretskii <[email-masked]>
Date: Sun, 23 Feb 2025 09:02:35 +0200

> From: Stephen Berman <[email-masked]>
> Cc: [email-masked],  [email-masked]
> Date: Sat, 22 Feb 2025 22:06:18 +0100
> 
> As you can see, after hitting the window.c breakpoint and stepping over
> set_marker_both, window-start is at 793, i.e. point-max, which is the
> expected position after invoking `(recenter 0)'.  After continuing and
> hitting the xdisp.c breakpoint, I stepped through the code and saw
> w->force_start get set to true, then stepped further until
> redisplay_window_0, then continued and hit the breakpoint again, and now
> w->force_start was false.  Then I stepped further until
> SET_TEXT_POS_FROM_MARKER (startp, w->start), after which window-start
> was now at 671, the value displayed in the message after one iteration,
> indicating the unexpected recentering.  I cannot tell what caused
> window-start to change from 793 to 671.

Thanks.

There are too many unknowns here that need to become knowns.

First, before the first call to redisplay_window returns, it does
this:

  /* Restore current_buffer and value of point in it.  The window
     update may have changed the buffer, so first make sure `opoint'
     is still valid (Bug#6177).  */
  if (CHARPOS (opoint) < BEGV)
    TEMP_SET_PT_BOTH (BEGV, BEGV_BYTE);
  else if (CHARPOS (opoint) > ZV)
    TEMP_SET_PT_BOTH (Z, Z_BYTE);
  else if (ochars_modiff == CHARS_MODIFF)
    TEMP_SET_PT_BOTH (CHARPOS (opoint), BYTEPOS (opoint));
  else
    {
      /* If the buffer was modified while we were redisplaying it, we
         cannot trust the correspondence between character and byte
         positions.  This can happen, for example, if we are
         redisplaying *Messages* and some Lisp, perhaps invoked by
         display_mode_lines, signals an error which caused something
         added/deleted to/from the buffer text.  */
      TEMP_SET_PT_BOTH (CHARPOS (opoint), CHAR_TO_BYTE (CHARPOS (opoint)));
    }
  set_buffer_internal_1 (old);
  /* Avoid an abort in TEMP_SET_PT_BOTH if the buffer has become
     shorter.  This can be caused by log truncation in *Messages*.  */
  if (CHARPOS (lpoint) <= ZV)
    {
      if (lchars_modiff == CHARS_MODIFF)
	TEMP_SET_PT_BOTH (CHARPOS (lpoint), BYTEPOS (lpoint));
      else
	TEMP_SET_PT_BOTH (CHARPOS (lpoint), CHAR_TO_BYTE (CHARPOS (lpoint)));
    }

Your trace indicates that TEMP_SET_PT_BOTH was called twice when
executing this fragment, but you haven't shown the values of point
after each one of them.  It is crucial to understand where point was
after the first call to redisplay_window, because any subsequent
scrolling of the window depends on whether point is fully visible in
the viewport.  So please show the value of point as set by these two
calls to TEMP_SET_PT_BOTH.

Next, at the very beginning of redisplay_window we have this:

  if (!just_this_one_p && needs_no_redisplay (w))
    return;

Thus, the second call to redisplay_window was supposed to return
immediately if these conditions were true.  Since it didn't, and
just_this_one_p is false, we can conclude that needs_no_redisplay
returned false, but which condition inside it caused that?

Next, this:

> 20520	  if (current_matrix_up_to_date_p
> (gdb)
> 20540	  else if (w->start_at_line_beg

indicates that current_matrix_up_to_date_p is false, but why is that?
It is set around line 20160:

  current_matrix_up_to_date_p
    = (w->window_end_valid
       && !current_buffer->clip_changed
       && !current_buffer->prevent_redisplay_optimizations_p
       && !window_outdated (w)
       && !composition_break_at_point
       && !hscrolling_current_line_p (w));

Which one of these causes current_matrix_up_to_date_p to become false?
Or maybe it starts at true, but becomes false below:

  /* When windows_or_buffers_changed is non-zero, we can't rely
     on the window end being valid, so set it to zero there.  */
  if (windows_or_buffers_changed)
    {
      /* If window starts on a continuation line, maybe adjust the
	 window start in case the window's width changed.  */
      if (XMARKER (w->start)->buffer == current_buffer)
	compute_window_start_on_continuation_line (w);

      w->window_end_valid = false;
      /* If so, we also can't rely on current matrix
	 and should not fool try_cursor_movement below.  */
      current_matrix_up_to_date_p = false;
    }

Next,

> (gdb)
> 20556	  else if ((tem = try_window_id (w)) != 0)
> (gdb)

indicates that try_window_id returned zero.  I'd be interested to know
why.

> 20570	  else if (CHARPOS (startp) >= BEGV

This indicates that one of the conditions below

  else if (CHARPOS (startp) >= BEGV
	   && CHARPOS (startp) <= ZV
	   && PT >= CHARPOS (startp)
	   && (CHARPOS (startp) < ZV
	       /* Avoid starting at end of buffer.  */
	       || CHARPOS (startp) == BEGV
	       || !window_outdated (w)))

yields false, but which one?

> 20684	  if ((0 < scroll_conservatively
> (gdb)
> 20732	  init_iterator (&it, w, PT, PT_BYTE, NULL, DEFAULT_FACE_ID);
> (gdb)
> 20733	  it.current_y = it.last_visible_y;
> (gdb)
> 20734	  if (centering_position < 0)

Here we arrive at a place where redisplay_window decided it must
redraw the window by recentering point in the viewport, and that's
what happens afterwards.  So the above questions should explain why
this happens, instead of the expected result: redisplay_window
deciding that the window's display is already up-to-day and needs no
display changes.