Subject: Re: bug#18923: Alternative scrolling model

From: Eli Zaretskii <[email-masked]>
Date: Sun, 02 Nov 2014 17:14:26 +0200

> From: E Sabof <[email-masked]>
> Date: Sun, 02 Nov 2014 01:15:52 +0000
> 
> I've made a prototype for an alternative way to scroll. Essentially scrolling 
> is done pixelwise irrespective of content. Whole lines are scrolled "normally", 
> and the remainder is vscrolled. If the end result is close to a line boundary 
> it gets "snapped" to it.

> This prevents unpleasant jumping when encountering an image. It doesn't handle 
> the "image taller than window" case, but it would if `st-height' could measure 
> more accurately.

Thanks.

What are the advantages of this alternative way of scrolling, beyond
being in Lisp and eliminating the jumps when encountering an image?
(Btw, a test case for the latter would be nice, perhaps as a separate
bug report.)  If the only advantage is better handling of in-line
images, then perhaps fixing the existing implementation is a better
path forward?

Allow me a few comments about the code below.

> (defvar st-ov nil)

Is this overlay used anywhere?  I couldn't find that.

> (defun st-height (&optional pos)
>   "Won't report accurately, if the line is higher than window."
>   (cl-flet (( posn-y ()
>               (cdr (posn-x-y (or (posn-at-point)
>                                  (progn
>                                    (vertical-motion 0)
>                                    (set-window-start nil (point))
>                                    (posn-at-point)))))))

Did you try using pos-visible-in-window-p?  I think it's what you
want.

>           (when (cl-plusp ws)

Why not use '(> ws 0)' instead?  It's marginally faster, I think.

>       (cl-incf ammount vscroll)

Why not use '(setq amount (+ amount vscroll))' instead?  It should be
marginally faster.

>       (when (cl-minusp direction)

Same question as for cl-plusp above.

>       (cl-loop do (push (st-height) rows)
>                until (or (zerop (vertical-motion direction))
>                          ;; >= ?
>                          (>= (cl-reduce '+ rows)
>                              (abs ammount))))

I don't understand why you needed this loop.  Can't you use
window-body-height instead?

Also, if you do need the loop, why is it a good idea to use cl-reduce
here, rather than keeping track of the running sum -- isn't the latter
faster?  (And I don't think you need to keep the 'rows' list, see
below.)

> (cl-defun st-move (lines vscroll)
>   ;; vscroll changes aren't always displayed. Haven't found a work-around for  this.

A recipe to reproduce this problem, perhaps as a separate bug report,
would be nice.

>          ( line-info (st-get-lines pixels-to-move))
>          ( heights (cadr line-info))
>          ( initial-vscroll (car line-info))
>          excess
>          enough-or-too-many-heights
>          too-few-heights)

>     (cl-incf pixels-to-move initial-vscroll)

>     (setq enough-or-too-many-heights (cl-reduce '+ heights)

It looks like you never need the info about the height of the
individual lines, only their sum.  So perhaps st-get-lines shouldn't
return the height info as a list, but as just one number?

>           too-few-heights (cl-reduce '+ (butlast heights) :initial-value 0)

OK, 2 numbers.

> ;; (global-set-key (kbd "<next>") (lambda () (interactive) (scroll-tick 10)))
> ;; (global-set-key (kbd "<prior>") (lambda () (interactive) (scroll-tick -10)))

This doesn't support the equivalent of a nil argument, which means
move by "near full screen".