bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Aaron Jensen]

Commit 1da4fca0647ebf1d5d6f12817301a17661560810 caused a regression of
bug#52231

The repro is the same:

(progn (setq scroll-margin 4)
       (pixel-scroll-precision-mode))

And scroll down a buffer with mouse wheel.

The buffer does not scroll properly, it jumps back unless you scroll
fast enough.


In GNU Emacs 30.0.50 (build 1, aarch64-apple-darwin22.6.0, NS
 appkit-2299.70 Version 13.6 (Build 22G120)) of 2023-10-17 built on
 Aarons-Laptop.local
Windowing system distributor 'Apple', version 10.3.2487
System Description:  macOS 14.1

Configured using:
 'configure --disable-dependency-tracking --disable-silent-rules
 --enable-locallisppath=/opt/homebrew/share/emacs/site-lisp
 --infodir=/opt/homebrew/Cellar/emacs-plus@30/30.0.50/share/info/emacs
 --prefix=/opt/homebrew/Cellar/emacs-plus@30/30.0.50 --with-xml2
 --with-gnutls --with-native-compilation --without-compress-install
 --without-dbus --without-imagemagick --with-modules --with-rsvg
 --with-webp --with-ns --disable-ns-self-contained 'CFLAGS=-Os -w -pipe
 -mmacosx-version-min=13
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk
 -DFD_SETSIZE=10000 -DDARWIN_UNLIMITED_SELECT'
 'CPPFLAGS=-I/opt/homebrew/opt/zlib/include
 -I/opt/homebrew/opt/jpeg/include -I/opt/homebrew/opt/icu4c/include
 -isystem/opt/homebrew/include -F/opt/homebrew/Frameworks
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk'
 'LDFLAGS=-L/opt/homebrew/opt/zlib/lib -L/opt/homebrew/opt/jpeg/lib
 -L/opt/homebrew/opt/icu4c/lib -L/opt/homebrew/lib
 -F/opt/homebrew/Frameworks -Wl,-headerpad_max_install_names
 -isysroot/Library/Developer/CommandLineTools/SDKs/MacOSX13.sdk
 -L/opt/homebrew/opt/libgccjit/lib''

Configured features:
ACL GIF GLIB GMP GNUTLS JPEG JSON LCMS2 LIBXML2 MODULES NATIVE_COMP
NOTIFY KQUEUE NS PDUMPER PNG RSVG SQLITE3 THREADS TIFF
TOOLKIT_SCROLL_BARS TREE_SITTER WEBP XIM ZLIB

Important settings:
  value of $LANG: en_US.UTF-8
  locale-coding-system: utf-8-unix

Major mode: Text

Minor modes in effect:
  eval-sexp-fu-flash-mode: t
  global-git-commit-mode: t
  transient-posframe-mode: t
  global-evil-mc-mode: t
  evil-mc-mode: t
  windmove-mode: t
  global-flycheck-mode: t
  flycheck-mode: t
  xterm-mouse-mode: t
  pixel-scroll-precision-mode: t
  org-roam-db-autosync-mode: t
  corfu-prescient-mode: t
  corfu-history-mode: t
  undo-fu-session-global-mode: t
  undo-fu-session-mode: t
  ws-butler-global-mode: t
  ws-butler-mode: t
  save-place-mode: t
  winner-mode: t
  tabspaces-mode: t
  savehist-mode: t
  delete-selection-mode: t
  yas-global-mode: t
  yas-minor-mode: t
  vertico-prescient-mode: t
  prescient-persist-mode: t
  vertico-mouse-mode: t
  vertico-mode: t
  mini-frame-mode: t
  better-jumper-mode: t
  better-jumper-local-mode: t
  global-auto-revert-mode: t
  ns-auto-titlebar-mode: t
  global-anzu-mode: t
  anzu-mode: t
  which-key-posframe-mode: t
  which-key-mode: t
  gcmh-mode: t
  recentf-mode: t
  repeat-mode: t
  +popup-mode: t
  evil-mode: t
  evil-local-mode: t
  server-mode: t
  leader-key-leader-override-mode: t
  global-leader-key-leader-override-mode: t
  elpaca-use-package-mode: t
  override-global-mode: t
  global-display-line-numbers-mode: t
  display-line-numbers-mode: t
  global-eldoc-mode: t
  show-paren-mode: t
  electric-indent-mode: t
  mouse-wheel-mode: t
  tab-bar-mode: t
  file-name-shadow-mode: t
  global-font-lock-mode: t
  font-lock-mode: t
  window-divider-mode: t
  minibuffer-regexp-mode: t
  line-number-mode: t
  auto-fill-function: yas--auto-fill
  transient-mark-mode: t
  auto-composition-mode: t
  auto-encryption-mode: t
  auto-compression-mode: t

