Subject: Re: bug#70038: 29.3.50; Shift up/down in buffer with images on M-x other-window with some fonts

From: Eli Zaretskii <[email-masked]>
Date: Sat, 06 Apr 2024 17:08:32 +0300

> Cc: Rahguzar <[email-masked]>, [email-masked], [email-masked]
> Date: Sat, 06 Apr 2024 15:33:59 +0300
> From: Eli Zaretskii <[email-masked]>
> 
> > From: Rahguzar <[email-masked]>
> > Cc: Eli Zaretskii <[email-masked]>, [email-masked]
> > Date: Thu, 28 Mar 2024 18:24:32 +0100
> > 
> > I can also reproduce this now! And vertico mode can be replaced with the
> > builtin icomplete-vertical-mode. So, the following recipe starting with
> > emacs -Q works for me:
> > 
> > 1) Paste
> > (let ((height (/ (* 2 (frame-pixel-height)) 15)))
> >   (icomplete-vertical-mode)
> >   (defun pin-vscroll-down (win)
> >     (set-window-vscroll win (/ height 2) t))
> >   (let ((image1 (create-image "~/Downloads/image1.png" nil nil :height height))
> >         (image2 (create-image "~/Downloads/image2.png" nil nil :height height))
> >         (image3 (create-image "~/Downloads/image3.png" nil nil :height height)))
> >     (with-current-buffer (get-buffer-create "*image-scroll-test*")
> >       (insert " \n \n \n \n \n \n")
> >       (put-image image1 1)
> >       (put-image image2 5)
> >       (put-image image3 9)
> >       ;; With larger image sizes (goto-char 3)
> >       ;; also consistently triggers the problem.
> >       (goto-char 11)
> >       (add-hook 'pre-redisplay-functions #'pin-vscroll-down nil t))
> >     (split-window-right)
> >     (other-window 1)
> >     (switch-to-buffer "*image-scroll-test*")))
> > 
> > into scratch buffer.
> > 
> > 2) Evaluate the form above using `C-M-x`.
> > 
> > 3) Type M-x t
> > 
> > 4) Wait till minibuffer expands to show completions, then type `C-g` to
> > quit minibuffer.
> > 
> > 5) Typing `C-x 0` results in the window with images losing vscroll.
> 
> Po Lu, I'm looking at this part of redisplay_window:
> 
>    force_start:
> 
>     /* Handle case where place to start displaying has been specified,
>        unless the specified location is outside the accessible range.  */
>     if (w->force_start)
>       {
> 	/* We set this later on if we have to adjust point.  */
> 	int new_vpos = -1;
> 
> 	w->force_start = false;
> 
> 	/* The vscroll should be preserved in this case, since
> 	   `pixel-scroll-precision-mode' must continue working normally
> 	   when a mini-window is resized.  (bug#55312) */
> 	if (!w->preserve_vscroll_p || !window_frozen_p (w))  <<<<<<<<<<<<<<<
> 	  w->vscroll = 0;
> 
> 	w->preserve_vscroll_p = false;
> 	w->window_end_valid = false;
> 
> where you added the condition for resetting w->vscroll in commit
> fd8eaa72a61, and I'm thinking that perhaps the condition should be
> 
> 	if (!w->preserve_vscroll_p && !window_frozen_p (w))
> 
> instead?  If not, can you explain why we use OR and not AND there?
> 
> Ramon, if you replace "||" with "&&" in the above condition, does the
> problem go away for you also when you change fonts, as you described
> in https://debbugs.gnu.org/cgi/bugreport.cgi?bug=70038#29 ?

There's one more aspect of this that bothers me: when we resize the
mini-window, we set the frame's frozen_window_starts flag, but we seem
to never reset it.

Martin, can you help out here?  I don't see shrink_mini_window being
called with non-zero DELTA anywhere, including when the mini-window is
exited and is resized to its normal one-line height.  Instead, this
resizing is performed by restore_window_configuration, called from
read_minibuf, but I don't see FRAME_WINDOWS_FROZEN being reset
anywhere there.  I don't think it's correct for us to leave the
frame's frozen_window_starts flag set forever once it was raised, so I
guess we should do something in minibuffer_unwind to reset that flag?