Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Stefan Monnier]

>     * lisp/emacs-lisp/timer.el (debounce, debounce-reduce): New macros.

Please use proper prefixes for those names.


        Stefan

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Juri Linkov]

>>     * lisp/emacs-lisp/timer.el (debounce, debounce-reduce): New macros.
>
> Please use proper prefixes for those names.

Maybe timer-debounce and timer-debounce-reduce.

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Stefan Monnier]

>>>     * lisp/emacs-lisp/timer.el (debounce, debounce-reduce): New macros.
>> Please use proper prefixes for those names.
> Maybe timer-debounce and timer-debounce-reduce.

I guess so, tho now that I looked at it I have other questions/comments:

- I don't see `debounce` used anywhere.  Do we need it?
- Why is `debounce-reduce` a macro rather than a function?
- What is the advantage of wrapping a `debounce-reduce` around
  `image--change-size` instead of doing something like:

    (defun image-decrease-size (&optional n)
      "Decrease the image size by a factor of N.
    If N is 3, then the image size will be decreased by 30%.  The
    default is 20%."
      (interactive "P")
      ;; Wait for a bit of idle-time before actually performing the change,
      ;; so as to batch together sequences of closely consecutive size changes.
      (run-with-idle-timer 0.3 nil
                           #'image--change-size
                           (if n
                               (- 1 (/ (prefix-numeric-value n) 10.0))
                             0.8)))

-- Stefan

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Juri Linkov]

>>>>     * lisp/emacs-lisp/timer.el (debounce, debounce-reduce): New macros.
>>> Please use proper prefixes for those names.
>> Maybe timer-debounce and timer-debounce-reduce.
>
> I guess so, tho now that I looked at it I have other questions/comments:
>
> - I don't see `debounce` used anywhere.  Do we need it?

I prepared a patch that uses `debounce`, but then waited until resolving
the question of prefixes.  But now I installed the patch in b31a966e88
and will add the prefixes afterwards.

> - Why is `debounce-reduce` a macro rather than a function?

In its initial versions that used `body` it needed to be a macro,
but now it can be a function indeed.

> - What is the advantage of wrapping a `debounce-reduce` around
>   `image--change-size` instead of doing something like:
>
>     (defun image-decrease-size (&optional n)
>       "Decrease the image size by a factor of N.
>     If N is 3, then the image size will be decreased by 30%.  The
>     default is 20%."
>       (interactive "P")
>       ;; Wait for a bit of idle-time before actually performing the change,
>       ;; so as to batch together sequences of closely consecutive size changes.
>       (run-with-idle-timer 0.3 nil
>                            #'image--change-size
>                            (if n
>                                (- 1 (/ (prefix-numeric-value n) 10.0))
>                              0.8)))

This version doesn't discard a sequence of consecutive calls
where every call is a costly operation.  For example, on slow hardware
every image resize takes about 1 sec.  Thus when mouse-wheel
generates 5 events, this version will wait for the next idle time,
then will resize the image sequentially 5 times during 5 seconds.

Whereas `debounce-reduce` accumulates all mouse wheel events, and then
calls only one resize operation with the collected scaling factor.

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Stefan Monnier]

>>                            #'image--change-size
>>                            (if n
>>                                (- 1 (/ (prefix-numeric-value n) 10.0))
>>                              0.8)))
> This version doesn't discard a sequence of consecutive calls
> where every call is a costly operation.  For example, on slow hardware
> every image resize takes about 1 sec.

I understand that but I don't see why you connect the two:
AFAICT `image--change-size` is a very lightweight function which just
changes one value in a plist.  The costly operation happens later during
redisplay.  So if those consecutive calls happen without any redisplay
between them, you should pay for the costly operation only once.

> Thus when mouse-wheel generates 5 events, this version will wait for
> the next idle time, then will resize the image sequentially 5 times
> during 5 seconds.

I would expect that after the idle time it will call
`image--change-size` five times and then perform one costly
recomputation with the resulting new size.


        Stefan

Re: master 4b5d04b: Use new macro debounce-reduce to make mouse scaling of images more responsive

[Juri Linkov]

>>>                            #'image--change-size
>>>                            (if n
>>>                                (- 1 (/ (prefix-numeric-value n) 10.0))
>>>                              0.8)))
>> This version doesn't discard a sequence of consecutive calls
>> where every call is a costly operation.  For example, on slow hardware
>> every image resize takes about 1 sec.
>
> I understand that but I don't see why you connect the two:
> AFAICT `image--change-size` is a very lightweight function which just
> changes one value in a plist.  The costly operation happens later during
> redisplay.  So if those consecutive calls happen without any redisplay
> between them, you should pay for the costly operation only once.
>
>> Thus when mouse-wheel generates 5 events, this version will wait for
>> the next idle time, then will resize the image sequentially 5 times
>> during 5 seconds.
>
> I would expect that after the idle time it will call
> `image--change-size` five times and then perform one costly
> recomputation with the resulting new size.

I didn't expect that the display engine is optimized to handle this case.
Eli asked to try to bind redisplay-dont-pause to nil, then I tried, but
redisplay-dont-pause has no effect.

But it seems run-with-idle-timer could help.  Not sure how I could confirm
that no images are created for intermediate scaling factors, but if visually
it feels more responsive, then run-with-idle-timer should be used.