Load-path shadows:
/Users/aaronjensen/.emacs.d/elpaca/builds/lispy/elpa hides /Users/aaronjensen/.emacs.d/elpaca/builds/ivy/elpa

Features:
(shadow sort mail-extr emacsbug copy-as-format all-the-icons
data-material data-weathericons data-octicons data-fileicons
data-faicons data-alltheicons all-the-icons-faces yaml-mode drag-stuff
org-clock cal-move magit-patch magit-subtree magit-gitignore semantic/ia
semantic/analyze/refs semantic/db-find semantic/db-ref semantic/senator
semantic/decorate semantic/analyze semantic/sort semantic/scope
semantic/analyze/fcn semantic/db eieio-base semantic/ctxt
semantic/format ezimage semantic/tag-ls semantic/find
semantic/util-modes semantic/util semantic semantic/tag cedet git-link
json-mode json-snatcher evil-matchit-ruby evil-matchit-simple
evil-matchit-prog evil-matchit evil-matchit-sdk semantic/lex semantic/fw
misearch multi-isearch novice wdired rotate vertico-repeat magit-ediff
evil-collection-ediff ediff ediff-merg ediff-mult ediff-wind ediff-diff
ediff-help ediff-init ediff-util consult-git-commit typescript-ts-mode
find-dired pulse consult-xref dumb-jump popup wgrep grep embark-org
embark-consult embark lsp-diagnostics lsp-modeline lsp-lens lsp-ui
lsp-ui-flycheck lsp-ui-doc lsp-ui-imenu lsp-ui-peek lsp-ui-sideline
lsp-ui-util evil-numbers speeddating git-rebase ffap goto-addr tabify
lsp-jq lsp-zig lsp-tilt lsp-steep lsp-svelte lsp-sqls
lsp-ruby-syntax-tree lsp-ruby-lsp lsp-yaml lsp-xml lsp-vimscript
lsp-vhdl lsp-volar lsp-vetur lsp-html lsp-verilog lsp-vala lsp-v
lsp-typeprof lsp-ttcn3 lsp-toml lsp-terraform lsp-tex lsp-sorbet
lsp-solargraph lsp-semgrep lsp-rust lsp-rubocop lsp-rf lsp-ruff-lsp
lsp-remark lsp-racket lsp-r lsp-purescript lsp-pylsp lsp-pyls lsp-pwsh
lsp-php lsp-pls lsp-perlnavigator lsp-perl lsp-openscad lsp-ocaml
lsp-magik lsp-nix lsp-nim lsp-nginx lsp-move lsp-mint lsp-mdx
lsp-marksman lsp-markdown lsp-lua lsp-kotlin lsp-json lsp-javascript
lsp-idris lsp-haxe lsp-groovy lsp-hack lsp-graphql lsp-glsl lsp-gleam
lsp-go lsp-completion lsp-gdscript lsp-fsharp lsp-fortran lsp-eslint
lsp-erlang lsp-emmet lsp-elixir lsp-elm lsp-dockerfile lsp-dhall lsp-d
lsp-css lsp-csharp lsp-crystal lsp-credo lsp-cmake lsp-clojure
lsp-semantic-tokens lsp-clangd lsp-beancount lsp-bash lsp-astro lsp-awk
lsp-ansible lsp-angular lsp-ada lsp-actionscript lsp-mode lsp-protocol
spinner network-stream inline ht dtrt-indent eval-sexp-fu eros
lispyville lispy lispy-inline etags fileloop lispy-tags mode-local
zoutline elisp-def ert ewoc evil-collection-xref xref sotlisp skeleton
evil-collection-vundo vundo ripgrep-capf dabbrev haml-mode js
c-ts-common cc-mode cc-fonts cc-guess cc-menus cc-cmds cc-styles
cc-align cc-engine css-mode magit-extras diary-lib diary-loaddefs view
org-indent org-appear orgonomic org-superstar form-feed org-journal
org-crypt cal-iso avy consult dired-aux evil-nerd-commenter
evil-nerd-commenter-operator evil-nerd-commenter-sdk sgml-mode facemenu
markdown-mode vc-hg vc-bzr vc-src vc-sccs vc-svn vc-cvs vc-rcs log-view
bug-reference flyspell ispell lua-mode magit-delta xterm-color
evil-collection-magit magit-bookmark magit-submodule magit-blame
magit-stash magit-reflog magit-bisect magit-push magit-pull magit-fetch
magit-clone magit-remote magit-commit magit-sequence magit-notes
magit-worktree magit-tag magit-merge magit-branch magit-reset
magit-files magit-refs magit-status magit package url-handlers
magit-repos magit-apply magit-wip magit-log which-func magit-diff
smerge-mode diff git-commit log-edit pcvs-util add-log magit-core
magit-autorevert magit-margin magit-transient magit-process with-editor
magit-mode transient-posframe transient magit-git magit-base crm
sh-script treesit executable tramp-cmds evil-mc evil-mc-command-execute
evil-mc-command-record evil-mc-cursor-make evil-mc-region
evil-mc-cursor-state evil-mc-undo evil-mc-vars evil-mc-known-commands
evil-mc-common evil-collection-helpful helpful cc-langs cc-vars cc-defs
trace evil-collection-edebug edebug evil-collection-debug debug
backtrace info-look info f f-shortdoc help-fns radix-tree elisp-refs
windmove tab-line hide-mode-line popup-mode-core evil-collection-vterm
vterm bookmark tramp trampver tramp-integration files-x tramp-message
tramp-compat tramp-loaddefs term ehelp vterm-module term/xterm xterm
oc-basic ol-eww eww url-queue mm-url ol-rmail ol-mhe ol-irc ol-info
ol-gnus nnselect gnus-art mm-uu mml2015 mm-view mml-smime smime gnutls
dig gnus-sum shr pixel-fill kinsoku url-file svg dom browse-url
gnus-group gnus-undo gnus-start gnus-dbus dbus xml gnus-cloud nnimap
nnmail mail-source utf7 nnoo parse-time gnus-spec gnus-int gnus-range
message sendmail yank-media rfc822 mml mml-sec epa epg rfc6068
epg-config mm-decode mm-bodies mm-encode mailabbrev gmm-utils mailheader
gnus-win gnus nnheader gnus-util mail-utils range ol-docview doc-view
image-mode exif ol-bibtex bibtex iso8601 ol-bbdb ol-w3m ol-doi
org-link-doi evil-terminal-cursor-changer color flycheck xt-mouse
pixel-scroll cua-base org-download url-http url url-proxy url-privacy
url-expand url-methods url-history mailcap url-auth mail-parse rfc2231
rfc2047 rfc2045 mm-util ietf-drums mail-prsvr url-cookie
generate-lisp-file url-domsuf url-gw nsm puny async vulpea vulpea-meta
vulpea-select vulpea-buffer vulpea-db s vulpea-utils vulpea-note
org-roam-migrate org-roam-log org-roam-mode org-roam-capture org-roam-id
org-roam-node org-roam-db emacsql-sqlite-builtin sqlite org-roam-utils
org-roam-compat org-roam org-mac-link org-goto org-capture org-attach
evil-org-agenda evil-org imenu ox-odt rng-loc rng-uri rng-parse
rng-match rng-dt rng-util rng-pttrn nxml-parse nxml-ns nxml-enc xmltok
nxml-util ox-latex ox-icalendar org-agenda ox-ascii ox-gfm ox-md ox-html
table ox-publish ox org-element org-persist xdg org-id org-refile
avl-tree generator org-tempo tempo ob-shell shell org ob ob-tangle
ob-ref ob-lob ob-table ob-exp org-macro org-src ob-comint org-pcomplete
pcomplete org-list org-footnote org-faces org-entities time-date
noutline outline ob-emacs-lisp ob-core ob-eval org-cycle org-table ol
org-fold org-fold-core org-keys oc org-loaddefs find-func cal-menu
calendar cal-loaddefs org-version org-compat org-macs emacsql-sqlite
emacsql-sqlite-common emacsql emacsql-compiler magit-section format-spec
dash jka-compr url-util cape corfu-prescient corfu-history corfu
evil-ruby-text-objects ruby-refactor elec-pair envrc inheritenv
evil-surround evil-matchit-evil-setup bundler inf-ruby compile
text-property-search comint ansi-osc ansi-color ruby-mode smie
undo-fu-session ws-butler vc-git diff-mode vertico-directory saveplace
winner tabspaces dired-x dired dired-loaddefs vc vc-dispatcher
cursor-sensor savehist delsel yasnippet vertico-prescient prescient
char-fold vertico-mouse vertico mini-frame better-jumper autorevert
filenotify ns-auto-titlebar evil-anzu anzu popup-mode-hacks
which-key-posframe posframe evil-collection-which-key which-key gcmh
recentf tree-widget repeat orderless popup-mode popup-mode-settings
evil-collection annalist evil-little-word cus-edit cus-start cus-load
wid-edit pp evil evil-integration evil-maps evil-commands reveal
evil-jumps evil-command-window evil-types evil-search evil-ex
evil-macros evil-repeat evil-states evil-core advice evil-common
thingatpt rect evil-vars memoize nano-modeline nano-light-theme
face-remap nano-theme disp-table gcmh-autoloads copy-as-format-autoloads
pdf-tools-autoloads tablist-autoloads restclient-autoloads
vterm-autoloads dumb-jump-autoloads popup-autoloads haml-mode-autoloads
emmet-mode-autoloads terraform-mode-autoloads hcl-mode-autoloads
dockerfile-mode-autoloads yaml-mode-autoloads json-mode-autoloads
json-snatcher-autoloads grip-mode-autoloads lua-mode-autoloads
bundler-autoloads inf-ruby-autoloads ruby-refactor-autoloads
evil-ruby-text-objects-autoloads sotlisp-autoloads elisp-def-autoloads
lispyville-autoloads lispy-autoloads zoutline-autoloads swiper-autoloads
ivy-autoloads iedit-autoloads eros-autoloads eval-sexp-fu-autoloads
web-mode-autoloads ripgrep-capf-autoloads git-link-autoloads
consult-git-commit-autoloads git-timemachine-autoloads
magit-delta-autoloads xterm-color-autoloads prettier-autoloads
editorconfig-autoloads nvm-autoloads iter2-autoloads flycheck-autoloads
pkg-info-autoloads epl-autoloads lsp-ui-autoloads lsp-mode-autoloads
markdown-mode-autoloads spinner-autoloads imenu-list-autoloads
org-superstar-autoloads ox-gfm-autoloads org-pandoc-import-autoloads
gnuplot-autoloads org-download-autoloads async-autoloads
org-journal-autoloads vulpea-autoloads org-roam-autoloads
emacsql-autoloads orgonomic-autoloads org-drill-autoloads
persist-autoloads org-appear-autoloads org-mac-link-autoloads
evil-org-autoloads evil-terminal-cursor-changer-autoloads
transient-posframe-autoloads better-jumper-autoloads
buffer-move-autoloads rotate-autoloads mini-frame-autoloads
embark-consult-autoloads embark-autoloads consult-autoloads
orderless-autoloads cape-autoloads corfu-prescient-autoloads
corfu-autoloads vertico-prescient-autoloads vertico-autoloads
prescient-autoloads tabspaces-autoloads which-key-posframe-autoloads
which-key-autoloads popup-mode-autoloads hide-mode-line-autoloads
evil-anzu-autoloads anzu-autoloads titlecase-autoloads wgrep-autoloads
yasnippet-autoloads form-feed-autoloads drag-stuff-autoloads
dtrt-indent-autoloads ws-butler-autoloads evil-collection-autoloads
annalist-autoloads evil-mc-autoloads evil-numbers-autoloads
speeddating-autoloads evil-little-word-autoloads evil-matchit-autoloads
evil-nerd-commenter-autoloads evil-visualstar-autoloads
evil-surround-autoloads vundo-autoloads undo-fu-session-autoloads
ztree-autoloads dwim-shell-command-autoloads treemacs-tab-bar-autoloads
treemacs-magit-autoloads magit-autoloads magit-section-autoloads
git-commit-autoloads with-editor-autoloads treemacs-evil-autoloads
evil-autoloads goto-chg-autoloads treemacs-autoloads cfrs-autoloads
ht-autoloads pfuture-autoloads ace-window-autoloads avy-autoloads
all-the-icons-autoloads rainbow-mode-autoloads posframe-autoloads
ns-auto-titlebar-autoloads nano-modeline-autoloads nano-theme-autoloads
memoize-autoloads envrc-autoloads inheritenv-autoloads helpful-autoloads
elisp-refs-autoloads f-autoloads s-autoloads dired-subtree-autoloads
dired-hacks-utils-autoloads dash-autoloads project server pcase hydra lv
url-parse auth-source eieio eieio-core password-cache json map url-vars
edmacro kmacro byte-opt ring compdef derived leader-key bind-map
no-littering compat compdef-autoloads hydra-autoloads lv-autoloads
leader-key-autoloads bind-map-autoloads no-littering-autoloads
compat-autoloads elpaca-use-package use-package use-package-ensure
use-package-delight use-package-diminish use-package-bind-key bind-key
easy-mmode use-package-core elpaca-use-package-autoloads elpaca
elpaca-process elpaca-autoloads comp comp-cstr warnings subr-x rx cl-seq
cl-macs gv bytecomp byte-compile cl-extra help-mode icons cl-loaddefs
cl-lib display-line-numbers rmc iso-transl tooltip cconv eldoc paren
electric uniquify ediff-hook vc-hooks lisp-float-type elisp-mode mwheel
term/ns-win ns-win ucs-normalize mule-util term/common-win touch-screen
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 kqueue
cocoa ns lcms2 multi-tty make-network-process native-compile emacs)

