Subject: bug#65214: 29.1; Tall images in 'image-mode' can't be scrolled with 'pixel-scroll-precision-mode' on

From: Rahguzar <[email-masked]>
Date: Fri, 11 Aug 2023 16:39:22 +0200

The diff in my last message was not correct, sorry about that. It should
be

@@ -524,12 +524,15 @@ pixel-scroll-precision-scroll-down-page
          (next-pos (save-excursion
                      (goto-char desired-start)
                      (when (zerop (vertical-motion (1+ scroll-margin)))
-                       (set-window-start nil desired-start)
-                       (signal 'end-of-buffer nil))
+                       (unless (let ((remaining-bottom (nth 3 (pos-visible-in-window-p desired-start nil t))))
+                                 (and remaining-bottom (not (zerop remaining-bottom))))
+                         (set-window-start nil desired-start t)
+                         (signal 'end-of-buffer nil)))
                      (while (when-let ((posn (posn-at-point)))
-                              (< (cdr (posn-x-y posn)) delta))
+                              (and (< (point) (point-min))
+                                   (< (cdr (posn-x-y posn)) delta)))
                        (when (zerop (vertical-motion 1))
-                         (set-window-start nil desired-start)
+                         (set-window-start nil desired-start t)
                          (signal 'end-of-buffer nil)))
                      (point)))
          (scroll-preserve-screen-position nil)

Rahguzar <[email-masked]> writes:

> Hi Po,
> In this case it is not the recentering which is causing this problem,
> it is rather the binding for next-pos in
> pixel-scroll-precision-scroll-down-page which is the issue. The
> binding signals an error when downward vertical motion is not
> possible. However in this scenario scrolling can still be done.
> Replacing that binding with desired-start allow me to scroll the
> images.
>
> This change for example allows for scrolling tall images,
> @@ -524,10 +524,12 @@ pixel-scroll-precision-scroll-down-page
>           (next-pos (save-excursion
>                       (goto-char desired-start)
>                       (when (zerop (vertical-motion (1+ scroll-margin)))
> -                       (set-window-start nil desired-start)
> -                       (signal 'end-of-buffer nil))
> +                       (unless (let ((remaining-bottom (nth 3 (pos-visible-in-window-p desired-start nil t))))
> +                                 (and remaining-bottom (not (zerop remaining-bottom))))
> +                         (signal 'end-of-buffer nil)))
>                       (while (when-let ((posn (posn-at-point)))
> -                              (< (cdr (posn-x-y posn)) delta))
> +                              (and (< (point) (point-min))
> +                                   (< (cdr (posn-x-y posn)) delta)))
>                         (when (zerop (vertical-motion 1))
>                           (set-window-start nil desired-start)
>                           (signal 'end-of-buffer nil)))
>
> I am not sure it is the correct change since I don't understand the
> while loop but comparing 'point' and 'point-min' doesn't seem the write
> thing there. On the others hand if there is 'point-min' is 1 and
> 'point-max' 2, (cdr (posn-x-y posn))
>
> Po Lu <[email-masked]> writes:
>
>> This is a known limitation of pixel-scroll-precision-mode: redisplay's
>> automatic recentering impededs adjustment of the vscroll as the image
>> moves outside of the window's text area.  Unfortunately, it's not slated
>> to be fixed any time soon.
>
> In this case it is not the recentering which is causing this problem,
> it is rather the binding for next-pos in
> pixel-scroll-precision-scroll-down-page which is the issue. The
> binding signals an error when downward vertical motion is not
> possible. However in this scenario scrolling can still be done.
> Replacing that binding with desired-start allow me to scroll the
> images.
>
> This change for example allows for scrolling tall images,
> @@ -524,10 +524,12 @@ pixel-scroll-precision-scroll-down-page
>           (next-pos (save-excursion
>                       (goto-char desired-start)
>                       (when (zerop (vertical-motion (1+ scroll-margin)))
> -                       (set-window-start nil desired-start)
> -                       (signal 'end-of-buffer nil))
> +                       (unless (let ((remaining-bottom (nth 3 (pos-visible-in-window-p desired-start nil t))))
> +                                 (and remaining-bottom (not (zerop remaining-bottom))))
> +                         (signal 'end-of-buffer nil)))
>                       (while (when-let ((posn (posn-at-point)))
> -                              (< (cdr (posn-x-y posn)) delta))
> +                              (and (< (point) (point-min))
> +                                   (< (cdr (posn-x-y posn)) delta)))
>                         (when (zerop (vertical-motion 1))
>                           (set-window-start nil desired-start)
>                           (signal 'end-of-buffer nil)))
>
> I am not sure it is the correct change since I don't understand the
> while loop but comparing 'point' and 'point-min' doesn't seem the write
> thing there. On the others hand if there is 'point-min' is 1 and
> 'point-max' 2, (cdr (posn-x-y posn)) is always zero, so that check makes
> the loop terminate without signaling an error.
>
> The behavior is also sub optimal in that it allows for scrolling past
> the lower edge of the image but how much depends on the speed of the
> scrolling, but that can be fixed easily.
>
> Rahguzar