[patch] add "underwave" face attribute

[patch] add "underwave" face attribute

[Aurélien Aptel]

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

Hi all,

I implemented a new face attribute "underwave". It does exactly what
this thread[1] asked for.
It only works on X11 currently. I've basically followed whatever
"overline" was doing across all the source tree so it should be ok. I
don't have a windows/mac machine to port this feature but it should be
easy to do: just port the changes made to xterm.c to other *term.c.

I've attached the diff and a screenshot of the splash screen with the
default and link face changed to use underwaving.
Note that it looks good on both X11 core fonts and Xft fonts.

I would love to see this merged in emacs so -- if everything is
alright -- what's the next step?

PS: if you receive this message twice you can ignore the other one, sorry.

1: http://thread.gmane.org/gmane.emacs.devel/147873/focus=147905

[-- Attachment #2: sample.png --]
[-- Type: image/png, Size: 53320 bytes --]

[-- Attachment #3: underwave.diff --]
[-- Type: text/x-patch, Size: 18217 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 381eaf6..1aa8712 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2089,6 +2089,10 @@ value @code{nil} means do not underline.
 Whether or not characters should be overlined, and in what color.
 The value is used like that of @code{:underline}.
 
+@item :underwave
+Whether or not characters should be underwaved, and in what color.
+The value is used like that of @code{:underline}.
+
 @item :strike-through
 Whether or not characters should be strike-through, and in what
 color.  The value is used like that of @code{:underline}.
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d725111..e7f2088 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -145,6 +145,13 @@
 	     (const :tag "On" t)
 	     (color :tag "Colored")))
 
+    (:underwave
+     (choice :tag "Underwave"
+	     :help-echo "Control text underwaving."
+	     (const :tag "Off" nil)
+	     (const :tag "On" t)
+	     (color :tag "Colored")))
+
     (:strike-through
      (choice :tag "Strike-through"
 	     :help-echo "Control text strike-through."
diff --git a/lisp/custom.el b/lisp/custom.el
index 132576a..deaaec5 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -368,7 +368,7 @@ ATTS is a list of face attributes followed by their values:
   (ATTR VALUE ATTR VALUE...)
 
 The possible attributes are `:family', `:width', `:height', `:weight',
-`:slant', `:underline', `:overline', `:strike-through', `:box',
+`:slant', `:underline', `:overline', `:underwave', `:strike-through', `:box',
 `:foreground', `:background', `:stipple', `:inverse-video', and `:inherit'.
 
 DISPLAY can be `default' (only in the first element), the symbol
diff --git a/lisp/faces.el b/lisp/faces.el
index 5d406ad..a3b8309 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -272,6 +272,7 @@ If FRAME is omitted or nil, use the selected frame."
     (:foreground (".attributeForeground" . "Face.AttributeForeground"))
     (:background (".attributeBackground" . "Face.AttributeBackground"))
     (:overline (".attributeOverline" . "Face.AttributeOverline"))
+    (:underwave (".attributeUnderwave" . "Face.AttributeUnderwave"))
     (:strike-through (".attributeStrikeThrough" . "Face.AttributeStrikeThrough"))
     (:box (".attributeBox" . "Face.AttributeBox"))
     (:underline (".attributeUnderline" . "Face.AttributeUnderline"))
@@ -628,6 +629,13 @@ VALUE is t, overline with foreground color of the face.  If VALUE is a
 string, overline with that color.  If VALUE is nil, explicitly don't
 overline.
 
+`:underwave'
+
+VALUE specifies whether characters in FACE should be underwaved.  If
+VALUE is t, underwave with foreground color of the face.  If VALUE is a
+string, underwave with that color.  If VALUE is nil, explicitly don't
+underwave.
+
 `:strike-through'
 
 VALUE specifies whether characters in FACE should be drawn with a line
@@ -992,7 +1000,7 @@ an integer value."
 	   (:inverse-video
 	    (mapcar #'(lambda (x) (cons (symbol-name x) x))
 		    (internal-lisp-face-attribute-values attribute)))
-           ((:underline :overline :strike-through :box)
+           ((:underline :overline :underwave :strike-through :box)
             (if (window-system frame)
                 (nconc (mapcar #'(lambda (x) (cons (symbol-name x) x))
                                (internal-lisp-face-attribute-values attribute))
@@ -1034,6 +1042,7 @@ an integer value."
     (:slant . "slant")
     (:underline . "underline")
     (:overline . "overline")
+    (:underwave . "underwave")
     (:strike-through . "strike-through")
     (:box . "box")
     (:inverse-video . "inverse-video display")
@@ -1323,6 +1332,7 @@ If FRAME is omitted or nil, use the selected frame."
 		  (:background . "Background")
 		  (:underline . "Underline")
 		  (:overline . "Overline")
+		  (:underwave . "Underwave")
 		  (:strike-through . "Strike-through")
 		  (:box . "Box")
 		  (:inverse-video . "Inverse")
diff --git a/src/dispextern.h b/src/dispextern.h
index 2c59f4f..7c93a11 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1484,6 +1484,7 @@ enum lface_attribute_index
   LFACE_BACKGROUND_INDEX,
   LFACE_STIPPLE_INDEX,
   LFACE_OVERLINE_INDEX,
+  LFACE_UNDERWAVE_INDEX,
   LFACE_STRIKE_THROUGH_INDEX,
   LFACE_BOX_INDEX,
   LFACE_FONT_INDEX,
@@ -1551,9 +1552,10 @@ struct face
   /* Pixel value or color index of underline color.  */
   unsigned long underline_color;
 
-  /* Pixel value or color index of overlined, strike-through, or box
-     color.  */
+  /* Pixel value or color index of overlined, underwaved,
+     strike-through, or box color.  */
   unsigned long overline_color;
+  unsigned long underwave_color;
   unsigned long strike_through_color;
   unsigned long box_color;
 
@@ -1586,9 +1588,10 @@ struct face
   unsigned use_box_color_for_shadows_p : 1;
 
   /* Non-zero if text in this face should be underlined, overlined,
-     strike-through or have a box drawn around it.  */
+     underwaved, strike-through or have a box drawn around it.  */
   unsigned underline_p : 1;
   unsigned overline_p : 1;
+  unsigned underwave_p : 1;
   unsigned strike_through_p : 1;
 
   /* 1 means that the colors specified for this face could not be
@@ -1606,6 +1609,7 @@ struct face
      attribute or that the specified color couldn't be loaded.
      Use the foreground color when drawing in that case. */
   unsigned overline_color_defaulted_p : 1;
+  unsigned underwave_color_defaulted_p : 1;
   unsigned strike_through_color_defaulted_p : 1;
   unsigned box_color_defaulted_p : 1;
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 617097d..4cfb061 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -53,10 +53,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    13. Whether or not characters should be strike-through, and in what
    color.
 
-   14. Whether or not a box should be drawn around characters, the box
+   14. Whether or not characters should be underwaved, and in what
+   color.
+
+   15. Whether or not a box should be drawn around characters, the box
    type, and, for simple boxes, in what color.
 
-   15. Font-spec, or nil.  This is a special attribute.
+   16. Font-spec, or nil.  This is a special attribute.
 
    A font-spec is a collection of font attributes (specs).
 
@@ -68,13 +71,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    On the other hand, if one of the other font-related attributes are
    specified, the corresponding specs in this attribute is set to nil.
 
-   15. A face name or list of face names from which to inherit attributes.
+   17. A face name or list of face names from which to inherit attributes.
 
-   16. A specified average font width, which is invisible from Lisp,
+   18. A specified average font width, which is invisible from Lisp,
    and is used to ensure that a font specified on the command line,
    for example, can be matched exactly.
 
-   17. A fontset name.  This is another special attribute.
+   19. A fontset name.  This is another special attribute.
 
    A fontset is a mappings from characters to font-specs, and the
    specs overwrite the font-spec in the 14th attribute.
@@ -313,7 +316,7 @@ Lisp_Object QCforeground, QCbackground;
 Lisp_Object QCwidth;
 static Lisp_Object QCfont, QCbold, QCitalic;
 static Lisp_Object QCreverse_video;
-static Lisp_Object QCoverline, QCstrike_through, QCbox, QCinherit;
+static Lisp_Object QCoverline, QCunderwave, QCstrike_through, QCbox, QCinherit;
 static Lisp_Object QCfontset;
 
 /* Symbols used for attribute values.  */
@@ -1311,11 +1314,11 @@ COLOR must be a valid color name.  */)
 /* Load color with name NAME for use by face FACE on frame F.
    TARGET_INDEX must be one of LFACE_FOREGROUND_INDEX,
    LFACE_BACKGROUND_INDEX, LFACE_UNDERLINE_INDEX, LFACE_OVERLINE_INDEX,
-   LFACE_STRIKE_THROUGH_INDEX, or LFACE_BOX_INDEX.  Value is the
-   pixel color.  If color cannot be loaded, display a message, and
-   return the foreground, background or underline color of F, but
-   record that fact in flags of the face so that we don't try to free
-   these colors.  */
+   LFACE_UNDERWAVE_INDEX, LFACE_STRIKE_THROUGH_INDEX, or LFACE_BOX_INDEX.
+   Value is the pixel color.  If color cannot be loaded, display a
+   message, and return the foreground, background or underline color
+   of F, but record that fact in flags of the face so that we don't
+   try to free these colors.  */
 
 unsigned long
 load_color (struct frame *f, struct face *face, Lisp_Object name,
@@ -1328,6 +1331,7 @@ load_color (struct frame *f, struct face *face, Lisp_Object name,
 	   || target_index == LFACE_BACKGROUND_INDEX
 	   || target_index == LFACE_UNDERLINE_INDEX
 	   || target_index == LFACE_OVERLINE_INDEX
+	   || target_index == LFACE_UNDERWAVE_INDEX
 	   || target_index == LFACE_STRIKE_THROUGH_INDEX
 	   || target_index == LFACE_BOX_INDEX);
 
@@ -1359,6 +1363,11 @@ load_color (struct frame *f, struct face *face, Lisp_Object name,
 	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
 	  break;
 
+	case LFACE_UNDERWAVE_INDEX:
+	  face->underwave_color_defaulted_p = 1;
+	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
+	  break;
+
 	case LFACE_STRIKE_THROUGH_INDEX:
 	  face->strike_through_color_defaulted_p = 1;
 	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
@@ -1477,6 +1486,13 @@ free_face_colors (struct frame *f, struct face *face)
       IF_DEBUG (--ncolors_allocated);
     }
 
+  if (face->underwave_p
+      && !face->underwave_color_defaulted_p)
+    {
+      x_free_colors (f, &face->underwave_color, 1);
+      IF_DEBUG (--ncolors_allocated);
+    }
+
   if (face->strike_through_p
       && !face->strike_through_color_defaulted_p)
     {
@@ -1842,6 +1858,7 @@ the WIDTH times as wide as FACE on FRAME.  */)
 #define LFACE_STIPPLE(LFACE)	    AREF ((LFACE), LFACE_STIPPLE_INDEX)
 #define LFACE_SWIDTH(LFACE)	    AREF ((LFACE), LFACE_SWIDTH_INDEX)
 #define LFACE_OVERLINE(LFACE)	    AREF ((LFACE), LFACE_OVERLINE_INDEX)
+#define LFACE_UNDERWAVE(LFACE)	    AREF ((LFACE), LFACE_UNDERWAVE_INDEX)
 #define LFACE_STRIKE_THROUGH(LFACE) AREF ((LFACE), LFACE_STRIKE_THROUGH_INDEX)
 #define LFACE_BOX(LFACE)	    AREF ((LFACE), LFACE_BOX_INDEX)
 #define LFACE_FONT(LFACE)	    AREF ((LFACE), LFACE_FONT_INDEX)
@@ -1894,6 +1911,10 @@ check_lface_attrs (Lisp_Object *attrs)
 	   || IGNORE_DEFFACE_P (attrs[LFACE_OVERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
 	   || STRINGP (attrs[LFACE_OVERLINE_INDEX]));
+  xassert (UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
+	   || IGNORE_DEFFACE_P (attrs[LFACE_UNDERWAVE_INDEX])
+	   || SYMBOLP (attrs[LFACE_UNDERWAVE_INDEX])
+	   || STRINGP (attrs[LFACE_UNDERWAVE_INDEX]));
   xassert (UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	   || SYMBOLP (attrs[LFACE_STRIKE_THROUGH_INDEX])
@@ -2534,6 +2555,15 @@ merge_face_ref (struct frame *f, Lisp_Object face_ref, Lisp_Object *to,
 		  else
 		    err = 1;
 		}
+	      else if (EQ (keyword, QCunderwave))
+		{
+		  if (EQ (value, Qt)
+		      || NILP (value)
+		      || STRINGP (value))
+		    to[LFACE_UNDERWAVE_INDEX] = value;
+		  else
+		    err = 1;
+		}
 	      else if (EQ (keyword, QCstrike_through))
 		{
 		  if (EQ (value, Qt)
@@ -2970,6 +3000,20 @@ FRAME 0 means change the face on all frames, and change the default
       old_value = LFACE_OVERLINE (lface);
       LFACE_OVERLINE (lface) = value;
     }
+  else if (EQ (attr, QCunderwave))
+    {
+      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
+	if ((SYMBOLP (value)
+	     && !EQ (value, Qt)
+	     && !EQ (value, Qnil))
+	    /* Underwave color.  */
+	    || (STRINGP (value)
+		&& SCHARS (value) == 0))
+	  signal_error ("Invalid face underwave", value);
+
+      old_value = LFACE_UNDERWAVE (lface);
+      LFACE_UNDERWAVE (lface) = value;
+    }
   else if (EQ (attr, QCstrike_through))
     {
       if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
@@ -3511,6 +3555,7 @@ DEFUN ("internal-set-lisp-face-attribute-from-resource",
     value = face_boolean_x_resource_value (value, 1);
   else if (EQ (attr, QCunderline)
 	   || EQ (attr, QCoverline)
+	   || EQ (attr, QCunderwave)
 	   || EQ (attr, QCstrike_through))
     {
       Lisp_Object boolean_value;
@@ -3720,6 +3765,8 @@ frames).  If FRAME is omitted or nil, use the selected frame.  */)
     value = LFACE_UNDERLINE (lface);
   else if (EQ (keyword, QCoverline))
     value = LFACE_OVERLINE (lface);
+  else if (EQ (keyword, QCunderwave))
+    value = LFACE_UNDERWAVE (lface);
   else if (EQ (keyword, QCstrike_through))
     value = LFACE_STRIKE_THROUGH (lface);
   else if (EQ (keyword, QCbox))
@@ -3766,6 +3813,8 @@ Value is nil if ATTR doesn't have a discrete set of valid values.  */)
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCoverline))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
+  else if (EQ (attr, QCunderwave))
+    result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCstrike_through))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCinverse_video) || EQ (attr, QCreverse_video))
@@ -4800,6 +4849,9 @@ x_supports_face_attributes_p (struct frame *f, Lisp_Object *attrs,
       || (!UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_OVERLINE_INDEX],
 				def_attrs[LFACE_OVERLINE_INDEX]))
+      || (!UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
+	  && face_attr_equal_p (attrs[LFACE_UNDERWAVE_INDEX],
+				def_attrs[LFACE_UNDERWAVE_INDEX]))
       || (!UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_STRIKE_THROUGH_INDEX],
 				def_attrs[LFACE_STRIKE_THROUGH_INDEX]))
@@ -4902,6 +4954,7 @@ tty_supports_face_attributes_p (struct frame *f, Lisp_Object *attrs,
       || !UNSPECIFIEDP (attrs[LFACE_HEIGHT_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_SWIDTH_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
+      || !UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_BOX_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_SLANT_INDEX]))
@@ -5370,6 +5423,9 @@ realize_default_face (struct frame *f)
   if (UNSPECIFIEDP (LFACE_OVERLINE (lface)))
     LFACE_OVERLINE (lface) = Qnil;
 
+  if (UNSPECIFIEDP (LFACE_UNDERWAVE (lface)))
+    LFACE_UNDERWAVE (lface) = Qnil;
+
   if (UNSPECIFIEDP (LFACE_STRIKE_THROUGH (lface)))
     LFACE_STRIKE_THROUGH (lface) = Qnil;
 
@@ -5563,7 +5619,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 #ifdef HAVE_WINDOW_SYSTEM
   struct face *default_face;
   struct frame *f;
-  Lisp_Object stipple, overline, strike_through, box;
+  Lisp_Object stipple, overline, underwave, strike_through, box;
 
   xassert (FRAME_WINDOW_P (cache->f));
 
@@ -5694,7 +5750,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 	}
     }
 
-  /* Text underline, overline, strike-through.  */
+  /* Text underline, overline, underwaved, strike-through.  */
 
   if (EQ (attrs[LFACE_UNDERLINE_INDEX], Qt))
     {
@@ -5734,6 +5790,21 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
       face->overline_p = 1;
     }
 
+  underwave = attrs[LFACE_UNDERWAVE_INDEX];
+  if (STRINGP (underwave))
+    {
+      face->underwave_color
+	= load_color (f, face, attrs[LFACE_UNDERWAVE_INDEX],
+		      LFACE_UNDERWAVE_INDEX);
+      face->underwave_p = 1;
+    }
+  else if (EQ (underwave, Qt))
+    {
+      face->underwave_color = face->foreground;
+      face->underwave_color_defaulted_p = 1;
+      face->underwave_p = 1;
+    }
+
   strike_through = attrs[LFACE_STRIKE_THROUGH_INDEX];
   if (STRINGP (strike_through))
     {
@@ -6457,6 +6528,7 @@ syms_of_xfaces (void)
   DEFSYM (QCbold, ":bold");
   DEFSYM (QCitalic, ":italic");
   DEFSYM (QCoverline, ":overline");
+  DEFSYM (QCunderwave, ":underwave");
   DEFSYM (QCstrike_through, ":strike-through");
   DEFSYM (QCbox, ":box");
   DEFSYM (QCinherit, ":inherit");
diff --git a/src/xterm.c b/src/xterm.c
index 4b34d63..2e9afb4 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2653,6 +2653,48 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = 1;
 }
 
+/*
+   Draw a wavy line. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+static void
+x_draw_underwave (Display *dpy, Window win, GC gc,
+                  unsigned long x0, unsigned long y0,
+                  unsigned long width,
+                  unsigned long wave_height, unsigned long wave_length)
+{
+  unsigned long dx = wave_length, dy = wave_height-1;
+  unsigned long x1, y1, x2, y2, i, times = width/dx;
+
+  for (i = 0; i < times; i++)
+    {
+      x1 = x0 + dx * i;
+      y1 = y0 + dy * (i%2);
+      x2 = x0 + dx * (i+1);
+      y2 = y0 + dy * ((i+1)%2);
+      XDrawLine(dpy, win, gc, x1, y1, x2, y2);
+    }
+
+  /* Draw remaining space */
+  if (x2 < x0+width)
+    {
+      unsigned long y = wave_height/(double)wave_length
+                        * (double)(x0 + width - x2);
+      x1 = x2;
+      y1 = y2;
+      x2 = x0 + width;
+      y2 = y0 + ((times % 2) ? wave_height - y : y);
+      XDrawLine(dpy, win, gc, x1, y1, x2, y2);
+    }
+}
+
 
 /* Draw glyph string S.  */
 
@@ -2836,6 +2878,25 @@ x_draw_glyph_string (struct glyph_string *s)
 	    }
 	}
 
+      /* Draw underwave.  */
+      if (s->face->underwave_p)
+	{
+          unsigned long h = 2, l = 3, y = s->ybase + 1;
+
+	  if (s->face->underwave_color_defaulted_p)
+	    x_draw_underwave (s->display, s->window, s->gc, s->x, y,
+                              s->width, h, l);
+	  else
+	    {
+	      XGCValues xgcv;
+	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+	      XSetForeground (s->display, s->gc, s->face->underwave_color);
+	      x_draw_underwave (s->display, s->window, s->gc, s->x, y,
+                                s->width, h, l);
+	      XSetForeground (s->display, s->gc, xgcv.foreground);
+	    }
+	}
+
       /* Draw strike-through.  */
       if (s->face->strike_through_p)
 	{

Re: [patch] add "underwave" face attribute

[Stefan Monnier]

> I would love to see this merged in Emacs so -- if everything is
> alright -- what's the next step?

The feature sounds good, I'll let others judge the implementation
quality, but I have one issue with the "user interface": do we really
want to let faces specify underline and underwave separately?
Maybe another approach is to include it as an additional value
for underline (which can currently be either a boolean or a color name,
so we'd have to extend the set of value so as to be able to specify
shape and color separately).


        Stefan

RE: [patch] add "underwave" face attribute

[Drew Adams]

> The feature sounds good, I'll let others judge the implementation
> quality, but I have one issue with the "user interface": do we really
> want to let faces specify underline and underwave separately?

No.

> Maybe another approach is to include it as an additional value
> for underline (which can currently be either a boolean or a 
> color name, so we'd have to extend the set of value so as to be
> able to specify shape and color separately).

+1

(And please implement it on other platforms as well.)

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

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

Ok so I've taken into account what has been said on the list. Here's
another attempt.

For a quick glance at the result, apply my patch, compile and run.
$ ./emacs -Q --eval "(set-face-attribute 'default nil :underline
'(:color \"red\" :style wave))

It works on X11, see below for the rest.

Summary of the changes
======================

The :underline attribute has now a new way to be set:

(:color color :style style)
    If `color' is a string, underline in it.
    If `color' is `foreground-color', underline with the
    foreground color of the face.

    If `style' is `wave' underline with a wave.
    If `style' is `line' underline with a line.

    If the attribute :color is omited, underline with the
    foreground color of the face.
    If the attribute :style is omited, underline with a line.

All the previous ways to set :underline behave the same.

Customize menu
--------------

The customize menu now looks like this:

  Underline: [Off/On]
    Color: [Foreground color/...]
    Style: [Line/Wave]

Implementation
--------------

-- src/dispextern.h --

Add a new enum for the underlining type.

  enum face_underline_type {
       FACE_UNDER_LINE,
       FACE_UNDER_WAVE,
  };

And a new field in struct face.

  struct face {
         ...
         enum face_underline_type underline_type;
         ...
  };

-- src/xterm.c --

Update X11 backend.
Add x_draw_underwave()
The code seems to use unsigned long for pixel offsets so that's what I used.
The wave is drown correctly now. No more wave breaks in C-h h.

   static void
   x_draw_underwave (Display *dpy, Window win, GC gc,
                     unsigned long x0, unsigned long y0,
                     unsigned long width,
                     unsigned long wave_height, unsigned long wave_length)

Add a new codepath in x_draw_glyph_string() to handle the new style.

  /* Draw underline.  */
  if (s->face->underline_p)
    {
      if (s->face->underline_type == FACE_UNDER_WAVE)
        {
           ...new code...
        }
      else if (s->face->underline_type == FACE_UNDER_LINE)
        {
            ...re-indented old code...
        }

-- src/w32term.m --
-- src/nsterm.c --

================================================================
 I do not have access to a Window/Mac OSX system.
 Someone needs to test and complete this.
 I don't event know if it compiles.
================================================================

First attempt at porting to Windows and Mac OSX.
Same change made to src/xterm.c, basically.
Just replaced line drawing primitive by the system one.

-- src/xfaces.c --

Add List_Object for symbol `line' and `wave'.
Reuse Qforeground_color, QCstyle, QCcolor.
Change checks made to :underline value to accept CONS construct.
Handle CONS construct when updating struct face attribute.

-- lisp/faces.el --
-- doc/lispref/display.texi --

Update :underline documentation

-- lisp/cus-face.el --

Update customize menu for :underline.

(:underline
 (choice :tag "Underline"
         :help-echo "Control text underlining."
         (const :tag "Off" nil)
         (list :tag "On"
    	   (const :format "" :value :color)
    	   (choice :tag "Color" (const :tag "Foreground Color"
foreground-color) color)
               (const :format "" :value :style)
               (choice :tag "Style"
                       (const :tag "Line" line)
                       (const :tag "Wave" wave)))))

[-- Attachment #2: underwave.patch --]
[-- Type: text/x-diff, Size: 31468 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 381eaf6..464dfaa 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2080,10 +2080,32 @@ Background color, a string.  The value can be a system-defined color
 name, or a hexadecimal color specification.  @xref{Color Names}.
 
 @item :underline
-Whether or not characters should be underlined, and in what color.  If
-the value is @code{t}, underlining uses the foreground color of the
-face.  If the value is a string, underlining uses that color.  The
-value @code{nil} means do not underline.
+Whether or not characters should be underlined, and in what
+color. Here are the possible values of the @code{:underline}
+attribute, and what they mean:
+
+@table @asis
+@item @code{nil}
+Don't underline.
+
+@item @code{t}
+Underline with the foreground color of the face.
+
+@item @var{color}
+Underline in color @var{color}.
+
+@item @code{(:color @var{color} :style @var{style})}
+If @var{color} is a string, underline in it.
+If @var{color} is @code{foreground-color}, underline with the
+foreground color of the face.
+
+If @var{style} is @code{wave} underline with a wave.
+If @var{style} is @code{line} underline with a line. 
+
+If the attribute @code{:color} is omited, underline with the
+foreground color of the face.
+If the attribute @code{:style} is omited, underline with a line.
+@end table
 
 @item :overline
 Whether or not characters should be overlined, and in what color.
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d725111..3680a26 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -135,8 +135,13 @@
      (choice :tag "Underline"
 	     :help-echo "Control text underlining."
 	     (const :tag "Off" nil)
-	     (const :tag "On" t)
-	     (color :tag "Colored")))
+	     (list :tag "On"
+		   (const :format "" :value :color)
+		   (choice :tag "Color" (const :tag "Foreground Color" foreground-color) color)
+                   (const :format "" :value :style)
+                   (choice :tag "Style"
+                           (const :tag "Line" line)
+                           (const :tag "Wave" wave)))))
 
     (:overline
      (choice :tag "Overline"
diff --git a/lisp/faces.el b/lisp/faces.el
index 5d406ad..e68abb1 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -616,10 +616,21 @@ VALUE must be a color name, a string.
 
 `:underline'
 
-VALUE specifies whether characters in FACE should be underlined.  If
-VALUE is t, underline with foreground color of the face.  If VALUE is
-a string, underline with that color.  If VALUE is nil, explicitly
-don't underline.
+VALUE specifies whether characters in FACE should be underlined.
+If VALUE is t, underline with foreground color of the face.
+If VALUE is a string, underline with that color.
+If VALUE is nil, explicitly don't underline.
+
+Otherwise, VALUE must be a property list of the form:
+
+`(:color COLOR :style STYLE)'.
+
+COLOR can be a either a color name string or `foreground-color'.
+STYLE can be either `line' or `wave'.
+If a keyword/value pair is missing from the property list, a
+default value will be used for the value.
+The default value of COLOR is the foreground color of the face.
+The default value of STYLE is `line'.
 
 `:overline'
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 2c59f4f..cdbbc16 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1510,6 +1510,13 @@ enum face_box_type
   FACE_SUNKEN_BOX
 };
 
+/* Underline type. */
+
+enum face_underline_type
+{
+  FACE_UNDER_LINE,
+  FACE_UNDER_WAVE
+};
 
 /* Structure describing a realized face.
 
@@ -1585,6 +1592,9 @@ struct face
      drawing shadows.  */
   unsigned use_box_color_for_shadows_p : 1;
 
+  /* Style of underlining. */
+  enum face_underline_type underline_type;
+
   /* Non-zero if text in this face should be underlined, overlined,
      strike-through or have a box drawn around it.  */
   unsigned underline_p : 1;
diff --git a/src/nsterm.m b/src/nsterm.m
index 70d3cc0..657072a 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2596,6 +2596,55 @@ ns_get_glyph_string_clip_rect (struct glyph_string *s, NativeRectangle *nr)
   return n;
 }
 
+/* -------------------------------------------------------------
+   Draw a wavy line. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+  -------------------------------------------------------------- */
+
+static void
+ns_draw_underwave (int x0, int y0,
+                   int width,
+                   int wave_height, int wave_length)
+{
+  int dx = wave_length, dy = wave_height-1;
+  int x1, y1, x2, y2;
+  double coef = dy/(double)dx;
+  NSPoint a, b;
+
+  /* Set the phase */
+  x1 = x0;
+  x2 = x0 + dx - (x0 % dx);
+
+  while (x1 < x0+width)
+    {
+      int odd = (x1/dx) % 2;
+      
+      if (odd)
+        {
+          y1 = y0 + (dy - coef * (x1 % dx));
+          y2 = y0 + coef * (x2 % dx);
+        } 
+      else
+        {
+          y1 = y0 + coef * (x1 % dx);
+          y2 = y0 + (dy - coef * (x2 % dx));
+        }
+
+      a.x = x1, a.y = y1;
+      b.x = x2, b.y = y2;
+      [NSBezierPath strokeLineFromPoint:a toPoint: b];
+
+      x1 = x2;
+      x2 += dx;
+    }
+}
+
+
 void
 ns_draw_text_decoration (struct glyph_string *s, struct face *face,
                          NSColor *defaultCol, CGFloat width, CGFloat x)
@@ -2609,63 +2658,77 @@ ns_draw_text_decoration (struct glyph_string *s, struct face *face,
   /* Do underline. */
   if (face->underline_p)
     {
-      NSRect r;
-      unsigned long thickness, position;
-
-      /* If the prev was underlined, match its appearance. */
-      if (s->prev && s->prev->face->underline_p
-          && s->prev->underline_thickness > 0)
+      if (s->face->underline_type == FACE_UNDER_WAVE)
         {
-          thickness = s->prev->underline_thickness;
-          position = s->prev->underline_position;
+          unsigned long height = 2, len = 3, y = s->ybase + 1;
+
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          
+          ns_draw_underwave (s->x, y, s->width, height, len);
         }
-      else
+      else if (s->face->underline_type == FACE_UNDER_LINE)
         {
-          struct font *font;
-          unsigned long descent;
-
-          font=s->font;
-          descent = s->y + s->height - s->ybase;
 
-          /* Use underline thickness of font, defaulting to 1. */
-          thickness = (font && font->underline_thickness > 0)
-            ? font->underline_thickness : 1;
+          NSRect r;
+          unsigned long thickness, position;
 
-          /* Determine the offset of underlining from the baseline. */
-          if (x_underline_at_descent_line)
-            position = descent - thickness;
-          else if (x_use_underline_position_properties
-                   && font && font->underline_position >= 0)
-            position = font->underline_position;
-          else if (font)
-            position = lround (font->descent / 2);
+          /* If the prev was underlined, match its appearance. */
+          if (s->prev && s->prev->face->underline_p
+              && s->prev->underline_thickness > 0)
+            {
+              thickness = s->prev->underline_thickness;
+              position = s->prev->underline_position;
+            }
           else
-            position = underline_minimum_offset;
+            {
+              struct font *font;
+              unsigned long descent;
 
-          position = max (position, underline_minimum_offset);
+              font=s->font;
+              descent = s->y + s->height - s->ybase;
 
-          /* Ensure underlining is not cropped. */
-          if (descent <= position)
-            {
-              position = descent - 1;
-              thickness = 1;
+              /* Use underline thickness of font, defaulting to 1. */
+              thickness = (font && font->underline_thickness > 0)
+                ? font->underline_thickness : 1;
+
+              /* Determine the offset of underlining from the baseline. */
+              if (x_underline_at_descent_line)
+                position = descent - thickness;
+              else if (x_use_underline_position_properties
+                       && font && font->underline_position >= 0)
+                position = font->underline_position;
+              else if (font)
+                position = lround (font->descent / 2);
+              else
+                position = underline_minimum_offset;
+
+              position = max (position, underline_minimum_offset);
+
+              /* Ensure underlining is not cropped. */
+              if (descent <= position)
+                {
+                  position = descent - 1;
+                  thickness = 1;
+                }
+              else if (descent < position + thickness)
+                thickness = 1;
             }
-          else if (descent < position + thickness)
-            thickness = 1;
-        }
 
-      s->underline_thickness = thickness;
-      s->underline_position = position;
+          s->underline_thickness = thickness;
+          s->underline_position = position;
 
-      r = NSMakeRect (x, s->ybase + position, width, thickness);
+          r = NSMakeRect (x, s->ybase + position, width, thickness);
 
-      if (face->underline_defaulted_p)
-        [defaultCol set];
-      else
-        [ns_lookup_indexed_color (face->underline_color, s->f) set];
-      NSRectFill (r);
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          NSRectFill (r);
+        }
     }
-
   /* Do overline. We follow other terms in using a thickness of 1
      and ignoring overline_margin. */
   if (face->overline_p)
diff --git a/src/w32term.c b/src/w32term.c
index f764ad9..27aa415 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -309,6 +309,60 @@ w32_set_clip_rectangle (HDC hdc, RECT *rect)
     SelectClipRgn (hdc, NULL);
 }
 
+/*
+   Draw a wavy line. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+void
+w32_draw_underwave (HDC hdc, COLORREF *color,
+                    int x0, int y0,
+                    int width,
+                    int wave_height, int wave_length)
+{
+  HPEN hp, oldhp;
+  int dx = wave_length, dy = wave_height-1;
+  int x1, y1, x2, y2;
+  double coef = dy/(double)dx;
+
+  hp = CreatePen (PS_SOLID, 0, color);
+  oldhb = SelectObject (hdc, hb);
+  oldhp = SelectObject (hdc, hp);
+
+  /* Set the phase */
+  x1 = x0;
+  x2 = x0 + dx - (x0 % dx);
+  MoveToEx (hdc, x1, y2, NULL);
+
+  while (x1 < x0+width)
+    {
+      int odd = (x1/dx) % 2;
+      
+      if (odd)
+        {
+          y1 = y0 + (dy - coef * (x1 % dx));
+          y2 = y0 + coef * (x2 % dx);
+        } 
+      else
+        {
+          y1 = y0 + coef * (x1 % dx);
+          y2 = y0 + (dy - coef * (x2 % dx));
+        }
+      
+      LineTo (hdc, x2, y2);
+      x1 = x2;
+      x2 += dx;
+    }
+
+  SelectObject (hdc, oldhp);
+  DeleteObject (hp);
+}
 
 /* Draw a hollow rectangle at the specified position.  */
 void
@@ -2343,60 +2397,76 @@ x_draw_glyph_string (struct glyph_string *s)
       /* Draw underline.  */
       if (s->face->underline_p)
         {
-          unsigned long thickness, position;
-          int y;
-
-          if (s->prev && s->prev->face->underline_p)
+	  if (s->face->underline_type == FACE_UNDER_WAVE)
             {
-              /* We use the same underline style as the previous one.  */
-              thickness = s->prev->underline_thickness;
-              position = s->prev->underline_position;
+              unsigned long height = 2, len = 3, y = s->ybase + 1;
+              COLORREF color;
+
+              if (s->face->underline_defaulted_p)
+                color = s->gc->foreground;
+              else
+                color = s->face->underline_color;
+
+              w32_draw_underwave (s->hdc, color, 
+                                  s->x, y, s->width, height, len);
             }
-          else
+          else if (s->face->underline_type == FACE_UNDER_LINE)
             {
-              /* Get the underline thickness.  Default is 1 pixel.  */
-              if (s->font && s->font->underline_thickness > 0)
-                thickness = s->font->underline_thickness;
-              else
-                thickness = 1;
-              if (x_underline_at_descent_line)
-                position = (s->height - thickness) - (s->ybase - s->y);
+              unsigned long thickness, position;
+              int y;
+
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
               else
                 {
-                /* Get the underline position.  This is the recommended
-                   vertical offset in pixels from the baseline to the top of
-                   the underline.  This is a signed value according to the
-                   specs, and its default is
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
 
-                   ROUND ((maximum_descent) / 2), with
-                   ROUND (x) = floor (x + 0.5)  */
+                         ROUND ((maximum_descent) / 2), with
+                         ROUND (x) = floor (x + 0.5)  */
 
-                if (x_use_underline_position_properties
-                    && s->font && s->font->underline_position >= 0)
-                  position = s->font->underline_position;
-                else if (s->font)
-                  position = (s->font->descent + 1) / 2;
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position =position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                {
+                  w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                                 y, s->width, 1);
+                }
+              else
+                {
+                  w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
+                                 y, s->width, 1);
                 }
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position =position;
-          y = s->ybase + position;
-          if (s->face->underline_defaulted_p)
-            {
-              w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-                             y, s->width, 1);
-            }
-          else
-            {
-              w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
-                             y, s->width, 1);
             }
         }
       /* Draw overline.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 617097d..8bee2d6 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -320,6 +320,7 @@ static Lisp_Object QCfontset;
 
 Lisp_Object Qnormal;
 Lisp_Object Qbold;
+static Lisp_Object Qline, Qwave;
 static Lisp_Object Qultra_light, Qextra_light, Qlight;
 static Lisp_Object Qsemi_light, Qsemi_bold, Qextra_bold, Qultra_bold;
 static Lisp_Object Qoblique, Qreverse_oblique, Qreverse_italic;
@@ -1889,7 +1890,8 @@ check_lface_attrs (Lisp_Object *attrs)
   xassert (UNSPECIFIEDP (attrs[LFACE_UNDERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_UNDERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_UNDERLINE_INDEX])
-	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX]));
+	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX])
+	   || CONSP (attrs[LFACE_UNDERLINE_INDEX]));
   xassert (UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_OVERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
@@ -2520,7 +2522,8 @@ merge_face_ref (struct frame *f, Lisp_Object face_ref, Lisp_Object *to,
 		{
 		  if (EQ (value, Qt)
 		      || NILP (value)
-		      || STRINGP (value))
+		      || STRINGP (value)
+		      || CONSP (value))
 		    to[LFACE_UNDERLINE_INDEX] = value;
 		  else
 		    err = 1;
@@ -2944,15 +2947,54 @@ FRAME 0 means change the face on all frames, and change the default
     }
   else if (EQ (attr, QCunderline))
     {
-      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
-	if ((SYMBOLP (value)
-	     && !EQ (value, Qt)
-	     && !EQ (value, Qnil))
-	    /* Underline color.  */
-	    || (STRINGP (value)
-		&& SCHARS (value) == 0))
-	  signal_error ("Invalid face underline", value);
+      int valid_p = 0;
+      
+      if (UNSPECIFIEDP (value) || IGNORE_DEFFACE_P (value))
+	valid_p = 1;
+      else if (NILP (value) || EQ (value, Qt))
+        valid_p = 1;
+      else if (STRINGP (value) && SCHARS (value) > 0)
+        valid_p = 1;
+      else if (CONSP (value))
+        {
+          Lisp_Object key, val, list;
 
+          list = value;
+          valid_p = 1;
+
+          while (!NILP (CAR_SAFE(list)))
+            {
+              key = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+              val = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+            
+              if(NILP (key) || NILP (val)) 
+                {
+                  valid_p = 0;
+                  break;
+                }
+
+              else if (EQ (key, QCcolor)
+                       && !(EQ (val, Qforeground_color)
+                            || (STRINGP (val) && SCHARS (val) > 0)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+              
+              else if (EQ (key, QCstyle) 
+                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+            }
+        }
+      
+      if (!valid_p)
+        signal_error ("Invalid face underline", value);
+      
       old_value = LFACE_UNDERLINE (lface);
       LFACE_UNDERLINE (lface) = value;
     }
@@ -3762,6 +3804,7 @@ Value is nil if ATTR doesn't have a discrete set of valid values.  */)
 
   CHECK_SYMBOL (attr);
 
+  /* XXX: no check for QCbox? */
   if (EQ (attr, QCunderline))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCoverline))