Memory information:
((conses 16 2080572 2381461) (symbols 48 77654 1056) (strings 32 451266 180979)
 (string-bytes 1 11467384) (vectors 16 155030) (vector-slots 8 3667916 2180885)
 (floats 8 1945 7523) (intervals 56 76874 1098) (buffers 992 36))

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

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

Aaron Jensen <aaronjensen@gmail.com> writes:

> Commit 1da4fca0647ebf1d5d6f12817301a17661560810 caused a regression of
> bug#52231
>
> The repro is the same:
>
> (progn (setq scroll-margin 4)
>        (pixel-scroll-precision-mode))
>
> And scroll down a buffer with mouse wheel.
>
> The buffer does not scroll properly, it jumps back unless you scroll
> fast enough.

Hmm, I'm not certain what the solution to this should be.

For images to scroll properly, the "target point" must be derived from
whether the point is visible after scrolling, instead of outside a set
number of rows from the window start or end.  Yet the latter information
is mandatory if the scroll margin is to be taken into account, and no
function supplies both besides posn-at-point, which is much too slow.

The immediate remedy is to restore the old code when scroll-margin is in
effect and document the consequent incapacity to scroll over large
images as an unfortunate corollary.  Is that acceptable by you?

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Eli Zaretskii]

> Cc: 66769@debbugs.gnu.org
> Date: Sat, 28 Oct 2023 10:17:51 +0800
> From:  Po Lu via "Bug reports for GNU Emacs,
>  the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>
> 
> Aaron Jensen <aaronjensen@gmail.com> writes:
> 
> > Commit 1da4fca0647ebf1d5d6f12817301a17661560810 caused a regression of
> > bug#52231
> >
> > The repro is the same:
> >
> > (progn (setq scroll-margin 4)
> >        (pixel-scroll-precision-mode))
> >
> > And scroll down a buffer with mouse wheel.
> >
> > The buffer does not scroll properly, it jumps back unless you scroll
> > fast enough.
> 
> Hmm, I'm not certain what the solution to this should be.
> 
> For images to scroll properly, the "target point" must be derived from
> whether the point is visible after scrolling, instead of outside a set
> number of rows from the window start or end.  Yet the latter information
> is mandatory if the scroll margin is to be taken into account, and no
> function supplies both besides posn-at-point, which is much too slow.

