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

From: Stephen Berman <[email-masked]>
Date: Tue, 25 Feb 2025 16:48:55 +0100

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

>> 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.

Thread 1 "emacs" hit Breakpoint 3, redisplay_window (window=XIL(0x555555c826c5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:21195
21195	  if (CHARPOS (opoint) < BEGV)
(gdb) p w->start
$1 = XIL(0x555555d2782d)
(gdb) pr $1
#<marker at 793 in smple>
(gdb) p PT
$2 = 793
(gdb) p ZV
$3 = 793
(gdb) n
[Thread 0x7fffde16e6c0 (LWP 28706) exited]
21197	  else if (CHARPOS (opoint) > ZV)
(gdb)
21199	  else if (ochars_modiff == CHARS_MODIFF)
(gdb)
21200	    TEMP_SET_PT_BOTH (CHARPOS (opoint), BYTEPOS (opoint));
(gdb)
21211	  set_buffer_internal_1 (old);
(gdb) p PT
$4 = 793
(gdb) n
21214	  if (CHARPOS (lpoint) <= ZV)
(gdb)
21216	      if (lchars_modiff == CHARS_MODIFF)
(gdb)
21217		TEMP_SET_PT_BOTH (CHARPOS (lpoint), BYTEPOS (lpoint));
(gdb)
21222	  unbind_to (count, Qnil);
(gdb) p PT
$5 = 793

> 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?

(gdb) break xdisp.c:20081
Breakpoint 4 at 0x5555555f8ae8: file /home/steve/src/emacs/emacs-master/src/xdisp.c, line 20081.
(gdb) c
Continuing.

Thread 1 "emacs" hit Breakpoint 4, redisplay_window (window=XIL(0x555555c828e5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:20081
20081	  if (!just_this_one_p && needs_no_redisplay (w))
(gdb) p !just_this_one_p
$6 = 1
(gdb) p needs_no_redisplay (w)
$7 = false
(gdb) p REDISPLAY_SOME_P ()
$8 = 0
(gdb) p !w->redisplay
$9 = 0
(gdb) p !w->update_mode_line
$10 = 0
(gdb) p !f->face_change
$11 = 1
(gdb) p !f->redisplay
$12 = 0
(gdb) p !buffer->text->redisplay
$13 = 0
(gdb) p window_point (w) == w->last_point
$14 = 0

> 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;
>     }

(gdb) break xdisp.c:20520
Breakpoint 5 at 0x5555555f92ea: file /home/steve/src/emacs/emacs-master/src/xdisp.c, line 20520.
(gdb) c
Continuing.

Thread 1 "emacs" hit Breakpoint 3, redisplay_window (window=XIL(0x555555c828e5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:21195
21195	  if (CHARPOS (opoint) < BEGV)
(gdb) c
Continuing.

Thread 1 "emacs" hit Breakpoint 4, redisplay_window (window=XIL(0x555555c826c5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:20081
20081	  if (!just_this_one_p && needs_no_redisplay (w))
(gdb)
Continuing.

Thread 1 "emacs" hit Breakpoint 5, redisplay_window (window=XIL(0x555555c826c5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:20520
20520	  if (current_matrix_up_to_date_p
(gdb) p current_matrix_up_to_date_p
$15 = <optimized out>
(gdb) p w->window_end_valid
$16 = false
(gdb) p !current_buffer->clip_changed
$17 = 1
(gdb) p !current_buffer->prevent_redisplay_optimizations_p
$18 = 0
(gdb) p !window_outdated (w)
$19 = 0
(gdb) p !composition_break_at_point
$20 = 1
(gdb) p !hscrolling_current_line_p (w)
$21 = 1
(gdb) n
20540	  else if (w->start_at_line_beg
(gdb) p current_matrix_up_to_date_p
$22 = <optimized out>

> 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.

(gdb) n
20556	  else if ((tem = try_window_id (w)) != 0)
(gdb) p try_window_id (w)
$23 = 0
(gdb) break xdisp.c:22148
Breakpoint 6 at 0x5555555f709a: file /home/steve/src/emacs/emacs-master/src/xdisp.c, line 22149.
(gdb) c
Continuing.

Thread 1 "emacs" hit Breakpoint 6, try_window_id (w=w@entry=0x555555c826c0)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:22149
22149	{
(gdb) n
22150	  struct frame *f = XFRAME (w->frame);
(gdb)
22155	  if (is_tty_root_frame_with_visible_child (f))
(gdb)
22158	  struct glyph_matrix *current_matrix = w->current_matrix;
(gdb)
22159	  struct glyph_matrix *desired_matrix = w->desired_matrix;
(gdb)
22166	  ptrdiff_t delta = 0, delta_bytes = 0, stop_pos;
(gdb)
22191	  SET_TEXT_POS_FROM_MARKER (start, w->start);
(gdb)

22195	  if (MINI_WINDOW_P (w))
(gdb) 22199	  if (windows_or_buffers_changed || f->cursor_type_changed)
(gdb)
redisplay_window (window=XIL(0x555555c826c5),
    just_this_one_p=just_this_one_p@entry=false)
    at /home/steve/src/emacs/emacs-master/src/xdisp.c:20570
20570	  else if (CHARPOS (startp) >= BEGV

>> 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?

(gdb) p CHARPOS (startp) >= BEGV
$24 = 1
(gdb) p CHARPOS (startp) <= ZV
$25 = 1
(gdb) p PT >= CHARPOS (startp)
$26 = 1
(gdb) p CHARPOS (startp) < ZV
$27 = 0
(gdb) p CHARPOS (startp) == BEGV
$28 = 0
(gdb) p !window_outdated (w)
$29 = 0

>> 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.

Does the trace answer your questions?  If so, can you connect the dots
for me, because I still don't see why the recentering happens.  If not,
can you tell what's still missing?

Steve Berman