Re: Question about display engine

[Ergus <spacibba@aol.com>]

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

On Wed, Aug 07, 2019 at 06:45:47PM +0300, Eli Zaretskii wrote:
>> Date: Wed, 7 Aug 2019 17:32:20 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: emacs-devel@gnu.org
>>
>> After thinking on that a little bit more since yesterday; maybe it is
>> possible to add another basic face for the rest of the line. That face
>> will be merged with the previous face as in the example code, so if it
>> specifies :underline then merging should work as specified; else, it
>> will just use the :underline from the latest glyph.
>
>Such a face will not be a fixed face, it will have to be recomputed
>whenever the face of the text changes, right?  E.g., if the face of
>the text specifies some color, you'd want this additional face to have
>the same colors, right?
>
We don't use the face itself, just to merge with the previous glyph.

>So it doesn't seem to be a face that can be customized in the usual
>sense.  We could let the users specify face attributes they don't want
>to see in face extension, though.

Please look the proposed patch. It may need some improvements, but at
least the functional part is a decent solution for all the issues in my
opinion.


[-- Attachment #2: fix_36858.patch --]
[-- Type: text/plain, Size: 8407 bytes --]

diff --git a/lisp/faces.el b/lisp/faces.el
index 5193c216d0..9c3eba0fff 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -2510,7 +2510,7 @@ unwanted effects."
 
 ;; Definition stolen from display-line-numbers.
 (defface fill-column-indicator
-  '((t :inherit shadow :weight normal :slant normal
+  '((t :inherit extend-to-end-of-line :weight normal :slant normal
        :underline nil :overline nil :strike-through nil
        :box nil :inverse-video nil :stipple nil))
   "Face for displaying fill column indicator.