What is the "target point" in the above text? target for what?

> The immediate remedy is to restore the old code when scroll-margin is in
> effect and document the consequent incapacity to scroll over large
> images as an unfortunate corollary.  Is that acceptable by you?

Why not use the too-slow posn-at-point, but only in this case?

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

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

Eli Zaretskii <eliz@gnu.org> writes:

> What is the "target point" in the above text? target for what?

The position where point will be moved after the window start and
vscroll are adjusted, which is such that redisplay will not recenter or
otherwise undermine the scrolling operation.

> Why not use the too-slow posn-at-point, but only in this case?

Because with that, precision scrolling slows down to a crawl.

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: aaronjensen@gmail.com,  66769@debbugs.gnu.org
> Date: Sat, 28 Oct 2023 15:35:13 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > What is the "target point" in the above text? target for what?
> 
> The position where point will be moved after the window start and
> vscroll are adjusted, which is such that redisplay will not recenter or
> otherwise undermine the scrolling operation.

And what are the problems in computing this target point in the
particular case described here?

> > Why not use the too-slow posn-at-point, but only in this case?
> 
> Because with that, precision scrolling slows down to a crawl.

Even if it's done "only in this case"?  It should slow down only this
case, no?

And what exactly is the crucial difference between "this case" and the
other cases, where scrolling works correctly?

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

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

