Re: Question about display engine

[Ergus via "Emacs development discussions." <emacs-devel@gnu.org>]

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

PATCH: (relative to  dd162a3f2264940e3e329d0bf)

On Mon, Sep 02, 2019 at 07:18:19PM +0300, Eli Zaretskii wrote:
>> Date: Mon, 2 Sep 2019 13:05:04 +0200
>> From: Ergus <spacibba@aol.com>
>> Cc: Eli Zaretskii <eliz@gnu.org>, emacs-devel@gnu.org
>>
>> >If so then we might consider the following optimization: The extra
>> >face pointer in each face is no more needed after the display engine
>> >has processed the face.  So we could build a "shadow" face for b+d,
>> >keep it in a separate, static face structure and realize a face from
>> >that structure whenever we want to (eagerly or lazily).  The display
>> >engine, when it finds a pointer to such an extra face at EOL, uses it
>> >(maybe realizing it on the fly).
>> >
>> >Also, the merger could nullify the extra face if it's the same as the
>> >normal one, that is no single merge step had an :extend false value
>> >override a former :extend true value.  So the display engine would
>> >know beforehand that it does not have to change the current face.
>> >
>> Yes; in my initial proposed approach the local pointer will be null in
>> that case.
>
>I don't think I understand why you are talking about face pointers.
>The iterator doesn't keep any face pointers, it keeps face IDs (which
>allow to obtain the face pointer, when needed, by using FACE_FROM_ID).
>
>So all you need is to have another face ID member in the iterator, to
>be used for extending past EOL; depending on how the :extend bits are
>set, that face ID may or may not be identical to the "normal" face ID,
>the one we have now and use for buffer text.
>
>> >Last but not least, the display engine has to, after processing the
>> >spaces at the EOL from b+d, restore the a+b+c+d+e face as its current
>> >face.  So we have two static pointers to keep around: One for the b+d
>> >face structure (or its already realized face) while the display engine
>> >processes normal text and one for the a+b+c+d+e realized face while
>> >the display engine processes the spaces at EOL.  Can you still agree?
>>
>> In the display engine we do this very
>> frequently. As extend_face_to_end_of_line is very localized we just need
>> to save a pointer to a+b+c+d on the beginning of the function and
>> restore it at the end.
>
>Again, not a pointer: a face ID.  And yes, we have saved_face_id
>member of the iterator for this purpose.
>
>> And that in X there is some extra code (somewhere) to extend the
>> background color using the color in the last inserted glyph (it is
>> something happening by default without calling even
>> extend_face_to_end_of_line). That code should be removed after this
>> change; but I don't know where is it. But for sure Eli will tell.
>
>It's just that we clear to EOL with the last background color, and
>avoid doing that if the background color is identical to the frame's
>background.  I don't think anything will have to be changed there, but
>we will see (I hope).
>

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

diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d73bce42c3..5a49a81043 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -233,7 +233,11 @@ custom-face-attributes
 	     (file :tag "File"
 		   :help-echo "Name of bitmap file."
 		   :must-match t)))
