[PATCH] Add function window-line-width

[PATCH] Add function window-line-width

[Sébastien Chapuis]

[-- Attachment #1.1: Type: text/plain, Size: 256 bytes --]

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.

[1] https://github.com/emacs-lsp/lsp-ui

[-- Attachment #1.2: Type: text/html, Size: 449 bytes --]

[-- Attachment #2: 0001-Add-function-window-line-width.patch --]
[-- Type: text/x-patch, Size: 5221 bytes --]

From b76852a07f9f2a6ef4b98fd90807e357a9ebb81c Mon Sep 17 00:00:00 2001
From: Sebastien Chapuis <sebastien@chapu.is>
Date: Thu, 7 Nov 2019 06:36:04 +0800
Subject: [PATCH] Add function window-line-width

* src/window.c: Define the function.
* etc/NEWS: Mention `window-line-width'.
---
 etc/NEWS     |   4 ++
 src/window.c | 112 +++++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 116 insertions(+)

diff --git a/etc/NEWS b/etc/NEWS
index bfcb7cf325..2f15185efe 100644
--- a/etc/NEWS
+++ b/etc/NEWS
@@ -221,6 +221,10 @@ To get the old, less-secure behavior, you can set the
 *** When run by root, emacsclient no longer connects to non-root sockets.
 (Instead you can use Tramp methods to run root commands in a non-root Emacs.)
 
+---
+** New function 'window-line-width'.
+Return width in pixels of a text line.
+
 ** New function 'network-lookup-address-info'.
 This does IPv4 and/or IPv6 address lookups on hostnames.
 
diff --git a/src/window.c b/src/window.c
index 0fa0bdf7b9..4bfdfc05de 100644
--- a/src/window.c
+++ b/src/window.c
@@ -1929,6 +1929,115 @@ DEFUN ("pos-visible-in-window-p", Fpos_visible_in_window_p,
   return in_window;
 }
 
+DEFUN ("window-line-width", Fwindow_line_width,
+       Swindow_line_width, 0, 3, 0,
+       doc: /* Return width in pixels of a text line LINE in WINDOW, in AREA.
+
+Return width of current line if LINE is omitted or nil.
+LINE can also be `tab-line', `header-line' and `mode-line'.
+Otherwise, LINE is a row number (zero-based).
+
+WINDOW must be a live window and defaults to the selected one.
+
+AREA can take one of those values:
+- `left-margin': Width of the left margin area
+- `right-margin': Width of the right margin area
+- `text-area': Width of the text area, until the new line (excluded)
+- `text-area-full': Similar to `text-area' but include glyphs after the
+   new line, if any.
+If omitted, AREA defaults to `text-area'.
+
+Return nil if the window display is not up-to-date.  */)
+  (Lisp_Object line, Lisp_Object window, Lisp_Object area)
+{
+  struct window *w = decode_live_window (window);
+  struct buffer *buffer UNINIT;
+  int vpos UNINIT;
+
+  if (noninteractive || w->pseudo_window_p)
+    return Qnil;
+
+  CHECK_BUFFER (w->contents);
+  buffer = XBUFFER (w->contents);
+
+  /* Fail if current matrix is not up-to-date.  */
+  if (!w->window_end_valid
+      || windows_or_buffers_changed
+      || buffer->clip_changed
+      || buffer->prevent_redisplay_optimizations_p
+      || window_outdated (w))
+    return Qnil;
+
+  struct glyph_matrix *matrix = w->current_matrix;
+
+  if (NILP (line))
+    vpos = w->cursor.vpos;
+  else if (EQ (line, Qtab_line))
+    {
+      if (!window_wants_tab_line (w) || !matrix->tab_line_p)
+	return Qnil;
+      vpos = 0;
+    }
+  else if (EQ (line, Qheader_line))
+    {
+      if (!window_wants_header_line (w) || !matrix->header_line_p)
+	return Qnil;
+      vpos = matrix->tab_line_p ? 1 : 0;
+    }
+  else if (EQ (line, Qmode_line))
+    {
+      if (!window_wants_mode_line (w))
+	return Qnil;
+      vpos = matrix->nrows - 1;
+    }
+  else
+    {
+      CHECK_FIXNUM (line);
+      vpos = XFIXNUM (line)
+        + window_wants_tab_line (w)
+        + window_wants_header_line (w);
+    }
+
+  if (vpos < 0 || vpos >= matrix->nrows)
+    error ("Invalid line");
+
+  enum glyph_row_area actual_area UNINIT;
+  bool full_text = false;
+
+  if (EQ (area, Qleft_margin))
+    actual_area = LEFT_MARGIN_AREA;
+  else if (EQ (area, Qright_margin))
+    actual_area = RIGHT_MARGIN_AREA;
+  else
+    {
+      if (EQ (area, Qtext_area_full))
+        full_text = true;
+      actual_area = TEXT_AREA;
+    }
+
+  struct glyph_row *row = MATRIX_ROW (matrix, vpos);
+
+  struct glyph *glyph = row->glyphs[actual_area];
+  int used = row->used[actual_area];
+  int width = 0;
+
+  for (int i = 0; i < used; ++i)
+    {
+      if (actual_area == TEXT_AREA && full_text == false
+          && NILP (glyph[i].object) && glyph[i].charpos >= 0)
+        /*
+          We reached the end of the line in TEXT_AREA.
+          display-line-numbers-mode inserts glyphs with NILP(glyph.object)
+          and glyph.charpos == -1, so we can't check only for
+          NILP (glyph.object)
+        */
+        break;
+      width += glyph[i].pixel_width;
+    }
+
+  return make_fixnum (width);
+}
+
 DEFUN ("window-line-height", Fwindow_line_height,
        Swindow_line_height, 0, 2, 0,
        doc: /* Return height in pixels of text line LINE in window WINDOW.
@@ -8179,6 +8288,8 @@ syms_of_window (void)
   DEFSYM (Qmode_line_format, "mode-line-format");
   DEFSYM (Qheader_line_format, "header-line-format");
   DEFSYM (Qtab_line_format, "tab-line-format");
+  DEFSYM (Qtext_area, "text-area");
+  DEFSYM (Qtext_area_full, "text-area-full");
 
   DEFVAR_LISP ("temp-buffer-show-function", Vtemp_buffer_show_function,
 	       doc: /* Non-nil means call as function to display a help buffer.
@@ -8443,6 +8554,7 @@ syms_of_window (void)
   defsubr (&Sset_frame_selected_window);
   defsubr (&Spos_visible_in_window_p);
   defsubr (&Swindow_line_height);
+  defsubr (&Swindow_line_width);
   defsubr (&Swindow_buffer);
   defsubr (&Swindow_old_buffer);
   defsubr (&Swindow_parent);
-- 
2.21.0

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

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

[martin rudalics]

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

martin

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

[Robert Pluim]

>>>>> On Thu, 7 Nov 2019 06:49:48 +0800, Sébastien Chapuis <sebastien@chapu.is> 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

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

[Sébastien Chapuis]

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

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.

[1]
https://github.com/emacs-mirror/emacs/blob/a070bd1c8b5213ad469d41dd80d392f924644aed/src/dispextern.h#L426-L428

Thanks,
Sebastien Chapuis.

Le jeu. 7 nov. 2019 à 16:50, martin rudalics <rudalics@gmx.at> a écrit :

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

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

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

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

[Eli Zaretskii]

> From: martin rudalics <rudalics@gmx.at>
> Date: Fri, 8 Nov 2019 10:21:45 +0100
> Cc: emacs-devel@gnu.org
> 
>  > 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.

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

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

[Sébastien Chapuis]

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

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 <rudalics@gmx.at> 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
>
>

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

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

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

[Sébastien Chapuis]

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

> 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 <rudalics@gmx.at> 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
>

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

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

[Eli Zaretskii]

> From: Sébastien Chapuis <sebastien@chapu.is>
> Date: Sun, 10 Nov 2019 06:09:56 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org

> 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?

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

[Eli Zaretskii]

> From: Sébastien Chapuis <sebastien@chapu.is>
> Date: Mon, 11 Nov 2019 03:02:23 +0800
> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
> 
> > 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.