bug#68374: min-width is not correctly treated by buffer-text-pixel-size

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[JD Smith]

When the ‘display min-width property is used on characters in a buffer, window/buffer-text-pixel-size does not include this min-width padding in its calculation of the text's pixel width.  

This can be seen by using a range of minimum character widths and noticing that the width is identical; see below.  Other ‘display properties like space :width and space :align-to do seem to be correctly handled by pixel size calculations. 

This is relevant because various elements on the mode-line use ‘display min-width, so performing pixel alignment on the mode-line text is impacted.

+++

(eval-when-compile 'cl-lib)
(cl-loop
 for w from 0. to 100. by 20. 
 collect
 (with-current-buffer (get-buffer-create "*pixel-sizes*")
   (erase-buffer)
   (insert ">>>" (propertize " " 'display `(min-width (,w))) "<<<")
   (cons w (buffer-text-pixel-size))))
  
;; ((0 49 . 14) (20 49 . 14) (40 49 . 14) (60 49 . 14) (80 49 . 14) (100 49 . 14))

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Wed, 10 Jan 2024 17:22:53 -0500
> 
> 
> When the ‘display min-width property is used on characters in a buffer, window/buffer-text-pixel-size does not include this min-width padding in its calculation of the text's pixel width.  

For some reason I cannot understand, we were ignoring min-width when
simulating display (as opposed to actually displaying stuff in a
window).  I can only assume it's some kind of left-over from when Lars
was developing this feature, where calls from functions that simulate
display caused him trouble, so he disabled that and forgot to
re-enable later.  At least I cannot find any discussion of this, and
the code which disables min-width in these situations was in the
initial version that landed on master.

So please try running with the patch below for a couple of weeks, and
if it doesn't cause you any trouble, I will install it on master.
Please configure your builds with --enable-checking='yes,glyphs' for
the duration of this test period, to activate some run-time tests and
assertions that will hopefully flag any problems this change might
have.

TIA

diff --git a/src/xdisp.c b/src/xdisp.c
index 14cf030..29ac4a4 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -5487,9 +5487,6 @@ display_min_width (struct it *it, ptrdiff_t bufpos,
   if (!NILP (it->min_width_property)
       && !EQ (width_spec, it->min_width_property))
     {
-      if (!it->glyph_row)
-	return;
-
       /* When called from display_string (i.e., the mode line),
 	 we're being called with a string as the object, and we
 	 may be called with many sub-strings belonging to the same

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[JD Smith]

Great, this small patch seems to work.  I’ve enabled glyph checking and will report here if anything untoward comes up.  Should I be keeping track of the *warnings* buffer?

> 
> On Jan 11, 2024, at 5:24 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> Date: Wed, 10 Jan 2024 17:22:53 -0500
>> 
>> 
>> When the ‘display min-width property is used on characters in a buffer, window/buffer-text-pixel-size does not include this min-width padding in its calculation of the text's pixel width.  
> 
> For some reason I cannot understand, we were ignoring min-width when
> simulating display (as opposed to actually displaying stuff in a
> window).  I can only assume it's some kind of left-over from when Lars
> was developing this feature, where calls from functions that simulate
> display caused him trouble, so he disabled that and forgot to
> re-enable later.  At least I cannot find any discussion of this, and
> the code which disables min-width in these situations was in the
> initial version that landed on master.
> 
> So please try running with the patch below for a couple of weeks, and
> if it doesn't cause you any trouble, I will install it on master.
> Please configure your builds with --enable-checking='yes,glyphs' for
> the duration of this test period, to activate some run-time tests and
> assertions that will hopefully flag any problems this change might
> have.
> 
> TIA
> 
> diff --git a/src/xdisp.c b/src/xdisp.c
> index 14cf030..29ac4a4 100644
> --- a/src/xdisp.c
> +++ b/src/xdisp.c
> @@ -5487,9 +5487,6 @@ display_min_width (struct it *it, ptrdiff_t bufpos,
>   if (!NILP (it->min_width_property)
>       && !EQ (width_spec, it->min_width_property))
>     {
> -      if (!it->glyph_row)
> -	return;
> -
>       /* When called from display_string (i.e., the mode line),
> 	 we're being called with a string as the object, and we
> 	 may be called with many sub-strings belonging to the same

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Thu, 11 Jan 2024 14:53:57 -0500
> Cc: 68374@debbugs.gnu.org
> 
> Great, this small patch seems to work.  I’ve enabled glyph checking and will report here if anything untoward comes up.  Should I be keeping track of the *warnings* buffer?

Yes, and also look out for redisplay errors in *Messages*.

Thanks.

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[JD Smith]

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

I suppose the occasional:

Error during redisplay: (jit-lock-function 3483) signaled (quit)
Error during redisplay: (jit-lock-function 3510) signaled (quit)

in *Messages* isn’t related or harmful?  Soon I’ll be using pixel width calculations to layout the mode line; and will keep a close eye on it.  

> On Jan 12, 2024, at 2:05 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> Date: Thu, 11 Jan 2024 14:53:57 -0500
>> Cc: 68374@debbugs.gnu.org
>> 
>> Great, this small patch seems to work.  I’ve enabled glyph checking and will report here if anything untoward comes up.  Should I be keeping track of the *warnings* buffer?
> 
> Yes, and also look out for redisplay errors in *Messages*.
> 
> Thanks.


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

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Fri, 12 Jan 2024 11:45:02 -0500
> Cc: 68374@debbugs.gnu.org
> 
> I suppose the occasional:
> 
>  Error during redisplay: (jit-lock-function 3483) signaled (quit)
>  Error during redisplay: (jit-lock-function 3510) signaled (quit)
> 
> in *Messages* isn’t related or harmful?

Did you type C-g because something in font-lock was taking too long?

But yes, in general this is not the kind of problems I'd expect from
the change.

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Thu, 11 Jan 2024 14:53:57 -0500
> Cc: 68374@debbugs.gnu.org
> 
> Great, this small patch seems to work.  I’ve enabled glyph checking and will report here if anything untoward comes up.

Any news?  Should I install this now?

Thanks.

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[JD Smith]

I haven't noticed anything unusual in a couple weeks of use, so please do.  Thanks.

> On Feb 11, 2024, at 3:47 AM, Eli Zaretskii <eliz@gnu.org> wrote:
> 
>> From: JD Smith <jdtsmith@gmail.com>
>> Date: Thu, 11 Jan 2024 14:53:57 -0500
>> Cc: 68374@debbugs.gnu.org
>> 
>> Great, this small patch seems to work.  I’ve enabled glyph checking and will report here if anything untoward comes up.
> 
> Any news?  Should I install this now?
> 
> Thanks.

bug#68374: min-width is not correctly treated by buffer-text-pixel-size

[Eli Zaretskii]

> From: JD Smith <jdtsmith@gmail.com>
> Date: Sun, 11 Feb 2024 07:20:21 -0500
> Cc: 68374@debbugs.gnu.org
> 
> I haven't noticed anything unusual in a couple weeks of use, so please do.  Thanks.

Thanks, installed on master, and closing the bug.