-
+    (:extend
+     (choice :tag "Extend"
+	     :help-echo "Control whether attributes should be extended after EOL."
+	     (const :tag "Off" nil)
+	     (const :tag "On" t)))
     (:inherit
      (repeat :tag "Inherit"
 	     :help-echo "List of faces to inherit attributes from."
diff --git a/lisp/faces.el b/lisp/faces.el
index 5193c216d0..11e42c5380 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -342,6 +342,7 @@ face-x-resources
     (:box (".attributeBox" . "Face.AttributeBox"))
     (:underline (".attributeUnderline" . "Face.AttributeUnderline"))
     (:inverse-video (".attributeInverse" . "Face.AttributeInverse"))
+    (:extend (".attributeExtend" . "Face.AttributeExtend"))
     (:stipple
      (".attributeStipple" . "Face.AttributeStipple")
      (".attributeBackgroundPixmap" . "Face.AttributeBackgroundPixmap"))
@@ -594,6 +595,13 @@ face-italic-p
   (let ((italic (face-attribute face :slant frame inherit)))
     (memq italic '(italic oblique))))
 
+(defun face-extend-p (face &optional frame inherit)
+ "Return non-nil if FACE specifies a non-nil extend.
+If the optional argument FRAME is given, report on face FACE in that frame.
+If FRAME is t, report on the defaults for face FACE (for new frames).
+If FRAME is omitted or nil, use the selected frame.
+Optional argument INHERIT is passed to `face-attribute'."
+ (eq (face-attribute face :extend frame inherit) t))
 
 \f
 ;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;;
@@ -760,6 +768,11 @@ set-face-attribute
 `:height', `:weight', and `:slant' may also be set in one step
 from an X font name:
 
+`:extend'
+
+VALUE specifies whether the FACE should be extended after EOL.
+VALUE must be one of t or nil.
+
 `:font'
 
 Set font-related face attributes from VALUE.
@@ -979,6 +992,18 @@ set-face-italic
 
 (define-obsolete-function-alias 'set-face-italic-p 'set-face-italic "24.4")
 
+(defun set-face-extend (face extend-p &optional frame)
+  "Specify whether face FACE should be extended.
+EXTEND-P nil means FACE explicitly doesn't extend after EOL.
+EXTEND-P t means FACE extends after EOL.
+
+FRAME nil or not specified means change face on all frames.
+Use `set-face-attribute' to \"unspecify\" underlining."
+  (interactive
+   (let ((list (read-face-and-attribute :extend)))
+     (list (car list) (if (cadr list) t))))
+  (set-face-attribute face frame :extend extend-p))
+
 
 (defalias 'set-face-background-pixmap 'set-face-stipple)
 
@@ -1102,7 +1127,7 @@ face-valid-attribute-values
 	   (:slant
 	    (mapcar #'(lambda (x) (cons (symbol-name (aref x 1)) (aref x 1)))
 		    font-slant-table))
-	   (:inverse-video
+	   ((or :inverse-video :extend)
 	    (mapcar #'(lambda (x) (cons (symbol-name x) x))
 		    (internal-lisp-face-attribute-values attribute)))
            ((or :underline :overline :strike-through :box)
@@ -1147,12 +1172,13 @@ face-attribute-name-alist
     (:slant . "slant")
     (:underline . "underline")
     (:overline . "overline")
+    (:extend . "extend")
     (:strike-through . "strike-through")
     (:box . "box")
     (:inverse-video . "inverse-video display")
     (:foreground . "foreground color")
     (:background . "background color")
-    (:stipple . "background stipple")
+    (:background . "background color")
     (:inherit . "inheritance"))
   "An alist of descriptive names for face attributes.
 Each element has the form (ATTRIBUTE-NAME . DESCRIPTION) where
@@ -2432,24 +2458,24 @@ highlight
 ;; if background is light.
 (defface region
   '((((class color) (min-colors 88) (background dark))
-     :background "blue3")
+     :background "blue3" :extend t)
     (((class color) (min-colors 88) (background light) (type gtk))
      :distant-foreground "gtk_selection_fg_color"
-     :background "gtk_selection_bg_color")
+     :background "gtk_selection_bg_color" :extend t)
     (((class color) (min-colors 88) (background light) (type ns))
      :distant-foreground "ns_selection_fg_color"
-     :background "ns_selection_bg_color")
+     :background "ns_selection_bg_color" :extend t)
     (((class color) (min-colors 88) (background light))
-     :background "lightgoldenrod2")
+     :background "lightgoldenrod2" :extend t)
     (((class color) (min-colors 16) (background dark))
-     :background "blue3")
+     :background "blue3" :extend t)
     (((class color) (min-colors 16) (background light))
-     :background "lightgoldenrod2")
+     :background "lightgoldenrod2" :extend t)
     (((class color) (min-colors 8))
-     :background "blue" :foreground "white")
+     :background "blue" :foreground "white" :extend t)
     (((type tty) (class mono))
      :inverse-video t)
-    (t :background "gray"))
+    (t :background "gray" :extend t))
   "Basic face for highlighting the region."
   :version "21.1"
   :group 'basic-faces)
diff --git a/src/dispextern.h b/src/dispextern.h
index 05f199ff35..e09d36944a 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1564,6 +1564,7 @@ #define FONT_TOO_HIGH(ft)						\
   LFACE_INHERIT_INDEX,
   LFACE_FONTSET_INDEX,
   LFACE_DISTANT_FOREGROUND_INDEX,
+  LFACE_EXTEND_INDEX,
   LFACE_VECTOR_SIZE
 };
 
@@ -1589,6 +1590,7 @@ #define FONT_TOO_HIGH(ft)						\
 
 enum face_underline_type
 {
+  FACE_NO_UNDERLINE = 0,
   FACE_UNDER_LINE,
   FACE_UNDER_WAVE
 };
@@ -1632,11 +1634,9 @@ #define FONT_TOO_HIGH(ft)						\
   /* Pixel value or color index of background color.  */
   unsigned long background;
 
