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

[Po Lu via "Bug reports for GNU Emacs, the Swiss army knife of text editors" <bug-gnu-emacs@gnu.org>]

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

Eli Zaretskii <eliz@gnu.org> writes:

> No, I think it's wrong to set the face GC in xdisp.c, because the GC
> is fundamentally terminal-specific, so it belongs to xterm/w32term
> etc.  I'm not even sure xdisp.c knows the exact type of GC, which
> differs according to the platform.  The logic of selecting the face
> should indeed be removed from x_set_mouse_face_gc, but the calls to
> XChangeGC etc. should remain there.

Thanks.  Here's a patch that should remove the issues you pointed out:


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: 0001-Fix-minor-issues-with-text-display-when-cursor-is-in.patch --]
[-- Type: text/x-patch, Size: 12644 bytes --]

From d9fa01cca632672efd20272131acd4e13b0b4106 Mon Sep 17 00:00:00 2001
From: Po Lu <luangruo@yahoo.com>
Date: Thu, 14 Oct 2021 18:38:26 +0800
Subject: [PATCH] Fix minor issues with text display when cursor is in mouse
 face

 * src/xdisp.c (get_cursor_offset_for_mouse_face): Don't calculate
offsets for the glyph the cursor is on.
(fill_composite_glyph_string)
(fill_gstring_glyph_string)
(fill_glyphless_glyph_string)
(fill_glyph_string)
(fill_image_glyph_string)
(fill_xwidget_glyph_string)
(fill_stretch_glyph_string): Set s->face to mouse face whenever
appropriate.
(set_glyph_string_background_width): Update background width and
s->width to take into account differing :box properties of the mouse
face, when producing strings for the cursor.
(erase_phys_cursor): Redraw mouse face when erasing a cursor on top of
the mouse face.
 * src/xterm.c (x_set_mouse_face_gc): Stop setting s->face when under
mouse face because redisplay now does that for us.
 * src/w32term.c (w32_set_mouse_face_gc): Likewise.
---
 src/w32term.c |  16 -----
 src/xdisp.c   | 158 +++++++++++++++++++++++++++++++++++++++++++-------
 src/xterm.c   |  16 -----
 3 files changed, 136 insertions(+), 54 deletions(-)

diff --git a/src/w32term.c b/src/w32term.c
index 9cf250cd73..07a5cd3564 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -954,22 +954,6 @@ w32_set_cursor_gc (struct glyph_string *s)
 static void
 w32_set_mouse_face_gc (struct glyph_string *s)
 {
-  int face_id;
-  struct face *face;
-
-  /* What face has to be used last for the mouse face?  */
-  face_id = MOUSE_HL_INFO (s->f)->mouse_face_face_id;
-  face = FACE_FROM_ID_OR_NULL (s->f, face_id);
-  if (face == NULL)
-    face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
-
-  if (s->first_glyph->type == CHAR_GLYPH)
-    face_id = FACE_FOR_CHAR (s->f, face, s->first_glyph->u.ch, -1, Qnil);
-  else
-    face_id = FACE_FOR_CHAR (s->f, face, 0, -1, Qnil);
-  s->face = FACE_FROM_ID (s->f, face_id);
-  prepare_face_for_display (s->f, s->face);
-
   /* If font in this face is same as S->font, use it.  */
   if (s->font == s->face->font)
     s->gc = s->face->gc;
diff --git a/src/xdisp.c b/src/xdisp.c
index 012c2ad8bf..c61d6a943b 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -28128,6 +28128,14 @@ fill_composite_glyph_string (struct glyph_string *s, struct face *base_face,
       s->font = s->face->font;
     }
 
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+      s->font = s->face->font;
+    }
+
   /* All glyph strings for the same composition has the same width,
      i.e. the width set for the first component of the composition.  */
   s->width = s->first_glyph->pixel_width;