Eli Zaretskii <eliz@gnu.org> writes:

> And what are the problems in computing this target point in the
> particular case described here?

It should be outside the scroll margin, so additional layout
computations must be performed after scrolling, compounding those
performed beforehand to establish the new window start.

> Even if it's done "only in this case"?  It should slow down only this
> case, no?
>
> And what exactly is the crucial difference between "this case" and the
> other cases, where scrolling works correctly?

The distinction is that scroll-margin is set.

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: aaronjensen@gmail.com,  66769@debbugs.gnu.org
> Date: Sat, 28 Oct 2023 16:34:17 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > And what are the problems in computing this target point in the
> > particular case described here?
> 
> It should be outside the scroll margin, so additional layout
> computations must be performed after scrolling, compounding those
> performed beforehand to establish the new window start.
> 
> > Even if it's done "only in this case"?  It should slow down only this
> > case, no?
> >
> > And what exactly is the crucial difference between "this case" and the
> > other cases, where scrolling works correctly?
> 
> The distinction is that scroll-margin is set.

That's what I thought, and which is why I asked whether calling the
slow posn-at-point only when scroll-margin is non-zero wouldn't be the
proper solution, as it should only slow down scrolling for those users
who set scroll-margin.  Or what am I missing?

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Aaron Jensen]

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