@@ -2694,12 +2694,20 @@ the same as `window-divider' face."
   :group 'basic-faces)
 
 (defface internal-border
-    '((t nil))
+  '((t nil))
   "Basic face for the internal border."
   :version "26.1"
   :group 'frames
   :group 'basic-faces)
 
+(defface extend-to-end-of-line
+  '((t :weight normal :slant normal
+       :underline nil :overline nil :strike-through nil
+       :box nil :inverse-video nil :stipple nil))
+  "Basic face to extend to end if line."
+  :version "27.1"
+  :group 'basic-faces)
+
 (defface minibuffer-prompt
   '((((background dark)) :foreground "cyan")
     ;; Don't use blue because many users of the MS-DOS port customize
diff --git a/src/dispextern.h b/src/dispextern.h
index 4e947daa25..9a8bad6d08 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1780,6 +1780,7 @@ enum face_id
   WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID,
   WINDOW_DIVIDER_LAST_PIXEL_FACE_ID,
   INTERNAL_BORDER_FACE_ID,
+  EXTEND_TO_END_OF_LINE_FACE_ID,
   BASIC_FACE_ID_SENTINEL
 };
 
diff --git a/src/xdisp.c b/src/xdisp.c
index 7338d2b7d4..738a6ca129 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -386,6 +386,7 @@ fill_column_indicator_column (struct it *it)
 {
   if (Vdisplay_fill_column_indicator
       && it->continuation_lines_width == 0
+      && IT_CHARPOS (*it) < ZV
       && CHARACTERP (Vdisplay_fill_column_indicator_character))
     {
       Lisp_Object col = (EQ (Vdisplay_fill_column_indicator_column, Qt)
@@ -20468,6 +20469,9 @@ extend_face_to_end_of_line (struct it *it)
       it->face_id = FACE_FOR_CHAR (f, face, 0, -1, Qnil);
     }
 
+  const int extend_face_merged_id =
+    merge_faces (it->w, Qextend_to_end_of_line, 0, it->face_id);
+
 #ifdef HAVE_WINDOW_SYSTEM
   if (FRAME_WINDOW_P (f))
     {
@@ -20519,12 +20523,14 @@ extend_face_to_end_of_line (struct it *it)
 	      const int char_width = (font->average_width
 				      ? font->average_width
 				      : font->space_width);
-	      int column_x;
-
-	      if (!INT_MULTIPLY_WRAPV (indicator_column, char_width, &column_x)
-		  && !INT_ADD_WRAPV (it->lnum_pixel_width, column_x, &column_x)
-		  && column_x >= it->current_x
-		  && column_x <= it->last_visible_x)
+	      int indicator_column_x;
+
+	      if (!INT_MULTIPLY_WRAPV (indicator_column,
+	                               char_width, &indicator_column_x)
+		  && !INT_ADD_WRAPV (it->lnum_pixel_width,
+		                     indicator_column_x, &indicator_column_x)
+		  && indicator_column_x >= it->current_x
+		  && indicator_column_x <= it->last_visible_x)
 	        {
 	          const char saved_char = it->char_to_display;
 	          const struct text_pos saved_pos = it->position;
@@ -20533,14 +20539,15 @@ extend_face_to_end_of_line (struct it *it)
 	          const bool saved_box_start = it->start_of_box_run_p;
 	          Lisp_Object save_object = it->object;
 
-	          /* The stretch width needs to considet the latter
+	          /* The stretch width needs to consider the latter
 	             added glyph.  */
 	          const int stretch_width
-		    = column_x - it->current_x - char_width;
+		    = indicator_column_x - it->current_x - char_width;
 
 	          memset (&it->position, 0, sizeof it->position);
 	          it->avoid_cursor_p = true;
 	          it->object = Qnil;
+		  it->face_id = extend_face_merged_id;
 
 	          /* Only generate a stretch glyph if there is distance
 	             between current_x and and the indicator position.  */
@@ -20548,6 +20555,7 @@ extend_face_to_end_of_line (struct it *it)
 		    {
 		      int stretch_ascent = (((it->ascent + it->descent)
 		                             * FONT_BASE (font)) / FONT_HEIGHT (font));
+
 		      append_stretch_glyph (it, Qnil, stretch_width,
 		                            it->ascent + it->descent,
 		                            stretch_ascent);
@@ -20555,24 +20563,29 @@ extend_face_to_end_of_line (struct it *it)
 
 	          /* Generate the glyph indicator only if
 	             append_space_for_newline didn't already.  */
-	          if (it->current_x < column_x)
+	          if (it->current_x < indicator_column_x)
 	            {
+		      it->face_id
+			= merge_faces (it->w, Qextend_to_end_of_line,
+			               0, extend_face_merged_id);
+
 		      it->char_to_display
 			= XFIXNAT (Vdisplay_fill_column_indicator_character);
-	              it->face_id
-			= merge_faces (it->w, Qfill_column_indicator,
-				       0, saved_face_id);
 	              PRODUCE_GLYPHS (it);
-	            }
 
-	          /* Restore the face after the indicator was generated.  */
-	          it->face_id = saved_face_id;
+		      it->face_id = extend_face_merged_id;
+	            }
 
 	          /* If there is space after the indicator generate an
 	             extra empty glyph to restore the face.  Issue was
 	             observed in X systems.  */
-	          it->char_to_display = ' ';
-	          PRODUCE_GLYPHS (it);
+		  if (it->current_x < it->last_visible_x) {
+		    it->char_to_display = ' ';
+		    PRODUCE_GLYPHS (it);
+		  }
+
+	          /* Restore the face after the indicator was generated.  */
+	          it->face_id = saved_face_id;
 
 	          it->char_to_display = saved_char;
 	          it->position = saved_pos;
@@ -20697,26 +20710,27 @@ extend_face_to_end_of_line (struct it *it)
       /* The last row's blank glyphs should get the default face, to
 	 avoid painting the rest of the window with the region face,
 	 if the region ends at ZV.  */
+
       if (it->glyph_row->ends_at_zv_p)
 	it->face_id = default_face->id;
       else
-	it->face_id = face->id;
+	it->face_id = extend_face_merged_id;
 
       /* Display fill-column indicator if needed.  */
       int indicator_column = fill_column_indicator_column (it);
       if (indicator_column >= 0
-	  && INT_ADD_WRAPV (it->lnum_pixel_width, indicator_column,
+          && INT_ADD_WRAPV (it->lnum_pixel_width, indicator_column,
 			    &indicator_column))
 	indicator_column = -1;
       do
 	{
 	  int saved_face_id;
-	  bool indicate = it->current_x == indicator_column;
+	  const bool indicate = it->current_x == indicator_column;
 	  if (indicate)
 	    {
 	      saved_face_id = it->face_id;
 	      it->face_id
-		= merge_faces (it->w, Qfill_column_indicator, 0, saved_face_id);
+		= merge_faces (it->w, Qfill_column_indicator, 0, extend_face_merged_id);
 	      it->c = it->char_to_display
 		= XFIXNAT (Vdisplay_fill_column_indicator_character);
 	    }
diff --git a/src/xfaces.c b/src/xfaces.c
index c3cae7e2a6..a0b0d9c6f8 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -4598,6 +4598,7 @@ lookup_basic_face (struct window *w, struct frame *f, int face_id)
     case WINDOW_DIVIDER_FIRST_PIXEL_FACE_ID:	name = Qwindow_divider_first_pixel;	break;
     case WINDOW_DIVIDER_LAST_PIXEL_FACE_ID:	name = Qwindow_divider_last_pixel;	break;
     case INTERNAL_BORDER_FACE_ID:	name = Qinternal_border; 	break;
+    case EXTEND_TO_END_OF_LINE_FACE_ID: name = Qextend_to_end_of_line;  break;
 
     default:
       emacs_abort (); /* the caller is supposed to pass us a basic face id */
@@ -5293,6 +5294,7 @@ realize_basic_faces (struct frame *f)
       realize_named_face (f, Qwindow_divider_last_pixel,
 			  WINDOW_DIVIDER_LAST_PIXEL_FACE_ID);
       realize_named_face (f, Qinternal_border, INTERNAL_BORDER_FACE_ID);
+      realize_named_face (f, Qextend_to_end_of_line, EXTEND_TO_END_OF_LINE_FACE_ID);
 
       /* Reflect changes in the `menu' face in menu bars.  */
       if (FRAME_FACE_CACHE (f)->menu_face_changed_p)
@@ -6592,6 +6594,7 @@ syms_of_xfaces (void)
   DEFSYM (Qwindow_divider_first_pixel, "window-divider-first-pixel");
   DEFSYM (Qwindow_divider_last_pixel, "window-divider-last-pixel");
   DEFSYM (Qinternal_border, "internal-border");
+  DEFSYM (Qextend_to_end_of_line, "extend-to-end-of-line");
 
   /* TTY color-related functions (defined in tty-colors.el).  */
   DEFSYM (Qtty_color_desc, "tty-color-desc");