@@ -5563,7 +5606,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 #ifdef HAVE_WINDOW_SYSTEM
   struct face *default_face;
   struct frame *f;
-  Lisp_Object stipple, overline, strike_through, box;
+  Lisp_Object stipple, underline, overline, strike_through, box;
 
   xassert (FRAME_WINDOW_P (cache->f));
 
@@ -5696,29 +5739,76 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 
   /* Text underline, overline, strike-through.  */
 
-  if (EQ (attrs[LFACE_UNDERLINE_INDEX], Qt))
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (EQ (underline, Qt))
     {
       /* Use default color (same as foreground color).  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 1;
       face->underline_color = 0;
     }
-  else if (STRINGP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (STRINGP (underline))
     {
       /* Use specified color.  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 0;
       face->underline_color
-	= load_color (f, face, attrs[LFACE_UNDERLINE_INDEX],
+	= load_color (f, face, underline,
 		      LFACE_UNDERLINE_INDEX);
     }
-  else if (NILP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (NILP (underline))
     {
       face->underline_p = 0;
       face->underline_defaulted_p = 0;
       face->underline_color = 0;
     }
+  else if (CONSP (underline))
+    {
+      /* `(:color COLOR :style STYLE)'.  
+         STYLE being one of `line' or `wave'. */
+      face->underline_p = 1;
+      face->underline_color = 0;
+      face->underline_defaulted_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
+
+      while (CONSP (underline))
+        {
+          Lisp_Object keyword, value;
+
+          keyword = XCAR (underline);
+          underline = XCDR (underline);
 
+          if (!CONSP (underline))
+            break;
+          value = XCAR (underline);
+          underline = XCDR (underline);
+
+          if (EQ (keyword, QCcolor))
+            {
+              if (EQ (value, Qforeground_color))
+                {
+                  face->underline_defaulted_p = 1;
+                  face->underline_color = 0;
+                }
+              else if (STRINGP (value))
+                {
+                  face->underline_defaulted_p = 0;
+                  face->underline_color = load_color (f, face, value,
+                                                      LFACE_UNDERLINE_INDEX);
+                }
+            }
+          else if (EQ (keyword, QCstyle))
+            {
+              if (EQ (value, Qline))
+                face->underline_type = FACE_UNDER_LINE;
+              else if (EQ (value, Qwave))
+                face->underline_type = FACE_UNDER_WAVE;
+            }
+        }
+    }
+  
   overline = attrs[LFACE_OVERLINE_INDEX];
   if (STRINGP (overline))
     {
@@ -6465,6 +6555,8 @@ syms_of_xfaces (void)
   DEFSYM (QCcolor, ":color");
   DEFSYM (QCline_width, ":line-width");
   DEFSYM (QCstyle, ":style");
+  DEFSYM (Qline, "line");
+  DEFSYM (Qwave, "wave");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qnormal, "normal");
diff --git a/src/xterm.c b/src/xterm.c
index 4b34d63..bd2597c 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2653,6 +2653,52 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = 1;
 }
 
