[PATCH] Add function window-line-width

classic Classic list List threaded Threaded
13 messages Options
Reply | Threaded
Open this post in threaded view
|

[PATCH] Add function window-line-width

Sébastien Chapuis
Hello,

I would like to propose the function `window-line-width'.
This is to use it in the package lsp-ui[1]

If the changes are accepted, can you send me the copyright assignment form ?

Thanks,
Sebastien Chapuis.


0001-Add-function-window-line-width.patch (6K) Download Attachment
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

martin rudalics
 > I would like to propose the function `window-line-width'.
 > This is to use it in the package lsp-ui[1]

Note that

(window-text-pixel-size
  nil (line-beginning-position) (line-end-position))

does already return the size of the current line.  Some sort of
x-limit is advised in order to handle overlong lines.

martin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

martin rudalics
And I obviously forgot to mention 'window-lines-pixel-dimensions' and
'line-pixel-height'.

martin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Robert Pluim
In reply to this post by Sébastien Chapuis
>>>>> On Thu, 7 Nov 2019 06:49:48 +0800, Sébastien Chapuis <[hidden email]> said:

    Sébastien> Hello,
    Sébastien> I would like to propose the function `window-line-width'.
    Sébastien> This is to use it in the package lsp-ui[1]

Doesnʼt 'window-text-pixel-size' provide most of this already?

(window-text-pixel-size nil (point-at-bol) (point-at-eol))

will give you the pixel length of the current line.

Robert

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Sébastien Chapuis
In reply to this post by martin rudalics
Ah, I didn't know about 'window-lines-pixel-dimensions'.
I knew 'window-line-height' and thought there was no equivalent for the width.

However 'window-lines-pixel-dimensions' has differences with the implementation I proposed:
- It counts the glyphs 'inserted by redisplay for its own purposes' [1].
  On a line with no characters, the function often return a width > 0 (1 or 2 characters, in my tests).
  Similarly, on a line with characters it returns more pixels than there really is.
- It allocates a cons and 2 integer for each line.
  In my use case, I have to get the width of _all_ the lines on the current window every time the cursor is
  changing line or when a character is inserted/removed in the buffer. So it occurs quite often.
  But that's micro optimization and I am not sure if there is a big impact.
- We can't choose the area in the window (left_margin_area, text_area or right_margin_area),
  but I don't need this for my use case, so it's not important to me.

The first point is important for my use case, so if 'window-line-width' is not accepted, would you agree for a patch
adding a parameter to 'window-lines-pixel-dimensions' controlling this behavior ?

'window-text-pixel-size' uses a display iterator to compute values, I don't know the details of this iterator but it seems to
be slower than fetching values directly from the glyph matrix. As mentioned, I need the width of all lines on window.


Thanks,
Sebastien Chapuis.

Le jeu. 7 nov. 2019 à 16:50, martin rudalics <[hidden email]> a écrit :
And I obviously forgot to mention 'window-lines-pixel-dimensions' and
'line-pixel-height'.

martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

martin rudalics
 > The first point is important for my use case, so if 'window-line-width' is
 > not accepted, would you agree for a patch
 > adding a parameter to 'window-lines-pixel-dimensions' controlling this
 > behavior ?

I certainly would but it's up to Eli to decide.

 > 'window-text-pixel-size' uses a display iterator to compute values, I don't
 > know the details of this iterator but it seems to
 > be slower than fetching values directly from the glyph matrix. As
 > mentioned, I need the width of all lines on window.

It would be good to know how much slower 'window-text-pixel-size' then
is.  If it is significantly slower, we probably should replace it with
a function based on directly accessing the glyph matrix.  Can you try
to measure the respective performances?

Thanks, martin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Eli Zaretskii
> From: martin rudalics <[hidden email]>
> Date: Fri, 8 Nov 2019 10:21:45 +0100
> Cc: [hidden email]
>
>  > The first point is important for my use case, so if 'window-line-width' is
>  > not accepted, would you agree for a patch
>  > adding a parameter to 'window-lines-pixel-dimensions' controlling this
>  > behavior ?
>
> I certainly would but it's up to Eli to decide.

If window-text-pixel-size fits the bill, I don't see why we need
another function or a new argument to window-lines-pixel-dimensions.

>  > 'window-text-pixel-size' uses a display iterator to compute values, I don't
>  > know the details of this iterator but it seems to
>  > be slower than fetching values directly from the glyph matrix. As
>  > mentioned, I need the width of all lines on window.
>
> It would be good to know how much slower 'window-text-pixel-size' then
> is.  If it is significantly slower, we probably should replace it with
> a function based on directly accessing the glyph matrix.  Can you try
> to measure the respective performances?

I would be very surprised to learn that it's significantly slower.  Do
you need to invoke this function in a tight loop or something?  If
not, a single invocation, even if slower, won't matter much.

In general, functions that use only the glyph matrices have a
significant disadvantage in that they will fail when display is not up
to date, something that can happen out of control of a Lisp program
that calls the function.  So I very much prefer window-text-pixel-size
to an alternative that uses the glyph matrices.  If we find that using
the glyph matrices is much faster, then I'd prefer to fix
window-text-pixel-size to use the glyph matrices when possible, and
otherwise fall back to its current method.  See move-point-visually
for one example of how this can be done.

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

martin rudalics
 > In general, functions that use only the glyph matrices have a
 > significant disadvantage in that they will fail when display is not up
 > to date, something that can happen out of control of a Lisp program
 > that calls the function.  So I very much prefer window-text-pixel-size
 > to an alternative that uses the glyph matrices.  If we find that using
 > the glyph matrices is much faster, then I'd prefer to fix
 > window-text-pixel-size to use the glyph matrices when possible, and
 > otherwise fall back to its current method.  See move-point-visually
 > for one example of how this can be done.

Agreed.  Sébastien can you try to show "that using the glyph matrices
is much faster"?

martin


Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Sébastien Chapuis
After thinking about it, I am wondering if the current behavior of 'window-lines-pixel-dimensions' is correct ?
As user of the function I do not expect the function to count glyphs that are inserted by redisplay only "for its own purpose",
glyphs that are invisible and non-existent for the end user.
IMHO, for an empty line,  'window-lines-pixel-dimensions' should return the width 0.
A line with X characters should have the width of all thoses characters.
I see that 'window-largest-empty-rectangle' uses this function, the widths of the rectangles it returns are smaller than they really are.

I think it makes sense to change this behavior and make 'window-lines-pixel-dimensions' not counting these "redisplay only" glyphs.
Thus, there would be no need for a new argument to the function.

> Agreed.  Sébastien can you try to show "that using the glyph matrices
> is much faster"?

I tested the following code:

```
(defmacro time (&rest forms)
  (let ((t1 (make-symbol "t1")))
    `(let (,t1)
       (redisplay t)
       (setq ,t1 (current-time))
       ,@forms
       (float-time (time-since ,t1)))))

(defun test1 nil
  (window-lines-pixel-dimensions nil nil nil t))

(defun test2 nil
  (save-excursion
    (goto-char (window-start))
    (let ((index 1)
          (height (window-body-height)))
      (while (<= index height)
        (window-text-pixel-size nil (line-beginning-position index) (line-end-position index))
        (setq index (1+ index))
        ))))
```

`(time (test1))` returns 0.000004869
`(time (test2))` returns 0.00376






Le sam. 9 nov. 2019 à 02:27, martin rudalics <[hidden email]> a écrit :
 > In general, functions that use only the glyph matrices have a
 > significant disadvantage in that they will fail when display is not up
 > to date, something that can happen out of control of a Lisp program
 > that calls the function.  So I very much prefer window-text-pixel-size
 > to an alternative that uses the glyph matrices.  If we find that using
 > the glyph matrices is much faster, then I'd prefer to fix
 > window-text-pixel-size to use the glyph matrices when possible, and
 > otherwise fall back to its current method.  See move-point-visually
 > for one example of how this can be done.

Agreed.  Sébastien can you try to show "that using the glyph matrices
is much faster"?

martin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

martin rudalics
 > A line with X characters should have the width of all thoses characters.
 > I see that 'window-largest-empty-rectangle' uses this function, the widths
 > of the rectangles it returns are smaller than they really are.

There's always the problem that we want to reserve space for a block
cursor at line end.

 > I tested the following code:
[...]
 > `(time (test1))` returns 0.000004869
 > `(time (test2))` returns 0.00376

Then we probably should rewrite 'window-text-pixel-size' as Eli
suggested earlier.

martin

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Sébastien Chapuis
> There's always the problem that we want to reserve space for a block
> cursor at line end.

IMO that's a case that should be handled by the user of the function, not by 'window-lines-pixel-dimensions' itself.
In my few tests, 'window-lines-pixel-dimensions' returns a width with 1 and 2 glyphs more, so it's not just 1 space for the block cursor,
there might be cases where there are more glyphs added (?).
And AFAIK, there is no way to know how many glyphs/pixels the redisplay add.
Anyone who wants to have an accurate number of pixels displayed will be unable to use this function.

> Then we probably should rewrite 'window-text-pixel-size' as Eli
> suggested earlier.

Agreed, but I would prefer to use 'window-lines-pixel-dimensions' since this is exactly the function I need.
As per the documentation [1]:
`window-text-pixel-size treats the text displayed in a window as a whole and does not care about
 the size of individual lines. The following function [window-lines-pixel-dimensions] does.`

[1] https://www.gnu.org/software/emacs/manual/html_node/elisp/Size-of-Displayed-Text.html#Size-of-Displayed-Text

Thanks,
Sebastien Chapuis.


Le dim. 10 nov. 2019 à 17:46, martin rudalics <[hidden email]> a écrit :
 > A line with X characters should have the width of all thoses characters.
 > I see that 'window-largest-empty-rectangle' uses this function, the widths
 > of the rectangles it returns are smaller than they really are.

There's always the problem that we want to reserve space for a block
cursor at line end.

 > I tested the following code:
[...]
 > `(time (test1))` returns 0.000004869
 > `(time (test2))` returns 0.00376

Then we probably should rewrite 'window-text-pixel-size' as Eli
suggested earlier.

martin
Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Eli Zaretskii
In reply to this post by Sébastien Chapuis
> From: Sébastien Chapuis <[hidden email]>
> Date: Sun, 10 Nov 2019 06:09:56 +0800
> Cc: Eli Zaretskii <[hidden email]>, [hidden email]

> After thinking about it, I am wondering if the current behavior of 'window-lines-pixel-dimensions' is correct ?
> As user of the function I do not expect the function to count glyphs that are inserted by redisplay only "for its
> own purpose",
> glyphs that are invisible and non-existent for the end user.

There are valid use cases where those glyphs do matter, see the use of
this function in Emacs's own code.

> I see that 'window-largest-empty-rectangle' uses this function, the widths of the rectangles it returns are
> smaller than they really are.

How do you mean "smaller"?  Can you show an example where the result
is incorrect in your opinion?

> I think it makes sense to change this behavior and make 'window-lines-pixel-dimensions' not counting these
> "redisplay only" glyphs.

I don't see the need, sorry.  If what that function does doesn't fit
your bill, it's probably because you should use another function
instead.

> `(time (test1))` returns 0.000004869
> `(time (test2))` returns 0.00376

So we are talking about 3 msec additional time?  Is that really
significant in your application?

Reply | Threaded
Open this post in threaded view
|

Re: [PATCH] Add function window-line-width

Eli Zaretskii
In reply to this post by Sébastien Chapuis
> From: Sébastien Chapuis <[hidden email]>
> Date: Mon, 11 Nov 2019 03:02:23 +0800
> Cc: Eli Zaretskii <[hidden email]>, [hidden email]
>
> > There's always the problem that we want to reserve space for a block
> > cursor at line end.
>
> IMO that's a case that should be handled by the user of the function, not by 'window-lines-pixel-dimensions'
> itself.

How can application code do anything like that?  The glyph matrices
aren't (and shouldn't be) exposed to Lisp.

> In my few tests, 'window-lines-pixel-dimensions' returns a width with 1 and 2 glyphs more, so it's not just 1
> space for the block cursor,
> there might be cases where there are more glyphs added (?).

Please show those test cases.  It is hard to have an effective
discussion without seeing the actual use cases being mentioned.

> And AFAIK, there is no way to know how many glyphs/pixels the redisplay add.

The display engine does know that, of course.

> Anyone who wants to have an accurate number of pixels displayed will be unable to use this function.

That's a pretty string assertion; the fact that we do use the function
in our own code seems to contradict it.

> > Then we probably should rewrite 'window-text-pixel-size' as Eli
> > suggested earlier.
>
> Agreed, but I would prefer to use 'window-lines-pixel-dimensions' since this is exactly the function I need.
> As per the documentation [1]:
> `window-text-pixel-size treats the text displayed in a window as a whole and does not care about
>  the size of individual lines. The following function [window-lines-pixel-dimensions] does.`

What is missing in window-lines-pixel-dimensions to make it satisfy
your use cases, and why?  I think it will be a better fix for your
needs, given the problems you have with window-lines-pixel-dimensions,
and I don't think we should change the latter in radical ways.