@@ -28164,7 +28172,14 @@ fill_gstring_glyph_string (struct glyph_string *s, int face_id,
   s->cmp_id = glyph->u.cmp.id;
   s->cmp_from = glyph->slice.cmp.from;
   s->cmp_to = glyph->slice.cmp.to + 1;
-  s->face = FACE_FROM_ID (s->f, face_id);
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+    }
+  else
+    s->face = FACE_FROM_ID (s->f, face_id);
   lgstring = composition_gstring_from_id (s->cmp_id);
   s->font = XFONT_OBJECT (LGSTRING_FONT (lgstring));
   /* The width of a composition glyph string is the sum of the
@@ -28218,7 +28233,14 @@ fill_glyphless_glyph_string (struct glyph_string *s, int face_id,
   glyph = s->row->glyphs[s->area] + start;
   last = s->row->glyphs[s->area] + end;
   voffset = glyph->voffset;
-  s->face = FACE_FROM_ID (s->f, face_id);
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+    }
+  else
+    s->face = FACE_FROM_ID (s->f, face_id);
   s->font = s->face->font ? s->face->font : FRAME_FONT (s->f);
   s->nchars = 1;
   s->width = glyph->pixel_width;
@@ -28281,6 +28303,16 @@ fill_glyph_string (struct glyph_string *s, int face_id,
 	break;
     }
 
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      struct face *face
+        = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+      s->face
+        = FACE_FROM_ID (s->f, FACE_FOR_CHAR (s->f, face,
+					     s->first_glyph->u.ch, -1, Qnil));
+    }
   s->font = s->face->font;
 
   /* If the specified font could not be loaded, use the frame's font,
@@ -28310,7 +28342,15 @@ fill_image_glyph_string (struct glyph_string *s)
   s->img = IMAGE_FROM_ID (s->f, s->first_glyph->u.img_id);
   eassert (s->img);
   s->slice = s->first_glyph->slice.img;
-  s->face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
+
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+    }
+  else
+    s->face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
   s->font = s->face->font;
   s->width = s->first_glyph->pixel_width;
 
@@ -28324,7 +28364,14 @@ fill_image_glyph_string (struct glyph_string *s)
 fill_xwidget_glyph_string (struct glyph_string *s)
 {
   eassert (s->first_glyph->type == XWIDGET_GLYPH);
-  s->face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+    }
+  else
+    s->face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
   s->font = s->face->font;
   s->width = s->first_glyph->pixel_width;
   s->ybase += s->first_glyph->voffset;
@@ -28349,7 +28396,14 @@ fill_stretch_glyph_string (struct glyph_string *s, int start, int end)
   glyph = s->row->glyphs[s->area] + start;
   last = s->row->glyphs[s->area] + end;
   face_id = glyph->face_id;
-  s->face = FACE_FROM_ID (s->f, face_id);
+  if (s->hl == DRAW_MOUSE_FACE
+      || (s->hl == DRAW_CURSOR && cursor_in_mouse_face_p (s->w)))
+    {
+      Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (s->f);
+      s->face = FACE_FROM_ID (s->f, hlinfo->mouse_face_face_id);
+    }
+  else
+    s->face = FACE_FROM_ID (s->f, face_id);
   s->font = s->face->font;
   s->width = glyph->pixel_width;
   s->nchars = 1;
@@ -28598,7 +28652,12 @@ right_overwriting (struct glyph_string *s)
 
 /* Set background width of glyph string S.  START is the index of the
    first glyph following S.  LAST_X is the right-most x-position + 1
-   in the drawing area.  */
+   in the drawing area.
+
+   If S's hl is DRAW_CURSOR, S->f is a window system frame, and the
+   cursor in S's window is currently under mouse face, s->width will
+   also be updated to take into account differing :box properties
+   between the original face and the mouse face. */
 
 static void
 set_glyph_string_background_width (struct glyph_string *s, int start, int last_x)
@@ -28620,7 +28679,67 @@ set_glyph_string_background_width (struct glyph_string *s, int start, int last_x
   if (s->extends_to_end_of_line_p)
     s->background_width = last_x - s->x + 1;
   else
-    s->background_width = s->width;
+    {
+      s->background_width = s->width;
+#ifdef HAVE_WINDOW_SYSTEM
+      if (FRAME_WINDOW_P (s->f)
+	  && s->hl == DRAW_CURSOR
+	  && cursor_in_mouse_face_p (s->w))
+	{
+	  /* We will have to adjust the background width of the string
+	     in this situation, because the glyph's pixel_width might
+	     be inconsistent with the box of the mouse face, which
+	     leads to an ugly over-wide cursor. */
+
+	  struct glyph *g = s->first_glyph;
+	  struct face *regular_face = FACE_FROM_ID (s->f, g->face_id);
+
+          bool do_left_box_p = g->left_box_line_p;
+          bool do_right_box_p = g->right_box_line_p;
+
+          /* This is required because we test some parameters
+             of the image slice before applying the box in
+             produce_image_glyph. */
+
+          if (g->type == IMAGE_GLYPH)
+            {
+	      if (!s->row->reversed_p)
+		{
+		  struct image *img = IMAGE_FROM_ID (s->f, g->u.img_id);
+		  do_left_box_p = g->left_box_line_p &&
+		    g->slice.img.x == 0;
+		  do_right_box_p = g->right_box_line_p &&
+		    g->slice.img.x + g->slice.img.width == img->width;
+		}
+	      else
+		{
+		  struct image *img = IMAGE_FROM_ID (s->f, g->u.img_id);
+		  do_left_box_p = g->left_box_line_p &&
+		    g->slice.img.x + g->slice.img.width == img->width;
+		  do_right_box_p = g->right_box_line_p &&
+		    g->slice.img.x == 0;
+		}
+            }
+
+          /* If the glyph has a left box line, subtract it from the
+	     offset.  */
+          if (do_left_box_p)
+            s->background_width -= max (0, regular_face->box_vertical_line_width);
+          /* Likewise with the right box line, as there may be a
+             box there as well.  */
+          if (do_right_box_p)
+            s->background_width -= max (0, regular_face->box_vertical_line_width);
+          /* Now add the line widths from the new face.  */
+          if (g->left_box_line_p)
+            s->background_width += max (0, s->face->box_vertical_line_width);
+          if (g->right_box_line_p)
+            s->background_width += max (0, s->face->box_vertical_line_width);
+
+	  /* s->width is probably worth adjusting here as well. */
+	  s->width = s->background_width;
+        }
+#endif
+    }
 }
 
 