+/*
+   Draw a wavy line. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+static void
+x_draw_underwave (Display *dpy, Window win, GC gc,
+                  unsigned long x0, unsigned long y0,
+                  unsigned long width,
+                  unsigned long wave_height, unsigned long wave_length)
+{
+  unsigned long dx = wave_length, dy = wave_height-1;
+  unsigned long x1, y1, x2, y2;
+  double coef = dy/(double)dx;
+
+  /* Set the phase */
+  x1 = x0;
+  x2 = x0 + dx - (x0 % dx);
+
+  while (x1 < x0+width)
+    {
+      int odd = (x1/dx) % 2;
+      
+      if (odd)
+        {
+          y1 = y0 + (dy - coef * (x1 % dx));
+          y2 = y0 + coef * (x2 % dx);
+        } 
+      else
+        {
+          y1 = y0 + coef * (x1 % dx);
+          y2 = y0 + (dy - coef * (x2 % dx));
+        }
+
+      XDrawLine (dpy, win, gc, x1, y1, x2, y2);
+      x1 = x2;
+      x2 += dx;
+    }
+}
+
 
 /* Draw glyph string S.  */
 
@@ -2756,67 +2802,86 @@ x_draw_glyph_string (struct glyph_string *s)
       /* Draw underline.  */
       if (s->face->underline_p)
 	{
-	  unsigned long thickness, position;
-	  int y;
-
-	  if (s->prev && s->prev->face->underline_p)
-	    {
-	      /* We use the same underline style as the previous one.  */
-	      thickness = s->prev->underline_thickness;
-	      position = s->prev->underline_position;
-	    }
-	  else
-	    {
-	      /* Get the underline thickness.  Default is 1 pixel.  */
-	      if (s->font && s->font->underline_thickness > 0)
-		thickness = s->font->underline_thickness;
-	      else
-		thickness = 1;
-	      if (x_underline_at_descent_line)
-		position = (s->height - thickness) - (s->ybase - s->y);
-	      else
-		{
-		  /* Get the underline position.  This is the recommended
-		     vertical offset in pixels from the baseline to the top of
-		     the underline.  This is a signed value according to the
-		     specs, and its default is
+	  if (s->face->underline_type == FACE_UNDER_WAVE)
+            {
+              unsigned long height = 2, len = 3, y = s->ybase + 1;
 
-		     ROUND ((maximum descent) / 2), with
-		     ROUND(x) = floor (x + 0.5)  */
+              if (s->face->underline_defaulted_p)
+                x_draw_underwave (s->display, s->window, s->gc,
+                                  s->x, y, s->width, height, len);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  x_draw_underwave (s->display, s->window, s->gc,
+                                    s->x, y, s->width, height, len);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+          else if (s->face->underline_type == FACE_UNDER_LINE)
+            {
+              unsigned long thickness, position;
+              int y;
+          
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
+              else
+                {
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
 
-		  if (x_use_underline_position_properties
-		      && s->font && s->font->underline_position >= 0)
-		    position = s->font->underline_position;
-		  else if (s->font)
-		    position = (s->font->descent + 1) / 2;
-		  else
-		    position = underline_minimum_offset;
-		}
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position = position;
-	  y = s->ybase + position;
-	  if (s->face->underline_defaulted_p)
-	    XFillRectangle (s->display, s->window, s->gc,
-			    s->x, y, s->width, thickness);
-	  else
-	    {
-	      XGCValues xgcv;
-	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-	      XSetForeground (s->display, s->gc, s->face->underline_color);
-	      XFillRectangle (s->display, s->window, s->gc,
-			      s->x, y, s->width, thickness);
-	      XSetForeground (s->display, s->gc, xgcv.foreground);
-	    }
-	}
+                         ROUND ((maximum descent) / 2), with
+                         ROUND(x) = floor (x + 0.5)  */
 
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                      else
+                        position = underline_minimum_offset;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position = position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                XFillRectangle (s->display, s->window, s->gc,
+                                s->x, y, s->width, thickness);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  XFillRectangle (s->display, s->window, s->gc,
+                                  s->x, y, s->width, thickness);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+        }
       /* Draw overline.  */
       if (s->face->overline_p)
 	{

Re: [patch] add "underwave" face attribute

[Leo]

On 2012-02-01 07:38 +0800, Aurélien Aptel wrote:
> The :underline attribute has now a new way to be set:
>
> (:color color :style style)
>     If `color' is a string, underline in it.
>     If `color' is `foreground-color', underline with the
>     foreground color of the face.
>
>     If `style' is `wave' underline with a wave.
>     If `style' is `line' underline with a line.
>
>     If the attribute :color is omited, underline with the
>     foreground color of the face.
>     If the attribute :style is omited, underline with a line.

This seems to take shape. Thank you for the work!

Leo

Re: [patch] add "underwave" face attribute

[Juri Linkov]

> (:underline
>  (choice :tag "Underline"
>          :help-echo "Control text underlining."
>          (const :tag "Off" nil)
>          (list :tag "On"
>     	   (const :format "" :value :color)
>     	   (choice :tag "Color" (const :tag "Foreground Color" foreground-color) color)
>                (const :format "" :value :style)
>                (choice :tag "Style"
>                        (const :tag "Line" line)
>                        (const :tag "Wave" wave)))))

Very nice, we could use it for `flyspell', `flymake', `next-error'
to highlight errors, etc.

But what to do with platforms where it's not yet supported?
There should be a way to detect its availability
and to fall back to other face attributes.

Currently `:underline' can test its support with `(supports :underline t)'.

Maybe `underwave' should do this with something like:

(defface underwave
  '((((supports :underline wave))
     :underline (:style wave :color "red"))
    (t
     :underline (:style line :color "red")))
  "Basic underwave face."
  :group 'basic-faces)

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

On Thu, Feb 2, 2012 at 12:11 AM, Juri Linkov <juri@jurta.org> wrote:
> Very nice, we could use it for `flyspell', `flymake', `next-error'
> to highlight errors, etc.

Indeed :)

> But what to do with platforms where it's not yet supported?

On a tty frame underwave underlines.
As for other platforms, I've tried to write some code in the patch,
but I can't compile or test it.
Someone who can build emacs on windows/mac osx needs to try it and report back

> There should be a way to detect its availability
> and to fall back to other face attributes.
>
> Currently `:underline' can test its support with `(supports :underline t)'.
>
> Maybe `underwave' should do this with something like:
>
> (defface underwave
>  '((((supports :underline wave))
>     :underline (:style wave :color "red"))
>    (t
>     :underline (:style line :color "red")))
>  "Basic underwave face."
>  :group 'basic-faces)

Maybe. I'm not too familiar with this face support hierarchy stuff.
I'll have a look.

Re: [patch] add "underwave" face attribute

[Juanma Barranquero]

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

On Thu, Feb 2, 2012 at 01:18, Aurélien Aptel
<aurelien.aptel+emacs@gmail.com> wrote:

> Someone who can build emacs on windows/mac osx needs to try it and report back

On Windows,

+void
+w32_draw_underwave (HDC hdc, COLORREF *color,

that should be COLORREF, not COLORREF *.

+                    int x0, int y0,
+                    int width,
+                    int wave_height, int wave_length)
+{
+  HPEN hp, oldhp;
+  int dx = wave_length, dy = wave_height-1;
+  int x1, y1, x2, y2;
+  double coef = dy/(double)dx;
+
+  hp = CreatePen (PS_SOLID, 0, color);
+  oldhb = SelectObject (hdc, hb);

oldhb and hb are not declared, and this line is not needed, I think.

Other than that, the patch works but there's still a few problems. See
attached image, after M-: (set-face-attribute 'default nil :underline
'(:color "red" :style wave))

1) The wave is drawn past the characters. Note that each row in the
image seems to have extra space.
2) Moving the cursor leaves some screen artifacts (these are the
vertical lines at the end of some rows of text).

    Juanma

[-- Attachment #2: bug.png --]
[-- Type: image/png, Size: 95518 bytes --]

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

Time for some updates.

I've fixed most of the bugs related to X11/Windows but I think I'll
have to change the approach to underwave.

Drawing chunk of waves (when width of the underwave < width of one
wave) often ends up with (little) artifacts due to rounding problem.
To have a perfect wave no matter the position or size I should clip
the region of the underwave and only draw full waves, letting the
clipping do the work.

On X11, I know how to change the clipping region but I don't know how
to restore it. The clipping region is stored in the GC but it *can't*
be requested with XGetGCValues() [1]. I could just remove the clipping
region altogether when I'm done it but I'm pretty sure this will have
some side effects. I've asked about it on stackoverflow[2] but I
haven't got any satisfying answer yet.

On Windows, I've looked at the GDI documentation[3] (which is what
w32term.c currently use) on MSDN and it looks doable.

On Mac OSX, I still don't know if my patch compile/works (could
someone please report back?). I haven't looked at the clipping stuff
either. I was hoping someone who actually knows what he's doing in
Objective C/COCOA could help me.

1: http://tronche.com/gui/x/xlib/GC/XGetGCValues.html
2: http://stackoverflow.com/questions/9037676/how-can-i-limit-the-surface-in-which-xlib-graphics-primitives-draw
3: http://msdn.microsoft.com/en-us/library/dd145203%28v=vs.85%29.aspx

Re: [patch] add "underwave" face attribute

[Jan Djärv]

3 feb 2012 kl. 20:37 skrev Aurélien Aptel:

> On X11, I know how to change the clipping region but I don't know how
> to restore it. The clipping region is stored in the GC but it *can't*
> be requested with XGetGCValues() [1]. I could just remove the clipping
> region altogether when I'm done it but I'm pretty sure this will have
> some side effects. I've asked about it on stackoverflow[2] but I
> haven't got any satisfying answer yet.


Just create a new GC.

	Jan D.

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

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

On Sat, Feb 4, 2012 at 9:14 AM, Jan Djärv <jan.h.d@swipnet.se> wrote:
> Just create a new GC.

That's what I ended up doing, but the previous GC already sets a
clipping region that I override when underwaving. This results in
unneeded underwaving ie. visual glitches.

I've looked deeply at the code and found a function that returns a
simple clip rectangle of a glyph_string and used it with
x_intersect_rectangles() to have the final clipping region. I then use
the clip field of glyph_string to restore the previous clipping
rectangles.

I've done the same thing for Windows, it should work. Juanma, could
you check again, please?
Mac OSX clipping works differently and I'm not sure what to do... See
XXX coments in nsterm.m

Patch attached.

[-- Attachment #2: underwave-clip-1.patch --]
[-- Type: text/x-patch, Size: 33316 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 381eaf6..464dfaa 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2080,10 +2080,32 @@ Background color, a string.  The value can be a system-defined color
 name, or a hexadecimal color specification.  @xref{Color Names}.
 
 @item :underline
-Whether or not characters should be underlined, and in what color.  If
-the value is @code{t}, underlining uses the foreground color of the
-face.  If the value is a string, underlining uses that color.  The
-value @code{nil} means do not underline.
+Whether or not characters should be underlined, and in what
+color. Here are the possible values of the @code{:underline}
+attribute, and what they mean:
+
+@table @asis
+@item @code{nil}
+Don't underline.
+
+@item @code{t}
+Underline with the foreground color of the face.
+
+@item @var{color}
+Underline in color @var{color}.
+
+@item @code{(:color @var{color} :style @var{style})}
+If @var{color} is a string, underline in it.
+If @var{color} is @code{foreground-color}, underline with the
+foreground color of the face.
+
+If @var{style} is @code{wave} underline with a wave.
+If @var{style} is @code{line} underline with a line. 
+
+If the attribute @code{:color} is omited, underline with the
+foreground color of the face.
+If the attribute @code{:style} is omited, underline with a line.
+@end table
 
 @item :overline
 Whether or not characters should be overlined, and in what color.
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d725111..3680a26 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -135,8 +135,13 @@
      (choice :tag "Underline"
 	     :help-echo "Control text underlining."
 	     (const :tag "Off" nil)
-	     (const :tag "On" t)
-	     (color :tag "Colored")))
+	     (list :tag "On"
+		   (const :format "" :value :color)
+		   (choice :tag "Color" (const :tag "Foreground Color" foreground-color) color)
+                   (const :format "" :value :style)
+                   (choice :tag "Style"
+                           (const :tag "Line" line)
+                           (const :tag "Wave" wave)))))
 
     (:overline
      (choice :tag "Overline"
diff --git a/lisp/faces.el b/lisp/faces.el
index 5d406ad..e68abb1 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -616,10 +616,21 @@ VALUE must be a color name, a string.
 
 `:underline'
 
-VALUE specifies whether characters in FACE should be underlined.  If
-VALUE is t, underline with foreground color of the face.  If VALUE is
-a string, underline with that color.  If VALUE is nil, explicitly
-don't underline.
+VALUE specifies whether characters in FACE should be underlined.
+If VALUE is t, underline with foreground color of the face.
+If VALUE is a string, underline with that color.
+If VALUE is nil, explicitly don't underline.
+
+Otherwise, VALUE must be a property list of the form:
+
+`(:color COLOR :style STYLE)'.
+
+COLOR can be a either a color name string or `foreground-color'.
+STYLE can be either `line' or `wave'.
+If a keyword/value pair is missing from the property list, a
+default value will be used for the value.
+The default value of COLOR is the foreground color of the face.
+The default value of STYLE is `line'.
 
 `:overline'
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 2c59f4f..cdbbc16 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1510,6 +1510,13 @@ enum face_box_type
   FACE_SUNKEN_BOX
 };
 
+/* Underline type. */
+
+enum face_underline_type
+{
+  FACE_UNDER_LINE,
+  FACE_UNDER_WAVE
+};
 
 /* Structure describing a realized face.
 
@@ -1585,6 +1592,9 @@ struct face
      drawing shadows.  */
   unsigned use_box_color_for_shadows_p : 1;
 
+  /* Style of underlining. */
+  enum face_underline_type underline_type;
+
   /* Non-zero if text in this face should be underlined, overlined,
      strike-through or have a box drawn around it.  */
   unsigned underline_p : 1;
diff --git a/src/nsterm.m b/src/nsterm.m
index 70d3cc0..71c2f83 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2596,6 +2596,86 @@ ns_get_glyph_string_clip_rect (struct glyph_string *s, NativeRectangle *nr)
   return n;
 }
 
+/* --------------------------------------------------------------------
+   Draw a wavy line under S. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+  --------------------------------------------------------------------- */
+
+static void
+ns_draw_underwave (int x0, int y0,
+                   int width,
+                   int wave_height, int wave_length)
+{
+  /* Helper macro to re-use NSPoint a and b to draw lines. */
+#define DRAWLINE(x1, y1, x2, y2)                        \
+  do                                                    \
+    {                                                   \
+      a.x = x1, a.y = y1;                               \
+      b.x = x2, b.y = y2;                               \
+      [NSBezierPath strokeLineFromPoint:a toPoint: b];  \
+    }                                                   \
+  while (0)
+
+  NSPoint a, b;
+  int wave_height = 2, wave_length = 3;
+  int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
+  XRectangle wave_clip, string_clip, final_clip;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  x0 = s->x;
+  y0 = s->ybase + 1;
+  width = s->width;
+  xmax = x0 + width;
+
+  /* Find and set clipping rectangle */
+
+  wave_clip = (XRectangle){ x0, y0, width, wave_height };
+  /* XXX: use ns_get_glyph_string_clip_rect() instead? */
+  get_glyph_string_clip_rect (s, &string_clip);
+
+  if (!x_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
+    return;
+
+  /*
+     XXX: set clip rectangle
+     XSetClipRectangles (s->display, s->gc, 0, 0, &final_clip, 1, Unsorted);
+  */
+
+  /* Draw the waves */
+
+  x1 = x0 - (x0 % dx);
+  x2 = x1 + dx;
+  odd = (x1/dx) % 2;
+  y1 = y2 = y0;
+
+  if (odd)
+    y1 += dy;
+  else
+    y2 += dy;
+
+  while (x1 <= xmax)
+    {
+      DRAWLINE (x1, y1, x2, y2);
+      x1  = x2, y1 = y2;
+      x2 += dx, y2 = y0 + odd*dy;
+      odd = !odd;
+    }
+
+  /*
+    XXX: Restore previous clipping rectangle(s)
+    XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted);
+  */
+#undef DRAWLINE
+}
+
+
+
 void
 ns_draw_text_decoration (struct glyph_string *s, struct face *face,
                          NSColor *defaultCol, CGFloat width, CGFloat x)
@@ -2609,63 +2689,77 @@ ns_draw_text_decoration (struct glyph_string *s, struct face *face,
   /* Do underline. */
   if (face->underline_p)
     {
-      NSRect r;
-      unsigned long thickness, position;
-
-      /* If the prev was underlined, match its appearance. */
-      if (s->prev && s->prev->face->underline_p
-          && s->prev->underline_thickness > 0)
+      if (s->face->underline_type == FACE_UNDER_WAVE)
         {
-          thickness = s->prev->underline_thickness;
-          position = s->prev->underline_position;
+          unsigned long height = 2, len = 3, y = s->ybase + 1;
+
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          
+          ns_draw_underwave (s->x, y, s->width, height, len);
         }
-      else
+      else if (s->face->underline_type == FACE_UNDER_LINE)
         {
-          struct font *font;
-          unsigned long descent;
-
-          font=s->font;
-          descent = s->y + s->height - s->ybase;
-
-          /* Use underline thickness of font, defaulting to 1. */
-          thickness = (font && font->underline_thickness > 0)
-            ? font->underline_thickness : 1;
-
-          /* Determine the offset of underlining from the baseline. */
-          if (x_underline_at_descent_line)
-            position = descent - thickness;
-          else if (x_use_underline_position_properties
-                   && font && font->underline_position >= 0)
-            position = font->underline_position;
-          else if (font)
-            position = lround (font->descent / 2);
-          else
-            position = underline_minimum_offset;
 
-          position = max (position, underline_minimum_offset);
+          NSRect r;
+          unsigned long thickness, position;
 
-          /* Ensure underlining is not cropped. */
-          if (descent <= position)
+          /* If the prev was underlined, match its appearance. */
+          if (s->prev && s->prev->face->underline_p
+              && s->prev->underline_thickness > 0)
             {
-              position = descent - 1;
-              thickness = 1;
+              thickness = s->prev->underline_thickness;
+              position = s->prev->underline_position;
             }
-          else if (descent < position + thickness)
-            thickness = 1;
-        }
+          else
+            {
+              struct font *font;
+              unsigned long descent;
+
+              font=s->font;
+              descent = s->y + s->height - s->ybase;
+
+              /* Use underline thickness of font, defaulting to 1. */
+              thickness = (font && font->underline_thickness > 0)
+                ? font->underline_thickness : 1;
+
+              /* Determine the offset of underlining from the baseline. */
+              if (x_underline_at_descent_line)
+                position = descent - thickness;
+              else if (x_use_underline_position_properties
+                       && font && font->underline_position >= 0)
+                position = font->underline_position;
+              else if (font)
+                position = lround (font->descent / 2);
+              else
+                position = underline_minimum_offset;
 
-      s->underline_thickness = thickness;
-      s->underline_position = position;
+              position = max (position, underline_minimum_offset);
 
-      r = NSMakeRect (x, s->ybase + position, width, thickness);
+              /* Ensure underlining is not cropped. */
+              if (descent <= position)
+                {
+                  position = descent - 1;
+                  thickness = 1;
+                }
+              else if (descent < position + thickness)
+                thickness = 1;
+            }
 
-      if (face->underline_defaulted_p)
-        [defaultCol set];
-      else
-        [ns_lookup_indexed_color (face->underline_color, s->f) set];
-      NSRectFill (r);
-    }
+          s->underline_thickness = thickness;
+          s->underline_position = position;
 
+          r = NSMakeRect (x, s->ybase + position, width, thickness);
+
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          NSRectFill (r);
+        }
+    }
   /* Do overline. We follow other terms in using a thickness of 1
      and ignoring overline_margin. */
   if (face->overline_p)
diff --git a/src/w32term.c b/src/w32term.c
index f764ad9..ebaa8f5 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -309,6 +309,93 @@ w32_set_clip_rectangle (HDC hdc, RECT *rect)
     SelectClipRgn (hdc, NULL);
 }
 