-  /* Pixel value or color index of underline color.  */
+  /* Pixel value or color index of underline, overlined,
+     strike-through, or box color.  */
   unsigned long underline_color;
-
-  /* Pixel value or color index of overlined, strike-through, or box
-     color.  */
   unsigned long overline_color;
   unsigned long strike_through_color;
   unsigned long box_color;
@@ -1663,7 +1663,7 @@ #define FONT_TOO_HIGH(ft)						\
   ENUM_BF (face_box_type) box : 2;
 
   /* Style of underlining. */
-  ENUM_BF (face_underline_type) underline_type : 1;
+  ENUM_BF (face_underline_type) underline : 2;
 
   /* If `box' above specifies a 3D type, true means use box_color for
      drawing shadows.  */
@@ -1671,7 +1671,6 @@ #define FONT_TOO_HIGH(ft)						\
 
   /* Non-zero if text in this face should be underlined, overlined,
      strike-through or have a box drawn around it.  */
-  bool_bf underline_p : 1;
   bool_bf overline_p : 1;
   bool_bf strike_through_p : 1;
 
@@ -1681,14 +1680,10 @@ #define FONT_TOO_HIGH(ft)						\
   bool_bf foreground_defaulted_p : 1;
   bool_bf background_defaulted_p : 1;
 
-  /* True means that either no color is specified for underlining or that
-     the specified color couldn't be loaded.  Use the foreground
-     color when drawing in that case. */
-  bool_bf underline_defaulted_p : 1;
-
   /* True means that either no color is specified for the corresponding
      attribute or that the specified color couldn't be loaded.
      Use the foreground color when drawing in that case. */
+  bool_bf underline_defaulted_p : 1;
   bool_bf overline_color_defaulted_p : 1;
   bool_bf strike_through_color_defaulted_p : 1;
   bool_bf box_color_defaulted_p : 1;
@@ -2448,6 +2443,9 @@ #define OVERLAY_STRING_CHUNK_SIZE 16
   /* Face to use.  */
   int face_id;
 
+  /* Face to extend at EOL/  */
+  int extend_face_id;
+
   /* Setting of buffer-local variable selective-display-ellipses.  */
   bool_bf selective_display_ellipsis_p : 1;
 
diff --git a/src/xdisp.c b/src/xdisp.c
index 75bc536cb9..015a80af42 100644
--- a/src/xdisp.c
+++ b/src/xdisp.c
@@ -7141,18 +7141,50 @@ lookup_glyphless_char_display (int c, struct it *it)
 }
 
 /* Merge escape glyph face and cache the result.  */
+static struct frame *last_extend_glyph_frame = NULL;
+static int last_extend_glyph_face_id = (1 << FACE_ID_BITS);
+static int last_extend_glyph_merged_face_id = 0;
 
+static int
+merge_extend_glyph_face (struct it *it)
+{
+  int extend_face_id = it->extend_face_id;
+
+  struct face *face = FACE_FROM_ID (it->f, it->extend_face_id);
+  fprintf(stderr, "extending1!!!\n");
+
+  if (!NILP (face->lface[LFACE_EXTEND_INDEX]))
+    {
+      fprintf(stderr,"extending2!!!\n");
+
+      if (it->f == last_extend_glyph_frame
+          && it->extend_face_id == last_extend_glyph_face_id)
+	return last_extend_glyph_merged_face_id;
+
+      /* Merge the `glyphless-char' face into the current face.  */
+      extend_face_id = merge_faces (it->w, Qt, it->face_id, extend_face_id);
+      last_extend_glyph_frame = it->f;
+      last_extend_glyph_face_id = it->extend_face_id;
+      last_extend_glyph_merged_face_id = extend_face_id;
+    }
+  return extend_face_id;
+}
+
+
+/* Merge escape glyph face and cache the result.  */
 static struct frame *last_escape_glyph_frame = NULL;
 static int last_escape_glyph_face_id = (1 << FACE_ID_BITS);
 static int last_escape_glyph_merged_face_id = 0;
 
 static int
-merge_escape_glyph_face (struct it *it)
+merge_escape_glyph_face (struct it *it, int lface_id)
 {
   int face_id;
 
-  if (it->f == last_escape_glyph_frame
-      && it->face_id == last_escape_glyph_face_id)
+  if (lface_id)
+    face_id = merge_faces (it->w, Qt, lface_id, it->face_id);
+  else if (it->f == last_escape_glyph_frame
+           && it->face_id == last_escape_glyph_face_id)
     face_id = last_escape_glyph_merged_face_id;
   else
     {
@@ -7162,6 +7194,8 @@ merge_escape_glyph_face (struct it *it)
       last_escape_glyph_face_id = it->face_id;
       last_escape_glyph_merged_face_id = face_id;
     }
+
+  merge_extend_glyph_face (it);
   return face_id;
 }
 
