Re: Making `x-underline-at-descent-line' a face attribute

[Po Lu <luangruo@yahoo.com>]

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

Eli Zaretskii <eliz@gnu.org> writes:

>> From: Po Lu <luangruo@yahoo.com>
>> Cc: emacs-devel@gnu.org
>> Date: Fri, 07 Jan 2022 15:14:45 +0800
>> 
>> Thanks, one last question (before I start to implement this): what would
>> you recommend as a special value of `:underline'?
>> 
>> Would it be okay to add a new property to the property list that is
>> already accepted as such a value?
>
> Yes, something like
>
>    (:color COLOR :style STYLE :position POS)
>
> where POS can be t (with the same effect as non-nil value of
> x-underline-at-descent-line) or (if you want to get fancy) a number,
> to specify an explicit offset from the baseline.

Thanks, I implemented that, though I haven't written any documentation
yet.  It would be nice if someone could test the attached diff on
MS-Windows.

BTW, there's a `hash' parameter in struct face, but I can't find where
it's calculated.  I assume I have to the hashing take any new fields
into account, but I don't know how.  Can you help with this?  Thanks.


[-- Warning: decoded text below may be mangled, UTF-8 assumed --]
[-- Attachment #2: test.diff --]
[-- Type: text/x-patch, Size: 12195 bytes --]

diff --git a/src/dispextern.h b/src/dispextern.h
index 954992a0ec..368507732c 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1720,6 +1720,12 @@ #define FONT_TOO_HIGH(ft)						\
   int box_vertical_line_width;
   int box_horizontal_line_width;
 
+
+  /* The amount of pixels above the descent line the underline should
+     be displayed.  It does not take effect unless
+     `underline_at_descent_line_p` is t.  */
+  int underline_pixels_above_descent_line;
+
   /* Type of box drawn.  A value of FACE_NO_BOX means no box is drawn
      around text in this face.  A value of FACE_SIMPLE_BOX means a box
      of width box_line_width is drawn in color box_color.  A value of
@@ -1753,6 +1759,9 @@ #define FONT_TOO_HIGH(ft)						\
   bool_bf strike_through_color_defaulted_p : 1;
   bool_bf box_color_defaulted_p : 1;
 
+  /* True means the underline should be drawn at the descent line.  */
+  bool_bf underline_at_descent_line_p : 1;
+
   /* TTY appearances.  Colors are found in `lface' with empty color
      string meaning the default color of the TTY.  */
   bool_bf tty_bold_p : 1;
diff --git a/src/haikuterm.c b/src/haikuterm.c
index 2239770de9..560ce913ac 100644
--- a/src/haikuterm.c
+++ b/src/haikuterm.c
@@ -608,7 +608,12 @@ haiku_draw_text_decoration (struct glyph_string *s, struct face *face,
 	  unsigned long thickness, position;
 	  int y;
 
-	  if (s->prev && s->prev && s->prev->hl == DRAW_MOUSE_FACE)
+	  if (s->prev
+	      && s->prev->face->underline == FACE_UNDER_LINE
+	      && (s->prev->face->underline_at_descent_line_p
+		  == s->face->underline_at_descent_line_p)
+	      && (s->prev->face->underline_pixels_above_descent_line
+		  == s->face->underline_pixels_above_descent_line))
 	    {
 	      struct face *prev_face = s->prev->face;
 
@@ -639,7 +644,8 @@ haiku_draw_text_decoration (struct glyph_string *s, struct face *face,
 	      val = (WINDOW_BUFFER_LOCAL_VALUE
 		     (Qx_underline_at_descent_line, s->w));
 	      underline_at_descent_line
-		= !(NILP (val) || EQ (val, Qunbound));
+		= (!(NILP (val) || EQ (val, Qunbound))
+		   || s->face->underline_at_descent_line_p);
 
 	      val = (WINDOW_BUFFER_LOCAL_VALUE
 		     (Qx_use_underline_position_properties, s->w));
@@ -652,7 +658,9 @@ haiku_draw_text_decoration (struct glyph_string *s, struct face *face,
 	      else
 		thickness = 1;
 	      if (underline_at_descent_line)
-		position = (s->height - thickness) - (s->ybase - s->y);
+		position = ((s->height - thickness)
+			    - (s->ybase - s->y)
+			    - s->face->underline_pixels_above_descent_line);
 	      else
 		{
 		  /* Get the underline position.  This is the
diff --git a/src/nsterm.m b/src/nsterm.m
index a15dc47a22..4f60cc737d 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -3265,7 +3265,11 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
           /* If the prev was underlined, match its appearance.  */
           if (s->prev
 	      && s->prev->face->underline == FACE_UNDER_LINE
-              && s->prev->underline_thickness > 0)
+              && s->prev->underline_thickness > 0
+	      && (s->prev->face->underline_at_descent_line_p
+		  == s->face->underline_at_descent_line_p)
+	      && (s->prev->face->underline_pixels_above_descent_line
+		  == s->face->underline_pixels_above_descent_line))
             {
               thickness = s->prev->underline_thickness;
               position = s->prev->underline_position;
@@ -3286,7 +3290,8 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
 
 	      val = (WINDOW_BUFFER_LOCAL_VALUE
 		     (Qx_underline_at_descent_line, s->w));
-	      underline_at_descent_line = !(NILP (val) || EQ (val, Qunbound));
+	      underline_at_descent_line = (!(NILP (val) || EQ (val, Qunbound))
+					   || s->face->underline_at_descent_line_p);
 
 	      val = (WINDOW_BUFFER_LOCAL_VALUE
 		     (Qx_use_underline_position_properties, s->w));
@@ -3299,7 +3304,8 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
 
               /* Determine the offset of underlining from the baseline.  */
               if (underline_at_descent_line)
-                position = descent - thickness;
+                position = (descent - thickness
+			    - s->face->underline_pixels_above_descent_line);
               else if (use_underline_position_properties
                        && font && font->underline_position >= 0)
                 position = font->underline_position;
@@ -3308,7 +3314,8 @@ Note that CURSOR_WIDTH is meaningful only for (h)bar cursors.
               else
                 position = minimum_offset;
 
-              position = max (position, minimum_offset);
+	      if (!s->face->underline_pixels_above_descent_line)
+		position = max (position, minimum_offset);
 
               /* Ensure underlining is not cropped.  */
               if (descent <= position)
diff --git a/src/w32term.c b/src/w32term.c
index 700c492cc3..78777f153c 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -2564,7 +2564,11 @@ w32_draw_glyph_string (struct glyph_string *s)
               int y;
 
               if (s->prev
-	          && s->prev->face->underline == FACE_UNDER_LINE)
+		  && s->prev->face->underline == FACE_UNDER_LINE
+		  && (s->prev->face->underline_at_descent_line_p
+		      == s->face->underline_at_descent_line_p)
+		  && (s->prev->face->underline_pixels_above_descent_line
+		      == s->face->underline_pixels_above_descent_line))
                 {
                   /* We use the same underline style as the previous one.  */
                   thickness = s->prev->underline_thickness;
@@ -2587,7 +2591,8 @@ w32_draw_glyph_string (struct glyph_string *s)
 		  val = (WINDOW_BUFFER_LOCAL_VALUE
 			 (Qx_underline_at_descent_line, s->w));
 		  underline_at_descent_line
-		    = !(NILP (val) || EQ (val, Qunbound));
+		    = (!(NILP (val) || EQ (val, Qunbound))
+		       || s->face->underline_at_descent_line_p);
 
 		  val = (WINDOW_BUFFER_LOCAL_VALUE
 			 (Qx_use_underline_position_properties, s->w));
@@ -2601,7 +2606,9 @@ w32_draw_glyph_string (struct glyph_string *s)
                     thickness = 1;
                   if (underline_at_descent_line
                       || !font)
-                    position = (s->height - thickness) - (s->ybase - s->y);
+		    position = ((s->height - thickness)
+				- (s->ybase - s->y)
+				- s->face->underline_pixels_above_descent_line);
                   else
                     {
                       /* Get the underline position.  This is the
@@ -2619,7 +2626,12 @@ w32_draw_glyph_string (struct glyph_string *s)
                       else
                         position = (font->descent + 1) / 2;
                     }
-                  position = max (position, minimum_offset);
+
+		  if (!(s->face->underline_at_descent_line_p
+			/* Ignore minimum_offset if the amount of pixels
+			   was explictly specified.  */
+			&& s->face->underline_pixels_above_descent_line))
+		    position = max (position, minimum_offset);
                 }
               /* Check the sanity of thickness and position.  We should
                  avoid drawing underline out of the current line area.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 3fd31b7f22..8064d47c94 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -6041,6 +6041,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
       face->underline = FACE_UNDER_LINE;
       face->underline_defaulted_p = true;
       face->underline_color = 0;
+      face->underline_at_descent_line_p = false;
+      face->underline_pixels_above_descent_line = 0;
     }
   else if (STRINGP (underline))
     {
@@ -6050,12 +6052,16 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
       face->underline_color
 	= load_color (f, face, underline,
 		      LFACE_UNDERLINE_INDEX);
+      face->underline_at_descent_line_p = false;
+      face->underline_pixels_above_descent_line = 0;
     }
   else if (NILP (underline))
     {
       face->underline = FACE_NO_UNDERLINE;
       face->underline_defaulted_p = false;
       face->underline_color = 0;
+      face->underline_at_descent_line_p = false;
+      face->underline_pixels_above_descent_line = 0;
     }
   else if (CONSP (underline))
     {
@@ -6064,6 +6070,8 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
       face->underline = FACE_UNDER_LINE;
       face->underline_color = 0;
       face->underline_defaulted_p = true;
+      face->underline_at_descent_line_p = false;
+      face->underline_pixels_above_descent_line = 0;
 
       /* FIXME?  This is also not robust about checking the precise form.
          See comments in Finternal_set_lisp_face_attribute.  */
@@ -6100,6 +6108,13 @@ realize_gui_face (struct face_cache *cache, Lisp_Object attrs[LFACE_VECTOR_SIZE]
               else if (EQ (value, Qwave))
                 face->underline = FACE_UNDER_WAVE;
             }
+	  else if (EQ (keyword, QCposition))
+	    {
+	      face->underline_at_descent_line_p = !NILP (value);
+
+	      if (FIXNATP (value))
+		face->underline_pixels_above_descent_line = XFIXNAT (value);
+	    }
         }
     }
 
@@ -6915,6 +6930,7 @@ syms_of_xfaces (void)
   DEFSYM (QCcolor, ":color");
   DEFSYM (QCline_width, ":line-width");
   DEFSYM (QCstyle, ":style");
+  DEFSYM (QCposition, ":position");
   DEFSYM (Qline, "line");
   DEFSYM (Qwave, "wave");
   DEFSYM (Qreleased_button, "released-button");
diff --git a/src/xterm.c b/src/xterm.c
index b284fdd312..717e946538 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -4131,8 +4131,12 @@ x_draw_glyph_string (struct glyph_string *s)
               unsigned long thickness, position;
               int y;
 
-              if (s->prev &&
-	          s->prev->face->underline == FACE_UNDER_LINE)
+              if (s->prev
+		  && s->prev->face->underline == FACE_UNDER_LINE
+		  && (s->prev->face->underline_at_descent_line_p
+		      == s->face->underline_at_descent_line_p)
+		  && (s->prev->face->underline_pixels_above_descent_line
+		      == s->face->underline_pixels_above_descent_line))
                 {
                   /* We use the same underline style as the previous one.  */
                   thickness = s->prev->underline_thickness;
@@ -4155,7 +4159,8 @@ x_draw_glyph_string (struct glyph_string *s)
 		  val = (WINDOW_BUFFER_LOCAL_VALUE
 			 (Qx_underline_at_descent_line, s->w));
 		  underline_at_descent_line
-		    = !(NILP (val) || EQ (val, Qunbound));
+		    = (!(NILP (val) || EQ (val, Qunbound))
+		       || s->face->underline_at_descent_line_p);
 
 		  val = (WINDOW_BUFFER_LOCAL_VALUE
 			 (Qx_use_underline_position_properties, s->w));
@@ -4168,7 +4173,9 @@ x_draw_glyph_string (struct glyph_string *s)
                   else
                     thickness = 1;
                   if (underline_at_descent_line)
-                    position = (s->height - thickness) - (s->ybase - s->y);
+		    position = ((s->height - thickness)
+				- (s->ybase - s->y)
+				- s->face->underline_pixels_above_descent_line);
                   else
                     {
                       /* Get the underline position.  This is the
@@ -4188,12 +4195,16 @@ x_draw_glyph_string (struct glyph_string *s)
                       else
                         position = minimum_offset;
                     }
-                  position = max (position, minimum_offset);
+
+		  /* Ignore minimum_offset if the amount of pixels was
+		     explictly specified.  */
+		  if (!s->face->underline_pixels_above_descent_line)
+		    position = max (position, minimum_offset);
                 }
               /* Check the sanity of thickness and position.  We should
                  avoid drawing underline out of the current line area.  */
-              if (s->y + s->height <= s->ybase + position)
-                position = (s->height - 1) - (s->ybase - s->y);
+	      if (s->y + s->height <= s->ybase + position)
+		position = (s->height - 1) - (s->ybase - s->y);
               if (s->y + s->height < s->ybase + position + thickness)
                 thickness = (s->y + s->height) - (s->ybase + position);
               s->underline_thickness = thickness;