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

From: Eli Zaretskii <[email-masked]>
Date: Sat, 16 Oct 2021 13:10:59 +0300

> From: Po Lu <[email-masked]>
> Cc: [email-masked],  [email-masked]
> Date: Sat, 16 Oct 2021 15:52:24 +0800
> 
> Eli Zaretskii <[email-masked]> writes:
> 
> >> I'm reasonably sure.  Under the old code in *term, moving the mouse over
> >> the entry for `glyphless-char' in list-faces-display results in nothing,
> >> while under the new code (where s->font == s->face->font even under
> >> mouse face) the section under mouse face overlaps with its surroundings
> >> and is otherwise glitchy, because the mouse face's font is larger than
> >> the original face's font.
> 
> > In the examples I used for testing the size of the font was the same,
> > so I'm no longer sure we are talking about the same thing.
> 
> Yes, this has gone off on some kind of tangent.

I don't think it's a tangent, see below.

> Okay.  The first issue, with the cursor put on the first character "s",
> is caused by this snippet of xterm.c (in x_draw_glyph_string_foreground):
> 
>   /* If first glyph of S has a left box line, start drawing the text
>      of S to the right of that box line.  */
>   if (s->face->box != FACE_NO_BOX
>       && s->first_glyph->left_box_line_p)
>     x = s->x + max (s->face->box_vertical_line_width, 0);
>   else
>     x = s->x;
> 
> An identical snippet exists in w32term.
> 
> This happens because s->face is not the mouse face when s->hl is
> DRAW_CURSOR and cursor_in_mouse_face_p, so it mistakenly assumes there
> is a box for s (when there is in fact no box), and adds the original
> face's vertical box width to x.

So you are saying that s->hl can only be either DRAW_CURSOR or
DRAW_MOUSE_FACE, whereas we need both?  If so, this matches what I
thought you were trying to solve.

So what happens here is that s->face is computed from the face of the
glyphs which "belong" to the glyph string s.  That face comes from the
glyph matrix which holds the glyphs.  That face was computed by
redisplay_window using FACE_FOR_CHAR, see get_next_display_element, so
it's the face at the character's buffer position adjusted for the font
suitable for the character at the cursor.  Now you want to display
that same character, but with the mouse-face.  FACE_FROM_ID gives you
that face, but it is for ASCII characters.  So you call FACE_FOR_CHAR
again, to obtain the mouse face adjusted for the font suitable for
displaying the character at the cursor.

The above sounds correct to me, so I don't understand why you want to
ignore the font of the face produced by FACE_FOR_CHAR.  What am I
missing?

> (Seeing this issue, you proposed to also move mouse face selection to
> draw_glyphs, and I proposed to move it to fill_XXX_glyph_string, leading
> to the above tangent about the semantics of s->font.)

Yes.  Btw, it would probably be cleaner to add an extra argument to
get_glyph_face_and_encoding, but make that argument be a pointer to
'struct face', not just an indication of which face to use.

> The second issue was caused by testing for "start <=" end instead of
> "start < end" in get_cursor_offset_for_mouse_face.

What was that second issue about? why did you need to change the
inequality?