@@ -7187,9 +7221,12 @@ merge_glyphless_glyph_face (struct it *it)
       last_glyphless_glyph_face_id = it->face_id;
       last_glyphless_glyph_merged_face_id = face_id;
     }
+
+  merge_extend_glyph_face (it);
   return face_id;
 }
 
+
 /* Forget the `escape-glyph' and `glyphless-char' faces.  This should
    be called before redisplaying windows, and when the frame's face
    cache is freed.  */
@@ -7355,9 +7392,7 @@ get_next_display_element (struct it *it)
 		      lface_id = GLYPH_CODE_FACE (gc);
 		    }
 
-		  face_id = (lface_id
-			     ? merge_faces (it->w, Qt, lface_id, it->face_id)
-			     : merge_escape_glyph_face (it));
+		  face_id = merge_escape_glyph_face (it, lface_id);
 
 		  XSETINT (it->ctl_chars[0], g);
 		  XSETINT (it->ctl_chars[1], c ^ 0100);
@@ -7373,6 +7408,8 @@ get_next_display_element (struct it *it)
 		  /* Merge `nobreak-space' into the current face.  */
 		  face_id = merge_faces (it->w, Qnobreak_space, 0,
 					 it->face_id);
+
+		  merge_extend_glyph_face (it);
 		  XSETINT (it->ctl_chars[0], ' ');
 		  ctl_len = 1;
 		  goto display_control;
@@ -7385,7 +7422,10 @@ get_next_display_element (struct it *it)
 		{
 		  /* Merge `nobreak-space' into the current face.  */
 		  face_id = merge_faces (it->w, Qnobreak_hyphen, 0,
-					 it->face_id);
+		                         it->face_id);
+
+		  merge_extend_glyph_face (it);
+
 		  XSETINT (it->ctl_chars[0], '-');
 		  ctl_len = 1;
 		  goto display_control;
@@ -7403,12 +7443,10 @@ get_next_display_element (struct it *it)
 		  lface_id = GLYPH_CODE_FACE (gc);
 		}
 
-	      face_id = (lface_id
-			 ? merge_faces (it->w, Qt, lface_id, it->face_id)
-			 : merge_escape_glyph_face (it));
+	      face_id = merge_escape_glyph_face (it, lface_id);
+	      
 
 	      /* Draw non-ASCII space/hyphen with escape glyph: */