+/* Restore clipping rectangle in S */
+static void
+w32_restore_glyph_string_clip (struct glyph_string *s)
+{
+  RECT *r = s->clip;
+  int n = s->num_clips;
+
+  if (n == 1)
+    w32_set_clip_rectangle (s->hdc, r);
+  else if (n > 1)
+    {
+      HRGN clip1 = CreateRectRgnIndirect (r);
+      HRGN clip2 = CreateRectRgnIndirect (r + 1);
+      if (CombineRgn (clip1, clip1, clip2, RGN_OR) != ERROR)
+        SelectClipRgn (s->hdc, clip1);
+      DeleteObject (clip1);
+      DeleteObject (clip2);
+    }
+}
+
+/*
+   Draw a wavy line under S. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+void
+w32_draw_underwave (struct glyph_string *s, COLORREF color)
+{
+  int wave_height = 2, wave_length = 3;
+  int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
+  XRectangle wave_clip, string_clip, final_clip;
+  RECT w32_final_clip;
+  HPEN hp, oldhp;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  x0 = s->x;
+  y0 = s->ybase + 1;
+  width = s->width;
+  xmax = x0 + width;
+
+  /* Find and set clipping rectangle */
+
+  wave_clip = (XRectangle){ x0, y0, width, wave_height };
+  get_glyph_string_clip_rect (s, &string_clip);
+
+  if (!x_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
+    return;
+
+  hp = CreatePen (PS_SOLID, 0, color);
+  oldhp = SelectObject (s->hdc, hp);
+  CONVERT_FROM_XRECT (final_clip, w32_final_clip);
+  w32_set_clip_rectangle (w32_final_clip);
+
+  /* Draw the waves */
+
+  x1 = x0 - (x0 % dx);
+  x2 = x1 + dx;
+  odd = (x1/dx) % 2;
+  y1 = y2 = y0;
+
+  if (odd)
+    y1 += dy;
+  else
+    y2 += dy;
+
+  MoveToEx (s->hdc, x1, y1, NULL);
+
+  while (x1 <= xmax)
+    {
+      LineTo (s->hdc, x2, y2);
+      x1  = x2, y1 = y2;
+      x2 += dx, y2 = y0 + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous pen and clipping rectangle(s) */
+  w32_restore_glyph_string_clip (s);
+  SelectObject (s->hdc, oldhp);
+  DeleteObject (hp);
+}
 
 /* Draw a hollow rectangle at the specified position.  */
 void
@@ -2343,60 +2430,74 @@ x_draw_glyph_string (struct glyph_string *s)
       /* Draw underline.  */
       if (s->face->underline_p)
         {
-          unsigned long thickness, position;
-          int y;
-
-          if (s->prev && s->prev->face->underline_p)
+          if (s->face->underline_type == FACE_UNDER_WAVE)
             {
-              /* We use the same underline style as the previous one.  */
-              thickness = s->prev->underline_thickness;
-              position = s->prev->underline_position;
+              COLORREF color;
+
+              if (s->face->underline_defaulted_p)
+                color = s->gc->foreground;
+              else
+                color = s->face->underline_color;
+
+              w32_draw_underwave (s, color);
             }
-          else
+          else if (s->face->underline_type == FACE_UNDER_LINE)
             {
-              /* Get the underline thickness.  Default is 1 pixel.  */
-              if (s->font && s->font->underline_thickness > 0)
-                thickness = s->font->underline_thickness;
+              unsigned long thickness, position;
+              int y;
+
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
               else
-                thickness = 1;
-              if (x_underline_at_descent_line)
-                position = (s->height - thickness) - (s->ybase - s->y);
+                {
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
+
+                         ROUND ((maximum_descent) / 2), with
+                         ROUND (x) = floor (x + 0.5)  */
+
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position =position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                {
+                  w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                                 y, s->width, 1);
+                }
               else
                 {
-                /* Get the underline position.  This is the recommended
-                   vertical offset in pixels from the baseline to the top of
-                   the underline.  This is a signed value according to the
-                   specs, and its default is
-
-                   ROUND ((maximum_descent) / 2), with
-                   ROUND (x) = floor (x + 0.5)  */
-
-                if (x_use_underline_position_properties
-                    && s->font && s->font->underline_position >= 0)
-                  position = s->font->underline_position;
-                else if (s->font)
-                  position = (s->font->descent + 1) / 2;
+                  w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
+                                 y, s->width, 1);
                 }
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position =position;
-          y = s->ybase + position;
-          if (s->face->underline_defaulted_p)
-            {
-              w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-                             y, s->width, 1);
-            }
-          else
-            {
-              w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
-                             y, s->width, 1);
             }
         }
       /* Draw overline.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 617097d..8bee2d6 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -320,6 +320,7 @@ static Lisp_Object QCfontset;
 
 Lisp_Object Qnormal;
 Lisp_Object Qbold;
+static Lisp_Object Qline, Qwave;
 static Lisp_Object Qultra_light, Qextra_light, Qlight;
 static Lisp_Object Qsemi_light, Qsemi_bold, Qextra_bold, Qultra_bold;
 static Lisp_Object Qoblique, Qreverse_oblique, Qreverse_italic;
@@ -1889,7 +1890,8 @@ check_lface_attrs (Lisp_Object *attrs)
   xassert (UNSPECIFIEDP (attrs[LFACE_UNDERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_UNDERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_UNDERLINE_INDEX])
-	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX]));
+	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX])
+	   || CONSP (attrs[LFACE_UNDERLINE_INDEX]));
   xassert (UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_OVERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
@@ -2520,7 +2522,8 @@ merge_face_ref (struct frame *f, Lisp_Object face_ref, Lisp_Object *to,
 		{
 		  if (EQ (value, Qt)
 		      || NILP (value)
-		      || STRINGP (value))
+		      || STRINGP (value)
+		      || CONSP (value))
 		    to[LFACE_UNDERLINE_INDEX] = value;
 		  else
 		    err = 1;
@@ -2944,15 +2947,54 @@ FRAME 0 means change the face on all frames, and change the default
     }
   else if (EQ (attr, QCunderline))
     {
-      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
-	if ((SYMBOLP (value)
-	     && !EQ (value, Qt)
-	     && !EQ (value, Qnil))
-	    /* Underline color.  */
-	    || (STRINGP (value)
-		&& SCHARS (value) == 0))
-	  signal_error ("Invalid face underline", value);
-
+      int valid_p = 0;
+      
+      if (UNSPECIFIEDP (value) || IGNORE_DEFFACE_P (value))
+	valid_p = 1;
+      else if (NILP (value) || EQ (value, Qt))
+        valid_p = 1;
+      else if (STRINGP (value) && SCHARS (value) > 0)
+        valid_p = 1;
+      else if (CONSP (value))
+        {
+          Lisp_Object key, val, list;
+
+          list = value;
+          valid_p = 1;
+
+          while (!NILP (CAR_SAFE(list)))
+            {
+              key = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+              val = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+            
+              if(NILP (key) || NILP (val)) 
+                {
+                  valid_p = 0;
+                  break;
+                }
+
+              else if (EQ (key, QCcolor)
+                       && !(EQ (val, Qforeground_color)
+                            || (STRINGP (val) && SCHARS (val) > 0)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+              
+              else if (EQ (key, QCstyle) 
+                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+            }
+        }
+      
+      if (!valid_p)
+        signal_error ("Invalid face underline", value);
+      
       old_value = LFACE_UNDERLINE (lface);
       LFACE_UNDERLINE (lface) = value;
     }
@@ -3762,6 +3804,7 @@ Value is nil if ATTR doesn't have a discrete set of valid values.  */)
 
   CHECK_SYMBOL (attr);
 
+  /* XXX: no check for QCbox? */
   if (EQ (attr, QCunderline))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCoverline))
