bug#14567: Scrolling of large images

[Eli Zaretskii]

> From: David Engster <deng@randomsample.de>
> Cc: Thomas Wiecki <thomas.wiecki@gmail.com>,  14567@debbugs.gnu.org
> Date: Tue, 11 Jun 2013 22:14:47 +0200
> 
> Eli Zaretskii writes:
> >> I can't really reproduce the line-move error either in this mode. It
> >> does show up occasionally but I haven't figured out how to
> >> explicitly trigger it thus far.
> >
> > I'll try reproducing it here.
> 
> I also tried the patch on current trunk. I use the following snippet to
> generate a buffer containing large and small images as well as text:

Thanks.  Unfortunately, using Emacs with Imagemagick support is a
no-starter for me.  However, I tried to cover every possibility of
using non-numeric values in numeric context.  Please try the patch
below (it's relative to the current trunk, not to the previous patch),
and see if it gives good results.  If you still get errors, please
manually load simple.el (not simple.elc) into your Emacs, run your
test snippet, and see if the backtrace can pinpoint the place where I
missed something.

Thanks.

=== modified file 'lisp/simple.el'
--- lisp/simple.el      2013-06-13 22:24:52 +0000
+++ lisp/simple.el      2013-06-15 07:57:08 +0000
@@ -4738,22 +4738,39 @@ lines."
           (vpos (nth 1 lh))
           (ypos (nth 2 lh))
           (rbot (nth 3 lh))
+          (this-lh (window-line-height))
+          (this-height (nth 0 this-lh))
+          (this-ypos (nth 2 this-lh))
+          (fch (frame-char-height))
           py vs)
       (when (or (null lh)
-               (>= rbot (frame-char-height))
-               (<= ypos (- (frame-char-height))))
+               (>= rbot fch)
+               (<= ypos (- fch))
+               (null this-lh)
+               (<= this-ypos (- fch)))
        (unless lh
          (let ((wend (pos-visible-in-window-p t nil t)))
            (setq rbot (nth 3 wend)
                  vpos (nth 5 wend))))
+       (unless this-lh
+         (let ((wstart (pos-visible-in-window-p nil nil t)))
+           (setq this-ypos (nth 2 wstart)
+                 this-height (nth 4 wstart))))
        (cond
-        ;; If last line of window is fully visible, move forward.
-        ((or (null rbot) (= rbot 0))
+        ;; If last line of window is fully visible, and vscrolling
+        ;; more would make this line invisible, move forward.
+        ((and (or (< (setq vs (window-vscroll nil t)) fch)
+                  (null this-height)
+                  (<= this-height fch))
+              (or (null rbot) (= rbot 0)))
          nil)
-        ;; If cursor is not in the bottom scroll margin, move forward.
-        ((and (> vpos 0)
+        ;; If cursor is not in the bottom scroll margin, and the
+        ;; current line is is not too tall, move forward.
+        ((and (or (null this-height) (<= this-height fch))
+              vpos
+              (> vpos 0)
               (< (setq py
-                       (or (nth 1 (window-line-height))
+                       (or (nth 1 this-lh)
                            (let ((ppos (posn-at-point)))
                              (cdr (or (posn-actual-col-row ppos)
                                       (posn-col-row ppos))))))
@@ -4761,19 +4778,21 @@ lines."
          nil)
         ;; When already vscrolled, we vscroll some more if we can,
         ;; or clear vscroll and move forward at end of tall image.
-        ((> (setq vs (window-vscroll nil t)) 0)
-         (when (> rbot 0)
-           (set-window-vscroll nil (+ vs (min rbot (frame-char-height))) t)))
+        ((> vs 0)
+         (when (or (and rbot (> rbot 0))
+                   (and this-height (> this-height fch)))
+           (set-window-vscroll nil (+ vs fch) t)))
         ;; If cursor just entered the bottom scroll margin, move forward,
         ;; but also vscroll one line so redisplay won't recenter.
-        ((and (> vpos 0)
+        ((and vpos
+              (> vpos 0)
               (= py (min (- (window-text-height) scroll-margin 1)
                          (1- vpos))))
          (set-window-vscroll nil (frame-char-height) t)
          (line-move-1 arg noerror to-end)
          t)
         ;; If there are lines above the last line, scroll-up one line.
-        ((> vpos 0)
+        ((and vpos (> vpos 0))
          (scroll-up 1)
          t)
         ;; Finally, start vscroll.
@@ -4808,7 +4827,14 @@ lines."
               ;; display-based motion doesn't make sense (because each
               ;; logical line occupies exactly one screen line).
               (not (> (window-hscroll) 0)))
-         (line-move-visual arg noerror)
+         (prog1 (line-move-visual arg noerror)
+           ;; If we moved into a tall line, set vscroll to make
+           ;; scrolling through tall images more smooth.
+           (let ((lh (line-pixel-height)))
+             (if (and (< arg 0)
+                      (< (point) (window-start))
+                      (> lh (frame-char-height)))
+                 (set-window-vscroll nil (- lh (frame-char-height)) t))))
        (line-move-1 arg noerror to-end)))))
 
 ;; Display-based alternative to line-move-1.

=== modified file 'src/xdisp.c'
--- src/xdisp.c 2013-06-10 04:36:06 +0000
+++ src/xdisp.c 2013-06-15 07:24:37 +0000
@@ -1217,6 +1217,24 @@ line_bottom_y (struct it *it)
   return line_top_y + line_height;
 }
 
+DEFUN ("line-pixel-height", Fline_pixel_height,
+       Sline_pixel_height, 0, 0, 0,
+       doc: /* Return height in pixels of text line in the selected window.
+
+Value is the height in pixels of the line at point.  */)
+  (void)
+{
+  struct it it;
+  struct text_pos pt;
+  struct window *w = XWINDOW (selected_window);
+
+  SET_TEXT_POS (pt, PT, PT_BYTE);
+  start_display (&it, w, pt);
+  it.vpos = it.current_y = 0;
+  last_height = 0;
+  return make_number (line_bottom_y (&it));
+}
+
 /* Subroutine of pos_visible_p below.  Extracts a display string, if
    any, from the display spec given as its argument.  */
 static Lisp_Object
@@ -28691,6 +28709,7 @@ syms_of_xdisp (void)
   defsubr (&Stool_bar_lines_needed);
   defsubr (&Slookup_image_map);
 #endif
+  defsubr (&Sline_pixel_height);
   defsubr (&Sformat_mode_line);
   defsubr (&Sinvisible_p);
   defsubr (&Scurrent_bidi_paragraph_direction);