bug#50660: 28.0.50; Text artifacting when the cursor moves over text under mouse face that originally displayed a box

[Po Lu]

Eli Zaretskii <eliz@gnu.org> writes:

> Thanks, this becomes clearer now.
>
> However, it is IMO wrong to "fix" the glyphs' pixel_width to account
> for the box face.  The glyphs didn't change, only their X-coordinate
> did.  By changing the width, we are in effect lying to any code that
> accesses the glyphs.  We should find another solution, perhaps similar
> to what the iterator does during the layout phase.

Interesting.  I don't quite know what the iterator does during the
layout phase, could you please point me to the relevant part of the
code?  Thanks.

> I still don't think I understand completely the problem you are
> talking about.  Is the problematic recipe as below?
>
>   . move the mouse pointer above text with box-face, so it is highlighted
>   . move the text cursor into the highlighted text
>
> Are there any other problems, or is the above the only problematic
> situation you saw and intended to fix?

That is the only situation I saw and intended to fix.

> Why do you need the two new flags?  If it's so you could avoid
> accounting for the box face too many times, isn't that a case of
> premature optimization?  A loop through a glyph-row's glyphs is
> straightforward and runs very fast.  The face of each glyph is stored
> in glyph->face_id, so you can easily see if its face includes the box
> attribute and get the box line thickness from that, and there are
> flags that tell you whether the box line is drawn on the left and on
> the right of the glyph.  What else is missing?

I suppose that is a case of premature optimization, thanks.

> Why did you need to recompute X in that case? why not fix the original
> computation instead?

Indeed, I have removed that change.

> I see only a couple of places, and they are all on the level of
> xterm.c/w32term.c, which is in an entirely different layer of the
> display code.  On the level of xdisp.c we only use mouse_face_face_id,
> AFAICT.

Hmm, it seems prudent to remove that then.  Thanks.

> Doesn't it logically belong to the job of display_and_set_cursor?

AFAIU, display_and_set_cursor only serves to set the position of the
cursor, and doesn't calculate or correct anything by itself.  Am I
missing something?