Depending on what "slow down to a crawl" means exactly in practice, I think
the reason is that it would cripple a feature. I don't know how many people
use scroll-margin, but I've used it for years. I suppose I just would have
to choose between precision scrolling and scroll margin, but I would have
to choose between them to support something that doesn't matter to me:
scrolling with images.

It also introduces additional complexity and variation in the scrolling
code, which in general, means higher overall maintenance costs (not that
it's my role to police this in Emacs).

I gather that it is redisplay that attempts to reconcile scroll-margin, is
that correct? If so, is there a way to flip scroll-margin on its head such
that it is only intentional point movement operations that cause
scroll-margin to trigger scrolling? i.e., when doing pixel scrolling, you
either temporarily disable the scroll margin (which has the negative impact
of once a user does move the point, it will cause a jump), or, after a
pixel scroll is done, you move the point to be outside of the bounds of the
scroll margin, rather than allowing the redisplay to change the scroll
position. Perhaps that is what you are describing and is what would require
posn-at-point.


Aaron


On Sat, Oct 28, 2023 at 4:42 AM, Eli Zaretskii <eliz@gnu.org> wrote:

> From: Po Lu <luangruo@yahoo.com>
> Cc: aaronjensen@gmail.com, 66769@debbugs.gnu.org
> Date: Sat, 28 Oct 2023 16:34:17 +0800
>
> Eli Zaretskii <eliz@gnu.org> writes:
>
> And what are the problems in computing this target point in the particular
> case described here?
>
> It should be outside the scroll margin, so additional layout computations
> must be performed after scrolling, compounding those performed beforehand
> to establish the new window start.
>
> Even if it's done "only in this case"? It should slow down only this case,
> no?
>
> And what exactly is the crucial difference between "this case" and the
> other cases, where scrolling works correctly?
>
> The distinction is that scroll-margin is set.
>
> That's what I thought, and which is why I asked whether calling the slow
> posn-at-point only when scroll-margin is non-zero wouldn't be the proper
> solution, as it should only slow down scrolling for those users who set
> scroll-margin. Or what am I missing?
>

[-- Attachment #2: Type: text/html, Size: 4119 bytes --]

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Eli Zaretskii]

> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Sat, 28 Oct 2023 05:33:06 -0700
> Cc: Po Lu <luangruo@yahoo.com>, 66769@debbugs.gnu.org
> 
> I gather that it is redisplay that attempts to reconcile scroll-margin, is that correct?

Yes, of course.  Moreover, the scrolling itself is done by the display
engine, and it keeps point out of the scroll-margin as part of that.

> If so, is there a way
> to flip scroll-margin on its head such that it is only intentional point movement operations that cause
> scroll-margin to trigger scrolling? i.e., when doing pixel scrolling, you either temporarily disable the
> scroll margin (which has the negative impact of once a user does move the point, it will cause a
> jump),

Maybe someone will come up with some clever trick, but if so, it will
be very fragile.

You seem to think that redisplay knows what moved point, or more
generally why there should be some change to be done on display, like
whether it was the fact that point moved or something else.  But
that's not what happens.  What actually happens is that redisplay is
called whenever Emacs is idle, and it (redisplay) then needs to decide
whether something should be done to redraw some part of the Emacs
display, and if so, which parts and how to do that.  The reason _why_
the display needs to be updated is mostly not known to the display
engine, at least not in terms of user commands, which is the level
which you seem to have in mind.

Moreover, even if by some trick we succeed to "persuade" redisplay to
refrain from some action, the very next redisplay cycle will almost
certainly "forget" that reason and do what we don't want anyway.  For
example, if redisplay finds that point is inside the scroll margin, it
will scroll the window or move point so as to have point out of
scroll-margin.  So any such temporary measure, even if it succeeds,
will be almost immediately undone.

> or, after a pixel scroll is done, you move the point to be outside of the bounds of the scroll
> margin, rather than allowing the redisplay to change the scroll position. Perhaps that is what you are
> describing and is what would require posn-at-point.

AFAIU, this is what Po Lu was thinking about, and this is what he
said would be too slow.

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Aaron Jensen]

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

