unofficial mirror of emacs-devel@gnu.org 
 help / color / mirror / code / Atom feed
* Temporarily select-window, without updating mode-line face and cursor fill?
@ 2021-05-01 18:46 JD Smith
  2021-05-01 19:31 ` Eli Zaretskii
  0 siblings, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-01 18:46 UTC (permalink / raw)
  To: emacs-devel

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

I’m working on a small “in mode-line” scroll bar, and have implemented mouse events for it, which are working well.  When I interact using the mouse with the mode line of a non-selected window, I need to have that window temporarily selected, so I can calculate its line offsets and move within it.  But (just as for regular scrollbars), I’d like to retain the originally active window afterwards.

Strangely, using `with-selected-window’, which seems like the obvious solution, leaves the mode line face and cursor in a spurious selected state (dark, and filled, respectively).  This is despite the fact that the original window is in fact re-selected after you leave the form.  This leads to two mode lines on the same frame with active mode line face (see below; notice the frame title)!

Wrapping the code in (select-window win)… (select-window old) instead of using `with-selected-window` solves this spurious "double-active" issue, but causes the mode-line to flash “active” while interacting.  Is there a way to select a window but NOT update the window’s mode line face and cursor “active” indicators?

Mouse handler below (attached via keymap text property only to the scroll bar at right).  GNU Emacs 27.2 (build 1, x86_64-apple-darwin18.7.0, Carbon Version 158 AppKit 1671.6) of 2021-03-27.

++++

