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

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

[Ramon Diaz-Uriarte]

[-- Attachment #1: Type: text/plain, Size: 1824 bytes --]


With some fonts, changing focus (M-x other-window) from a buffer with images, makes the content in the buffer with images to shift up and down.

I am seeing this in Debian, with both the GTK and Lucid builds, under X11.

The code below reproduces the problem. This problem has been identified while debugging a change on focus issue with pdf-tools (https://github.com/vedang/pdf-tools/pull/224#issuecomment-2014151358 and ff.)


How to reproduce

- Copy the images to /tmp  (or place there three reasonably sized images, named image1.png, image2.png, image3.png)

- emacs -Q

- eval the following code

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
;; Place images in /tmp
(progn
  (defun pin-vscroll-down (win)
    (set-window-vscroll win 200 t))
  ;; Any of the following leads to the bug
  (set-frame-font "JuliaMono" nil t)
  ;; (set-frame-font "DM Mono" nil t)
  ;; (set-frame-font "Intel One Mono" nil t)
  (let* ((height (/ (* 2 (frame-pixel-height)) 15))
         (image1 (create-image "/tmp/image1.png" nil nil :height height))
         (image2 (create-image "/tmp/image2.png" nil nil :height height))
         (image3 (create-image "/tmp/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 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*")))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

- M-x other-window

- Notice how the images on the buffer on the right move up and down.



[-- Attachment #2: image1.png --]
[-- Type: image/png, Size: 20327 bytes --]

[-- Attachment #3: image2.png --]
[-- Type: image/png, Size: 22369 bytes --]

[-- Attachment #4: image3.png --]
[-- Type: image/png, Size: 20595 bytes --]

[-- Attachment #5: Type: text/plain, Size: 11753 bytes --]



In GNU Emacs 29.3.50 (build 1, x86_64-pc-linux-gnu, GTK+ Version
 3.24.38, cairo version 1.18.0) of 2024-03-26 built on Phelsuma
Repository revision: 38faacf353fb4c8efb027019a4619a386edfe62c
Repository branch: emacs-29
Windowing system distributor 'The X.Org Foundation', version 11.0.12101008
System Description: Debian GNU/Linux trixie/sid

Configured using:
 'configure --disable-silent-rules --with-native-compilation=aot
 --with-json --with-xwidgets --without-xaw3d --with-x-toolkit=gtk3
 --with-xinput2 --with-tree-sitter
 --prefix=/home/ramon/Sources/emacs29-bin 'CFLAGS=-g -O2 -mtune=native
 -march=native' 'CC=ccache gcc''

Configured features:
ACL CAIRO DBUS FREETYPE GIF GLIB GMP GNUTLS GPM GSETTINGS HARFBUZZ JPEG
JSON LCMS2 LIBOTF LIBSELINUX LIBSYSTEMD LIBXML2 M17N_FLT MODULES
NATIVE_COMP NOTIFY INOTIFY PDUMPER PNG RSVG SECCOMP SOUND SQLITE3
THREADS TIFF TOOLKIT_SCROLL_BARS TREE_SITTER WEBP X11 XDBE XIM XINPUT2
XPM XWIDGETS GTK3 ZLIB

Important settings:
  value of $LANG: en_GB.UTF-8
  value of $XMODIFIERS: @im=ibus
  locale-coding-system: utf-8-unix

Major mode: Lisp Interaction

Minor modes in effect:
  dogears-mode: t
  global-jinx-mode: t
  jinx-mode: t
  company-quickhelp-mode: t
  company-quickhelp-local-mode: t
  hideshowvis-minor-mode: t
  rainbow-delimiters-mode: t
  outli-mode: t
  outline-minor-mode: t
  symbol-overlay-mode: t
  display-fill-column-indicator-mode: t
  hl-line-mode: t
  display-line-numbers-mode: t
  company-tng-mode: t
  global-company-mode: t
  company-mode: t
  pixel-scroll-precision-mode: t
  which-function-mode: t
  recentf-mode: t
  vertico-indexed-mode: t
  vertico-prescient-mode: t
  prescient-persist-mode: t
  vertico-multiform-mode: t
  vertico-mode: t
  marginalia-mode: t
  helm-ff-icon-mode: t
  shell-dirtrack-mode: t
  helm-autoresize-mode: 1
  async-bytecomp-package-mode: t
  pulsar-global-mode: t
  pulsar-mode: t
  global-hl-todo-mode: t
  hl-todo-mode: t
  global-aggressive-indent-mode: t
  aggressive-indent-mode: t
  shackle-mode: t
  winner-mode: t
  global-anzu-mode: t
  anzu-mode: t
  savehist-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  electric-pair-mode: t
  which-key-mode: t
  minibuffer-depth-indicate-mode: t
  global-auto-revert-mode: t
  override-global-mode: t
  gcmh-mode: t
  tooltip-mode: t
  global-eldoc-mode: t
  eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  file-name-shadow-mode: t
  context-menu-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  blink-cursor-mode: t
  column-number-mode: t
  line-number-mode: t
  indent-tabs-mode: t
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t
  hs-minor-mode: t

Load-path shadows:
/home/ramon/.emacs.d/elpa/ef-themes-1.5.1/theme-loaddefs hides /home/ramon/.emacs.d/elpa/modus-themes-20240303.1023/theme-loaddefs
/home/ramon/.emacs.d/elpa/emacsql-sqlite-builtin-20240119.2314/emacsql-sqlite-builtin hides /home/ramon/.emacs.d/elpa/emacsql-20240124.1601/emacsql-sqlite-builtin
/home/ramon/.emacs.d/elpa/transient-20240226.2332/transient hides /home/ramon/Sources/emacs29-bin/share/emacs/29.3.50/lisp/transient
/home/ramon/.emacs.d/elpa/ef-themes-1.5.1/theme-loaddefs hides /home/ramon/Sources/emacs29-bin/share/emacs/29.3.50/lisp/theme-loaddefs
/home/ramon/.emacs.d/elpa/eldoc-1.15.0/eldoc hides /home/ramon/Sources/emacs29-bin/share/emacs/29.3.50/lisp/emacs-lisp/eldoc

Features:
(shadow sort virtual-auto-fill visual-fill-column adaptive-wrap
writegood-mode mail-extr emacsbug message yank-media puny rfc822 mml
mml-sec epa derived epg rfc6068 epg-config gnus-util mm-decode mm-bodies
mm-encode mail-parse rfc2231 mailabbrev gmm-utils sendmail rfc2047
rfc2045 ietf-drums mm-util mail-prsvr mule-util tramp-cmds dogears
project bookmark text-property-search add-log jinx company-quickhelp
pos-tip hideshowvis rainbow-delimiters hideshow outli org-faces org-keys
oc org-compat org-version org-macs noutline outline symbol-overlay
display-fill-column-indicator hl-line display-line-numbers
company-abbrev company-dabbrev-code company-dabbrev company-keywords
company-files company-semantic company-template company-yasnippet
company-capf company-tng company pcase pixel-scroll cua-base
rdu-miscell-funcs modus-operandi-theme modus-themes
rdu-emacs-fonts-funcs-0 pdf-macs nerd-icons nerd-icons-faces
nerd-icons-data nerd-icons-data-mdicon nerd-icons-data-flicon
nerd-icons-data-codicon nerd-icons-data-devicon nerd-icons-data-sucicon
nerd-icons-data-wicon nerd-icons-data-faicon nerd-icons-data-powerline
nerd-icons-data-octicon nerd-icons-data-pomicon nerd-icons-data-ipsicon
which-func imenu recentf tree-widget vertico-indexed vertico-prescient
prescient char-fold vertico-multiform vertico marginalia helm-mode
helm-misc helm-files image-dired image-dired-tags image-dired-external
image-dired-util xdg image-mode dired-hide-permissions dired
dired-loaddefs exif tramp tramp-loaddefs trampver tramp-integration
files-x tramp-compat shell pcomplete comint ansi-osc parse-time iso8601
time-date helm-buffers all-the-icons all-the-icons-faces data-material
data-weathericons data-octicons data-fileicons data-faicons
data-alltheicons helm-occur helm-tags helm-locate helm-grep helm-regexp
helm-utils helm-help helm-types helm helm-global-bindings helm-core
async-bytecomp helm-source helm-multi-match helm-lib async
flycheck-grammarly grammarly websocket bindat request mailheader
mail-utils s dom exec-path-from-shell flycheck ansi-color find-func
pulsar pulse color hl-todo aggressive-indent shackle trace winner anzu
advice thingatpt savehist yasnippet transient format-spec compat
elec-pair cus-edit pp wid-edit hydra ring lv which-key pdf-loader
edmacro kmacro use-package-bind-key mb-depth comp comp-cstr warnings
icons rx autorevert filenotify time cus-load cl bind-key easy-mmode gcmh
cl-extra help-mode use-package-ensure use-package-core finder-inf info
ace-jump-zap-autoloads ace-jump-mode-autoloads ace-link-autoloads
ace-window-autoloads aggressive-indent-autoloads all-the-icons-autoloads
anaphora-autoloads anki-editor-autoloads anzu-autoloads
atomic-chrome-autoloads avy-autoloads bicycle-autoloads
buffer-move-autoloads burly-autoloads capf-autosuggest-autoloads
casual-autoloads citar-embark-autoloads citar-autoloads
citeproc-autoloads company-auctex-autoloads auctex-autoloads tex-site
company-math-autoloads company-prescient-autoloads
company-quickhelp-autoloads company-reftex-autoloads
company-shell-autoloads company-stan-autoloads
default-text-scale-autoloads deferred-autoloads deft-autoloads
dogears-autoloads dumb-jump-autoloads dwim-shell-command-autoloads
easy-kill-autoloads ef-themes-autoloads eglot-jl-autoloads
eldoc-box-autoloads eldoc-stan-autoloads elisp-demos-autoloads
emacsql-sqlite-builtin-autoloads embark-consult-autoloads
embark-autoloads eshell-syntax-highlighting-autoloads
eshell-vterm-autoloads ess-autoloads exec-path-from-shell-autoloads
expand-region-autoloads flycheck-grammarly-autoloads
flycheck-stan-autoloads flycheck-autoloads
flyspell-correct-helm-autoloads flyspell-correct-autoloads
gcmh-autoloads git-timemachine-autoloads google-this-autoloads
grammarly-autoloads haskell-mode-autoloads helm-bibtex-autoloads
bibtex-completion-autoloads biblio-autoloads biblio-core-autoloads
helm-c-yasnippet-autoloads helm-company-autoloads company-autoloads
helm-descbinds-autoloads helm-mu-autoloads helm-easymenu
helm-swoop-autoloads helm-autoloads helm-core-autoloads
helpful-autoloads elisp-refs-autoloads hide-mode-line-autoloads
highlight-indent-guides-autoloads hl-todo-autoloads hydra-autoloads
iedit-autoloads imenu-anywhere-autoloads jinx-autoloads
julia-repl-autoloads julia-ts-mode-autoloads julia-mode-autoloads
jump-char-autoloads lsp-latex-autoloads consult-autoloads
lsp-ui-autoloads lsp-mode-autoloads eldoc-autoloads lv-autoloads
magit-autoloads git-commit-autoloads marginalia-autoloads
markdown-toc-autoloads math-symbol-lists-autoloads mixed-pitch-autoloads
modus-themes-autoloads multiple-cursors-autoloads nerd-icons-autoloads
orderless-autoloads org-attach-screenshot-autoloads
org-download-autoloads async-autoloads org-modern-autoloads
org-roam-ui-autoloads org-roam-autoloads magit-section-autoloads
emacsql-autoloads org-side-tree-autoloads org-sidebar-autoloads
org-ql-autoloads f-autoloads org-sticky-header-autoloads
org-super-agenda-autoloads ht-autoloads ov-autoloads parsebib-autoloads
pdf-tools-autoloads pdfgrep-autoloads peg-autoloads poly-R-autoloads
poly-markdown-autoloads markdown-mode-autoloads poly-noweb-autoloads
polymode-autoloads popup-autoloads pos-tip-autoloads posframe-autoloads
pulsar-autoloads puni-autoloads queue-autoloads
rainbow-delimiters-autoloads request-autoloads rg-autoloads
rotate-autoloads scratch-autoloads shackle-autoloads
side-hustle-autoloads simple-httpd-autoloads spinner-autoloads
sr-speedbar-autoloads stan-mode-autoloads stream-autoloads
string-inflection-autoloads symbol-overlay-autoloads tablist-autoloads
transient-autoloads transpose-frame-autoloads ts-autoloads s-autoloads
dash-autoloads vertico-prescient-autoloads vertico-autoloads
prescient-autoloads virtual-auto-fill-autoloads adaptive-wrap-autoloads
visual-fill-column-autoloads vterm-autoloads vundo-autoloads
websocket-autoloads wfnames-autoloads wgrep-ag-autoloads wgrep-autoloads
which-key-autoloads whole-line-or-region-autoloads with-editor-autoloads
compat-autoloads workgroups2-autoloads writegood-mode-autoloads
yasnippet-autoloads zmq-autoloads ztree-autoloads package browse-url url
url-proxy url-privacy url-expand url-methods url-history url-cookie
generate-lisp-file url-domsuf url-util mailcap url-handlers url-parse
auth-source cl-seq eieio eieio-core cl-macs password-cache json subr-x
map byte-opt gv bytecomp byte-compile url-vars cl-loaddefs cl-lib rmc
iso-transl tooltip cconv eldoc paren electric uniquify ediff-hook
vc-hooks lisp-float-type elisp-mode mwheel term/x-win x-win
term/common-win x-dnd tool-bar dnd fontset image regexp-opt fringe
tabulated-list replace newcomment text-mode lisp-mode prog-mode register
page tab-bar menu-bar rfn-eshadow isearch easymenu timer select
scroll-bar mouse jit-lock font-lock syntax font-core term/tty-colors
frame minibuffer nadvice seq simple cl-generic indonesian philippine
cham georgian utf-8-lang misc-lang vietnamese tibetan thai tai-viet lao
korean japanese eucjp-ms cp51932 hebrew greek romanian slovak czech
european ethiopic indian cyrillic chinese composite emoji-zwj charscript
charprop case-table epa-hook jka-cmpr-hook help abbrev obarray oclosure
cl-preloaded button loaddefs theme-loaddefs faces cus-face macroexp
files window text-properties overlay sha1 md5 base64 format env
code-pages mule custom widget keymap hashtable-print-readable backquote
threads xwidget-internal dbusbind inotify lcms2 dynamic-setting
system-font-setting font-render-setting cairo move-toolbar gtk x-toolkit
xinput2 x multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 484796 318366)
 (symbols 48 30627 9)
 (strings 32 135401 49483)
 (string-bytes 1 4269554)
 (vectors 16 51708)
 (vector-slots 8 1089854 540164)
 (floats 8 1329 1287)
 (intervals 56 825 472)
 (buffers 984 12))


-- 
Ramon Diaz-Uriarte
Department of Biochemistry, Lab B-31
Facultad de Medicina 
Universidad Autónoma de Madrid 
Arzobispo Morcillo, 4
28029 Madrid
Spain

Phone: +34-91-497-2412

Email: rdiaz02@gmail.com
       r.diaz@uam.es
       ramon.diaz@iib.uam.es

https://ligarto.org/rdiaz

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

[Eli Zaretskii]

> Cc: Rahguzar <rahguzar@zohomail.eu>, Ramon Diaz-Uriarte <rdiaz02@gmail.com>
> From: Ramon Diaz-Uriarte <rdiaz02@gmail.com>
> Date: Wed, 27 Mar 2024 21:25:34 +0100
> 
> With some fonts, changing focus (M-x other-window) from a buffer with images, makes the content in the buffer with images to shift up and down.
> 
> I am seeing this in Debian, with both the GTK and Lucid builds, under X11.
> 
> The code below reproduces the problem. This problem has been identified while debugging a change on focus issue with pdf-tools (https://github.com/vedang/pdf-tools/pull/224#issuecomment-2014151358 and ff.)

Crystal ball says that this happens because when a frame loses focus,
we redraw the cursor as a hollow rectangle, and (for boring technical
reasons, which I can explain if someone wants to know) that hollow
cursor can have a different height than the block cursor we draw on a
selected frame.  You can verify this guess of mine if you customize
cursor-type to one of the non-default values -- the problem should go
away for any cursor-type but the default one.

If my guess is correct, fixing this is not easy (but patches are
welcome, of course), and at the time I decided to leave this rare case
be.

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

[Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Eli,

Eli Zaretskii <eliz@gnu.org> writes:

> Crystal ball says that this happens because when a frame loses focus,
> we redraw the cursor as a hollow rectangle, and (for boring technical
> reasons, which I can explain if someone wants to know) that hollow
> cursor can have a different height than the block cursor we draw on a
> selected frame.  You can verify this guess of mine if you customize
> cursor-type to one of the non-default values -- the problem should go
> away for any cursor-type but the default one.
>
> If my guess is correct, fixing this is not easy (but patches are
> welcome, of course), and at the time I decided to leave this rare case
> be.

The original problem where we encountered this the cursor wasn't shown
at all. So I think this might be different. I have been unable to
reproduce it on my machine (it might be because pgtk build doesn't have
this problem) but from what I can tell the problem is that when the
window is not selected the vscroll gets reset to 0 and trying to set it
again seems to have no effect.

In the minimal reproducer there is a pre-redisplay-function that always
sets vscroll for the window to 200. When the window is deselected, the
function runs and Ramon checked by inserting a message that the vscroll
gets reported to be non-zero however the window gets drawn as if the
vscroll was 0. On selecting the window again, it gets redrawn with the
expected value of rescroll and this causes the jumps.

Rahguzar

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

[Eli Zaretskii]

> From: Rahguzar <rahguzar@zohomail.eu>
> Cc: Ramon Diaz-Uriarte <rdiaz02@gmail.com>, 70038@debbugs.gnu.org
> Date: Thu, 28 Mar 2024 08:52:21 +0100
> 
> The original problem where we encountered this the cursor wasn't shown
> at all. So I think this might be different. I have been unable to
> reproduce it on my machine (it might be because pgtk build doesn't have
> this problem) but from what I can tell the problem is that when the
> window is not selected the vscroll gets reset to 0 and trying to set it
> again seems to have no effect.
> 
> In the minimal reproducer there is a pre-redisplay-function that always
> sets vscroll for the window to 200. When the window is deselected, the
> function runs and Ramon checked by inserting a message that the vscroll
> gets reported to be non-zero however the window gets drawn as if the
> vscroll was 0. On selecting the window again, it gets redrawn with the
> expected value of rescroll and this causes the jumps.

In that case, I don't understand what the font has to do with this: if
redisplay resets the window's vscroll, it should always do that, no?
Also, I don't have the fonts you used, so I cannot try the recipe as
is.  I tried a few monospaced fonts I do have, and couldn't reproduce.
A recipe that doesn't depend on rare fonts would be appreciated.
Bonus points for providing a recipe that doesn't depend on fonts at
all.

Thanks.

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

[Ramon Diaz-Uriarte]

> In that case, I don't understand what the font has to do with this: if
> redisplay resets the window's vscroll, it should always do that, no?
> Also, I don't have the fonts you used, so I cannot try the recipe as
> is.  I tried a few monospaced fonts I do have, and couldn't reproduce.
> A recipe that doesn't depend on rare fonts would be appreciated.
> Bonus points for providing a recipe that doesn't depend on fonts at
> all.
> 
> Thanks.

I am afraid I don't get the bonus points, because all simple recipes I've found so far depend on fonts (even more, on the "rare fonts", which I use routinely.)

Best,

R.

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

[Ramon Diaz-Uriarte]

Actually, maybe I can claim those bonus points: this does not depend on fonts, though I am triggering it using the package vertico (so maybe this example is vertico's fault):


Steps:

1. emacs -Q
2. eval this code

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(package-initialize)
(vertico-mode 1)

(progn
  (defun pin-vscroll-down (win)
    (set-window-vscroll win 200 t))
   (let* ((height (/ (* 2 (frame-pixel-height)) 15))
         (image1 (create-image "/tmp/image1.png" nil nil :height height))
         (image2 (create-image "/tmp/image2.png" nil nil :height height))
         (image3 (create-image "/tmp/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*")))
;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;

3. Do `M-x` (or C-x b). No need to execute anything or switch buffers, just have the minibuffer show options.

4. `C-x o`  a few times. You'll see the images move up and down.

I am seeing this in Lucid and GTK builds.

Best,

R.




On Thu, 28-March-2024, at 17:12:45, Ramon Diaz-Uriarte <rdiaz02@gmail.com> wrote:
>> In that case, I don't understand what the font has to do with this: if
>> redisplay resets the window's vscroll, it should always do that, no?
>> Also, I don't have the fonts you used, so I cannot try the recipe as
>> is.  I tried a few monospaced fonts I do have, and couldn't reproduce.
>> A recipe that doesn't depend on rare fonts would be appreciated.
>> Bonus points for providing a recipe that doesn't depend on fonts at
>> all.
>> 
>> Thanks.
>
> I am afraid I don't get the bonus points, because all simple recipes I've found so far depend on fonts (even more, on the "rare fonts", which I use routinely.)
>
> Best,
>
> R.

-- 
Ramon Diaz-Uriarte
Department of Biochemistry, Lab B-31
Facultad de Medicina 
Universidad Autónoma de Madrid 
Arzobispo Morcillo, 4
28029 Madrid
Spain

Phone: +34-91-497-2412

Email: rdiaz02@gmail.com
       r.diaz@uam.es
       ramon.diaz@iib.uam.es

https://ligarto.org/rdiaz

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

[Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Hi Ramon,

Ramon Diaz-Uriarte <rdiaz02@gmail.com> writes:

> Actually, maybe I can claim those bonus points: this does not depend on fonts, though I am triggering it using the package vertico (so maybe this example is vertico's fault):
>
>
> Steps:
>
> 1. emacs -Q
> 2. eval this code
>
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
> (package-initialize)
> (vertico-mode 1)
>
> (progn
>   (defun pin-vscroll-down (win)
>     (set-window-vscroll win 200 t))
>    (let* ((height (/ (* 2 (frame-pixel-height)) 15))
>          (image1 (create-image "/tmp/image1.png" nil nil :height height))
>          (image2 (create-image "/tmp/image2.png" nil nil :height height))
>          (image3 (create-image "/tmp/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*")))
> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>
> 3. Do `M-x` (or C-x b). No need to execute anything or switch buffers, just have the minibuffer show options.
>
> 4. `C-x o`  a few times. You'll see the images move up and down.
>
> I am seeing this in Lucid and GTK builds.

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.

> Best,
>
> R.

Rahguzar

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

[Rahguzar via Bug reports for GNU Emacs, the Swiss army knife of text editors]

On further testing,

(setq read-minibuffer-restore-windows nil)

makes the problem go away.

Rahguzar <rahguzar@zohomail.eu> writes:

> Hi Ramon,
>
> Ramon Diaz-Uriarte <rdiaz02@gmail.com> writes:
>
>> Actually, maybe I can claim those bonus points: this does not depend on fonts, though I am triggering it using the package vertico (so maybe this example is vertico's fault):
>>
>>
>> Steps:
>>
>> 1. emacs -Q
>> 2. eval this code
>>
>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>> (package-initialize)
>> (vertico-mode 1)
>>
>> (progn
>>   (defun pin-vscroll-down (win)
>>     (set-window-vscroll win 200 t))
>>    (let* ((height (/ (* 2 (frame-pixel-height)) 15))
>>          (image1 (create-image "/tmp/image1.png" nil nil :height height))
>>          (image2 (create-image "/tmp/image2.png" nil nil :height height))
>>          (image3 (create-image "/tmp/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*")))
>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>
>> 3. Do `M-x` (or C-x b). No need to execute anything or switch buffers, just have the minibuffer show options.
>>
>> 4. `C-x o`  a few times. You'll see the images move up and down.
>>
>> I am seeing this in Lucid and GTK builds.
>
> 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.
>
>> Best,
>>
>> R.
>
> Rahguzar

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

[Ramon Diaz-Uriarte]

For me, this setting fixes the problem when there is no dependence on fonts:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(setq read-minibuffer-restore-windows nil)
(progn
  (defun pin-vscroll-down (win)
    (set-window-vscroll win 200 t))
   (let* ((height (/ (* 2 (frame-pixel-height)) 5))
         (image1 (create-image "/tmp/image1.png" nil nil :height height))
         (image2 (create-image "/tmp/image2.png" nil nil :height height))
         (image3 (create-image "/tmp/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*")))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;


However, it does not solve the problem when I change fonts:

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
(setq read-minibuffer-restore-windows nil)
(progn
  (defun pin-vscroll-down (win)
    (set-window-vscroll win 200 t))
  ;; Any of the following leads to the bug
  (set-frame-font "JuliaMono" nil t)
  ;; (set-frame-font "DM Mono" nil t)
  ;; (set-frame-font "Intel One Mono" nil t)
  (let* ((height (/ (* 2 (frame-pixel-height)) 15))
         (image1 (create-image "/tmp/image1.png" nil nil :height height))
         (image2 (create-image "/tmp/image2.png" nil nil :height height))
         (image3 (create-image "/tmp/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 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*")))

;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;


As mentioned initially, this issue was found when trying to solve a similar problem with pdf-tools: https://github.com/vedang/pdf-tools/pull/224#issuecomment-2014151358

Setting

(setq read-minibuffer-restore-windows nil)

solves the pdf-tools continuous scrolling issue when there is no change in font but not when we change font (https://github.com/vedang/pdf-tools/pull/224#issuecomment-2026142343 )


Best,

R.




On Thu, 28-March-2024, at 21:50:49, Rahguzar <rahguzar@zohomail.eu> wrote:
> On further testing,
>
> (setq read-minibuffer-restore-windows nil)
>
> makes the problem go away.
>
> Rahguzar <rahguzar@zohomail.eu> writes:
>
>> Hi Ramon,
>>
>> Ramon Diaz-Uriarte <rdiaz02@gmail.com> writes:
>>
>>> Actually, maybe I can claim those bonus points: this does not depend on fonts, though I am triggering it using the package vertico (so maybe this example is vertico's fault):
>>>
>>>
>>> Steps:
>>>
>>> 1. emacs -Q
>>> 2. eval this code
>>>
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>> (package-initialize)
>>> (vertico-mode 1)
>>>
>>> (progn
>>>   (defun pin-vscroll-down (win)
>>>     (set-window-vscroll win 200 t))
>>>    (let* ((height (/ (* 2 (frame-pixel-height)) 15))
>>>          (image1 (create-image "/tmp/image1.png" nil nil :height height))
>>>          (image2 (create-image "/tmp/image2.png" nil nil :height height))
>>>          (image3 (create-image "/tmp/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*")))
>>> ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
>>>
>>> 3. Do `M-x` (or C-x b). No need to execute anything or switch buffers, just have the minibuffer show options.
>>>
>>> 4. `C-x o`  a few times. You'll see the images move up and down.
>>>
>>> I am seeing this in Lucid and GTK builds.
>>
>> 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.
>>
>>> Best,
>>>
>>> R.
>>
>> Rahguzar

-- 
Ramon Diaz-Uriarte
Department of Biochemistry, Lab B-31
Facultad de Medicina 
Universidad Autónoma de Madrid 
Arzobispo Morcillo, 4
28029 Madrid
Spain

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

[Eli Zaretskii]

> From: Rahguzar <rahguzar@zohomail.eu>
> Cc: Eli Zaretskii <eliz@gnu.org>, 70038@debbugs.gnu.org
> 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 ?

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

[Eli Zaretskii]

> Cc: Rahguzar <rahguzar@zohomail.eu>, rdiaz02@gmail.com, 70038@debbugs.gnu.org
> Date: Sat, 06 Apr 2024 15:33:59 +0300
> From: Eli Zaretskii <eliz@gnu.org>
> 
> > From: Rahguzar <rahguzar@zohomail.eu>
> > Cc: Eli Zaretskii <eliz@gnu.org>, 70038@debbugs.gnu.org
> > 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?

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

Eli Zaretskii <eliz@gnu.org> writes:

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

I think you are correct.

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

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

Just for the record: Here I once used a version of shrink_mini_window
that went as

/** Shrink mini-window W to its minimum height.  */
void
shrink_mini_window (struct window *w)
{
   /* Just attempt to shrink it to zero, grow_mini_window makes sure it
      does not get to small.  */
   FRAME_WINDOWS_FROZEN (WINDOW_XFRAME (w)) = false;
   grow_mini_window (w, -WINDOW_PIXEL_HEIGHT (w));
}

where grow_mini_window took care of the rest.  But I don't call
shrink_mini_window any more and so the flag remains stuck here as well.

 > so I
 > guess we should do something in minibuffer_unwind to reset that flag?

Would that be sufficient?  Don't we freeze also when resizing the echo
area?

martin

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

[Eli Zaretskii]

> Date: Sun, 7 Apr 2024 10:24:29 +0200
> Cc: r.diaz@uam.es, luangruo@yahoo.com, rahguzar@zohomail.eu,
>  rdiaz02@gmail.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > so I
>  > guess we should do something in minibuffer_unwind to reset that flag?
> 
> Would that be sufficient?  Don't we freeze also when resizing the echo
> area?

I guess we do, but where is that resized back to its normal height?

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 >> Would that be sufficient?  Don't we freeze also when resizing the echo
 >> area?
 >
 > I guess we do, but where is that resized back to its normal height?

In shrink_mini_window hopefully so this should be covered.  If the only
problem is that of restore_window_configuration, then minibuffer_unwind
looks like the right place.  But maybe read_char_help_form_unwind would
require the same treatment.

martin

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

[Eli Zaretskii]

> Date: Sun, 7 Apr 2024 12:12:55 +0200
> Cc: r.diaz@uam.es, luangruo@yahoo.com, rahguzar@zohomail.eu,
>  rdiaz02@gmail.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  >> Would that be sufficient?  Don't we freeze also when resizing the echo
>  >> area?
>  >
>  > I guess we do, but where is that resized back to its normal height?
> 
> In shrink_mini_window hopefully so this should be covered.  If the only
> problem is that of restore_window_configuration, then minibuffer_unwind
> looks like the right place.  But maybe read_char_help_form_unwind would
> require the same treatment.

Hmm...  read_char_help_form_unwind is called after we invoke
help-form-show, and that one pops up a special buffer:

  (defun help-form-show ()
    "Display the output of a non-nil `help-form'."
    (let ((msg (eval help-form t)))
      (if (stringp msg)
	  (with-output-to-temp-buffer " *Char Help*"
	    (princ msg)))))

Is there a way to make that use the echo-area? if so, can you tell how
to do that?

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

IIUC the basic idea of all this is to preserve window start positions
when enlarging a mini window unless that would move a window's point
outside that window.  Once upon a time, resize_mini_window used window
configurations to get back the previous start positions but that was
maybe considered too costly and Gerd decided to "Don't save window
configuration, freeze window starts instead."

A possibly heretical question is in which way freezing start positions
permanently can harm.  IIUC, after a minibuffer interaction, currently
the only way to unfreeze start positions is to resize the minibuffer
window and trigger the corresponding call in shrink_mini_window.  But
setting the start position of any window while a minibuffer interaction
is going on seems to work here without problems.

Let's assume it can harm:

 >> If the only
 >> problem is that of restore_window_configuration, then minibuffer_unwind
 >> looks like the right place.

That was silly.  minibuffer_unwind seems to care only about replacing
one minibuffer with another.  read_minibuf_unwind already should handle
this here (don't ask me what a future_mini_window is)

   /* When we get to the outmost level, make sure we resize the
      mini-window back to its normal size.  */
   if (minibuf_level == 0
       || !EQ (selected_frame, WINDOW_FRAME (XWINDOW (future_mini_window))))
     resize_mini_window (XWINDOW (minibuf_window), 0);

The only problem is that if the mini window was _not_ enlarged,
shrink_mini_window won't unfreeze starts.  Unconditionally unfreezing
start positions there as I mentioned in my first mail should fix that.

 > Hmm...  read_char_help_form_unwind is called after we invoke
 > help-form-show, and that one pops up a special buffer:
 >
 >    (defun help-form-show ()
 >      "Display the output of a non-nil `help-form'."
 >      (let ((msg (eval help-form t)))
 >        (if (stringp msg)
 > 	  (with-output-to-temp-buffer " *Char Help*"
 > 	    (princ msg)))))
 >
 > Is there a way to make that use the echo-area? if so, can you tell how
 > to do that?

IIUC that is called in a situation where the minibuffer is active.
Showing that text in the echo area (albeit shortly) doesn't look like
TRT to me.

IIUC read_char can resize the mini window here

	  if (minibuf_level
	      && EQ (minibuf_window, echo_area_window)
	      /* The case where minibuffer-message-timeout is a number
		 was already handled near the beginning of command_loop_1.  */
	      && !NUMBERP (Vminibuffer_message_timeout))
	    resize_mini_window (XWINDOW (minibuf_window), false);

so this should be covered by read_minibuf_unwind already.

martin

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

[Ramon Diaz-Uriarte]

Sorry, I did not see this.

On Sat, 06-April-2024, at 14:33:59, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Rahguzar <rahguzar@zohomail.eu>
>> Cc: Eli Zaretskii <eliz@gnu.org>, 70038@debbugs.gnu.org
>> 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 ?


Yes: 
the problem goes away (and I do not need to use
(setq read-minibuffer-restore-windows nil)
).

Moreover, the original problem with pdf-tools that lead this bug report (https://github.com/vedang/pdf-tools/pull/224?notification_referrer_id=NT_kwDOABnEI7I3MDQyMzY2MDYwOjE2ODg2MTE#issuecomment-2014151358) also goes away.

Best,

R.

-- 
Ramon Diaz-Uriarte
Department of Biochemistry, Lab B-31
Facultad de Medicina 
Universidad Autónoma de Madrid 
Arzobispo Morcillo, 4
28029 Madrid
Spain

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

[Eli Zaretskii]

> From: Ramon Diaz-Uriarte <r.diaz@uam.es>
> Cc: Po Lu <luangruo@yahoo.com>,  rdiaz02@gmail.com,  Rahguzar
>  <rahguzar@zohomail.eu>,  70038@debbugs.gnu.org
> Date: Thu, 11 Apr 2024 15:56:30 +0200
> 
> Sorry, I did not see this.

No sweat.

> On Sat, 06-April-2024, at 14:33:59, Eli Zaretskii <eliz@gnu.org> wrote:
> >> From: Rahguzar <rahguzar@zohomail.eu>
> >> Cc: Eli Zaretskii <eliz@gnu.org>, 70038@debbugs.gnu.org
> >> Date: Thu, 28 Mar 2024 18:24:32 +0100
> >> 
> > 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 ?
> 
> 
> Yes: 
> the problem goes away (and I do not need to use
> (setq read-minibuffer-restore-windows nil)
> ).
> 
> Moreover, the original problem with pdf-tools that lead this bug report (https://github.com/vedang/pdf-tools/pull/224?notification_referrer_id=NT_kwDOABnEI7I3MDQyMzY2MDYwOjE2ODg2MTE#issuecomment-2014151358) also goes away.

Thanks for testing.  So I've now installed this change on the emacs-29
branch (soon to be merged to master).

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

[Ramon Diaz-Uriarte]

On Thu, 11-April-2024, at 17:36:48, Eli Zaretskii <eliz@gnu.org> wrote:
>> From: Ramon Diaz-Uriarte <r.diaz@uam.es>
>> Cc: Po Lu <luangruo@yahoo.com>,  rdiaz02@gmail.com,  Rahguzar
>>  <rahguzar@zohomail.eu>,  70038@debbugs.gnu.org
>> Date: Thu, 11 Apr 2024 15:56:30 +0200
>>
>> Sorry, I did not see this.
>
> No sweat.
>
>> On Sat, 06-April-2024, at 14:33:59, Eli Zaretskii <eliz@gnu.org> wrote:
>> >> From: Rahguzar <rahguzar@zohomail.eu>
>> >> Cc: Eli Zaretskii <eliz@gnu.org>, 70038@debbugs.gnu.org
>> >> Date: Thu, 28 Mar 2024 18:24:32 +0100
>> >>
>> > 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 ?
>>
>>
>> Yes:
>> the problem goes away (and I do not need to use
>> (setq read-minibuffer-restore-windows nil)
>> ).
>>
>> Moreover, the original problem with pdf-tools that lead this bug report
>> (https://github.com/vedang/pdf-tools/pull/224?notification_referrer_id=NT_kwDOABnEI7I3MDQyMzY2MDYwOjE2ODg2MTE#issuecomment-2014151358)
>> also goes away.
>
> Thanks for testing.  So I've now installed this change on the emacs-29
> branch (soon to be merged to master).


Thanks you!!

--
Ramon Diaz-Uriarte
Department of Biochemistry, Lab B-31
Facultad de Medicina
Universidad Autónoma de Madrid
Arzobispo Morcillo, 4
28029 Madrid
Spain

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

[Eli Zaretskii]

> Date: Mon, 8 Apr 2024 11:07:50 +0200
> Cc: r.diaz@uam.es, luangruo@yahoo.com, rahguzar@zohomail.eu,
>  rdiaz02@gmail.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
> A possibly heretical question is in which way freezing start positions
> permanently can harm.  IIUC, after a minibuffer interaction, currently
> the only way to unfreeze start positions is to resize the minibuffer
> window and trigger the corresponding call in shrink_mini_window.  But
> setting the start position of any window while a minibuffer interaction
> is going on seems to work here without problems.

The (slight) harm this does is that it might make redisplay of the
window slightly more expensive, because it disables some
optimizations, like if nothing changed except the position of point.
Another kind of harm is what triggered this bug report: it could cause
us to reset the window's vscroll for now good reason, because when
start positions are frozen, we set the window's force_start flag, and
that sometimes forces us to reset vscroll.

> That was silly.  minibuffer_unwind seems to care only about replacing
> one minibuffer with another.  read_minibuf_unwind already should handle
> this here (don't ask me what a future_mini_window is)
> 
>    /* When we get to the outmost level, make sure we resize the
>       mini-window back to its normal size.  */
>    if (minibuf_level == 0
>        || !EQ (selected_frame, WINDOW_FRAME (XWINDOW (future_mini_window))))
>      resize_mini_window (XWINDOW (minibuf_window), 0);
> 
> The only problem is that if the mini window was _not_ enlarged,
> shrink_mini_window won't unfreeze starts.  Unconditionally unfreezing
> start positions there as I mentioned in my first mail should fix that.

Thanks.

So the patch below is the only change we need to make sure the frame's
frozen_window_starts is reset when the mini-window is resized back?

Should it matter whether, if the minibuffer is active, we do that only
at the outer level of minibuffer?

diff --git a/src/window.c b/src/window.c
index fe26311..0945b24 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5407,13 +5407,13 @@ shrink_mini_window (struct window *w)
 
   eassert (MINI_WINDOW_P (w));
 
+  FRAME_WINDOWS_FROZEN (f) = false;
   if (delta > 0)
     {
       Lisp_Object root = FRAME_ROOT_WINDOW (f);
       struct window *r = XWINDOW (root);
       Lisp_Object grow;
 
-      FRAME_WINDOWS_FROZEN (f) = false;
       grow = call3 (Qwindow__resize_root_window_vertically,
 		    root, make_fixnum (delta), Qt);
 

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > The (slight) harm this does is that it might make redisplay of the
 > window slightly more expensive, because it disables some
 > optimizations, like if nothing changed except the position of point.
 > Another kind of harm is what triggered this bug report: it could cause
 > us to reset the window's vscroll for now good reason, because when
 > start positions are frozen, we set the window's force_start flag, and
 > that sometimes forces us to reset vscroll.

 From looking at the code it's hard to understand what you say here.  It
might be a good idea to add this as a comment.

 > So the patch below is the only change we need to make sure the frame's
 > frozen_window_starts is reset when the mini-window is resized back?

Probably not.  If 'resize-mini-windows' is t, we never unfreeze window
starts because we don't call shrink_mini_window in that case.

 > Should it matter whether, if the minibuffer is active, we do that only
 > at the outer level of minibuffer?

It shouldn't.  Window starts would be frozen immediately as soon as the
mini window grows again.  I think that to cover most cases we should
unfreeze window starts every time the mini window gets smaller as in the
patch below.

diff --git a/src/window.c b/src/window.c
index fe26311fbb2..9c8fc12ee3e 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5376,7 +5376,8 @@ grow_mini_window (struct window *w, int delta)
        struct window *r = XWINDOW (root);
        Lisp_Object grow;

-      FRAME_WINDOWS_FROZEN (f) = true;
+      FRAME_WINDOWS_FROZEN (f) = (delta > 0) ? true : false;
+
        grow = call3 (Qwindow__resize_root_window_vertically,
  		    root, make_fixnum (- delta), Qt);

martin

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

[Eli Zaretskii]

> Date: Sun, 14 Apr 2024 10:31:15 +0200
> Cc: luangruo@yahoo.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > The (slight) harm this does is that it might make redisplay of the
>  > window slightly more expensive, because it disables some
>  > optimizations, like if nothing changed except the position of point.
>  > Another kind of harm is what triggered this bug report: it could cause
>  > us to reset the window's vscroll for now good reason, because when
>  > start positions are frozen, we set the window's force_start flag, and
>  > that sometimes forces us to reset vscroll.
> 
>  From looking at the code it's hard to understand what you say here.  It
> might be a good idea to add this as a comment.

If the window is frozen, we do this:

  /* If someone specified a new starting point but did not insist,
     check whether it can be used.  */
  if ((w->optional_new_start || window_frozen_p (w))
      && CHARPOS (startp) >= BEGV
      && CHARPOS (startp) <= ZV)
    {
      ptrdiff_t it_charpos;

      w->optional_new_start = false;
      if (!w->force_start)
	{
	[...]
	  /* Make sure we set the force_start flag only if the cursor
	     row will be fully visible.  Otherwise, the code under
	     force_start label below will try to move point back into
	     view, which is not what the code which sets
	     optional_new_start wants.  */
	  if (it.current_y == 0 || line_bottom_y (&it) < it.last_visible_y)
	    {
	      if (it_charpos == PT)
		w->force_start = true;
	      /* IT may overshoot PT if text at PT is invisible.  */
	      else if (it_charpos > PT && CHARPOS (startp) <= PT)
		w->force_start = true;

So this sets w->force_start.  Then the code under this condition will
be executed:

  /* Handle case where place to start displaying has been specified,
     unless the specified location is outside the accessible range.  */
  if (w->force_start)

That code calls try_window, so it's more expensive than the
optimization under this condition:

 ignore_start:

  /* Handle case where text has not changed, only point, and it has
     not moved off the frame, and we are not retrying after hscroll.
     (current_matrix_up_to_date_p is true when retrying.)  */
  if (current_matrix_up_to_date_p
      && (rc = try_cursor_movement (window, startp, &temp_scroll_step),
	  rc != CURSOR_MOVEMENT_CANNOT_BE_USED))

which is what we want to do when text has not changed and point didn't
move far away.

Did I succeed explaining the issue?

>  > So the patch below is the only change we need to make sure the frame's
>  > frozen_window_starts is reset when the mini-window is resized back?
> 
> Probably not.  If 'resize-mini-windows' is t, we never unfreeze window
> starts because we don't call shrink_mini_window in that case.
> 
>  > Should it matter whether, if the minibuffer is active, we do that only
>  > at the outer level of minibuffer?
> 
> It shouldn't.  Window starts would be frozen immediately as soon as the
> mini window grows again.  I think that to cover most cases we should
> unfreeze window starts every time the mini window gets smaller as in the
> patch below.

Is that in addition to what I suggested to do in shrink_mini_window?

Also, shouldn't we do this instead:

> -      FRAME_WINDOWS_FROZEN (f) = true;
> +      FRAME_WINDOWS_FROZEN (f) = (old_height + delta > min_height) ? true : false;

IOW, shouldn't we unfreeze only when resizing to the default one-line
height?

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Did I succeed explaining the issue?

Not yet.  When we freeze window starts in the case at hand we do that to
approximate the behavior of a 'save-window-excursion'.  But doesn't
Emacs always try to preserve window start positions regardless of
whether it enlarges or shrinks a window?  So what, if possibly, could
motivate Emacs to "willfully" change a start position?  It's obvious that
'recenter' asks for it and sets optional_new_start.  It's already not
clear to me why 'delete-other-windows' should want it.  So why treating
frozen starts like asking for new start positions is still a mystery to
me ...

I suppose that freezing should handle one situation only: Emacs enlarges
the mini window, shrinks another window for that purpose and - for a
reason that is still unclear to me as mentioned above - would like to
change that window's start position.  To avoid that - Emacs will still
have to change the start position to avoid that point goes off-screen -
we set that flag.  Now it seems obvious that shrinking the mini window
and thus enlarging another window cannot harm in this regard since
otherwise we couldn't have reset the flag when shrinking a mini window
as we did earlier.  Is it about making the cursor line fully visible?

 > Is that in addition to what I suggested to do in shrink_mini_window?

It should replace it.

 > Also, shouldn't we do this instead:
 >
 >> -      FRAME_WINDOWS_FROZEN (f) = true;
 >> +      FRAME_WINDOWS_FROZEN (f) = (old_height + delta > min_height) ? true : false;
 >
 > IOW, shouldn't we unfreeze only when resizing to the default one-line
 > height?

Since every enlarging of the mini window freezes starts again, this
shouldn't be necessary.  But to make sure that we cover all possible
scenarios we could use the below.

martin

diff --git a/src/window.c b/src/window.c
index fe26311fbb2..34d0def7d72 100644
--- a/src/window.c
+++ b/src/window.c
@@ -5376,7 +5376,6 @@ grow_mini_window (struct window *w, int delta)
        struct window *r = XWINDOW (root);
        Lisp_Object grow;

-      FRAME_WINDOWS_FROZEN (f) = true;
        grow = call3 (Qwindow__resize_root_window_vertically,
  		    root, make_fixnum (- delta), Qt);

@@ -5390,6 +5389,9 @@ grow_mini_window (struct window *w, int delta)
  	  && window_resize_check (r, false))
  	resize_mini_window_apply (w, -XFIXNUM (grow));
      }
+
+  FRAME_WINDOWS_FROZEN (f)
+    = window_body_height (w, WINDOW_BODY_IN_PIXELS) > FRAME_LINE_HEIGHT (f);
  }

  /**
@@ -5413,7 +5415,6 @@ shrink_mini_window (struct window *w)
        struct window *r = XWINDOW (root);
        Lisp_Object grow;

-      FRAME_WINDOWS_FROZEN (f) = false;
        grow = call3 (Qwindow__resize_root_window_vertically,
  		    root, make_fixnum (delta), Qt);

@@ -5425,6 +5426,8 @@ shrink_mini_window (struct window *w)
         bar.  */
      grow_mini_window (w, -delta);

+  FRAME_WINDOWS_FROZEN (f)
+    = window_body_height (w, WINDOW_BODY_IN_PIXELS) > FRAME_LINE_HEIGHT (f);
  }

  DEFUN ("resize-mini-window-internal", Fresize_mini_window_internal,

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

[Eli Zaretskii]

> Date: Mon, 15 Apr 2024 11:23:18 +0200
> Cc: luangruo@yahoo.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Did I succeed explaining the issue?
> 
> Not yet.

You asked what harm could unnecessarily frozen window-start do, and I
explained that: it causes us to run more expensive code.  Now you are
asking a different question: why do we freeze window-start points at
all?  Let me try to answer that as well.

> When we freeze window starts in the case at hand we do that to
> approximate the behavior of a 'save-window-excursion'.  But doesn't
> Emacs always try to preserve window start positions regardless of
> whether it enlarges or shrinks a window?

It does, but what it will do if the current window-start doesn't show
point depends on whether or not we take the "force_start" path.  See
below.

> So what, if possibly, could
> motivate Emacs to "willfully" change a start position?

For example, if point is not visible in the window, or is on a screen
line that is only partially visible, or is inside the scroll-margin.

> It's obvious that
> 'recenter' asks for it and sets optional_new_start.  It's already not
> clear to me why 'delete-other-windows' should want it.  So why treating
> frozen starts like asking for new start positions is still a mystery to
> me ...

See above.  Basically, anything that changes the window geometry can
potentially cause us to want to move the window-start point.

> I suppose that freezing should handle one situation only: Emacs enlarges
> the mini window, shrinks another window for that purpose and - for a
> reason that is still unclear to me as mentioned above - would like to
> change that window's start position.  To avoid that - Emacs will still
> have to change the start position to avoid that point goes off-screen -
> we set that flag.  Now it seems obvious that shrinking the mini window
> and thus enlarging another window cannot harm in this regard since
> otherwise we couldn't have reset the flag when shrinking a mini window
> as we did earlier.  Is it about making the cursor line fully visible?

Yes.  More generally, if the force_start flag is NOT set, and using
the current window-start doesn't produce point in a fully-visible
screen line and out of the scroll margins, then Emacs will try to find
a different window-start point (even recentering the window around
point if nothing else works).  By contrast, if the force_start flag
_is_ set, Emacs will instead move point to bring it inside the window,
without changing window-start.  When the window-start is frozen, we
quickly determine whether using the current window-start will leave
point inside the window, and if so, we set the force_start flag
ourselves, to prevent redisplay_window from changing window-start.

I hope this is now more clear.

>  > Is that in addition to what I suggested to do in shrink_mini_window?
> 
> It should replace it.

Hmm... I don't like leaving shrink_mini_window without the reset.

>  > Also, shouldn't we do this instead:
>  >
>  >> -      FRAME_WINDOWS_FROZEN (f) = true;
>  >> +      FRAME_WINDOWS_FROZEN (f) = (old_height + delta > min_height) ? true : false;
>  >
>  > IOW, shouldn't we unfreeze only when resizing to the default one-line
>  > height?
> 
> Since every enlarging of the mini window freezes starts again, this
> shouldn't be necessary.  But to make sure that we cover all possible
> scenarios we could use the below.

Thanks, I like this much better.  Now installed on master.

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > I hope this is now more clear.

Thanks.  So when we enter the part headed by

   if ((w->optional_new_start || window_frozen_p (w))

then the single aim is apparently to make sure that the cursor line
remains fully visible.

When we enter that part because window_frozen_p (w) is set, we ignore
the scroll margins because we assume that preserving the window start
position is more important in this situation.

When we enter that part because w->optional_new_start was set, we assume
that the caller ('recenter', 'delete-other-windows-internal') has done
its part to obey the scroll margins but still might have failed to keep
the cursor visible.

Is that interpretation correct?  If so, it might make sense to put some
explanation into a comment for that part because, at least for me, it's
a priori not clear that the same treatment is wanted for keeping the
previous start position of a window and for one that has been explicitly
changed.  I've spent almost an hour to arrive at the conclusion above.
Things like this comment in 'delete-other-windows-internal'

	  /* We need to do this, so that the window-scroll-functions
	     get called.  */
	  w->optional_new_start = true;

were distracting even further.

martin

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

[Eli Zaretskii]

> Date: Wed, 17 Apr 2024 10:02:12 +0200
> Cc: luangruo@yahoo.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > I hope this is now more clear.
> 
> Thanks.  So when we enter the part headed by
> 
>    if ((w->optional_new_start || window_frozen_p (w))
> 
> then the single aim is apparently to make sure that the cursor line
> remains fully visible.

Yes.  If it isn't, we will either move point or, if even that is
impossible, ignore the request to preserve window-start.

> When we enter that part because window_frozen_p (w) is set, we ignore
> the scroll margins because we assume that preserving the window start
> position is more important in this situation.
> 
> When we enter that part because w->optional_new_start was set, we assume
> that the caller ('recenter', 'delete-other-windows-internal') has done
> its part to obey the scroll margins but still might have failed to keep
> the cursor visible.
> 
> Is that interpretation correct?

No, we actually check that point doesn't end up inside scroll margins
(search for "Some people insist"), and move point or reject the
window-start if it did.

> If so, it might make sense to put some
> explanation into a comment for that part because, at least for me, it's
> a priori not clear that the same treatment is wanted for keeping the
> previous start position of a window and for one that has been explicitly
> changed.  I've spent almost an hour to arrive at the conclusion above.
> Things like this comment in 'delete-other-windows-internal'
> 
> 	  /* We need to do this, so that the window-scroll-functions
> 	     get called.  */
> 	  w->optional_new_start = true;
> 
> were distracting even further.

Where would you like to put the comment, and what should that comment
say?

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 > Where would you like to put the comment, and what should that comment
 > say?

There are three issues I don't understand well in this context.  The
first one concerns the preserve_vscroll_p flag we currently check as:

       if (!w->preserve_vscroll_p && !window_frozen_p (w))
	w->vscroll = 0;

Here, in all scenarios I tried, I observed that w->preserve_vscroll_p
implies window_frozen_p (w) which IIUC means that the preserve_vscroll_p
flag is not needed and we could simplify the above to

       if (!window_frozen_p (w))
	w->vscroll = 0;

What am I missing here?


The second issue is why we freeze window starts at all.  Conceptually,
when window starts are frozen, redisplay should not change the start
position of _any_ window whenever we enlarge the mini window.  But
window_frozen_p returns false for the selected and minibuffer scroll
windows.  Hence, freezing does not set force_start for these windows.

While this discrimination seems unwanted (in particular when resizing
the echo area), it apparently doesn't matter in any of the scenarios I
tried here - the start positions of all windows remain unchanged in
precisely the same way regardless of whether they are selected or not.
So IIUC we could do away with freezing as well.

The only new issue now is that if window starts are no more frozen, we'd
always (or never) reset w->vscroll in force_start.  But, as mentioned
before, resetting w->vscroll only in non-selected, non-minibuffer
windows doesn't seem very optimal anyway.  And resetting w->vscroll is
IMHO problematic in another way: After having set vscroll for a window,
why does moving point _within_ that window reset it?  Wouldn't it be
more logical to reset vscroll iff a new start position for the window
has to be found - either explicitly due to a scroll command or
implicitly to move point back into view (including the case where
vscroll itself caused point to move off-screen)?

I suppose that I'm missing some optimization issue here but fail to see
what it is.


The third issue is that of the optional_new_start flag: All 'recenter'
effectively does is to calculate a new start position of a window and
ask redisplay to apply it.  Now why does 'recenter' in particular set
that flag and why does none of the other functions that set a window's
start position like 'scroll-up' or 'scroll-down' set it?  IIUC nothing
should prevent us from doing away with that flag too.  Unless, again I'm
missing something important here too.


Maybe also some of the force vs ignore window starts dichotomy stems
from earlier versions of redisplay where, as for example, in the

   if ((w->optional_new_start || window_frozen_p (w))

section, we did not care about scroll margins but somewhere below we now
check them anyway.  Or pathological cases we didn't handle earlier like
that of a cursor not becoming fully visible when the line it is on is
too high to fit into its window's body.


Note that I'm not asking to revise code that works sufficiently well
now.  But clarifying the issues I tried to sketch above in the comment
preceding redisplay_window could be useful to better understand bugs
like the one that triggered this thread.

Thanks, martin

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

[Po Lu via Bug reports for GNU Emacs, the Swiss army knife of text editors]

martin rudalics <rudalics@gmx.at> writes:

>> Where would you like to put the comment, and what should that comment
>> say?
>
> There are three issues I don't understand well in this context.  The
> first one concerns the preserve_vscroll_p flag we currently check as:
>
>       if (!w->preserve_vscroll_p && !window_frozen_p (w))
> 	w->vscroll = 0;
>
> Here, in all scenarios I tried, I observed that w->preserve_vscroll_p

???  Have you enabled pixel-scroll-precision-mode, for which this flag
was introduced?  p-s-p-m relies on vscroll values that it sets being
preserved by force_start in all scenarios, which naturally includes
those where the window start is frozen, just as it does those where it
is variable.

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

[martin rudalics via Bug reports for GNU Emacs, the Swiss army knife of text editors]

 >> There are three issues I don't understand well in this context.  The
 >> first one concerns the preserve_vscroll_p flag we currently check as:
 >>
 >>        if (!w->preserve_vscroll_p && !window_frozen_p (w))
 >> 	w->vscroll = 0;
 >>
 >> Here, in all scenarios I tried, I observed that w->preserve_vscroll_p
 >
 > ???  Have you enabled pixel-scroll-precision-mode, for which this flag
 > was introduced?

Yes.

 > p-s-p-m relies on vscroll values that it sets being
 > preserved by force_start in all scenarios, which naturally includes
 > those where the window start is frozen, just as it does those where it
 > is variable.

When I ran the above with various debugger breakpoints I was not able to
trigger a configuration with w->preserve_vscroll_p true on an unfrozen
window.  In all the scenarios I tried, w->preserve_vscroll_p was reset
in mark_window_display_accurate_1 before arriving there.  Can you please
post a scenario where the difference matters?  If so, it might be worth
to shortly document it somewhere.

Thanks, martin

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

[Eli Zaretskii]

> Date: Sun, 28 Apr 2024 10:51:38 +0200
> Cc: luangruo@yahoo.com, 70038@debbugs.gnu.org
> From: martin rudalics <rudalics@gmx.at>
> 
>  > Where would you like to put the comment, and what should that comment
>  > say?
> 
> There are three issues I don't understand well in this context.  The
> first one concerns the preserve_vscroll_p flag we currently check as:
> 
>        if (!w->preserve_vscroll_p && !window_frozen_p (w))
> 	w->vscroll = 0;
> 
> Here, in all scenarios I tried, I observed that w->preserve_vscroll_p
> implies window_frozen_p (w) which IIUC means that the preserve_vscroll_p
> flag is not needed and we could simplify the above to
> 
>        if (!window_frozen_p (w))
> 	w->vscroll = 0;
> 
> What am I missing here?

I'll let Po Lu answer that, since the preserve_vscroll_p flag was
introduced to support precise pixelwise scrolling.

> The second issue is why we freeze window starts at all.  Conceptually,
> when window starts are frozen, redisplay should not change the start
> position of _any_ window whenever we enlarge the mini window.  But
> window_frozen_p returns false for the selected and minibuffer scroll
> windows.  Hence, freezing does not set force_start for these windows.
> 
> While this discrimination seems unwanted (in particular when resizing
> the echo area), it apparently doesn't matter in any of the scenarios I
> tried here - the start positions of all windows remain unchanged in
> precisely the same way regardless of whether they are selected or not.
> So IIUC we could do away with freezing as well.

Your conclusions are too drastic, IMO.  Freezing window-starts is an
advisory, not a mandatory, setting.  So the fact that it isn't always
obeyed doesn't mean it has no place: when it _is_ obeyed, it enables
some optimizations, as I described several messages up-thread.  Giving
up those optimizations when we can use them is not wise.

> I suppose that I'm missing some optimization issue here but fail to see
> what it is.

I described those optimizations in so many words in one of my previous
messages.

> The third issue is that of the optional_new_start flag: All 'recenter'
> effectively does is to calculate a new start position of a window and
> ask redisplay to apply it.  Now why does 'recenter' in particular set
> that flag and why does none of the other functions that set a window's
> start position like 'scroll-up' or 'scroll-down' set it?

Scrolling commands set force_start, which is a stronger condition than
optional_new_start.  I guess recenter doesn't have to make sure the
window starts exactly where it thinks it should, or something.

> Note that I'm not asking to revise code that works sufficiently well
> now.  But clarifying the issues I tried to sketch above in the comment
> preceding redisplay_window could be useful to better understand bugs
> like the one that triggered this thread.

I still don't know what to comment, because you seem to ask the same
questions again, although I already answered them.  So I'm probably
missing something here.  I can think about comments once you say that
you understand and agree with the explanations, but would like to see
them spelled out in comments, but not before that.