@@ -31755,10 +31874,6 @@ erase_phys_cursor (struct window *w)
   Mouse_HLInfo *hlinfo = MOUSE_HL_INFO (f);
   int hpos = w->phys_cursor.hpos;
   int vpos = w->phys_cursor.vpos;
-#ifdef HAVE_WINDOW_SYSTEM
-  int mouse_delta;
-  int phys_x = w->phys_cursor.x;
-#endif
   bool mouse_face_here_p = false;
   struct glyph_matrix *active_glyphs = w->current_matrix;
   struct glyph_row *cursor_row;
@@ -31829,13 +31944,16 @@ erase_phys_cursor (struct window *w)
     mouse_face_here_p = true;
 
 #ifdef HAVE_WINDOW_SYSTEM
-  /* Adjust the physical cursor's X coordinate if needed.  The problem
-     solved by the code below is outlined in the comment above
-     'get_cursor_offset_for_mouse_face'.  */
-  if (mouse_face_here_p)
+  /* Since erasing the phys cursor will probably lead to corruption of
+     the mouse face display if the glyph's pixel_width is not kept up
+     to date with the :box property of the mouse face, just redraw the
+     mouse face. */
+  if (FRAME_WINDOW_P (WINDOW_XFRAME (w)) && mouse_face_here_p)
     {
-      get_cursor_offset_for_mouse_face (w, cursor_row, &mouse_delta);
-      w->phys_cursor.x += mouse_delta;
+      w->phys_cursor_on_p = false;
+      w->phys_cursor_type = NO_CURSOR;
+      show_mouse_face (MOUSE_HL_INFO (WINDOW_XFRAME (w)), DRAW_MOUSE_FACE);
+      return;
     }
 #endif
 
@@ -31874,10 +31992,6 @@ erase_phys_cursor (struct window *w)
   draw_phys_cursor_glyph (w, cursor_row, hl);
 
  mark_cursor_off:
-#ifdef HAVE_WINDOW_SYSTEM
-  /* Restore the original cursor position.  */
-  w->phys_cursor.x = phys_x;
-#endif
   w->phys_cursor_on_p = false;
   w->phys_cursor_type = NO_CURSOR;
 }
@@ -36042,7 +36156,7 @@ get_cursor_offset_for_mouse_face (struct window *w, struct glyph_row *row,
   /* Calculate the offset to correct phys_cursor x if we are
      drawing the cursor inside mouse-face highlighted text.  */
 
-  for (; row->reversed_p ? start >= end : start <= end;
+  for (; row->reversed_p ? start > end : start < end;
        row->reversed_p ? --start : ++start)
     {
       struct glyph *g = start;
diff --git a/src/xterm.c b/src/xterm.c
index 89885e0d88..961c61c245 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -1563,22 +1563,6 @@ x_set_cursor_gc (struct glyph_string *s)
 static void
 x_set_mouse_face_gc (struct glyph_string *s)
 {
-  int face_id;
-  struct face *face;
-
-  /* What face has to be used last for the mouse face?  */
-  face_id = MOUSE_HL_INFO (s->f)->mouse_face_face_id;
-  face = FACE_FROM_ID_OR_NULL (s->f, face_id);
-  if (face == NULL)
-    face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
-
-  if (s->first_glyph->type == CHAR_GLYPH)
-    face_id = FACE_FOR_CHAR (s->f, face, s->first_glyph->u.ch, -1, Qnil);
-  else
-    face_id = FACE_FOR_CHAR (s->f, face, 0, -1, Qnil);
-  s->face = FACE_FROM_ID (s->f, face_id);
-  prepare_face_for_display (s->f, s->face);
-
   if (s->font == s->face->font)
     s->gc = s->face->gc;
   else
-- 
2.31.1