-
 	      if (nonascii_space_p || nonascii_hyphen_p)
 		{
 		  XSETINT (it->ctl_chars[0], escape_glyph);
@@ -8032,8 +8070,11 @@ next_element_from_display_vector (struct it *it)
 	{
 	  int lface_id = GLYPH_CODE_FACE (gc);
 	  if (lface_id > 0)
-	    it->face_id = merge_faces (it->w, Qt, lface_id,
-				       it->saved_face_id);
+	    {
+	      it->face_id = merge_faces (it->w, Qt, lface_id,
+	                                 it->saved_face_id);
+	      merge_extend_glyph_face (it);
+	    }
 	}
 
       /* Glyphs in the display vector could have the box face, so we
@@ -8061,8 +8102,11 @@ next_element_from_display_vector (struct it *it)
 		GLYPH_CODE_FACE (it->dpvec[it->current.dpvec_index + 1]);
 
 	      if (lface_id > 0)
-		next_face_id = merge_faces (it->w, Qt, lface_id,
-					    it->saved_face_id);
+		{
+		  next_face_id = merge_faces (it->w, Qt, lface_id,
+		                              it->saved_face_id);
+		  merge_extend_glyph_face (it);
+		}
 	    }
 	}
       next_face = FACE_FROM_ID_OR_NULL (it->f, next_face_id);
@@ -20324,7 +20368,7 @@ append_space_for_newline (struct it *it, bool default_face_p)
 		     = XFIXNAT (Vdisplay_fill_column_indicator_character);
 	           it->face_id
 		     = merge_faces (it->w, Qfill_column_indicator,
-				    0, saved_face_id);
+		                    0, it->extend_face_id);
 	           face = FACE_FROM_ID (it->f, it->face_id);
 	           goto produce_glyphs;
 	         }
@@ -20471,33 +20515,33 @@ extend_face_to_end_of_line (struct it *it)
      1-``pixel'' wide, so they hit the equality too early.  This grace
      is needed only for R2L rows that are not continued, to produce
      one extra blank where we could display the cursor.  */
-  if ((it->current_x >= it->last_visible_x
-       + (!FRAME_WINDOW_P (f)
-	  && it->glyph_row->reversed_p
-	  && !it->glyph_row->continued_p))
-      /* If the window has display margins, we will need to extend
-	 their face even if the text area is filled.  */
-      && !(WINDOW_LEFT_MARGIN_WIDTH (it->w) > 0
-	   || WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0))
-    return;
-
-  /* Face extension extends the background and box of IT->face_id
-     to the end of the line.  If the background equals the background
-     of the frame, we don't have to do anything.  */
+/*   if ((it->current_x >= it->last_visible_x */
+/*        + (!FRAME_WINDOW_P (f) */
+/*           && it->glyph_row->reversed_p */
+/*           && !it->glyph_row->continued_p)) */
+/*       /\* If the window has display margins, we will need to extend */
+/* 	 their face even if the text area is filled.  *\/ */
+/*       && !(WINDOW_LEFT_MARGIN_WIDTH (it->w) > 0 */
+/* 	   || WINDOW_RIGHT_MARGIN_WIDTH (it->w) > 0)) */
+/*     return; */
+
+/*   /\* Face extension extends the background and box of IT->face_id */
+/*      to the end of the line.  If the background equals the background */
+/*      of the frame, we don't have to do anything.  *\/ */
   face = FACE_FROM_ID (f, (it->face_before_selective_p
 			   ? it->saved_face_id
-			   : it->face_id));
-
-  if (FRAME_WINDOW_P (f)
-      && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row)
-      && face->box == FACE_NO_BOX
-      && FACE_COLOR_TO_PIXEL (face->background, f) == FRAME_BACKGROUND_PIXEL (f)
-#ifdef HAVE_WINDOW_SYSTEM
-      && !face->stipple
-#endif
-      && !it->glyph_row->reversed_p
-      && !Vdisplay_fill_column_indicator)
-    return;
+			   : it->extend_face_id));
+
+/*   if (FRAME_WINDOW_P (f) */
+/*       && MATRIX_ROW_DISPLAYS_TEXT_P (it->glyph_row) */
+/*       && face->box == FACE_NO_BOX */
+/*       && FACE_COLOR_TO_PIXEL (face->background, f) == FRAME_BACKGROUND_PIXEL (f) */
+/* #ifdef HAVE_WINDOW_SYSTEM */
+/*       && !face->stipple */
+/* #endif */
+/*       && !it->glyph_row->reversed_p */
+/*       && !Vdisplay_fill_column_indicator) */
+/*     return; */
 
   /* Set the glyph row flag indicating that the face of the last glyph
      in the text area has to be drawn to the end of the text area.  */
@@ -20560,79 +20604,88 @@ extend_face_to_end_of_line (struct it *it)
 	  /* Display fill column indicator if not in modeline or
 	     toolbar and display fill column indicator mode is
 	     active.  */
+	  const char saved_char = it->char_to_display;
+	  const struct text_pos saved_pos = it->position;
+	  const bool saved_avoid_cursor = it->avoid_cursor_p;
+	  const int saved_face_id = it->face_id;
+	  const bool saved_box_start = it->start_of_box_run_p;
+	  Lisp_Object save_object = it->object;
+
+	  it->avoid_cursor_p = true;
+	  it->object = Qnil;
+	  memset (&it->position, 0, sizeof it->position);
+
 	  int indicator_column = (it->w->pseudo_window_p == 0
 				  ? fill_column_indicator_column (it)
 				  : -1);
