Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Po Lu]

Po Lu via Mailing list for Emacs changes <emacs-diffs@gnu.org> writes:

>     Prevent text decorations from overwriting surrounding areas on X
>     
>     * src/xterm.c (x_draw_underwave): New parameter
>     `decoration_width'.
>     (x_draw_glyph_string): Constrain decoration width to current
>     text area.

This happens on X because nothing actually clips the decoration to the
string's area when we draw the decoration.

You can test by running custom-set-faces on default, enabling an
underline or overline, and typing in a field that is extended to the end
of the line.

It doesn't happen on NS or Haiku -- but I have no idea if a similar
adjustment is needed on MS-Windows.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Date: Mon, 10 Jan 2022 19:58:12 +0800
> 
> Po Lu via Mailing list for Emacs changes <emacs-diffs@gnu.org> writes:
> 
> >     Prevent text decorations from overwriting surrounding areas on X
> >     
> >     * src/xterm.c (x_draw_underwave): New parameter
> >     `decoration_width'.
> >     (x_draw_glyph_string): Constrain decoration width to current
> >     text area.
> 
> This happens on X because nothing actually clips the decoration to the
> string's area when we draw the decoration.

I'm not sure I understand what clipping did you have in mind.  In
x_draw_underwave, for example, we have this:

  /* Find and set clipping rectangle */

  wave_clip.x = x0;
  wave_clip.y = y0;
  wave_clip.width = width;
  wave_clip.height = wave_height;
  get_glyph_string_clip_rect (s, &string_clip);

  if (!gui_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
    return;

  XSetClipRectangles (display, s->gc, 0, 0, &final_clip, 1, Unsorted);

Isn't this the clipping?  Or what am I missing?

> You can test by running custom-set-faces on default, enabling an
> underline or overline, and typing in a field that is extended to the end
> of the line.

Please show some actual Lisp to try this, so we wouldn't need to guess
whether the problem doesn't exist or we failed to reproduce correctly.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> I'm not sure I understand what clipping did you have in mind.  In
> x_draw_underwave, for example, we have this:
>
>   /* Find and set clipping rectangle */
>
>   wave_clip.x = x0;
>   wave_clip.y = y0;
>   wave_clip.width = width;
>   wave_clip.height = wave_height;
>   get_glyph_string_clip_rect (s, &string_clip);
>
>   if (!gui_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
>     return;
>
>   XSetClipRectangles (display, s->gc, 0, 0, &final_clip, 1, Unsorted);
>
> Isn't this the clipping?  Or what am I missing?

That's only done in x_draw_underwave, and nowhere else where text
decorations (underlines and overlines) are drawn.

>> You can test by running custom-set-faces on default, enabling an
>> underline or overline, and typing in a field that is extended to the end
>> of the line.

> Please show some actual Lisp to try this, so we wouldn't need to guess
> whether the problem doesn't exist or we failed to reproduce correctly.

After loading the definition of widget-field, just do this in an empty
buffer:

  (set-face-attribute 'default nil :underline t)
  (save-excursion
    (insert (propertize "foo\n" 'face 'widget-field)))
  (redisplay)
  (insert "a")

The underline of the first line will then overwrite the right fringe.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Tue, 11 Jan 2022 08:37:11 +0800
> 
> >   wave_clip.x = x0;
> >   wave_clip.y = y0;
> >   wave_clip.width = width;
> >   wave_clip.height = wave_height;
> >   get_glyph_string_clip_rect (s, &string_clip);
> >
> >   if (!gui_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
> >     return;
> >
> >   XSetClipRectangles (display, s->gc, 0, 0, &final_clip, 1, Unsorted);
> >
> > Isn't this the clipping?  Or what am I missing?
> 
> That's only done in x_draw_underwave, and nowhere else where text
> decorations (underlines and overlines) are drawn.

For underline, we have this:

              /* Check the sanity of thickness and position.  We should
                 avoid drawing underline out of the current line area.  */
	      if (s->y + s->height <= s->ybase + position)
		position = (s->height - 1) - (s->ybase - s->y);
              if (s->y + s->height < s->ybase + position + thickness)
                thickness = (s->y + s->height) - (s->ybase + position);
              s->underline_thickness = thickness;
              s->underline_position = position;
              y = s->ybase + position;
              if (s->face->underline_defaulted_p)
                x_fill_rectangle (s->f, s->gc,
				  s->x, y, decoration_width, thickness);
              else
                {
                  Display *display = FRAME_X_DISPLAY (s->f);
                  XGCValues xgcv;
                  XGetGCValues (display, s->gc, GCForeground, &xgcv);
                  XSetForeground (display, s->gc, s->face->underline_color);
                  x_fill_rectangle (s->f, s->gc,
				    s->x, y, decoration_width, thickness);
                  XSetForeground (display, s->gc, xgcv.foreground);
                }

The call to x_fill_rectangle was with s->width, which you now replaced
with decoration_width, computed as follows:

      decoration_width = s->width;
      if (area_max_x < (s->x + decoration_width - 1))
	decoration_width -= (s->x + decoration_width - 1) - area_max_x;

I have a problem with this correction, because I don't understand the
need for it.  s->width is the width of the glyph string, which is
computed from the glyphs that comprise the glyph string.  Those glyphs
were laid out by display_line, which should never add a glyph to a
glyph row if that glyph doesn't fit on the screen.  So how come
s->width in your case was too large?  Or maybe it's s->x that was
wrong (does the underline start exactly under the first glyph)?

So I suspect that your fix hides a much more serious problem
elsewhere.  Please investigate the problem you saw before the change.

> > Please show some actual Lisp to try this, so we wouldn't need to guess
> > whether the problem doesn't exist or we failed to reproduce correctly.
> 
> After loading the definition of widget-field, just do this in an empty
> buffer:
> 
>   (set-face-attribute 'default nil :underline t)
>   (save-excursion
>     (insert (propertize "foo\n" 'face 'widget-field)))
>   (redisplay)
>   (insert "a")
> 
> The underline of the first line will then overwrite the right fringe.

It doesn't overwrite the fringe here on MS-Windows.  Which is what I'd
expect, given the above rationale.  There's something else at work
here, and we shouldn't so easily decide that no one thought about the
need for clipping in this case.

Thanks.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Po Lu]

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

 [Sorry for the webmail, Yahoo SMTP is wedged; I hope Reply All worked
and this reaches the list]

> I have a problem with this correction, because I don't understand the
> need for it. s->width is the width of the glyph string, which is
> computed from the glyphs that comprise the glyph string. Those glyphs
> were laid out by display_line, which should never add a glyph to a
> glyph row if that glyph doesn't fit on the screen. So how come
> s->width in your case was too large? Or maybe it's s->x that was
> wrong (does the underline start exactly under the first glyph)?

> So I suspect that your fix hides a much more serious problem
> elsewhere. Please investigate the problem you saw before the change.

Thanks, will do. Should I revert this fix?
  

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

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Eli Zaretskii]

> Date: Tue, 11 Jan 2022 13:18:20 +0000 (UTC)
> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> 
> [Sorry for the webmail, Yahoo SMTP is wedged; I hope Reply All worked
> and this reaches the list]

It did work.

> > I have a problem with this correction, because I don't understand the
> > need for it. s->width is the width of the glyph string, which is
> > computed from the glyphs that comprise the glyph string. Those glyphs
> > were laid out by display_line, which should never add a glyph to a
> > glyph row if that glyph doesn't fit on the screen. So how come
> > s->width in your case was too large? Or maybe it's s->x that was
> > wrong (does the underline start exactly under the first glyph)?
> 
> > So I suspect that your fix hides a much more serious problem
> > elsewhere. Please investigate the problem you saw before the change.
> 
> Thanks, will do. Should I revert this fix?

I don't see a reason to revert immediately, as the change doesn't do
any particular harm, being on master.  It's up to you.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> It did work.

Great!

> I don't see a reason to revert immediately, as the change doesn't do
> any particular harm, being on master.  It's up to you.

The glyphs with over-wide decorations (that extend over the fringes)
seem to be stretch glyphs, and only occur inside text that has been
extended to the end of a line.

Maybe you can see the problem from this information alone, but if not
I'll keep debugging.  Thanks in advance.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Jose Antonio Ortega Ruiz]

Hi,

I am afraid i cannot at the moment debug this further, but i've noticed
that, with this change, an underline text in the mode-line extending the
full mode-line will get cut at the fringe too, which, to me, is
unexpected, because the modeline covers the fringe, so i expect the
underline to cover it too (and that's the behaviour before this change).

(i actually use this trick to sometimes show an empty mode line, with ""
as format, with a mode-line face set to, say, :underline "red"... before
this change the underline would go from the left to the right vertical
lines delimiting the buffer; after it, the underline starts at the left
vertical bar, but stops before arriving to the right one.  i presume it
is because it stops at the fringe).

Cheers,
jao

On Wed, Jan 12 2022, Po Lu wrote:

> Eli Zaretskii <eliz@gnu.org> writes:
>
>> It did work.
>
> Great!
>
>> I don't see a reason to revert immediately, as the change doesn't do
>> any particular harm, being on master.  It's up to you.
>
> The glyphs with over-wide decorations (that extend over the fringes)
> seem to be stretch glyphs, and only occur inside text that has been
> extended to the end of a line.
>
> Maybe you can see the problem from this information alone, but if not
> I'll keep debugging.  Thanks in advance.
>

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Po Lu]

Jose Antonio Ortega Ruiz <mail@jao.io> writes:

> I am afraid i cannot at the moment debug this further, but i've noticed
> that, with this change, an underline text in the mode-line extending the
> full mode-line will get cut at the fringe too, which, to me, is
> unexpected, because the modeline covers the fringe, so i expect the
> underline to cover it too (and that's the behaviour before this change).

> (i actually use this trick to sometimes show an empty mode line, with ""
> as format, with a mode-line face set to, say, :underline "red"... before
> this change the underline would go from the left to the right vertical
> lines delimiting the buffer; after it, the underline starts at the left
> vertical bar, but stops before arriving to the right one.  i presume it
> is because it stops at the fringe).

Thanks, that'll be fixed shortly.

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Jose Antonio Ortega Ruiz]

On Wed, Jan 12 2022, Po Lu wrote:

> Jose Antonio Ortega Ruiz <mail@jao.io> writes:
>
>> I am afraid i cannot at the moment debug this further, but i've noticed
>> that, with this change, an underline text in the mode-line extending the
>> full mode-line will get cut at the fringe too, which, to me, is
>> unexpected, because the modeline covers the fringe, so i expect the
>> underline to cover it too (and that's the behaviour before this change).
>
>> (i actually use this trick to sometimes show an empty mode line, with ""
>> as format, with a mode-line face set to, say, :underline "red"... before
>> this change the underline would go from the left to the right vertical
>> lines delimiting the buffer; after it, the underline starts at the left
>> vertical bar, but stops before arriving to the right one.  i presume it
>> is because it stops at the fringe).
>
> Thanks, that'll be fixed shortly.

Many thanks for the swift answer and fix: i can confirm it's working
again as expected in current master.

Cheers,
jao
-- 
It does not do to leave a live dragon out of your calculations, if you
live near him. -J.R.R. Tolkien, novelist and philologist (1892-1973)

Re: master 77c3d41adc: Prevent text decorations from overwriting surrounding areas on X

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: emacs-devel@gnu.org
> Date: Wed, 12 Jan 2022 09:56:15 +0800
> 
> The glyphs with over-wide decorations (that extend over the fringes)
> seem to be stretch glyphs, and only occur inside text that has been
> extended to the end of a line.

That means we compute the width of those stretch glyphs incorrectly
somewhere.

And I still wonder how come I don't see the problem on MS-Windows.