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

[Eli Zaretskii]

> From: Po Lu <luangruo@yahoo.com>
> Cc: larsi@gnus.org,  50660@debbugs.gnu.org
> Date: Mon, 20 Sep 2021 16:18:01 +0800
> 
> Eli Zaretskii <eliz@gnu.org> writes:
> 
> > Actually, it looks like we already do everything we need to account
> > for the box border, when it exists, while drawing the glyphs (in
> > xterm.c/w32term.c).  The pixel_width of the glyphs is not used by the
> > back-end code which actually draws to the glass.  So the only place
> > which needs fixing is probably draw_phys_cursor_glyph and maybe also
> > erase_phys_cursor.  Assuming we are indeed talking about problems with
> > the glyph under the cursor.
> 
> My understanding is that when the cursor is drawn, the string in
> RIF->draw_glyph_string contains _only_ the glyph underneath the cursor,

That is correct.

> while the terminals only compensate for the box if the first glyph in
> the string has a left box line; when drawing a cursor on the area with a
> mouse face, that is only true if the cursor lands on the start of the
> box.

Also correct.  But the cursor glyph that is not the first or last of
the box area doesn't need any compensation, it just needs to be drawn
at the correct X-coordinate.

So I think all that's needed is to adjust the value of
w->phys_cursor.x, when needed, in draw_phys_cursor_glyph, before
passing it to draw_glyphs, or perhaps in its callers.  The value of
w->phys_cursor.x should stay unaltered, but what we pass to
draw_glyphs should be offset if needed.

If you think this will not be enough, what else is needed, and why?

> In addition to that, draw_phys_cursor_glyph uses the value
> w->output_cursor.x as the X offset passed to draw_glyphs, so I still
> don't think the problem lies in draw_phys_cursor_glyph, but either in
> where the cursor position is calculated (by tallying up the
> pixel_widths), or where the mouse face is drawn without updating the
> contents of the glyph row to reflect the potentially changed dimensions.

The glyph row doesn't store any dimensions, it only stores the
pixel_width of each glyph and the starting X-coordinate of the row.
So where the mouse face is drawn, we don't need any updating.

> Personally, I still think the problem lies in the latter area and not
> the former, but I'll leave that up to your judgement.

The cursor position of a window, stored in w->phys_cursor, is
calculated when the cursor is moved.  It is never recalculated
thereafter, until point is moved again.  In particular, displaying
mouse-sensitive text in mouse-face just reuses the glyphs already
produced, it doesn't recalculate them.  But since we always redraw the
entire sequence of glyphs in the mouse face or in the box face, the
glyphs gets moved horizontally because we see the first glyph with the
box and handle that accordingly.  But when we then redraw the cursor,
we reuse the information in w->phys_cursor, which is slightly off when
the box attributes of the mouse face is different from that of the
"regular" face.  This is what causes the cursor glyph be drawn in the
wrong location.

The correct place to fix this is therefore somewhere under
note_mouse_highlight, which is where we handle redrawing of the
mouse-sensitive face, including the cursor.

> BTW, in x_set_cursor_gc, I notice that s->face isn't being set to the
> mouse face even when the cursor lies inside the mouse face.  Perhaps
> checking for cursor_in_mouse_face_p (s->w), and setting s->face to the
> mouse face when that is the case would be prudent?  AFAIK, something
> similar is already being done in x_draw_stretch_glyph_string (see this
> chunk of code):
> 
>         if (s->row->mouse_face_p
>             && cursor_in_mouse_face_p (s->w))
>           {
>             x_set_mouse_face_gc (s);
>             gc = s->gc;
>           }

x_draw_stretch_glyph_string does that only if needs to clear an area
that is wider than the width of the frame's default font, which cannot
happen in our case.