On Fri, Oct 27, 2023 at 10:17 PM, Po Lu <luangruo@yahoo.com> wrote:

> Aaron Jensen <aaronjensen@gmail.com> writes:
>
> Commit 1da4fca0647ebf1d5d6f12817301a17661560810 caused a regression of
> bug#52231
>
> The repro is the same:
>
> (progn (setq scroll-margin 4)
> (pixel-scroll-precision-mode))
>
> And scroll down a buffer with mouse wheel.
>
> The buffer does not scroll properly, it jumps back unless you scroll fast
> enough.
>
> Hmm, I'm not certain what the solution to this should be.
>
> For images to scroll properly, the "target point" must be derived from
> whether the point is visible after scrolling, instead of outside a set
> number of rows from the window start or end. Yet the latter information is
> mandatory if the scroll margin is to be taken into account, and no function
> supplies both besides posn-at-point, which is much too slow.
>
> The immediate remedy is to restore the old code when scroll-margin is in
> effect and document the consequent incapacity to scroll over large images
> as an unfortunate corollary. Is that acceptable by you?
>

It looks like the current code uses posn-at-point already, yes? What is it
that would make it too slow to use it again for the point? I'm trying to
understand the code and making some headway, but it's still not totally
clear what's happening and why. It does seem that if you force a redisplay
after the set-window-vscroll, the window-start will move in the case that
it butts up against the scroll margin.