(defun mlscroll-mouse (start-event)
  "Handle click and drag mouse events on the mode line scroll bar."
  (interactive "e")
  (let* ((start-posn (event-start start-event))
	 (start-win (posn-window start-posn))
	 (lcr (cdr-safe (posn-object start-posn)))
	 (x (car (posn-object-x-y start-posn)))
	 (xstart-abs (car (posn-x-y start-posn)))
	 (mouse-fine-grained-tracking t)
	 (old-selected-window (selected-window))
	 xstart event end xnew)
    (select-window start-win)
    (setq xstart (mlscroll-scroll-to x lcr)) 
    (track-mouse
      (setq track-mouse 'dragging)
      (while (progn (setq event (read-event))
		    (mouse-movement-p event))
	(setq end (event-end event)
	      xnew (+ xstart (- (car (posn-x-y end)) xstart-abs)))
	(when (and
	       (>= xnew 0)
	       (<= xnew (- mlscroll-width mlscroll-border))
	       (eq start-win (posn-window end)))
	  (mlscroll-scroll-to xnew))))
    (select-window old-selected-window)))




[-- Attachment #2.1: Type: text/html, Size: 4650 bytes --]

[-- Attachment #2.2: PastedGraphic-1.png --]
[-- Type: image/png, Size: 139808 bytes --]

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 18:46 Temporarily select-window, without updating mode-line face and cursor fill? JD Smith
@ 2021-05-01 19:31 ` Eli Zaretskii
  2021-05-01 20:32   ` JD Smith
  2021-05-01 22:17   ` JD Smith
  0 siblings, 2 replies; 22+ messages in thread
From: Eli Zaretskii @ 2021-05-01 19:31 UTC (permalink / raw)
  To: JD Smith; +Cc: emacs-devel

> From: JD Smith <jdtsmith@gmail.com>
> Date: Sat, 1 May 2021 14:46:31 -0400
> 
> I’m working on a small “in mode-line” scroll bar, and have implemented mouse events for it, which are working well.  When I interact using the mouse with the mode line of a non-selected window, I need to have that window temporarily selected, so I can calculate its line offsets and move within it.  But (just as for regular scrollbars), I’d like to retain the originally active window afterwards.

Please explain in more detail why do you think you need to select the
window.  What are those "line offsets", and why "move within it"
requires to select the window?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 19:31 ` Eli Zaretskii
@ 2021-05-01 20:32   ` JD Smith
  2021-05-02  6:49     ` Eli Zaretskii
  2021-05-02  7:40     ` martin rudalics
  2021-05-01 22:17   ` JD Smith
  1 sibling, 2 replies; 22+ messages in thread
From: JD Smith @ 2021-05-01 20:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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



> On May 1, 2021, at 3:31 PM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> Date: Sat, 1 May 2021 14:46:31 -0400
>> 
>> I’m working on a small “in mode-line” scroll bar, and have implemented mouse events for it, which are working well.  When I interact using the mouse with the mode line of a non-selected window, I need to have that window temporarily selected, so I can calculate its line offsets and move within it.  But (just as for regular scrollbars), I’d like to retain the originally active window afterwards.
> 
> Please explain in more detail why do you think you need to select the
> window.  What are those "line offsets", and why "move within it"
> requires to select the window?

Sure, thanks.  I compute line positions at a number of points (window-start, window-end, and point-max) inside a mode line :eval form.  To compute line numbers at these positions, for speed in very long files, I’m actually using:

(defun mlscroll-fast-line-number-at-pos (pos)
  (save-excursion
    (goto-char pos)
    (string-to-number (format-mode-line "%l"))))

The operative bits of the “move within” look like:

   (when (/= targ start)
      (forward-line (- targ start))
      (recenter))

During normal automatic mode-line update, the selected window is automatically bound correctly while the mode line string is computed (without updating the mode line face, I might add!).  But during the mouse event callback, I must use the starting window of the mouse event (a press) as the window to target, since the user could click on any of them. 

I believe I can re-task all of the “computing lines” code to take a window argument, i.e. to avoid actually selecting the window itself.  But less clear is how to move forward and back a given number of lines (forward-line) and recenter in an unselected window, without selecting it temporarily and causing the mode line face flashing.


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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 19:31 ` Eli Zaretskii
  2021-05-01 20:32   ` JD Smith
@ 2021-05-01 22:17   ` JD Smith
  2021-05-02  6:55     ` Eli Zaretskii
  1 sibling, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-01 22:17 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel


> Please explain in more detail why do you think you need to select the
> window.  What are those "line offsets", and why "move within it"
> requires to select the window?


Update: I managed to avoid selecting the window by using a somewhat more complex line move, based on set-window-start:

    (when (/= targ start)
      (set-window-start
       win
       (with-current-buffer (window-buffer win)
	 (save-excursion
	   (goto-char wstart)
	   (forward-line (- targ start))
	   (point)))))

Here wstart is the window start in the target window.  I presume with-current-buffer in the same buffer and set-window-start in the selected window is pretty efficient.  I also modified my fast line number function to work with a non-selected window as well (also somewhat convoluted, feedback welcome):

(defun mlscroll-fast-line-number-at-pos (pos &optional win)
  "Line number at position.
Compute line number at position POS. Uses fast mode-line
formatting.  If WIN is non-nil, find line number at position within
that window."
  (string-to-number
   (if win
       (let ((old (window-point win)))
	 (set-window-point win pos)
	 (prog1
	     (format-mode-line "%l" 0 win)
	   (set-window-point win old)))
	 (save-excursion
	   (goto-char pos)
	   (format-mode-line "%l" 0)))))


^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 20:32   ` JD Smith
@ 2021-05-02  6:49     ` Eli Zaretskii
  2021-05-03  2:15       ` JD Smith
  2021-05-02  7:40     ` martin rudalics
  1 sibling, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-05-02  6:49 UTC (permalink / raw)
  To: JD Smith; +Cc: emacs-devel

> From: JD Smith <jdtsmith@gmail.com>
> Date: Sat, 1 May 2021 16:32:22 -0400
> Cc: emacs-devel@gnu.org
> 
> Sure, thanks.  I compute line positions at a number of points (window-start, window-end, and point-max)
> inside a mode line :eval form.  To compute line numbers at these positions, for speed in very long files, I’m
> actually using:
> 
> (defun mlscroll-fast-line-number-at-pos (pos)
>   (save-excursion
>     (goto-char pos)
>     (string-to-number (format-mode-line "%l"))))

I think your are doing premature optimization here.  Which alternative
method of counting lines you thought would be too slow?

> The operative bits of the “move within” look like:
> 
>    (when (/= targ start)
>       (forward-line (- targ start))
>       (recenter))

forward-line works on the current buffer, it doesn't care about the
window, AFAIK.

> During normal automatic mode-line update, the selected window is automatically bound correctly while the
> mode line string is computed (without updating the mode line face, I might add!).  But during the mouse event
> callback, I must use the starting window of the mouse event (a press) as the window to target, since the user
> could click on any of them. 

Sure, but the event gives you the window, and through it the buffer of
that window, so I see no particular reason to make the window
selected.

Alternatively, you could try doing this as mwheel.el does it.



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 22:17   ` JD Smith
@ 2021-05-02  6:55     ` Eli Zaretskii
  2021-05-03  2:08       ` JD Smith
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-05-02  6:55 UTC (permalink / raw)
  To: JD Smith; +Cc: emacs-devel

> From: JD Smith <jdtsmith@gmail.com>
> Date: Sat, 1 May 2021 18:17:46 -0400
> Cc: emacs-devel@gnu.org
> 
> (defun mlscroll-fast-line-number-at-pos (pos &optional win)
>   "Line number at position.
> Compute line number at position POS. Uses fast mode-line
> formatting.  If WIN is non-nil, find line number at position within
> that window."
>   (string-to-number
>    (if win
>        (let ((old (window-point win)))
> 	 (set-window-point win pos)
> 	 (prog1
> 	     (format-mode-line "%l" 0 win)
> 	   (set-window-point win old)))
> 	 (save-excursion
> 	   (goto-char pos)
> 	   (format-mode-line "%l" 0)))))

I don't recommend using format-mode-line for counting lines.  Why not
use count-lines instead?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-01 20:32   ` JD Smith
  2021-05-02  6:49     ` Eli Zaretskii
@ 2021-05-02  7:40     ` martin rudalics
  2021-05-03  2:23       ` JD Smith
  1 sibling, 1 reply; 22+ messages in thread
From: martin rudalics @ 2021-05-02  7:40 UTC (permalink / raw)
  To: JD Smith, Eli Zaretskii; +Cc: emacs-devel

 > Sure, thanks.  I compute line positions at a number of points
 > (window-start, window-end, and point-max) inside a mode line :eval
 > form.

But `window-start' and `window-end' already provide the buffer positions
you need.  So why do think you have to care about the window after all
and do things like `set-window-start' and `set-window-point' in your
following post?

martin



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-02  6:55     ` Eli Zaretskii
@ 2021-05-03  2:08       ` JD Smith
  2021-05-03  2:25         ` Stefan Monnier
  0 siblings, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-03  2:08 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

For very large file, line-number-at-pos, which uses count-lines, if far too slow to be used in a modeline :eval.  I suspect this is why the “%l” mode line formatter does not itself use count-lines.  Here’s my analysis of that (on top of some 7yr old complaints about the speed of line-number-at-pos): https://emacs.stackexchange.com/questions/3821/a-faster-method-to-obtain-line-number-at-pos-in-large-buffers/64656#64656

Summary: format-mode-line is >10x faster on “random” lines, and thousands of times faster for “nearby lines” (presumably due to caching).  The latter is particular important for rapid scrolling updates. 

I managed to get this working quite well on large files (like /usr/dict/words); see https://github.com/jdtsmith/mlscroll.  I indeed use count-lines for incrementing line count from window-start to window-end, since they are “close”, and have simplified the fast line count to:

(defun mlscroll-fast-line-number-at-pos (pos &optional win)
  (let ((old (window-point win)))
    (set-window-point win pos)
    (prog1
	(string-to-number (format-mode-line "%l" 0 win))
      (set-window-point win old))))

If you had any concrete suggestions for calculating window positions (like window-start and window-end, not just buffer positions) in a non-active window without selecting it, which is superior to my approach, I’d be happy to hear. 

> On May 2, 2021, at 2:55 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> Date: Sat, 1 May 2021 18:17:46 -0400
>> Cc: emacs-devel@gnu.org
>> 
>> (defun mlscroll-fast-line-number-at-pos (pos &optional win)
>>  "Line number at position.
>> Compute line number at position POS. Uses fast mode-line
>> formatting.  If WIN is non-nil, find line number at position within
>> that window."
>>  (string-to-number
>>   (if win
>>       (let ((old (window-point win)))
>> 	 (set-window-point win pos)
>> 	 (prog1
>> 	     (format-mode-line "%l" 0 win)
>> 	   (set-window-point win old)))
>> 	 (save-excursion
>> 	   (goto-char pos)
>> 	   (format-mode-line "%l" 0)))))
> 
> I don't recommend using format-mode-line for counting lines.  Why not
> use count-lines instead?




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-02  6:49     ` Eli Zaretskii
@ 2021-05-03  2:15       ` JD Smith
  2021-05-03  7:50         ` martin rudalics
  0 siblings, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-03  2:15 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: emacs-devel

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



> On May 2, 2021, at 2:49 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
> Sure, but the event gives you the window, and through it the buffer of
> that window, so I see no particular reason to make the window
> selected.


A given buffer may appear in multiple windows, which likely will have different “window-start” and “window-end”.  So you have to find the line position in the buffer relevant for that particular window.  Also tying me to this is the fact that the format-mode-line method obviously uses windows, not buffers.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-02  7:40     ` martin rudalics
@ 2021-05-03  2:23       ` JD Smith
  0 siblings, 0 replies; 22+ messages in thread
From: JD Smith @ 2021-05-03  2:23 UTC (permalink / raw)
  To: martin rudalics; +Cc: emacs-devel


I need line position, not points in a buffer, so I need to calculate those. The set-window-point is really just so I can harvest the speed of (format-mode-line “%l”), which works in windows.  I think without that need, I indeed could just use a with-current-buffer, and do all my line position calculating there, as you suggest.  Either way I would still need final set-window-start to scroll a particular window once I have a new location. 

But perhaps there is a simpler approach; happy for more input. 

> On May 2, 2021, at 3:40 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> > Sure, thanks.  I compute line positions at a number of points
> > (window-start, window-end, and point-max) inside a mode line :eval
> > form.
> 
> But `window-start' and `window-end' already provide the buffer positions
> you need.  So why do think you have to care about the window after all
> and do things like `set-window-start' and `set-window-point' in your
> following post?
> 
> martin




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  2:08       ` JD Smith
@ 2021-05-03  2:25         ` Stefan Monnier
  2021-05-03  2:49           ` JD Smith
                             ` (2 more replies)
  0 siblings, 3 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-05-03  2:25 UTC (permalink / raw)
  To: JD Smith; +Cc: Eli Zaretskii, emacs-devel

> If you had any concrete suggestions for calculating window positions
> (like window-start and window-end, not just buffer positions) in
> a non-active window without selecting it, which is superior to my
> approach, I’d be happy to hear. 

We really should simply speed up `line-number-at-pos`.
It shouldn't be hard.  See below what I do in nlinum.el.


        Stefan



(defvar nlinum--line-number-cache nil)
(make-variable-buffer-local 'nlinum--line-number-cache)

;; We could try and avoid flushing the cache at every change, e.g. with:
;;   (defun nlinum--before-change (start _end)
;;     (if (and nlinum--line-number-cache
;;              (< start (car nlinum--line-number-cache)))
;;         (save-excursion (goto-char start) (nlinum--line-number-at-pos))))
;; But it's far from clear that it's worth the trouble.  The current simplistic
;; approach seems to be good enough in practice.

(defun nlinum--after-change (&rest _args)
  (setq nlinum--line-number-cache nil))

(defun nlinum--line-number-at-pos ()
  "Like `line-number-at-pos' but sped up with a cache.
Only works right if point is at BOL."
  ;; (cl-assert (bolp))
  (if nlinum-widen
      (save-excursion
        (save-restriction
          (widen)
          (forward-line 0)              ;In case (point-min) was not at BOL.
          (let ((nlinum-widen nil))
            (nlinum--line-number-at-pos))))
    (let ((pos
           (if (and nlinum--line-number-cache
                    (> (- (point) (point-min))
                       (abs (- (point) (car nlinum--line-number-cache)))))
               (funcall (if (> (point) (car nlinum--line-number-cache))
                            #'+ #'-)
                        (cdr nlinum--line-number-cache)
                        (count-lines (point) (car nlinum--line-number-cache)))
             (line-number-at-pos))))
      ;;(assert (= pos (line-number-at-pos)))
      (add-hook 'after-change-functions #'nlinum--after-change nil :local)
      (setq nlinum--line-number-cache (cons (point) pos))
      pos)))




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  2:25         ` Stefan Monnier
@ 2021-05-03  2:49           ` JD Smith
  2021-05-04 19:28           ` JD Smith
  2021-05-05  0:49           ` Stefan Kangas
  2 siblings, 0 replies; 22+ messages in thread
From: JD Smith @ 2021-05-03  2:49 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

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



> On May 2, 2021, at 10:25 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
> 
> We really should simply speed up `line-number-at-pos`.
> It shouldn't be hard.  See below what I do in nlinum.el.

Thanks.  I saw your interesting cached approach mentioned on S.E., but didn’t test it.  It may in fact be good enough for the actual usage case I was having issues with (scrolling in large files).  I don’t love the after-change function, but using one would also permit long-term caching of the “final” line at (point-max), assuming narrow/widen triggers it.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  2:15       ` JD Smith
@ 2021-05-03  7:50         ` martin rudalics
  2021-05-03 16:16           ` JD Smith
  0 siblings, 1 reply; 22+ messages in thread
From: martin rudalics @ 2021-05-03  7:50 UTC (permalink / raw)
  To: JD Smith, Eli Zaretskii; +Cc: emacs-devel

 >> Sure, but the event gives you the window, and through it the buffer of
 >> that window, so I see no particular reason to make the window
 >> selected.
 >
 >
 > A given buffer may appear in multiple windows, which likely will have
 > different “window-start” and “window-end”.  So you have to find the
 > line position in the buffer relevant for that particular window.  Also
 > tying me to this is the fact that the format-mode-line method
 > obviously uses windows, not buffers.

I'm still missing you.  Both `window-start' and `window-point' have a
WINDOW argument.  So running `get-buffer-window-list' for the buffer you
want to manage and calling the former for each window you get that way
should all do what you want.

And please keep in mind: Calling `select-window' in an :eval form within
`mode-line-format' means asking for trouble.  Calling `set-window-start'
and `set-window-point' anywhere within `mode-line-format' or a hook like
`window-scroll-functions' or `window-state-change-functions' means
asking for more trouble.  All these serve to react to changes in the
window configuration but should never change that configuration itself.

martin




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  7:50         ` martin rudalics
@ 2021-05-03 16:16           ` JD Smith
  2021-05-03 17:32             ` martin rudalics
  0 siblings, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-03 16:16 UTC (permalink / raw)
  To: martin rudalics; +Cc: Eli Zaretskii, emacs-devel

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



> On May 3, 2021, at 3:50 AM, martin rudalics <rudalics@gmx.at> wrote:
> 
> >> Sure, but the event gives you the window, and through it the buffer of
> >> that window, so I see no particular reason to make the window
> >> selected.
> >
> >
> > A given buffer may appear in multiple windows, which likely will have
> > different “window-start” and “window-end”.  So you have to find the
> > line position in the buffer relevant for that particular window.  Also
> > tying me to this is the fact that the format-mode-line method
> > obviously uses windows, not buffers.
> 
> I'm still missing you.  Both `window-start' and `window-point' have a
> WINDOW argument.  So running `get-buffer-window-list' for the buffer you
> want to manage and calling the former for each window you get that way
> should all do what you want.

Since I’m working on the mode line, I am managing windows, not buffers. Mode line :eval forms handle this issue for you automatically, by quietly selecting the appropriate window (even inactive ones).  So nothing is needed there.  Only on modeline _mouse events_ do I need to target a specific window (the one mentioned in the event).  

For calculating the line number in a window which may or may not be selected, (format-mode-line "%l” 0 win) has a window argument, but it does *not* have a position argument.  It takes its position from (as far as I can tell) the window-point of its window argument.  So I need to move window-point and immediately restore it if I want to use format-mode-line.  If the window were selected, a simple save-excursion would be enough.  But I cannot first select the window or I get “mode line flashing".  I need a mythical `save-excursion-in-window', if you will.

But perhaps I am missing you?  If you think there’s a simpler approach that permits the format-mode-line line number workaround without set-window-point, could you share some pseudo-code?

I recognize I could avoid this issue if an optimization like Stefan’s nlinum caching would work well enough, in which case I can drop the use of format-mode-line, and avoid moving window-point at all.  The cost is an after-change-function always running, which some people might object to (me included).   And now simply 

If line-number-at-pos were more performant, and not dependent on position within the buffer, I could avoid both of these things (format-mode-line and an after-change-function).  Quick overview of timing per line-num calc on /usr/dict/words (236k lines) @ (point-max):

line-number-at-pos: 21ms
set-window-point, (string-to-number (format-line-mode %l), restore window-point: 2ms

> And please keep in mind: Calling `select-window' in an :eval form within
> `mode-line-format' means asking for trouble.  Calling `set-window-start'
> and `set-window-point' anywhere within `mode-line-format' or a hook like
> `window-scroll-functions' or `window-state-change-functions' means
> asking for more trouble.  All these serve to react to changes in the
> window configuration but should never change that configuration itself.

I’m not using select-window at all now.  On set-window-point, see above; I admit it’s not clear to me why setting and restoring window-point is any worse than a save-excursion.  

But I will certainly need set-window-start for handling mouse-based events on the mode line (click/drag/scroll).  Perhaps I didn’t make it clear that set-window-start will only be called in mouse-based event callbacks on the mode line; apologies if so.  If even this is problematic in your view, could you clarify the sort of “trouble” it would cause?  Other mouse events in the mode-line call things like ‘previous-buffer, so it’s not clear to me why set-window-start would lead to any special issues when driven by mouse events. 

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03 16:16           ` JD Smith
@ 2021-05-03 17:32             ` martin rudalics
  0 siblings, 0 replies; 22+ messages in thread
From: martin rudalics @ 2021-05-03 17:32 UTC (permalink / raw)
  To: JD Smith; +Cc: Eli Zaretskii, emacs-devel

 > For calculating the line number in a window which may or may not be
 > selected, (format-mode-line "%l” 0 win) has a window argument, but it
 > does *not* have a position argument.  It takes its position from (as
 > far as I can tell) the window-point of its window argument.  So I need
 > to move window-point and immediately restore it if I want to use
 > format-mode-line.  If the window were selected, a simple
 > save-excursion would be enough.  But I cannot first select the window
 > or I get “mode line flashing".  I need a mythical
 > `save-excursion-in-window', if you will.

If you just want to evaluate forms like the above (format-mode-line "%l"
0 win) then all I said does not apply.  I thought you wanted to do such
things via an :eval in the mode line format.

So I probably misunderstood you.  If all you want to do is to exploit
the "%l" construct for getting the line number at window start or window
end, then `set-window-point' should at least not harm (though it might
not work due to scrolling).

 > But I will certainly need set-window-start for handling mouse-based
 > events on the mode line (click/drag/scroll).  Perhaps I didn’t make it
 > clear that set-window-start will only be called in mouse-based event
 > callbacks on the mode line; apologies if so.  If even this is
 > problematic in your view, could you clarify the sort of “trouble” it
 > would cause?  Other mouse events in the mode-line call things like
 > ‘previous-buffer, so it’s not clear to me why set-window-start would
 > lead to any special issues when driven by mouse events.

If you mean `set-window-start' as the effect (or a side effect) of a
mouse-based event on the mode line I see no problem.

martin




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  2:25         ` Stefan Monnier
  2021-05-03  2:49           ` JD Smith
@ 2021-05-04 19:28           ` JD Smith
  2021-05-04 19:40             ` Stefan Monnier
  2021-05-05  0:49           ` Stefan Kangas
  2 siblings, 1 reply; 22+ messages in thread
From: JD Smith @ 2021-05-04 19:28 UTC (permalink / raw)
  To: Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

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


> On May 2, 2021, at 10:25 PM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
> 
> (setq nlinum--line-number-cache nil))

Taking a closer look at Stefan’s idea here, and considering the concerns people have about the set-window-point/format-mode-line approach, I have implemented a new caching system based on nlinum, with the regular old line-number-at-pos and count-lines underneath; see:

https://github.com/jdtsmith/mlscroll/blob/07a657d28d779c3bc56b0a2ea04cfc6d23c54897/mlscroll.el#L87

Neither format-mode-line nor window-point manipulation is needed now.  Unlike nlinum, it also does not require an after-change-function, opting instead to save a buffer tick and point-min/max to look for buffer modifications and any narrowing/widening.  In practice it entirely solves the long file scrolling bottleneck, since effectively all line lookups during scrolling hit the cache, and cached computation of all 3 line positions is >50x faster than even format-mode-line was.

Thanks all for your suggestions/criticism/feedback.  What I can conclude is that the default approach line-number-at-pos takes of “counting all newlines from the start of the file” is really painfully inefficient.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-04 19:28           ` JD Smith
@ 2021-05-04 19:40             ` Stefan Monnier
  0 siblings, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-05-04 19:40 UTC (permalink / raw)
  To: JD Smith; +Cc: Eli Zaretskii, emacs-devel

> Neither format-mode-line nor window-point manipulation is needed now.
> Unlike nlinum, it also does not require an after-change-function, opting
> instead to save a buffer tick and point-min/max to look for buffer
> modifications and any narrowing/widening.

Oh, that's a great idea.  I think this could really be retrofitted
directly into `line-number-at-pos` (and eventually used by
`format-mode-line` as well ;-).


        Stefan




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-03  2:25         ` Stefan Monnier
  2021-05-03  2:49           ` JD Smith
  2021-05-04 19:28           ` JD Smith
@ 2021-05-05  0:49           ` Stefan Kangas
  2021-05-05 11:54             ` Eli Zaretskii
  2 siblings, 1 reply; 22+ messages in thread
From: Stefan Kangas @ 2021-05-05  0:49 UTC (permalink / raw)
  To: Stefan Monnier, JD Smith; +Cc: Eli Zaretskii, emacs-devel

Stefan Monnier <monnier@iro.umontreal.ca> writes:

> We really should simply speed up `line-number-at-pos`.
> It shouldn't be hard.  See below what I do in nlinum.el.

Interesting, it seems much faster at least in this simplistic test:

    (benchmark-run 100000 (line-number-at-pos))
    => (5.868677411999999 0 0.0)

    (benchmark-run 100000 (line-number-at-pos))
    => (0.9772498589999999 1 0.5954873589998897)

Why not to just rename `line-number-at-pos' to
`internal--line-number-at-pos and install something much like the below
as `line-number-at-pos'?

> (defvar nlinum--line-number-cache nil)
> (make-variable-buffer-local 'nlinum--line-number-cache)
>
> ;; We could try and avoid flushing the cache at every change, e.g. with:
> ;;   (defun nlinum--before-change (start _end)
> ;;     (if (and nlinum--line-number-cache
> ;;              (< start (car nlinum--line-number-cache)))
> ;;         (save-excursion (goto-char start) (nlinum--line-number-at-pos))))
> ;; But it's far from clear that it's worth the trouble.  The current simplistic
> ;; approach seems to be good enough in practice.
>
> (defun nlinum--after-change (&rest _args)
>   (setq nlinum--line-number-cache nil))
>
> (defun nlinum--line-number-at-pos ()
>   "Like `line-number-at-pos' but sped up with a cache.
> Only works right if point is at BOL."
>   ;; (cl-assert (bolp))
>   (if nlinum-widen
>       (save-excursion
>         (save-restriction
>           (widen)
>           (forward-line 0)              ;In case (point-min) was not at BOL.
>           (let ((nlinum-widen nil))
>             (nlinum--line-number-at-pos))))
>     (let ((pos
>            (if (and nlinum--line-number-cache
>                     (> (- (point) (point-min))
>                        (abs (- (point) (car nlinum--line-number-cache)))))
>                (funcall (if (> (point) (car nlinum--line-number-cache))
>                             #'+ #'-)
>                         (cdr nlinum--line-number-cache)
>                         (count-lines (point) (car nlinum--line-number-cache)))
>              (line-number-at-pos))))
>       ;;(assert (= pos (line-number-at-pos)))
>       (add-hook 'after-change-functions #'nlinum--after-change nil :local)
>       (setq nlinum--line-number-cache (cons (point) pos))
>       pos)))



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-05  0:49           ` Stefan Kangas
@ 2021-05-05 11:54             ` Eli Zaretskii
  2021-05-05 19:32               ` Stefan Kangas
  0 siblings, 1 reply; 22+ messages in thread
From: Eli Zaretskii @ 2021-05-05 11:54 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: monnier, jdtsmith, emacs-devel

> From: Stefan Kangas <stefankangas@gmail.com>
> Date: Tue, 4 May 2021 19:49:25 -0500
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> Stefan Monnier <monnier@iro.umontreal.ca> writes:
> 
> > We really should simply speed up `line-number-at-pos`.
> > It shouldn't be hard.  See below what I do in nlinum.el.
> 
> Interesting, it seems much faster at least in this simplistic test:
> 
>     (benchmark-run 100000 (line-number-at-pos))
>     => (5.868677411999999 0 0.0)
> 
>     (benchmark-run 100000 (line-number-at-pos))
>     => (0.9772498589999999 1 0.5954873589998897)

Doesn't this measure the line number of the same position?  If so,
that's about as favorable benchmark for caching as possible, no?



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-05 11:54             ` Eli Zaretskii
@ 2021-05-05 19:32               ` Stefan Kangas
  2021-05-05 19:47                 ` Stefan Monnier
  2021-05-06  0:16                 ` JD Smith
  0 siblings, 2 replies; 22+ messages in thread
From: Stefan Kangas @ 2021-05-05 19:32 UTC (permalink / raw)
  To: Eli Zaretskii; +Cc: monnier, jdtsmith, emacs-devel

Eli Zaretskii <eliz@gnu.org> writes:

>> > We really should simply speed up `line-number-at-pos`.
>> > It shouldn't be hard.  See below what I do in nlinum.el.
>>
>> Interesting, it seems much faster at least in this simplistic test:
>>
>>     (benchmark-run 100000 (line-number-at-pos))
>>     => (5.868677411999999 0 0.0)
>>
>>     (benchmark-run 100000 (line-number-at-pos))
>>     => (0.9772498589999999 1 0.5954873589998897)
>
> Doesn't this measure the line number of the same position?  If so,
> that's about as favorable benchmark for caching as possible, no?

Yup.

Here's another one I threw together, tested here in src/keymap.c:

(defvar bench-run-times 10000)

(defun line-number-at-pos-benchmark (lines)
  (let (results)
    (dolist (fun '(line-number-at-pos nlinum--line-number-at-pos))
      (goto-char (/ (point-max) 2))
      (save-excursion
        (push
         (cons fun
               (cons lines
                     (benchmark-run bench-run-times
                       (let ((n (- (random (/ lines 2))
                                   (random (/ lines 2)))))
                         (forward-line n))
                       (funcall fun))))
         results)))
    results))

(defun run-bench (from to inc)
  (with-current-buffer (get-buffer-create "*Bench-Results*")
    (erase-buffer))
  (mapc
   (lambda (res)
     ;; ((nlinum--line-number-at-pos 0 0.93488997 0 0.0)
     ;;  (line-number-at-pos 0 1.0096957629999999 0 0.0))
     (let ((a (car res))
           (b (cadr res)))
      (with-current-buffer (get-buffer-create "*Bench-Results*")
        (insert (format (concat "%3s: "
                                "%s %6.3f %2d %6.3f | "
                                "%s %6.3f %2d %6.3f  [%5.2f %% faster]\n")
                        (cadr b)
                        (car a) (caddr a) (cadddr a) (car (cddddr a))
                        (car b) (caddr b) (cadddr b) (car (cddddr b))
                        (* (- 1.0 (/ (caddr a) (caddr b))) 100))))))
   (mapcar #'line-number-at-pos-benchmark
           (number-sequence from to inc)))
  (pop-to-buffer "*Bench-Results*"))

Now, (run-bench 0 100 5) gives me:

  0: nlinum--line-number-at-pos  0.963  0  0.000 | line-number-at-pos
1.107  0  0.000  [13.04 % faster]
  5: nlinum--line-number-at-pos  0.027  0  0.000 | line-number-at-pos
0.118  0  0.000  [77.13 % faster]
 10: nlinum--line-number-at-pos  0.030  0  0.000 | line-number-at-pos
0.174  0  0.000  [82.56 % faster]
 15: nlinum--line-number-at-pos  0.031  0  0.000 | line-number-at-pos
0.170  0  0.000  [81.80 % faster]
 20: nlinum--line-number-at-pos  0.033  0  0.000 | line-number-at-pos
0.209  0  0.000  [84.26 % faster]
 25: nlinum--line-number-at-pos  0.035  0  0.000 | line-number-at-pos
0.170  0  0.000  [79.45 % faster]
 30: nlinum--line-number-at-pos  0.037  0  0.000 | line-number-at-pos
0.109  0  0.000  [65.96 % faster]
 35: nlinum--line-number-at-pos  0.039  0  0.000 | line-number-at-pos
0.094  0  0.000  [58.57 % faster]
 40: nlinum--line-number-at-pos  0.040  0  0.000 | line-number-at-pos
0.077  0  0.000  [48.01 % faster]
 45: nlinum--line-number-at-pos  0.044  0  0.000 | line-number-at-pos
0.117  0  0.000  [62.09 % faster]
 50: nlinum--line-number-at-pos  0.044  0  0.000 | line-number-at-pos
0.181  0  0.000  [75.77 % faster]
 55: nlinum--line-number-at-pos  0.046  0  0.000 | line-number-at-pos
0.106  0  0.000  [57.01 % faster]
 60: nlinum--line-number-at-pos  0.048  0  0.000 | line-number-at-pos
0.085  0  0.000  [43.54 % faster]
 65: nlinum--line-number-at-pos  0.052  0  0.000 | line-number-at-pos
0.114  0  0.000  [54.87 % faster]
 70: nlinum--line-number-at-pos  0.052  0  0.000 | line-number-at-pos
0.115  0  0.000  [54.48 % faster]
 75: nlinum--line-number-at-pos  0.055  0  0.000 | line-number-at-pos
0.208  0  0.000  [73.71 % faster]
 80: nlinum--line-number-at-pos  0.058  0  0.000 | line-number-at-pos
0.185  0  0.000  [68.89 % faster]
 85: nlinum--line-number-at-pos  0.059  0  0.000 | line-number-at-pos
0.157  0  0.000  [62.57 % faster]
 90: nlinum--line-number-at-pos  0.061  0  0.000 | line-number-at-pos
0.185  0  0.000  [67.22 % faster]
 95: nlinum--line-number-at-pos  0.063  0  0.000 | line-number-at-pos
0.102  0  0.000  [38.34 % faster]
100: nlinum--line-number-at-pos  0.064  0  0.000 | line-number-at-pos
0.729  1  0.597  [91.24 % faster]

However, if I ramp up bench-run-times to 100000 I start seeing GC hits,
and then nlinum--line-number-at-pos can be up to 10 times slower
instead...



^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-05 19:32               ` Stefan Kangas
@ 2021-05-05 19:47                 ` Stefan Monnier
  2021-05-06  0:16                 ` JD Smith
  1 sibling, 0 replies; 22+ messages in thread
From: Stefan Monnier @ 2021-05-05 19:47 UTC (permalink / raw)
  To: Stefan Kangas; +Cc: Eli Zaretskii, jdtsmith, emacs-devel

> However, if I ramp up bench-run-times to 100000 I start seeing GC hits,
> and then nlinum--line-number-at-pos can be up to 10 times slower
> instead...

I think the approach proposed by JD Smith to use modified-ticks rather
than `after-change-functions` to detect changes should make it possible
to have a replacement that doesn't allocate memory at all (except on the
first use in a buffer to create the cache).


        Stefan




^ permalink raw reply	[flat|nested] 22+ messages in thread

* Re: Temporarily select-window, without updating mode-line face and cursor fill?
  2021-05-05 19:32               ` Stefan Kangas
  2021-05-05 19:47                 ` Stefan Monnier
@ 2021-05-06  0:16                 ` JD Smith
  1 sibling, 0 replies; 22+ messages in thread
From: JD Smith @ 2021-05-06  0:16 UTC (permalink / raw)
  To: Stefan Kangas, Stefan Monnier; +Cc: Eli Zaretskii, emacs-devel

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


> 
> Here's another one I threw together, tested here in src/keymap.c:
> 
> (defvar bench-run-times 10000)
> ...
> However, if I ramp up bench-run-times to 100000 I start seeing GC hits,
> and then nlinum--line-number-at-pos can be up to 10 times slower
> Instead…


I got some speedup for free because I’m always interested in the line number at (point-max), and that remains fixed until the buffer changes. But otherwise, for random jumps that span a large fraction of a file’s length, I found “most recent line” caching doesn’t help you too much, as you might expect.  But far more likely than jumping at random in a buffer is asking for many nearby line numbers in rapid succession (e.g. scrolling).  I simulated that by skipping say 3000±1500 characters ahead at a time.  For that access pattern, the speedup of “most recent” caching is tremendous, and increases linearly with position in a file.  

I should point out that in my testing, format-mode-line %l performs even at worst case 10x better than line-number-at-pos, but both of them scale together with position in the file; i.e. both are counting lines from the beginning of the file (but one is doing it in C).  When a file hasn’t changed, this is just tremendously inefficient.  It does seem to have some kind of very nearby caching built in, since taking small steps (like a couple characters) leads to freakish speed in the 10µs range.

To support more random access patterns, it may pay to cache more than just the “last line”.  E.g. if the distance from the closest last position is >20% of the (narrowed) buffer length, save another line on the cache (up to some fixed number of lines), in addition to the most recent.  Finding the nearest cached point wouldn’t be free, and a deeper cache might slow down sequential (scrolling) cached line calculation a bit, but a modest amount of spread might be worth it to improve performance in the more general case.  I’d personally favor `line-number-at-pos-cached' or perhaps a new optional CACHED argument, since there may be other caching strategies people have employed.

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

^ permalink raw reply	[flat|nested] 22+ messages in thread

end of thread, other threads:[~2021-05-06  0:16 UTC | newest]

Thread overview: 22+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2021-05-01 18:46 Temporarily select-window, without updating mode-line face and cursor fill? JD Smith
2021-05-01 19:31 ` Eli Zaretskii
2021-05-01 20:32   ` JD Smith
2021-05-02  6:49     ` Eli Zaretskii
2021-05-03  2:15       ` JD Smith
2021-05-03  7:50         ` martin rudalics
2021-05-03 16:16           ` JD Smith
2021-05-03 17:32             ` martin rudalics
2021-05-02  7:40     ` martin rudalics
2021-05-03  2:23       ` JD Smith
2021-05-01 22:17   ` JD Smith
2021-05-02  6:55     ` Eli Zaretskii
2021-05-03  2:08       ` JD Smith
2021-05-03  2:25         ` Stefan Monnier
2021-05-03  2:49           ` JD Smith
2021-05-04 19:28           ` JD Smith
2021-05-04 19:40             ` Stefan Monnier
2021-05-05  0:49           ` Stefan Kangas
2021-05-05 11:54             ` Eli Zaretskii
2021-05-05 19:32               ` Stefan Kangas
2021-05-05 19:47                 ` Stefan Monnier
2021-05-06  0:16                 ` JD Smith

Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/emacs.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).