-	  if (indicator_column >= 0)
-            {
-	      struct font *font = (default_face->font
+
+	  struct font *font = (default_face->font
 				   ? default_face->font
 				   : FRAME_FONT (f));
-	      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)
-	        {
-	          const char saved_char = it->char_to_display;
-	          const struct text_pos saved_pos = it->position;
-	          const bool saved_avoid_cursor = it->avoid_cursor_p;
-	          const int saved_face_id = it->face_id;
-	          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
-	             added glyph.  */
-	          const int stretch_width
-		    = column_x - it->current_x - char_width;
-
-	          memset (&it->position, 0, sizeof it->position);
-	          it->avoid_cursor_p = true;
-	          it->object = Qnil;
-
-	          /* Only generate a stretch glyph if there is distance
-	             between current_x and and the indicator position.  */
-	          if (stretch_width > 0)
-		    {
-		      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);
-		    }
+	  const int char_width = (font->average_width
+	                          ? font->average_width
+	                          : font->space_width);
+	  int column_x;
+
+	  if (indicator_column >= 0
+	      && !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)
+	    {
+
+	      /* The stretch width needs to considet the latter
+		 added glyph.  */
+	      const int stretch_width
+		= column_x - it->current_x - char_width;
+
+	      /* Only generate a stretch glyph if there is distance
+		 between current_x and and the indicator position.  */
+	      if (stretch_width > 0)
+		{
+		  it->face_id = it->extend_face_id;
 
-	          /* Generate the glyph indicator only if
-	             append_space_for_newline didn't already.  */
-	          if (it->current_x < column_x)
-	            {
-		      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;
-
-	          /* 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);
-
-	          it->char_to_display = saved_char;
-	          it->position = saved_pos;
-	          it->avoid_cursor_p = saved_avoid_cursor;
-	          it->start_of_box_run_p = saved_box_start;
-	          it->object = save_object;
-	        }
-            }
+		  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);
+		}
+
+	      /* Generate the glyph indicator only if
+		 append_space_for_newline didn't already.  */
+	      if (it->current_x < column_x)
+		{
+		  it->char_to_display
+		    = XFIXNAT (Vdisplay_fill_column_indicator_character);
+		  it->face_id
+		    = merge_faces (it->w, Qfill_column_indicator,
+		                   0, it->extend_face_id);
+		  PRODUCE_GLYPHS (it);
+		  it->char_to_display = saved_char;
+		}
+
+	    }
+
+	  const int stretch_width = it->last_visible_x - it->current_x;
+
+	  if (stretch_width > 0)
+	    {
+	      it->face_id = it->extend_face_id;
+
+	      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);
+	    }
+
+	  it->char_to_display = saved_char;
+	  it->position = saved_pos;
+	  it->avoid_cursor_p = saved_avoid_cursor;
+	  it->face_id = saved_face_id;
+	  it->start_of_box_run_p = saved_box_start;
+	  it->object = save_object;
 	}
       if (it->glyph_row->reversed_p)
 	{
@@ -20718,7 +20771,7 @@ extend_face_to_end_of_line (struct it *it)
       it->len = 1;
 
       if (WINDOW_LEFT_MARGIN_WIDTH (it->w) > 0
-	  && (it->glyph_row->used[LEFT_MARGIN_AREA]
+          && (it->glyph_row->used[LEFT_MARGIN_AREA]
 	      < WINDOW_LEFT_MARGIN_WIDTH (it->w))
 	  && !it->glyph_row->mode_line_p
 	  && FACE_COLOR_TO_PIXEL (face->background, f) != FRAME_BACKGROUND_PIXEL (f))
@@ -20762,24 +20815,25 @@ extend_face_to_end_of_line (struct it *it)
 	indicator_column = -1;
       do
 	{
-	  int saved_face_id;
-	  bool indicate = it->current_x == indicator_column;
-	  if (indicate)
+	  if (it->current_x == indicator_column)
 	    {
-	      saved_face_id = it->face_id;
+	      int 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,
+		               it->extend_face_id);
 	      it->c = it->char_to_display
 		= XFIXNAT (Vdisplay_fill_column_indicator_character);
-	    }
 
-	  PRODUCE_GLYPHS (it);
+	      PRODUCE_GLYPHS (it);
 
-	  if (indicate)
-	    {
 	      it->face_id = saved_face_id;
 	      it->c = it->char_to_display = ' ';
 	    }
+	  else
+	    {
+	      PRODUCE_GLYPHS (it);
+	    }
 	}
       while (it->current_x <= it->last_visible_x);
 
@@ -21760,6 +21814,7 @@ display_line (struct it *it, int cursor_vpos)
   it->starts_in_middle_of_char_p = false;
   it->tab_offset = 0;
   it->line_number_produced_p = false;
+  it->extend_face_id = DEFAULT_FACE_ID;
 
   /* Arrange the overlays nicely for our purposes.  Usually, we call
      display_line on only one line at a time, in which case this
@@ -27354,7 +27409,7 @@ font_for_underline_metrics (struct glyph_string *s)
   for (g = s->first_glyph - 1; g >= g0; g--)
     {
       struct face *prev_face = FACE_FROM_ID (s->f, g->face_id);
-      if (!(prev_face && prev_face->underline_p))
+      if (!(prev_face && prev_face->underline != FACE_NO_UNDERLINE))
 	break;
     }
 
diff --git a/src/xfaces.c b/src/xfaces.c
index c3cae7e2a6..092e110de5 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -1209,7 +1209,7 @@ free_face_colors (struct frame *f, struct face *face)
       IF_DEBUG (--ncolors_allocated);
     }
 
-  if (face->underline_p
+  if (face->underline
       && !face->underline_defaulted_p)
     {
       x_free_colors (f, &face->underline_color, 1);
@@ -1590,6 +1590,7 @@ #define LFACE_BOX(LFACE)	    AREF ((LFACE), LFACE_BOX_INDEX)
 #define LFACE_FONT(LFACE)	    AREF ((LFACE), LFACE_FONT_INDEX)
 #define LFACE_INHERIT(LFACE)	    AREF ((LFACE), LFACE_INHERIT_INDEX)
 #define LFACE_FONTSET(LFACE)	    AREF ((LFACE), LFACE_FONTSET_INDEX)
+#define LFACE_EXTEND(LFACE)	    AREF ((LFACE), LFACE_EXTEND_INDEX)
 #define LFACE_DISTANT_FOREGROUND(LFACE) \
   AREF ((LFACE), LFACE_DISTANT_FOREGROUND_INDEX)
 
@@ -2512,6 +2513,13 @@ merge_face_ref (struct window *w,
 					err_msgs, named_merge_points))
 		    err = true;
 		}
+	      else if (EQ (keyword, QCextend))
+		{
+		  if (EQ (value, Qt) || NILP (value))
+		    to[LFACE_EXTEND_INDEX] = value;
+		  else
+		    err = true;
+		}
 	      else
 		err = true;
 
@@ -3030,6 +3038,17 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
       old_value = LFACE_INVERSE (lface);
       ASET (lface, LFACE_INVERSE_INDEX, value);
     }
+  else if (EQ (attr, QCextend))
+    {
+      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
+	{
+	  CHECK_SYMBOL (value);
+	  if (!EQ (value, Qt) && !NILP (value))
+	    signal_error ("Invalid inverse-video face attribute value", value);
+	}
+      old_value = LFACE_EXTEND (lface);
+      ASET (lface, LFACE_EXTEND_INDEX, value);
+    }
   else if (EQ (attr, QCforeground))
     {
       /* Compatibility with 20.x.  */
@@ -3203,6 +3222,14 @@ DEFUN ("internal-set-lisp-face-attribute", Finternal_set_lisp_face_attribute,
       ASET (lface, LFACE_SLANT_INDEX, NILP (value) ? Qnormal : Qitalic);
       prop_index = FONT_SLANT_INDEX;
     }
+  else if (EQ (attr, QCitalic))
+    {
+      attr = QCslant;
+      old_value = LFACE_SLANT (lface);
+      ASET (lface, LFACE_SLANT_INDEX, NILP (value) ? Qnormal : Qitalic);
+      prop_index = FONT_SLANT_INDEX;
+    }
+
   else
     signal_error ("Invalid face attribute name", attr);
 
@@ -3503,7 +3530,9 @@ DEFUN ("internal-set-lisp-face-attribute-from-resource",
     value = face_boolean_x_resource_value (value, true);
   else if (EQ (attr, QCweight) || EQ (attr, QCslant) || EQ (attr, QCwidth))
     value = intern (SSDATA (value));
-  else if (EQ (attr, QCreverse_video) || EQ (attr, QCinverse_video))
+  else if (EQ (attr, QCreverse_video)
+           || EQ (attr, QCinverse_video)
+           || EQ (attr, QCextend))
     value = face_boolean_x_resource_value (value, true);
   else if (EQ (attr, QCunderline)
 	   || EQ (attr, QCoverline)
@@ -3727,6 +3756,8 @@ DEFUN ("internal-get-lisp-face-attribute", Finternal_get_lisp_face_attribute,
     value = LFACE_SWIDTH (lface);
   else if (EQ (keyword, QCinherit))
     value = LFACE_INHERIT (lface);
+  else if (EQ (keyword, QCextend))
+    value = LFACE_EXTEND (lface);
   else if (EQ (keyword, QCfont))
     value = LFACE_FONT (lface);
   else if (EQ (keyword, QCfontset))
@@ -5694,16 +5725,14 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
   if (EQ (underline, Qt))
     {
       /* Use default color (same as foreground color).  */
-      face->underline_p = true;
-      face->underline_type = FACE_UNDER_LINE;
+      face->underline = FACE_UNDER_LINE;
       face->underline_defaulted_p = true;
       face->underline_color = 0;
     }
   else if (STRINGP (underline))
     {
       /* Use specified color.  */
-      face->underline_p = true;
-      face->underline_type = FACE_UNDER_LINE;
+      face->underline = FACE_UNDER_LINE;
       face->underline_defaulted_p = false;
       face->underline_color
 	= load_color (f, face, underline,
@@ -5711,7 +5740,7 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
     }
   else if (NILP (underline))
     {
-      face->underline_p = false;
+      face->underline = FACE_NO_UNDERLINE;
       face->underline_defaulted_p = false;
       face->underline_color = 0;
     }
@@ -5719,10 +5748,9 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
     {
       /* `(:color COLOR :style STYLE)'.
          STYLE being one of `line' or `wave'. */
-      face->underline_p = true;
+      face->underline = FACE_UNDER_LINE;
       face->underline_color = 0;
       face->underline_defaulted_p = true;
-      face->underline_type = FACE_UNDER_LINE;
 
       /* FIXME?  This is also not robust about checking the precise form.
          See comments in Finternal_set_lisp_face_attribute.  */
@@ -5755,9 +5783,9 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
           else if (EQ (keyword, QCstyle))
             {
               if (EQ (value, Qline))
-                face->underline_type = FACE_UNDER_LINE;
+                face->underline = FACE_UNDER_LINE;
               else if (EQ (value, Qwave))
-                face->underline_type = FACE_UNDER_WAVE;
+                face->underline = FACE_UNDER_WAVE;
             }
         }
     }
@@ -6292,9 +6320,8 @@ merge_faces (struct window *w, Lisp_Object face_name, int face_id,
 {
   struct frame *f = WINDOW_XFRAME (w);
   Lisp_Object attrs[LFACE_VECTOR_SIZE];
-  struct face *base_face;
+  struct face *base_face = FACE_FROM_ID_OR_NULL (f, base_face_id);
 
-  base_face = FACE_FROM_ID_OR_NULL (f, base_face_id);
   if (!base_face)
     return base_face_id;
 
@@ -6319,12 +6346,14 @@ merge_faces (struct window *w, Lisp_Object face_name, int face_id,
     }
   else
     {
-      struct face *face;
       if (face_id < 0)
 	return base_face_id;
-      face = FACE_FROM_ID_OR_NULL (f, face_id);
+
+      struct face *face = FACE_FROM_ID_OR_NULL (f, face_id);
+
       if (!face)
 	return base_face_id;
+
       merge_face_vectors (w, f, face->lface, attrs, 0);
     }
 
@@ -6412,7 +6441,7 @@ dump_realized_face (struct face *face)
 #endif
   fprintf (stderr, "fontset: %d\n", face->fontset);
   fprintf (stderr, "underline: %d (%s)\n",
-	   face->underline_p,
+	   face->underline,
 	   SDATA (Fsymbol_name (face->lface[LFACE_UNDERLINE_INDEX])));
   fprintf (stderr, "hash: %" PRIuPTR "\n", face->hash);
 }
@@ -6537,6 +6566,7 @@ syms_of_xfaces (void)
   DEFSYM (QCstrike_through, ":strike-through");
   DEFSYM (QCbox, ":box");
   DEFSYM (QCinherit, ":inherit");
+  DEFSYM (QCextend, ":extend");
 
   /* Symbols used for Lisp face attribute values.  */
   DEFSYM (QCcolor, ":color");
diff --git a/src/xterm.c b/src/xterm.c
index b761eaf4d1..b8f8db56a7 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -3798,9 +3798,9 @@ x_draw_glyph_string (struct glyph_string *s)
   if (!s->for_overlaps)
     {
       /* Draw underline.  */
-      if (s->face->underline_p)
+      if (s->face->underline)
         {
-          if (s->face->underline_type == FACE_UNDER_WAVE)
+          if (s->face->underline == FACE_UNDER_WAVE)
             {
               if (s->face->underline_defaulted_p)
                 x_draw_underwave (s);
@@ -3814,13 +3814,13 @@ x_draw_glyph_string (struct glyph_string *s)
                   XSetForeground (display, s->gc, xgcv.foreground);
                 }
             }
-          else if (s->face->underline_type == FACE_UNDER_LINE)
+          else if (s->face->underline == FACE_UNDER_LINE)
             {
               unsigned long thickness, position;
               int y;
 
-              if (s->prev && s->prev->face->underline_p
-		  && s->prev->face->underline_type == FACE_UNDER_LINE)
+              if (s->prev &&
+	          s->prev->face->underline == FACE_UNDER_LINE)
                 {
                   /* We use the same underline style as the previous one.  */
                   thickness = s->prev->underline_thickness;