@@ -5563,7 +5606,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 #ifdef HAVE_WINDOW_SYSTEM
   struct face *default_face;
   struct frame *f;
-  Lisp_Object stipple, overline, strike_through, box;
+  Lisp_Object stipple, underline, overline, strike_through, box;
 
   xassert (FRAME_WINDOW_P (cache->f));
 
@@ -5696,29 +5739,76 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 
   /* Text underline, overline, strike-through.  */
 
-  if (EQ (attrs[LFACE_UNDERLINE_INDEX], Qt))
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (EQ (underline, Qt))
     {
       /* Use default color (same as foreground color).  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 1;
       face->underline_color = 0;
     }
-  else if (STRINGP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (STRINGP (underline))
     {
       /* Use specified color.  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 0;
       face->underline_color
-	= load_color (f, face, attrs[LFACE_UNDERLINE_INDEX],
+	= load_color (f, face, underline,
 		      LFACE_UNDERLINE_INDEX);
     }
-  else if (NILP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (NILP (underline))
     {
       face->underline_p = 0;
       face->underline_defaulted_p = 0;
       face->underline_color = 0;
     }
-
+  else if (CONSP (underline))
+    {
+      /* `(:color COLOR :style STYLE)'.  
+         STYLE being one of `line' or `wave'. */
+      face->underline_p = 1;
+      face->underline_color = 0;
+      face->underline_defaulted_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
+
+      while (CONSP (underline))
+        {
+          Lisp_Object keyword, value;
+
+          keyword = XCAR (underline);
+          underline = XCDR (underline);
+
+          if (!CONSP (underline))
+            break;
+          value = XCAR (underline);
+          underline = XCDR (underline);
+
+          if (EQ (keyword, QCcolor))
+            {
+              if (EQ (value, Qforeground_color))
+                {
+                  face->underline_defaulted_p = 1;
+                  face->underline_color = 0;
+                }
+              else if (STRINGP (value))
+                {
+                  face->underline_defaulted_p = 0;
+                  face->underline_color = load_color (f, face, value,
+                                                      LFACE_UNDERLINE_INDEX);
+                }
+            }
+          else if (EQ (keyword, QCstyle))
+            {
+              if (EQ (value, Qline))
+                face->underline_type = FACE_UNDER_LINE;
+              else if (EQ (value, Qwave))
+                face->underline_type = FACE_UNDER_WAVE;
+            }
+        }
+    }
+  
   overline = attrs[LFACE_OVERLINE_INDEX];
   if (STRINGP (overline))
     {
@@ -6465,6 +6555,8 @@ syms_of_xfaces (void)
   DEFSYM (QCcolor, ":color");
   DEFSYM (QCline_width, ":line-width");
   DEFSYM (QCstyle, ":style");
+  DEFSYM (Qline, "line");
+  DEFSYM (Qwave, "wave");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qnormal, "normal");
diff --git a/src/xterm.c b/src/xterm.c
index 4b34d63..a58e366 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2653,6 +2653,65 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = 1;
 }
 
+/*
+   Draw a wavy line under S. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+static void
+x_draw_underwave (struct glyph_string *s)
+{
+  int wave_height = 2, wave_length = 3;
+  int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
+  XRectangle wave_clip, string_clip, final_clip;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  x0 = s->x;
+  y0 = s->ybase + 1;
+  width = s->width;
+  xmax = x0 + width;
+
+  /* Find and set clipping rectangle */
+
+  wave_clip = (XRectangle){ x0, y0, width, wave_height };
+  get_glyph_string_clip_rect (s, &string_clip);
+
+  if (!x_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
+    return;
+
+  XSetClipRectangles (s->display, s->gc, 0, 0, &final_clip, 1, Unsorted);
+
+  /* Draw the waves */
+
+  x1 = x0 - (x0 % dx);
+  x2 = x1 + dx;
+  odd = (x1/dx) % 2;
+  y1 = y2 = y0;
+
+  if (odd)
+    y1 += dy;
+  else
+    y2 += dy;
+
+  while (x1 <= xmax)
+    {
+      XDrawLine (s->display, s->window, s->gc, x1, y1, x2, y2);
+      x1  = x2, y1 = y2;
+      x2 += dx, y2 = y0 + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous clipping rectangle(s) */
+  XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted);
+}
+
 
 /* Draw glyph string S.  */
 