Is there a fast way to compute the position that is scroll-margin lines
away from the window start and then compare the point to that? Or is the
bigger problem when scrolling up?

Aaron

[-- Attachment #2: Type: text/html, Size: 2738 bytes --]

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

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

Aaron Jensen <aaronjensen@gmail.com> writes:

> It looks like the current code uses posn-at-point already, yes? What
> is it that would make it too slow to use it again for the point?

posn-at-point is presently not used by p-s-p-m.

> I'm trying to understand the code and making some headway, but it's
> still not totally clear what's happening and why. It does seem that if
> you force a redisplay after the set-window-vscroll, the window-start
> will move in the case that it butts up against the scroll margin.

Yes, because redisplay is the process responsible for enforcing the
scroll margin in the process of maintaining point within the window.

> Is there a fast way to compute the position that is scroll-margin
> lines away from the window start and then compare the point to that?
> Or is the bigger problem when scrolling up?

The problem is two-fold: a position must be calculated that is
scroll-margin rows from the window start or end, but that position must
be replaced by the position of the row farthest from the window boundary
opposite the direction being scrolled in if there are fewer than
scroll-margin rows displayed in the window _after_ the scroll completes.

bug#66769: 30.0.50; pixel-scroll-precision-mode and scroll-margin regression

[Eli Zaretskii]

> Cc: 66769@debbugs.gnu.org
> From: Aaron Jensen <aaronjensen@gmail.com>
> Date: Wed, 1 Nov 2023 22:49:40 -0700
> 
> Is there a fast way to compute the position that is scroll-margin lines away from the window start

Try something like

  (save-excursion
  (goto-char (window-start))
  (vertical-motion scroll-margin)
  (point))

Caveat: I have no idea if this is "fast enough" for the purposes of
pixel-scroll-precision-mode.