@@ -2755,68 +2814,83 @@ x_draw_glyph_string (struct glyph_string *s)
     {
       /* Draw underline.  */
       if (s->face->underline_p)
-	{
-	  unsigned long thickness, position;
-	  int y;
-
-	  if (s->prev && s->prev->face->underline_p)
-	    {
-	      /* We use the same underline style as the previous one.  */
-	      thickness = s->prev->underline_thickness;
-	      position = s->prev->underline_position;
-	    }
-	  else
-	    {
-	      /* Get the underline thickness.  Default is 1 pixel.  */
-	      if (s->font && s->font->underline_thickness > 0)
-		thickness = s->font->underline_thickness;
-	      else
-		thickness = 1;
-	      if (x_underline_at_descent_line)
-		position = (s->height - thickness) - (s->ybase - s->y);
-	      else
-		{
-		  /* Get the underline position.  This is the recommended
-		     vertical offset in pixels from the baseline to the top of
-		     the underline.  This is a signed value according to the
-		     specs, and its default is
-
-		     ROUND ((maximum descent) / 2), with
-		     ROUND(x) = floor (x + 0.5)  */
-
-		  if (x_use_underline_position_properties
-		      && s->font && s->font->underline_position >= 0)
-		    position = s->font->underline_position;
-		  else if (s->font)
-		    position = (s->font->descent + 1) / 2;
-		  else
-		    position = underline_minimum_offset;
-		}
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position = position;
-	  y = s->ybase + position;
-	  if (s->face->underline_defaulted_p)
-	    XFillRectangle (s->display, s->window, s->gc,
-			    s->x, y, s->width, thickness);
-	  else
-	    {
-	      XGCValues xgcv;
-	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-	      XSetForeground (s->display, s->gc, s->face->underline_color);
-	      XFillRectangle (s->display, s->window, s->gc,
-			      s->x, y, s->width, thickness);
-	      XSetForeground (s->display, s->gc, xgcv.foreground);
-	    }
-	}
+        {
+          if (s->face->underline_type == FACE_UNDER_WAVE)
+            {
+              if (s->face->underline_defaulted_p)
+                x_draw_underwave (s);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  x_draw_underwave (s);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+          else if (s->face->underline_type == FACE_UNDER_LINE)
+            {
+              unsigned long thickness, position;
+              int y;
 
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
+              else
+                {
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
+
+                         ROUND ((maximum descent) / 2), with
+                         ROUND(x) = floor (x + 0.5)  */
+
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                      else
+                        position = underline_minimum_offset;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position = position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                XFillRectangle (s->display, s->window, s->gc,
+                                s->x, y, s->width, thickness);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  XFillRectangle (s->display, s->window, s->gc,
+                                  s->x, y, s->width, thickness);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+        }
       /* Draw overline.  */
       if (s->face->overline_p)
 	{

Re: [patch] add "underwave" face attribute

[Alp Aker]

[-- Attachment #1.1: Type: text/plain, Size: 828 bytes --]

Aurélien Aptel wrote:

> Mac OSX clipping works differently and I'm not sure what to do... See
> XXX coments in nsterm.m

I fixed up the NextStep portion; attached is a new diff reflecting the
changes to nsterm.m.  In particular, in ns_draw_underwave:

(1) I changed the API calls for the clipping.
(2) Because of the way the NS port handles cursor drawing, one can't
extract the x-origin and width for the underwave directly from the glyph
string as you did.  I made the appropriate changes.
(3) On the various displays I tried, changing wave_height from 2 to 3
dramatically improved the appearance. I took the liberty of using the
latter value.  (Perhaps wave_height and wave_length should be exposed to
the user as options?)
(4) I rearranged the code to make it more obviously parallel
to x_draw_underwave.

[-- Attachment #1.2: Type: text/html, Size: 1162 bytes --]

[-- Attachment #2: underwave-clip-2.diff --]
[-- Type: application/octet-stream, Size: 6344 bytes --]

=== modified file 'src/nsterm.m'
--- src/nsterm.m	2012-02-04 15:10:54 +0000
+++ src/nsterm.m	2012-02-05 23:52:29 +0000
@@ -2598,6 +2598,66 @@
   return n;
 }
 
+/* --------------------------------------------------------------------
+   Draw a wavy line under glyph string s. The wave fills wave_height
+   pixels from y.
+
+                    x          wave_length = 2
+                                 --
+                y    *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+  --------------------------------------------------------------------- */
+
+static void
+ns_draw_underwave (struct glyph_string *s, CGFloat width, CGFloat x)
+{
+  int wave_height = 3, wave_length = 3;
+  int y, dx, dy, odd, xmax;
+  NSPoint a, b;
+  NSRect waveClip, stringClip, finalClip;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  y =  s->ybase + 1;
+  xmax = x + width;
+
+  /* Find and set clipping rectangle */
+
+  waveClip = NSMakeRect (x, y, width, wave_height);
+  ns_get_glyph_string_clip_rect (s, &stringClip);
+  finalClip = NSIntersectionRect (waveClip, stringClip);
+
+  if (NSIsEmptyRect (finalClip))          
+    return;
+
+  [[NSGraphicsContext currentContext] saveGraphicsState];
+  NSRectClip (finalClip);
+
+  /* Draw the waves */
+
+  a.x = x - ((int)(x) % dx); 
+  b.x = a.x + dx;
+  odd = ((int)(a.x)/dx) % 2;
+  a.y = b.y = y;
+
+  if (odd)
+    a.y += dy;
+  else
+    b.y += dy;
+
+  while (a.x <= xmax)
+    {
+      [NSBezierPath strokeLineFromPoint:a toPoint: b];
+      a.x = b.x, a.y = b.y;
+      b.x += dx, b.y = y + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous clipping rectangle(s) */
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
+}
+
 void
 ns_draw_text_decoration (struct glyph_string *s, struct face *face,
                          NSColor *defaultCol, CGFloat width, CGFloat x)
@@ -2611,63 +2671,75 @@
   /* Do underline. */
   if (face->underline_p)
     {
-      NSRect r;
-      unsigned long thickness, position;
-
-      /* If the prev was underlined, match its appearance. */
-      if (s->prev && s->prev->face->underline_p
-          && s->prev->underline_thickness > 0)
-        {
-          thickness = s->prev->underline_thickness;
-          position = s->prev->underline_position;
-        }
-      else
-        {
-          struct font *font;
-          unsigned long descent;
-
-          font=s->font;
-          descent = s->y + s->height - s->ybase;
-
-          /* Use underline thickness of font, defaulting to 1. */
-          thickness = (font && font->underline_thickness > 0)
-            ? font->underline_thickness : 1;
-
-          /* Determine the offset of underlining from the baseline. */
-          if (x_underline_at_descent_line)
-            position = descent - thickness;
-          else if (x_use_underline_position_properties
-                   && font && font->underline_position >= 0)
-            position = font->underline_position;
-          else if (font)
-            position = lround (font->descent / 2);
-          else
-            position = underline_minimum_offset;
-
-          position = max (position, underline_minimum_offset);
-
-          /* Ensure underlining is not cropped. */
-          if (descent <= position)
-            {
-              position = descent - 1;
-              thickness = 1;
-            }
-          else if (descent < position + thickness)
-            thickness = 1;
-        }
-
-      s->underline_thickness = thickness;
-      s->underline_position = position;
-
-      r = NSMakeRect (x, s->ybase + position, width, thickness);
-
-      if (face->underline_defaulted_p)
-        [defaultCol set];
-      else
-        [ns_lookup_indexed_color (face->underline_color, s->f) set];
-      NSRectFill (r);
+      if (s->face->underline_type == FACE_UNDER_WAVE)
+        {
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+
+          ns_draw_underwave (s, width, x);
+        }
+      else if (s->face->underline_type == FACE_UNDER_LINE)
+        {
+
+          NSRect r;
+          unsigned long thickness, position;
+
+          /* If the prev was underlined, match its appearance. */
+          if (s->prev && s->prev->face->underline_p
+              && s->prev->underline_thickness > 0)
+            {
+              thickness = s->prev->underline_thickness;
+              position = s->prev->underline_position;
+            }
+          else
+            {
+              struct font *font;
+              unsigned long descent;
+
+              font=s->font;
+              descent = s->y + s->height - s->ybase;
+
+              /* Use underline thickness of font, defaulting to 1. */
+              thickness = (font && font->underline_thickness > 0)
+                ? font->underline_thickness : 1;
+
+              /* Determine the offset of underlining from the baseline. */
+              if (x_underline_at_descent_line)
+                position = descent - thickness;
+              else if (x_use_underline_position_properties
+                       && font && font->underline_position >= 0)
+                position = font->underline_position;
+              else if (font)
+                position = lround (font->descent / 2);
+              else
+                position = underline_minimum_offset;
+
+              position = max (position, underline_minimum_offset);
+
+              /* Ensure underlining is not cropped. */
+              if (descent <= position)
+                {
+                  position = descent - 1;
+                  thickness = 1;
+                }
+              else if (descent < position + thickness)
+                thickness = 1;
+            }
+
+          s->underline_thickness = thickness;
+          s->underline_position = position;
+
+          r = NSMakeRect (x, s->ybase + position, width, thickness);
+
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          NSRectFill (r);
+        }
     }
-
   /* Do overline. We follow other terms in using a thickness of 1
      and ignoring overline_margin. */
   if (face->overline_p)

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

On Mon, Feb 6, 2012 at 1:18 AM, Alp Aker <alptekin.aker@gmail.com> wrote:
> Aurélien Aptel wrote:
>> Mac OSX clipping works differently and I'm not sure what to do... See
>> XXX coments in nsterm.m
>
> I fixed up the NextStep portion; attached is a new diff reflecting the
> changes to nsterm.m.  In particular, in ns_draw_underwave:
>
> (1) I changed the API calls for the clipping.
> (2) Because of the way the NS port handles cursor drawing, one can't extract
> the x-origin and width for the underwave directly from the glyph string as
> you did.  I made the appropriate changes.
> (3) On the various displays I tried, changing wave_height from 2 to 3
> dramatically improved the appearance. I took the liberty of using the latter
> value.  (Perhaps wave_height and wave_length should be exposed to the user
> as options?)
> (4) I rearranged the code to make it more obviously parallel
> to x_draw_underwave.

Hooray! Could you provide a screen shot? I'm curious :)
I don't have to continue this osx install anymore \o/

Re: [patch] add "underwave" face attribute

[Alp Aker]

[-- Attachment #1.1: Type: text/plain, Size: 118 bytes --]

Aurélien Aptel wrote:

> Hooray! Could you provide a screen shot? I'm curious :)

No problem.  See the attached.

[-- Attachment #1.2: Type: text/html, Size: 203 bytes --]

[-- Attachment #2: underwave-nextstep.png --]
[-- Type: image/png, Size: 23157 bytes --]

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

On Mon, Feb 6, 2012 at 1:18 AM, Alp Aker <alptekin.aker@gmail.com> wrote:
> (1) I changed the API calls for the clipping.

Are you sure this won't break the rest of the code? (which relies
heavily on clipping) It sure looks cleaner but the rest of the code
uses ns_focus/unfocus.

> (2) Because of the way the NS port handles cursor drawing, one can't extract
> the x-origin and width for the underwave directly from the glyph string as
> you did.  I made the appropriate changes.

You're correct. I should have looked closer at the underline code.
Then again I wrote the code blindly :p

> (3) On the various displays I tried, changing wave_height from 2 to 3
> dramatically improved the appearance. I took the liberty of using the latter
> value.  (Perhaps wave_height and wave_length should be exposed to the user
> as options?)

Set them to the best looking value. The screenshot looks nice.  We
could expose it as options but wave_height has a short upper limit:
the wave gets cropped above a certain value. I don't know if there's a
*simple* way to make the "line space" higher to handle this.

Re: [patch] add "underwave" face attribute

[Alp Aker]

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

Aurélien Aptel wrote:

> Are you sure this won't break the rest of the code? (which relies
> heavily on clipping) It sure looks cleaner but the rest of the code
> uses ns_focus/unfocus.

Fairly sure.  What's needed here is a temporary change to the graphics
state in the middle of a redisplay; we change the clipping rectangle, draw
the underwave, then restore the previous clipping rectangle, if any.
ns_focus/ns_unfocus can't be used to do that; they're intended to be used
at the beginning and end of redisplay.

[-- Attachment #2: Type: text/html, Size: 680 bytes --]

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

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

Ok, I will send the attached patch to the bug mailing list.

[-- Attachment #2: underwave-clip-3.patch --]
[-- Type: text/x-patch, Size: 32623 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 381eaf6..464dfaa 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2080,10 +2080,32 @@ Background color, a string.  The value can be a system-defined color
 name, or a hexadecimal color specification.  @xref{Color Names}.
 
 @item :underline
-Whether or not characters should be underlined, and in what color.  If
-the value is @code{t}, underlining uses the foreground color of the
-face.  If the value is a string, underlining uses that color.  The
-value @code{nil} means do not underline.
+Whether or not characters should be underlined, and in what
+color. Here are the possible values of the @code{:underline}
+attribute, and what they mean:
+
+@table @asis
+@item @code{nil}
+Don't underline.
+
+@item @code{t}
+Underline with the foreground color of the face.
+
+@item @var{color}
+Underline in color @var{color}.
+
+@item @code{(:color @var{color} :style @var{style})}
+If @var{color} is a string, underline in it.
+If @var{color} is @code{foreground-color}, underline with the
+foreground color of the face.
+
+If @var{style} is @code{wave} underline with a wave.
+If @var{style} is @code{line} underline with a line. 
+
+If the attribute @code{:color} is omited, underline with the
+foreground color of the face.
+If the attribute @code{:style} is omited, underline with a line.
+@end table
 
 @item :overline
 Whether or not characters should be overlined, and in what color.
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d725111..3680a26 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -135,8 +135,13 @@
      (choice :tag "Underline"
 	     :help-echo "Control text underlining."
 	     (const :tag "Off" nil)
-	     (const :tag "On" t)
-	     (color :tag "Colored")))
+	     (list :tag "On"
+		   (const :format "" :value :color)
+		   (choice :tag "Color" (const :tag "Foreground Color" foreground-color) color)
+                   (const :format "" :value :style)
+                   (choice :tag "Style"
+                           (const :tag "Line" line)
+                           (const :tag "Wave" wave)))))
 
     (:overline
      (choice :tag "Overline"
diff --git a/lisp/faces.el b/lisp/faces.el
index 5d406ad..e68abb1 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -616,10 +616,21 @@ VALUE must be a color name, a string.
 
 `:underline'
 
-VALUE specifies whether characters in FACE should be underlined.  If
-VALUE is t, underline with foreground color of the face.  If VALUE is
-a string, underline with that color.  If VALUE is nil, explicitly
-don't underline.
+VALUE specifies whether characters in FACE should be underlined.
+If VALUE is t, underline with foreground color of the face.
+If VALUE is a string, underline with that color.
+If VALUE is nil, explicitly don't underline.
+
+Otherwise, VALUE must be a property list of the form:
+
+`(:color COLOR :style STYLE)'.
+
+COLOR can be a either a color name string or `foreground-color'.
+STYLE can be either `line' or `wave'.
+If a keyword/value pair is missing from the property list, a
+default value will be used for the value.
+The default value of COLOR is the foreground color of the face.
+The default value of STYLE is `line'.
 
 `:overline'
 
diff --git a/src/dispextern.h b/src/dispextern.h
index 2c59f4f..cdbbc16 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1510,6 +1510,13 @@ enum face_box_type
   FACE_SUNKEN_BOX
 };
 
+/* Underline type. */
+
+enum face_underline_type
+{
+  FACE_UNDER_LINE,
+  FACE_UNDER_WAVE
+};
 
 /* Structure describing a realized face.
 
@@ -1585,6 +1592,9 @@ struct face
      drawing shadows.  */
   unsigned use_box_color_for_shadows_p : 1;
 
+  /* Style of underlining. */
+  enum face_underline_type underline_type;
+
   /* Non-zero if text in this face should be underlined, overlined,
      strike-through or have a box drawn around it.  */
   unsigned underline_p : 1;
diff --git a/src/nsterm.m b/src/nsterm.m
index 70d3cc0..0378330 100644
--- a/src/nsterm.m
+++ b/src/nsterm.m
@@ -2596,6 +2596,68 @@ ns_get_glyph_string_clip_rect (struct glyph_string *s, NativeRectangle *nr)
   return n;
 }
 
+/* --------------------------------------------------------------------
+   Draw a wavy line under glyph string s. The wave fills wave_height
+   pixels from y.
+
+                    x          wave_length = 2
+                                 --
+                y    *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+  --------------------------------------------------------------------- */
+
+static void
+ns_draw_underwave (struct glyph_string *s, CGFloat width, CGFloat x)
+{
+  int wave_height = 3, wave_length = 3;
+  int y, dx, dy, odd, xmax;
+  NSPoint a, b;
+  NSRect waveClip, stringClip, finalClip;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  y =  s->ybase + 1;
+  xmax = x + width;
+
+  /* Find and set clipping rectangle */
+
+  waveClip = NSMakeRect (x, y, width, wave_height);
+  ns_get_glyph_string_clip_rect (s, &stringClip);
+  finalClip = NSIntersectionRect (waveClip, stringClip);
+
+  if (NSIsEmptyRect (finalClip))
+    return;
+
+  [[NSGraphicsContext currentContext] saveGraphicsState];
+  NSRectClip (finalClip);
+
+  /* Draw the waves */
+
+  a.x = x - ((int)(x) % dx);
+  b.x = a.x + dx;
+  odd = ((int)(a.x)/dx) % 2;
+  a.y = b.y = y;
+
+  if (odd)
+    a.y += dy;
+  else
+    b.y += dy;
+
+  while (a.x <= xmax)
+    {
+      [NSBezierPath strokeLineFromPoint:a toPoint: b];
+      a.x = b.x, a.y = b.y;
+      b.x += dx, b.y = y + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous clipping rectangle(s) */
+  [[NSGraphicsContext currentContext] restoreGraphicsState];
+}
+
+
+
 void
 ns_draw_text_decoration (struct glyph_string *s, struct face *face,
                          NSColor *defaultCol, CGFloat width, CGFloat x)
@@ -2609,63 +2671,75 @@ ns_draw_text_decoration (struct glyph_string *s, struct face *face,
   /* Do underline. */
   if (face->underline_p)
     {
-      NSRect r;
-      unsigned long thickness, position;
-
-      /* If the prev was underlined, match its appearance. */
-      if (s->prev && s->prev->face->underline_p
-          && s->prev->underline_thickness > 0)
+      if (s->face->underline_type == FACE_UNDER_WAVE)
         {
-          thickness = s->prev->underline_thickness;
-          position = s->prev->underline_position;
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+
+          ns_draw_underwave (s, width, x);
         }
-      else
+      else if (s->face->underline_type == FACE_UNDER_LINE)
         {
-          struct font *font;
-          unsigned long descent;
-
-          font=s->font;
-          descent = s->y + s->height - s->ybase;
-
-          /* Use underline thickness of font, defaulting to 1. */
-          thickness = (font && font->underline_thickness > 0)
-            ? font->underline_thickness : 1;
-
-          /* Determine the offset of underlining from the baseline. */
-          if (x_underline_at_descent_line)
-            position = descent - thickness;
-          else if (x_use_underline_position_properties
-                   && font && font->underline_position >= 0)
-            position = font->underline_position;
-          else if (font)
-            position = lround (font->descent / 2);
-          else
-            position = underline_minimum_offset;
 
-          position = max (position, underline_minimum_offset);
+          NSRect r;
+          unsigned long thickness, position;
 
-          /* Ensure underlining is not cropped. */
-          if (descent <= position)
+          /* If the prev was underlined, match its appearance. */
+          if (s->prev && s->prev->face->underline_p
+              && s->prev->underline_thickness > 0)
             {
-              position = descent - 1;
-              thickness = 1;
+              thickness = s->prev->underline_thickness;
+              position = s->prev->underline_position;
             }
-          else if (descent < position + thickness)
-            thickness = 1;
-        }
+          else
+            {
+              struct font *font;
+              unsigned long descent;
+
+              font=s->font;
+              descent = s->y + s->height - s->ybase;
+
+              /* Use underline thickness of font, defaulting to 1. */
+              thickness = (font && font->underline_thickness > 0)
+                ? font->underline_thickness : 1;
+
+              /* Determine the offset of underlining from the baseline. */
+              if (x_underline_at_descent_line)
+                position = descent - thickness;
+              else if (x_use_underline_position_properties
+                       && font && font->underline_position >= 0)
+                position = font->underline_position;
+              else if (font)
+                position = lround (font->descent / 2);
+              else
+                position = underline_minimum_offset;
+
+              position = max (position, underline_minimum_offset);
 
-      s->underline_thickness = thickness;
-      s->underline_position = position;
+              /* Ensure underlining is not cropped. */
+              if (descent <= position)
+                {
+                  position = descent - 1;
+                  thickness = 1;
+                }
+              else if (descent < position + thickness)
+                thickness = 1;
+            }
 
-      r = NSMakeRect (x, s->ybase + position, width, thickness);
+          s->underline_thickness = thickness;
+          s->underline_position = position;
 
-      if (face->underline_defaulted_p)
-        [defaultCol set];
-      else
-        [ns_lookup_indexed_color (face->underline_color, s->f) set];
-      NSRectFill (r);
-    }
+          r = NSMakeRect (x, s->ybase + position, width, thickness);
 
+          if (face->underline_defaulted_p)
+            [defaultCol set];
+          else
+            [ns_lookup_indexed_color (face->underline_color, s->f) set];
+          NSRectFill (r);
+        }
+    }
   /* Do overline. We follow other terms in using a thickness of 1
      and ignoring overline_margin. */
   if (face->overline_p)
diff --git a/src/w32term.c b/src/w32term.c
index f764ad9..d53f0ab 100644
--- a/src/w32term.c
+++ b/src/w32term.c
@@ -309,6 +309,94 @@ w32_set_clip_rectangle (HDC hdc, RECT *rect)
     SelectClipRgn (hdc, NULL);
 }
 
+/* Restore clipping rectangle in S */
+static void
+w32_restore_glyph_string_clip (struct glyph_string *s)
+{
+  RECT *r = s->clip;
+  int n = s->num_clips;
+
+  if (n == 1)
+    w32_set_clip_rectangle (s->hdc, r);
+  else if (n > 1)
+    {
+      HRGN clip1 = CreateRectRgnIndirect (r);
+      HRGN clip2 = CreateRectRgnIndirect (r + 1);
+      if (CombineRgn (clip1, clip1, clip2, RGN_OR) != ERROR)
+        SelectClipRgn (s->hdc, clip1);
+      DeleteObject (clip1);
+      DeleteObject (clip2);
+    }
+}
+
+/*
+   Draw a wavy line under S. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+void
+w32_draw_underwave (struct glyph_string *s, COLORREF color)
+{
+  int wave_height = 2, wave_length = 3;
+  int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
+  XRectangle wave_clip, string_clip, final_clip;
+  RECT w32_final_clip, w32_string_clip;
+  HPEN hp, oldhp;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  x0 = s->x;
+  y0 = s->ybase + 1;
+  width = s->width;
+  xmax = x0 + width;
+
+  /* Find and set clipping rectangle */
+
+  wave_clip = (XRectangle){ x0, y0, width, wave_height };
+  get_glyph_string_clip_rect (s, &w32_string_clip);
+  CONVERT_TO_XRECT (string_clip, w32_string_clip);
+
+  if (!x_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
+    return;
+
+  hp = CreatePen (PS_SOLID, 0, color);
+  oldhp = SelectObject (s->hdc, hp);
+  CONVERT_FROM_XRECT (final_clip, w32_final_clip);
+  w32_set_clip_rectangle (s->hdc, &w32_final_clip);
+
+  /* Draw the waves */
+
+  x1 = x0 - (x0 % dx);
+  x2 = x1 + dx;
+  odd = (x1/dx) % 2;
+  y1 = y2 = y0;
+
+  if (odd)
+    y1 += dy;
+  else
+    y2 += dy;
+
+  MoveToEx (s->hdc, x1, y1, NULL);
+
+  while (x1 <= xmax)
+    {
+      LineTo (s->hdc, x2, y2);
+      x1  = x2, y1 = y2;
+      x2 += dx, y2 = y0 + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous pen and clipping rectangle(s) */
+  w32_restore_glyph_string_clip (s);
+  SelectObject (s->hdc, oldhp);
+  DeleteObject (hp);
+}
 
 /* Draw a hollow rectangle at the specified position.  */
 void
@@ -2343,60 +2431,74 @@ x_draw_glyph_string (struct glyph_string *s)
       /* Draw underline.  */
       if (s->face->underline_p)
         {
-          unsigned long thickness, position;
-          int y;
-
-          if (s->prev && s->prev->face->underline_p)
+          if (s->face->underline_type == FACE_UNDER_WAVE)
             {
-              /* We use the same underline style as the previous one.  */
-              thickness = s->prev->underline_thickness;
-              position = s->prev->underline_position;
+              COLORREF color;
+
+              if (s->face->underline_defaulted_p)
+                color = s->gc->foreground;
+              else
+                color = s->face->underline_color;
+
+              w32_draw_underwave (s, color);
             }
-          else
+          else if (s->face->underline_type == FACE_UNDER_LINE)
             {
-              /* Get the underline thickness.  Default is 1 pixel.  */
-              if (s->font && s->font->underline_thickness > 0)
-                thickness = s->font->underline_thickness;
+              unsigned long thickness, position;
+              int y;
+
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
               else
-                thickness = 1;
-              if (x_underline_at_descent_line)
-                position = (s->height - thickness) - (s->ybase - s->y);
+                {
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
+
+                         ROUND ((maximum_descent) / 2), with
+                         ROUND (x) = floor (x + 0.5)  */
+
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position =position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                {
+                  w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
+                                 y, s->width, 1);
+                }
               else
                 {
-                /* Get the underline position.  This is the recommended
-                   vertical offset in pixels from the baseline to the top of
-                   the underline.  This is a signed value according to the
-                   specs, and its default is
-
-                   ROUND ((maximum_descent) / 2), with
-                   ROUND (x) = floor (x + 0.5)  */
-
-                if (x_use_underline_position_properties
-                    && s->font && s->font->underline_position >= 0)
-                  position = s->font->underline_position;
-                else if (s->font)
-                  position = (s->font->descent + 1) / 2;
+                  w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
+                                 y, s->width, 1);
                 }
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position =position;
-          y = s->ybase + position;
-          if (s->face->underline_defaulted_p)
-            {
-              w32_fill_area (s->f, s->hdc, s->gc->foreground, s->x,
-                             y, s->width, 1);
-            }
-          else
-            {
-              w32_fill_area (s->f, s->hdc, s->face->underline_color, s->x,
-                             y, s->width, 1);
             }
         }
       /* Draw overline.  */
diff --git a/src/xfaces.c b/src/xfaces.c
index 617097d..8bee2d6 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -320,6 +320,7 @@ static Lisp_Object QCfontset;
 
 Lisp_Object Qnormal;
 Lisp_Object Qbold;
+static Lisp_Object Qline, Qwave;
 static Lisp_Object Qultra_light, Qextra_light, Qlight;
 static Lisp_Object Qsemi_light, Qsemi_bold, Qextra_bold, Qultra_bold;
 static Lisp_Object Qoblique, Qreverse_oblique, Qreverse_italic;
@@ -1889,7 +1890,8 @@ check_lface_attrs (Lisp_Object *attrs)
   xassert (UNSPECIFIEDP (attrs[LFACE_UNDERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_UNDERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_UNDERLINE_INDEX])
-	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX]));
+	   || STRINGP (attrs[LFACE_UNDERLINE_INDEX])
+	   || CONSP (attrs[LFACE_UNDERLINE_INDEX]));
   xassert (UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_OVERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
@@ -2520,7 +2522,8 @@ merge_face_ref (struct frame *f, Lisp_Object face_ref, Lisp_Object *to,
 		{
 		  if (EQ (value, Qt)
 		      || NILP (value)
-		      || STRINGP (value))
+		      || STRINGP (value)
+		      || CONSP (value))
 		    to[LFACE_UNDERLINE_INDEX] = value;
 		  else
 		    err = 1;
@@ -2944,15 +2947,54 @@ FRAME 0 means change the face on all frames, and change the default
     }
   else if (EQ (attr, QCunderline))
     {
-      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
-	if ((SYMBOLP (value)
-	     && !EQ (value, Qt)
-	     && !EQ (value, Qnil))
-	    /* Underline color.  */
-	    || (STRINGP (value)
-		&& SCHARS (value) == 0))
-	  signal_error ("Invalid face underline", value);
-
+      int valid_p = 0;
+      
+      if (UNSPECIFIEDP (value) || IGNORE_DEFFACE_P (value))
+	valid_p = 1;
+      else if (NILP (value) || EQ (value, Qt))
+        valid_p = 1;
+      else if (STRINGP (value) && SCHARS (value) > 0)
+        valid_p = 1;
+      else if (CONSP (value))
+        {
+          Lisp_Object key, val, list;
+
+          list = value;
+          valid_p = 1;
+
+          while (!NILP (CAR_SAFE(list)))
+            {
+              key = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+              val = CAR_SAFE (list);
+              list = CDR_SAFE (list);
+            
+              if(NILP (key) || NILP (val)) 
+                {
+                  valid_p = 0;
+                  break;
+                }
+
+              else if (EQ (key, QCcolor)
+                       && !(EQ (val, Qforeground_color)
+                            || (STRINGP (val) && SCHARS (val) > 0)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+              
+              else if (EQ (key, QCstyle) 
+                       && !(EQ (val, Qline) || EQ (val, Qwave)))
+                {
+                  valid_p = 0;
+                  break;
+                }
+            }
+        }
+      
+      if (!valid_p)
+        signal_error ("Invalid face underline", value);
+      
       old_value = LFACE_UNDERLINE (lface);
       LFACE_UNDERLINE (lface) = value;
     }
@@ -3762,6 +3804,7 @@ Value is nil if ATTR doesn't have a discrete set of valid values.  */)
 
   CHECK_SYMBOL (attr);
 
+  /* XXX: no check for QCbox? */
   if (EQ (attr, QCunderline))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCoverline))
@@ -5563,7 +5606,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 #ifdef HAVE_WINDOW_SYSTEM
   struct face *default_face;
   struct frame *f;
-  Lisp_Object stipple, overline, strike_through, box;
+  Lisp_Object stipple, underline, overline, strike_through, box;
 
   xassert (FRAME_WINDOW_P (cache->f));
 
@@ -5696,29 +5739,76 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 
   /* Text underline, overline, strike-through.  */
 
-  if (EQ (attrs[LFACE_UNDERLINE_INDEX], Qt))
+  underline = attrs[LFACE_UNDERLINE_INDEX];
+  if (EQ (underline, Qt))
     {
       /* Use default color (same as foreground color).  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 1;
       face->underline_color = 0;
     }
-  else if (STRINGP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (STRINGP (underline))
     {
       /* Use specified color.  */
       face->underline_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
       face->underline_defaulted_p = 0;
       face->underline_color
-	= load_color (f, face, attrs[LFACE_UNDERLINE_INDEX],
+	= load_color (f, face, underline,
 		      LFACE_UNDERLINE_INDEX);
     }
-  else if (NILP (attrs[LFACE_UNDERLINE_INDEX]))
+  else if (NILP (underline))
     {
       face->underline_p = 0;
       face->underline_defaulted_p = 0;
       face->underline_color = 0;
     }
-
+  else if (CONSP (underline))
+    {
+      /* `(:color COLOR :style STYLE)'.  
+         STYLE being one of `line' or `wave'. */
+      face->underline_p = 1;
+      face->underline_color = 0;
+      face->underline_defaulted_p = 1;
+      face->underline_type = FACE_UNDER_LINE;
+
+      while (CONSP (underline))
+        {
+          Lisp_Object keyword, value;
+
+          keyword = XCAR (underline);
+          underline = XCDR (underline);
+
+          if (!CONSP (underline))
+            break;
+          value = XCAR (underline);
+          underline = XCDR (underline);
+
+          if (EQ (keyword, QCcolor))
+            {
+              if (EQ (value, Qforeground_color))
+                {
+                  face->underline_defaulted_p = 1;
+                  face->underline_color = 0;
+                }
+              else if (STRINGP (value))
+                {
+                  face->underline_defaulted_p = 0;
+                  face->underline_color = load_color (f, face, value,
+                                                      LFACE_UNDERLINE_INDEX);
+                }
+            }
+          else if (EQ (keyword, QCstyle))
+            {
+              if (EQ (value, Qline))
+                face->underline_type = FACE_UNDER_LINE;
+              else if (EQ (value, Qwave))
+                face->underline_type = FACE_UNDER_WAVE;
+            }
+        }
+    }
+  
   overline = attrs[LFACE_OVERLINE_INDEX];
   if (STRINGP (overline))
     {
@@ -6465,6 +6555,8 @@ syms_of_xfaces (void)
   DEFSYM (QCcolor, ":color");
   DEFSYM (QCline_width, ":line-width");
   DEFSYM (QCstyle, ":style");
+  DEFSYM (Qline, "line");
+  DEFSYM (Qwave, "wave");
   DEFSYM (Qreleased_button, "released-button");
   DEFSYM (Qpressed_button, "pressed-button");
   DEFSYM (Qnormal, "normal");
diff --git a/src/xterm.c b/src/xterm.c
index 4b34d63..a58e366 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2653,6 +2653,65 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = 1;
 }
 
+/*
+   Draw a wavy line under S. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+static void
+x_draw_underwave (struct glyph_string *s)
+{
+  int wave_height = 2, wave_length = 3;
+  int dx, dy, x0, y0, width, x1, y1, x2, y2, odd, xmax;
+  XRectangle wave_clip, string_clip, final_clip;
+
+  dx = wave_length;
+  dy = wave_height - 1;
+  x0 = s->x;
+  y0 = s->ybase + 1;
+  width = s->width;
+  xmax = x0 + width;
+
+  /* Find and set clipping rectangle */
+
+  wave_clip = (XRectangle){ x0, y0, width, wave_height };
+  get_glyph_string_clip_rect (s, &string_clip);
+
+  if (!x_intersect_rectangles (&wave_clip, &string_clip, &final_clip))
+    return;
+
+  XSetClipRectangles (s->display, s->gc, 0, 0, &final_clip, 1, Unsorted);
+
+  /* Draw the waves */
+
+  x1 = x0 - (x0 % dx);
+  x2 = x1 + dx;
+  odd = (x1/dx) % 2;
+  y1 = y2 = y0;
+
+  if (odd)
+    y1 += dy;
+  else
+    y2 += dy;
+
+  while (x1 <= xmax)
+    {
+      XDrawLine (s->display, s->window, s->gc, x1, y1, x2, y2);
+      x1  = x2, y1 = y2;
+      x2 += dx, y2 = y0 + odd*dy;
+      odd = !odd;
+    }
+
+  /* Restore previous clipping rectangle(s) */
+  XSetClipRectangles (s->display, s->gc, 0, 0, s->clip, s->num_clips, Unsorted);
+}
+
 
 /* Draw glyph string S.  */
 
@@ -2755,68 +2814,83 @@ x_draw_glyph_string (struct glyph_string *s)
     {
       /* Draw underline.  */
       if (s->face->underline_p)
-	{
-	  unsigned long thickness, position;
-	  int y;
-
-	  if (s->prev && s->prev->face->underline_p)
-	    {
-	      /* We use the same underline style as the previous one.  */
-	      thickness = s->prev->underline_thickness;
-	      position = s->prev->underline_position;
-	    }
-	  else
-	    {
-	      /* Get the underline thickness.  Default is 1 pixel.  */
-	      if (s->font && s->font->underline_thickness > 0)
-		thickness = s->font->underline_thickness;
-	      else
-		thickness = 1;
-	      if (x_underline_at_descent_line)
-		position = (s->height - thickness) - (s->ybase - s->y);
-	      else
-		{
-		  /* Get the underline position.  This is the recommended
-		     vertical offset in pixels from the baseline to the top of
-		     the underline.  This is a signed value according to the
-		     specs, and its default is
-
-		     ROUND ((maximum descent) / 2), with
-		     ROUND(x) = floor (x + 0.5)  */
-
-		  if (x_use_underline_position_properties
-		      && s->font && s->font->underline_position >= 0)
-		    position = s->font->underline_position;
-		  else if (s->font)
-		    position = (s->font->descent + 1) / 2;
-		  else
-		    position = underline_minimum_offset;
-		}
-	      position = max (position, underline_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 + thickness)
-	    thickness = (s->y + s->height) - (s->ybase + position);
-	  s->underline_thickness = thickness;
-	  s->underline_position = position;
-	  y = s->ybase + position;
-	  if (s->face->underline_defaulted_p)
-	    XFillRectangle (s->display, s->window, s->gc,
-			    s->x, y, s->width, thickness);
-	  else
-	    {
-	      XGCValues xgcv;
-	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
-	      XSetForeground (s->display, s->gc, s->face->underline_color);
-	      XFillRectangle (s->display, s->window, s->gc,
-			      s->x, y, s->width, thickness);
-	      XSetForeground (s->display, s->gc, xgcv.foreground);
-	    }
-	}
+        {
+          if (s->face->underline_type == FACE_UNDER_WAVE)
+            {
+              if (s->face->underline_defaulted_p)
+                x_draw_underwave (s);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  x_draw_underwave (s);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+          else if (s->face->underline_type == FACE_UNDER_LINE)
+            {
+              unsigned long thickness, position;
+              int y;
 
+              if (s->prev && s->prev->face->underline_p)
+                {
+                  /* We use the same underline style as the previous one.  */
+                  thickness = s->prev->underline_thickness;
+                  position = s->prev->underline_position;
+                }
+              else
+                {
+                  /* Get the underline thickness.  Default is 1 pixel.  */
+                  if (s->font && s->font->underline_thickness > 0)
+                    thickness = s->font->underline_thickness;
+                  else
+                    thickness = 1;
+                  if (x_underline_at_descent_line)
+                    position = (s->height - thickness) - (s->ybase - s->y);
+                  else
+                    {
+                      /* Get the underline position.  This is the recommended
+                         vertical offset in pixels from the baseline to the top of
+                         the underline.  This is a signed value according to the
+                         specs, and its default is
+
+                         ROUND ((maximum descent) / 2), with
+                         ROUND(x) = floor (x + 0.5)  */
+
+                      if (x_use_underline_position_properties
+                          && s->font && s->font->underline_position >= 0)
+                        position = s->font->underline_position;
+                      else if (s->font)
+                        position = (s->font->descent + 1) / 2;
+                      else
+                        position = underline_minimum_offset;
+                    }
+                  position = max (position, underline_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 + thickness)
+                thickness = (s->y + s->height) - (s->ybase + position);
+              s->underline_thickness = thickness;
+              s->underline_position = position;
+              y = s->ybase + position;
+              if (s->face->underline_defaulted_p)
+                XFillRectangle (s->display, s->window, s->gc,
+                                s->x, y, s->width, thickness);
+              else
+                {
+                  XGCValues xgcv;
+                  XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+                  XSetForeground (s->display, s->gc, s->face->underline_color);
+                  XFillRectangle (s->display, s->window, s->gc,
+                                  s->x, y, s->width, thickness);
+                  XSetForeground (s->display, s->gc, xgcv.foreground);
+                }
+            }
+        }
       /* Draw overline.  */
       if (s->face->overline_p)
 	{

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

The patch is accessible on the bug tracker at this url:
http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10736

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

On Mon, Feb 6, 2012 at 6:20 PM, Aurélien Aptel
<aurelien.aptel+emacs@gmail.com> wrote:
> The patch is accessible on the bug tracker at this url:
> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10736

Any news about the patch? Anything that needs fixing? I would be glad
to help if there is something I can do.
Can we expect to see this in Emacs 24? It's one of those easy-to-show
new graphic features ;)

Re: [patch] add "underwave" face attribute

[Stefan Monnier]

>> The patch is accessible on the bug tracker at this url:
>> http://debbugs.gnu.org/cgi/bugreport.cgi?bug=10736

> Any news about the patch? Anything that needs fixing? I would be glad
> to help if there is something I can do.
> Can we expect to see this in Emacs 24? It's one of those easy-to-show
> new graphic features ;)

We've been in feature freeze for the pretest of Emacs-24.1 for
a while now.  So you're basically waiting for us to re-open the trunk
for general modifications (which might then make it into Emacs-24.2).


        Stefan

Re: [patch] add "underwave" face attribute

[Leo]

On 2012-02-13 00:59 +0800, Stefan Monnier wrote:
>> Any news about the patch? Anything that needs fixing? I would be glad
>> to help if there is something I can do.
>> Can we expect to see this in Emacs 24? It's one of those easy-to-show
>> new graphic features ;)
>
> We've been in feature freeze for the pretest of Emacs-24.1 for
> a while now.  So you're basically waiting for us to re-open the trunk
> for general modifications (which might then make it into Emacs-24.2).

I am looking forward to this patch. It is made for highlighting
errors/warnings for syntax validators, noticeable and not intrusive.

Leo

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

> We've been in feature freeze for the pretest of Emacs-24.1 for
> a while now.  So you're basically waiting for us to re-open the trunk
> for general modifications (which might then make it into Emacs-24.2).

Alright, thanks!

[patch] add "underwave" face attribute

[Aurélien Aptel]

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

Hi all,

I implemented a new face attribute "underwave". It does exactly what
this thread[1] asked for.
It only works on X11 currently. I don't have a windows/mac machine to
port this feature but it should be easy to do: just port the changes
made to xterm.c to other *term.c. I've basically followed whatever
"overline" was doing so it should be ok.

I've attached the diff and a screenshot of the splash screen with the
default and link face changed to use underwaving. Note that it looks
good on both X11 core fonts and Xft fonts.

I would love to this this merged in emacs so -- if everything's
alright -- what's the next step?

1: http://thread.gmane.org/gmane.emacs.devel/147873/focus=147905

[-- Attachment #2: underwave.diff --]
[-- Type: text/x-patch, Size: 18217 bytes --]

diff --git a/doc/lispref/display.texi b/doc/lispref/display.texi
index 381eaf6..1aa8712 100644
--- a/doc/lispref/display.texi
+++ b/doc/lispref/display.texi
@@ -2089,6 +2089,10 @@ value @code{nil} means do not underline.
 Whether or not characters should be overlined, and in what color.
 The value is used like that of @code{:underline}.
 
+@item :underwave
+Whether or not characters should be underwaved, and in what color.
+The value is used like that of @code{:underline}.
+
 @item :strike-through
 Whether or not characters should be strike-through, and in what
 color.  The value is used like that of @code{:underline}.
diff --git a/lisp/cus-face.el b/lisp/cus-face.el
index d725111..e7f2088 100644
--- a/lisp/cus-face.el
+++ b/lisp/cus-face.el
@@ -145,6 +145,13 @@
 	     (const :tag "On" t)
 	     (color :tag "Colored")))
 
+    (:underwave
+     (choice :tag "Underwave"
+	     :help-echo "Control text underwaving."
+	     (const :tag "Off" nil)
+	     (const :tag "On" t)
+	     (color :tag "Colored")))
+
     (:strike-through
      (choice :tag "Strike-through"
 	     :help-echo "Control text strike-through."
diff --git a/lisp/custom.el b/lisp/custom.el
index 132576a..deaaec5 100644
--- a/lisp/custom.el
+++ b/lisp/custom.el
@@ -368,7 +368,7 @@ ATTS is a list of face attributes followed by their values:
   (ATTR VALUE ATTR VALUE...)
 
 The possible attributes are `:family', `:width', `:height', `:weight',
-`:slant', `:underline', `:overline', `:strike-through', `:box',
+`:slant', `:underline', `:overline', `:underwave', `:strike-through', `:box',
 `:foreground', `:background', `:stipple', `:inverse-video', and `:inherit'.
 
 DISPLAY can be `default' (only in the first element), the symbol
diff --git a/lisp/faces.el b/lisp/faces.el
index 5d406ad..a3b8309 100644
--- a/lisp/faces.el
+++ b/lisp/faces.el
@@ -272,6 +272,7 @@ If FRAME is omitted or nil, use the selected frame."
     (:foreground (".attributeForeground" . "Face.AttributeForeground"))
     (:background (".attributeBackground" . "Face.AttributeBackground"))
     (:overline (".attributeOverline" . "Face.AttributeOverline"))
+    (:underwave (".attributeUnderwave" . "Face.AttributeUnderwave"))
     (:strike-through (".attributeStrikeThrough" . "Face.AttributeStrikeThrough"))
     (:box (".attributeBox" . "Face.AttributeBox"))
     (:underline (".attributeUnderline" . "Face.AttributeUnderline"))
@@ -628,6 +629,13 @@ VALUE is t, overline with foreground color of the face.  If VALUE is a
 string, overline with that color.  If VALUE is nil, explicitly don't
 overline.
 
+`:underwave'
+
+VALUE specifies whether characters in FACE should be underwaved.  If
+VALUE is t, underwave with foreground color of the face.  If VALUE is a
+string, underwave with that color.  If VALUE is nil, explicitly don't
+underwave.
+
 `:strike-through'
 
 VALUE specifies whether characters in FACE should be drawn with a line
@@ -992,7 +1000,7 @@ an integer value."
 	   (:inverse-video
 	    (mapcar #'(lambda (x) (cons (symbol-name x) x))
 		    (internal-lisp-face-attribute-values attribute)))
-           ((:underline :overline :strike-through :box)
+           ((:underline :overline :underwave :strike-through :box)
             (if (window-system frame)
                 (nconc (mapcar #'(lambda (x) (cons (symbol-name x) x))
                                (internal-lisp-face-attribute-values attribute))
@@ -1034,6 +1042,7 @@ an integer value."
     (:slant . "slant")
     (:underline . "underline")
     (:overline . "overline")
+    (:underwave . "underwave")
     (:strike-through . "strike-through")
     (:box . "box")
     (:inverse-video . "inverse-video display")
@@ -1323,6 +1332,7 @@ If FRAME is omitted or nil, use the selected frame."
 		  (:background . "Background")
 		  (:underline . "Underline")
 		  (:overline . "Overline")
+		  (:underwave . "Underwave")
 		  (:strike-through . "Strike-through")
 		  (:box . "Box")
 		  (:inverse-video . "Inverse")
diff --git a/src/dispextern.h b/src/dispextern.h
index 2c59f4f..7c93a11 100644
--- a/src/dispextern.h
+++ b/src/dispextern.h
@@ -1484,6 +1484,7 @@ enum lface_attribute_index
   LFACE_BACKGROUND_INDEX,
   LFACE_STIPPLE_INDEX,
   LFACE_OVERLINE_INDEX,
+  LFACE_UNDERWAVE_INDEX,
   LFACE_STRIKE_THROUGH_INDEX,
   LFACE_BOX_INDEX,
   LFACE_FONT_INDEX,
@@ -1551,9 +1552,10 @@ struct face
   /* Pixel value or color index of underline color.  */
   unsigned long underline_color;
 
-  /* Pixel value or color index of overlined, strike-through, or box
-     color.  */
+  /* Pixel value or color index of overlined, underwaved,
+     strike-through, or box color.  */
   unsigned long overline_color;
+  unsigned long underwave_color;
   unsigned long strike_through_color;
   unsigned long box_color;
 
@@ -1586,9 +1588,10 @@ struct face
   unsigned use_box_color_for_shadows_p : 1;
 
   /* Non-zero if text in this face should be underlined, overlined,
-     strike-through or have a box drawn around it.  */
+     underwaved, strike-through or have a box drawn around it.  */
   unsigned underline_p : 1;
   unsigned overline_p : 1;
+  unsigned underwave_p : 1;
   unsigned strike_through_p : 1;
 
   /* 1 means that the colors specified for this face could not be
@@ -1606,6 +1609,7 @@ struct face
      attribute or that the specified color couldn't be loaded.
      Use the foreground color when drawing in that case. */
   unsigned overline_color_defaulted_p : 1;
+  unsigned underwave_color_defaulted_p : 1;
   unsigned strike_through_color_defaulted_p : 1;
   unsigned box_color_defaulted_p : 1;
 
diff --git a/src/xfaces.c b/src/xfaces.c
index 617097d..4cfb061 100644
--- a/src/xfaces.c
+++ b/src/xfaces.c
@@ -53,10 +53,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    13. Whether or not characters should be strike-through, and in what
    color.
 
-   14. Whether or not a box should be drawn around characters, the box
+   14. Whether or not characters should be underwaved, and in what
+   color.
+
+   15. Whether or not a box should be drawn around characters, the box
    type, and, for simple boxes, in what color.
 
-   15. Font-spec, or nil.  This is a special attribute.
+   16. Font-spec, or nil.  This is a special attribute.
 
    A font-spec is a collection of font attributes (specs).
 
@@ -68,13 +71,13 @@ along with GNU Emacs.  If not, see <http://www.gnu.org/licenses/>.  */
    On the other hand, if one of the other font-related attributes are
    specified, the corresponding specs in this attribute is set to nil.
 
-   15. A face name or list of face names from which to inherit attributes.
+   17. A face name or list of face names from which to inherit attributes.
 
-   16. A specified average font width, which is invisible from Lisp,
+   18. A specified average font width, which is invisible from Lisp,
    and is used to ensure that a font specified on the command line,
    for example, can be matched exactly.
 
-   17. A fontset name.  This is another special attribute.
+   19. A fontset name.  This is another special attribute.
 
    A fontset is a mappings from characters to font-specs, and the
    specs overwrite the font-spec in the 14th attribute.
@@ -313,7 +316,7 @@ Lisp_Object QCforeground, QCbackground;
 Lisp_Object QCwidth;
 static Lisp_Object QCfont, QCbold, QCitalic;
 static Lisp_Object QCreverse_video;
-static Lisp_Object QCoverline, QCstrike_through, QCbox, QCinherit;
+static Lisp_Object QCoverline, QCunderwave, QCstrike_through, QCbox, QCinherit;
 static Lisp_Object QCfontset;
 
 /* Symbols used for attribute values.  */
@@ -1311,11 +1314,11 @@ COLOR must be a valid color name.  */)
 /* Load color with name NAME for use by face FACE on frame F.
    TARGET_INDEX must be one of LFACE_FOREGROUND_INDEX,
    LFACE_BACKGROUND_INDEX, LFACE_UNDERLINE_INDEX, LFACE_OVERLINE_INDEX,
-   LFACE_STRIKE_THROUGH_INDEX, or LFACE_BOX_INDEX.  Value is the
-   pixel color.  If color cannot be loaded, display a message, and
-   return the foreground, background or underline color of F, but
-   record that fact in flags of the face so that we don't try to free
-   these colors.  */
+   LFACE_UNDERWAVE_INDEX, LFACE_STRIKE_THROUGH_INDEX, or LFACE_BOX_INDEX.
+   Value is the pixel color.  If color cannot be loaded, display a
+   message, and return the foreground, background or underline color
+   of F, but record that fact in flags of the face so that we don't
+   try to free these colors.  */
 
 unsigned long
 load_color (struct frame *f, struct face *face, Lisp_Object name,
@@ -1328,6 +1331,7 @@ load_color (struct frame *f, struct face *face, Lisp_Object name,
 	   || target_index == LFACE_BACKGROUND_INDEX
 	   || target_index == LFACE_UNDERLINE_INDEX
 	   || target_index == LFACE_OVERLINE_INDEX
+	   || target_index == LFACE_UNDERWAVE_INDEX
 	   || target_index == LFACE_STRIKE_THROUGH_INDEX
 	   || target_index == LFACE_BOX_INDEX);
 
@@ -1359,6 +1363,11 @@ load_color (struct frame *f, struct face *face, Lisp_Object name,
 	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
 	  break;
 
+	case LFACE_UNDERWAVE_INDEX:
+	  face->underwave_color_defaulted_p = 1;
+	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
+	  break;
+
 	case LFACE_STRIKE_THROUGH_INDEX:
 	  face->strike_through_color_defaulted_p = 1;
 	  color.pixel = FRAME_FOREGROUND_PIXEL (f);
@@ -1477,6 +1486,13 @@ free_face_colors (struct frame *f, struct face *face)
       IF_DEBUG (--ncolors_allocated);
     }
 
+  if (face->underwave_p
+      && !face->underwave_color_defaulted_p)
+    {
+      x_free_colors (f, &face->underwave_color, 1);
+      IF_DEBUG (--ncolors_allocated);
+    }
+
   if (face->strike_through_p
       && !face->strike_through_color_defaulted_p)
     {
@@ -1842,6 +1858,7 @@ the WIDTH times as wide as FACE on FRAME.  */)
 #define LFACE_STIPPLE(LFACE)	    AREF ((LFACE), LFACE_STIPPLE_INDEX)
 #define LFACE_SWIDTH(LFACE)	    AREF ((LFACE), LFACE_SWIDTH_INDEX)
 #define LFACE_OVERLINE(LFACE)	    AREF ((LFACE), LFACE_OVERLINE_INDEX)
+#define LFACE_UNDERWAVE(LFACE)	    AREF ((LFACE), LFACE_UNDERWAVE_INDEX)
 #define LFACE_STRIKE_THROUGH(LFACE) AREF ((LFACE), LFACE_STRIKE_THROUGH_INDEX)
 #define LFACE_BOX(LFACE)	    AREF ((LFACE), LFACE_BOX_INDEX)
 #define LFACE_FONT(LFACE)	    AREF ((LFACE), LFACE_FONT_INDEX)
@@ -1894,6 +1911,10 @@ check_lface_attrs (Lisp_Object *attrs)
 	   || IGNORE_DEFFACE_P (attrs[LFACE_OVERLINE_INDEX])
 	   || SYMBOLP (attrs[LFACE_OVERLINE_INDEX])
 	   || STRINGP (attrs[LFACE_OVERLINE_INDEX]));
+  xassert (UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
+	   || IGNORE_DEFFACE_P (attrs[LFACE_UNDERWAVE_INDEX])
+	   || SYMBOLP (attrs[LFACE_UNDERWAVE_INDEX])
+	   || STRINGP (attrs[LFACE_UNDERWAVE_INDEX]));
   xassert (UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	   || IGNORE_DEFFACE_P (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	   || SYMBOLP (attrs[LFACE_STRIKE_THROUGH_INDEX])
@@ -2534,6 +2555,15 @@ merge_face_ref (struct frame *f, Lisp_Object face_ref, Lisp_Object *to,
 		  else
 		    err = 1;
 		}
+	      else if (EQ (keyword, QCunderwave))
+		{
+		  if (EQ (value, Qt)
+		      || NILP (value)
+		      || STRINGP (value))
+		    to[LFACE_UNDERWAVE_INDEX] = value;
+		  else
+		    err = 1;
+		}
 	      else if (EQ (keyword, QCstrike_through))
 		{
 		  if (EQ (value, Qt)
@@ -2970,6 +3000,20 @@ FRAME 0 means change the face on all frames, and change the default
       old_value = LFACE_OVERLINE (lface);
       LFACE_OVERLINE (lface) = value;
     }
+  else if (EQ (attr, QCunderwave))
+    {
+      if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
+	if ((SYMBOLP (value)
+	     && !EQ (value, Qt)
+	     && !EQ (value, Qnil))
+	    /* Underwave color.  */
+	    || (STRINGP (value)
+		&& SCHARS (value) == 0))
+	  signal_error ("Invalid face underwave", value);
+
+      old_value = LFACE_UNDERWAVE (lface);
+      LFACE_UNDERWAVE (lface) = value;
+    }
   else if (EQ (attr, QCstrike_through))
     {
       if (!UNSPECIFIEDP (value) && !IGNORE_DEFFACE_P (value))
@@ -3511,6 +3555,7 @@ DEFUN ("internal-set-lisp-face-attribute-from-resource",
     value = face_boolean_x_resource_value (value, 1);
   else if (EQ (attr, QCunderline)
 	   || EQ (attr, QCoverline)
+	   || EQ (attr, QCunderwave)
 	   || EQ (attr, QCstrike_through))
     {
       Lisp_Object boolean_value;
@@ -3720,6 +3765,8 @@ frames).  If FRAME is omitted or nil, use the selected frame.  */)
     value = LFACE_UNDERLINE (lface);
   else if (EQ (keyword, QCoverline))
     value = LFACE_OVERLINE (lface);
+  else if (EQ (keyword, QCunderwave))
+    value = LFACE_UNDERWAVE (lface);
   else if (EQ (keyword, QCstrike_through))
     value = LFACE_STRIKE_THROUGH (lface);
   else if (EQ (keyword, QCbox))
@@ -3766,6 +3813,8 @@ Value is nil if ATTR doesn't have a discrete set of valid values.  */)
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCoverline))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
+  else if (EQ (attr, QCunderwave))
+    result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCstrike_through))
     result = Fcons (Qt, Fcons (Qnil, Qnil));
   else if (EQ (attr, QCinverse_video) || EQ (attr, QCreverse_video))
@@ -4800,6 +4849,9 @@ x_supports_face_attributes_p (struct frame *f, Lisp_Object *attrs,
       || (!UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_OVERLINE_INDEX],
 				def_attrs[LFACE_OVERLINE_INDEX]))
+      || (!UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
+	  && face_attr_equal_p (attrs[LFACE_UNDERWAVE_INDEX],
+				def_attrs[LFACE_UNDERWAVE_INDEX]))
       || (!UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
 	  && face_attr_equal_p (attrs[LFACE_STRIKE_THROUGH_INDEX],
 				def_attrs[LFACE_STRIKE_THROUGH_INDEX]))
@@ -4902,6 +4954,7 @@ tty_supports_face_attributes_p (struct frame *f, Lisp_Object *attrs,
       || !UNSPECIFIEDP (attrs[LFACE_HEIGHT_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_SWIDTH_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_OVERLINE_INDEX])
+      || !UNSPECIFIEDP (attrs[LFACE_UNDERWAVE_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_STRIKE_THROUGH_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_BOX_INDEX])
       || !UNSPECIFIEDP (attrs[LFACE_SLANT_INDEX]))
@@ -5370,6 +5423,9 @@ realize_default_face (struct frame *f)
   if (UNSPECIFIEDP (LFACE_OVERLINE (lface)))
     LFACE_OVERLINE (lface) = Qnil;
 
+  if (UNSPECIFIEDP (LFACE_UNDERWAVE (lface)))
+    LFACE_UNDERWAVE (lface) = Qnil;
+
   if (UNSPECIFIEDP (LFACE_STRIKE_THROUGH (lface)))
     LFACE_STRIKE_THROUGH (lface) = Qnil;
 
@@ -5563,7 +5619,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 #ifdef HAVE_WINDOW_SYSTEM
   struct face *default_face;
   struct frame *f;
-  Lisp_Object stipple, overline, strike_through, box;
+  Lisp_Object stipple, overline, underwave, strike_through, box;
 
   xassert (FRAME_WINDOW_P (cache->f));
 
@@ -5694,7 +5750,7 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
 	}
     }
 
-  /* Text underline, overline, strike-through.  */
+  /* Text underline, overline, underwaved, strike-through.  */
 
   if (EQ (attrs[LFACE_UNDERLINE_INDEX], Qt))
     {
@@ -5734,6 +5790,21 @@ realize_x_face (struct face_cache *cache, Lisp_Object *attrs)
       face->overline_p = 1;
     }
 
+  underwave = attrs[LFACE_UNDERWAVE_INDEX];
+  if (STRINGP (underwave))
+    {
+      face->underwave_color
+	= load_color (f, face, attrs[LFACE_UNDERWAVE_INDEX],
+		      LFACE_UNDERWAVE_INDEX);
+      face->underwave_p = 1;
+    }
+  else if (EQ (underwave, Qt))
+    {
+      face->underwave_color = face->foreground;
+      face->underwave_color_defaulted_p = 1;
+      face->underwave_p = 1;
+    }
+
   strike_through = attrs[LFACE_STRIKE_THROUGH_INDEX];
   if (STRINGP (strike_through))
     {
@@ -6457,6 +6528,7 @@ syms_of_xfaces (void)
   DEFSYM (QCbold, ":bold");
   DEFSYM (QCitalic, ":italic");
   DEFSYM (QCoverline, ":overline");
+  DEFSYM (QCunderwave, ":underwave");
   DEFSYM (QCstrike_through, ":strike-through");
   DEFSYM (QCbox, ":box");
   DEFSYM (QCinherit, ":inherit");
diff --git a/src/xterm.c b/src/xterm.c
index 4b34d63..2e9afb4 100644
--- a/src/xterm.c
+++ b/src/xterm.c
@@ -2653,6 +2653,48 @@ x_draw_stretch_glyph_string (struct glyph_string *s)
   s->background_filled_p = 1;
 }
 
+/*
+   Draw a wavy line. The wave fills wave_height pixels from y0.
+
+                    x0         wave_length = 2
+                                 --
+                y0   *   *   *   *   *
+                     |* * * * * * * * *
+    wave_height = 3  | *   *   *   *
+
+*/
+
+static void
+x_draw_underwave (Display *dpy, Window win, GC gc,
+                  unsigned long x0, unsigned long y0,
+                  unsigned long width,
+                  unsigned long wave_height, unsigned long wave_length)
+{
+  unsigned long dx = wave_length, dy = wave_height-1;
+  unsigned long x1, y1, x2, y2, i, times = width/dx;
+
+  for (i = 0; i < times; i++)
+    {
+      x1 = x0 + dx * i;
+      y1 = y0 + dy * (i%2);
+      x2 = x0 + dx * (i+1);
+      y2 = y0 + dy * ((i+1)%2);
+      XDrawLine(dpy, win, gc, x1, y1, x2, y2);
+    }
+
+  /* Draw remaining space */
+  if (x2 < x0+width)
+    {
+      unsigned long y = wave_height/(double)wave_length
+                        * (double)(x0 + width - x2);
+      x1 = x2;
+      y1 = y2;
+      x2 = x0 + width;
+      y2 = y0 + ((times % 2) ? wave_height - y : y);
+      XDrawLine(dpy, win, gc, x1, y1, x2, y2);
+    }
+}
+
 
 /* Draw glyph string S.  */
 
@@ -2836,6 +2878,25 @@ x_draw_glyph_string (struct glyph_string *s)
 	    }
 	}
 
+      /* Draw underwave.  */
+      if (s->face->underwave_p)
+	{
+          unsigned long h = 2, l = 3, y = s->ybase + 1;
+
+	  if (s->face->underwave_color_defaulted_p)
+	    x_draw_underwave (s->display, s->window, s->gc, s->x, y,
+                              s->width, h, l);
+	  else
+	    {
+	      XGCValues xgcv;
+	      XGetGCValues (s->display, s->gc, GCForeground, &xgcv);
+	      XSetForeground (s->display, s->gc, s->face->underwave_color);
+	      x_draw_underwave (s->display, s->window, s->gc, s->x, y,
+                                s->width, h, l);
+	      XSetForeground (s->display, s->gc, xgcv.foreground);
+	    }
+	}
+
       /* Draw strike-through.  */
       if (s->face->strike_through_p)
 	{

[-- Attachment #3: sample.png --]
[-- Type: image/png, Size: 53320 bytes --]

Re: [patch] add "underwave" face attribute

[Leo]

On 2012-01-27 01:55 +0800, Aurélien Aptel wrote:
> I implemented a new face attribute "underwave". It does exactly what
> this thread[1] asked for.
> It only works on X11 currently. I don't have a windows/mac machine to
> port this feature but it should be easy to do: just port the changes
> made to xterm.c to other *term.c. I've basically followed whatever
> "overline" was doing so it should be ok.

Thank you for taking the trouble to implement it. Looks nice.

> I've attached the diff and a screenshot of the splash screen with the
> default and link face changed to use underwaving. Note that it looks
> good on both X11 core fonts and Xft fonts.
>
> I would love to this this merged in emacs so -- if everything's
> alright -- what's the next step?

Probably submit the patch to the bug tracker M-x report-emacs-bug. Get
someone to review it.

Thanks again.

Leo

Re: [patch] add "underwave" face attribute

[Andreas Schwab]

Aurélien Aptel <aurelien.aptel@gmail.com> writes:

> I've attached the diff and a screenshot of the splash screen with the
> default and link face changed to use underwaving. Note that it looks
> good on both X11 core fonts and Xft fonts.

It doesn't yet look good on variable width fonts.  The underwave really
needs to be drawn independent of the characters.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

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

On Fri, Jan 27, 2012 at 11:25 AM, Andreas Schwab <schwab@linux-m68k.org> wrote:
> Aurélien Aptel <aurelien.aptel@gmail.com> writes:
>
>> I've attached the diff and a screenshot of the splash screen with the
>> default and link face changed to use underwaving. Note that it looks
>> good on both X11 core fonts and Xft fonts.
>
> It doesn't yet look good on variable width fonts.  The underwave really
> needs to be drawn independent of the characters.

I've tested every font available on my system and it looks good. I've
attached a screenshot using bitstream vera.
Could you post a font name and a file that doesn't look good?

[-- Attachment #2: variable-width.png --]
[-- Type: image/png, Size: 68551 bytes --]

Re: [patch] add "underwave" face attribute

[Andreas Schwab]

Aurélien Aptel <aurelien.aptel@gmail.com> writes:

> Could you post a font name and a file that doesn't look good?

Just try C-h h and put an underwave over it.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [patch] add "underwave" face attribute

[Aurélien Aptel]

I think you are nitpicking. It looks good enough.

Re: [patch] add "underwave" face attribute

[Andreas Schwab]

The wave is broken in many places.  The phase should always be computed
relative to the left margin.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."