bug#10736: Add "underwave" face attribute

bug#10736: Add "underwave" face attribute

[Aurélien Aptel]

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

Changelog
=========

2012-02-06  Aurélien Aptel  <aurelien.aptel@gmail.com>

	Add underwave face attribute for X11, W32 and NextStep.


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 wave is computed from the distance to the left margin so that
  there is no artifact when drawing 2 consecutive waves.
* The drawing is clipped in order to not overlap.

  static void
  x_draw_underwave (struct glyph_string *s)

* 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 --

Same change made to src/xterm.c, basically.
Just replaced line drawing primitive by the system one.

* Juanma Barranquero helped on the W32 port.
* Alp Aker helped on the NextStep port.

-- 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-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)
 	{

bug#10736: Add "underwave" face attribute

[Lars Magne Ingebrigtsen]

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

> 2012-02-06  Aurélien Aptel  <aurelien.aptel@gmail.com>
>
> 	Add underwave face attribute for X11, W32 and NextStep.
>
> 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.

The screen shots look really good, but I'm not familiar enough with the
C level face stuff to judge that.

Looking at the mailing list, it seems like it was agreed that this is
useful, and that this interface is correct.  Since we're in Emacs 24.2
territory now, should this go in?

-- 
(domestic pets only, the antidote for overdose, milk.)
  bloggy blog http://lars.ingebrigtsen.no/

bug#10736: Add "underwave" face attribute

[Stefan Monnier]

> Looking at the mailing list, it seems like it was agreed that this is
> useful, and that this interface is correct.  Since we're in Emacs 24.2
> territory now, should this go in?

Assuming the code is OK, yes.   I'd like to hear someone's opinion on
the code, tho (Jan, maybe?).


        Stefan

bug#10736: Add "underwave" face attribute

[Alp Aker]

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

On Thu, Apr 12, 2012 at 5:57 PM, Stefan Monnier
<monnier@iro.umontreal.ca> wrote:
> Assuming the code is OK, yes.   I'd like to hear someone's opinion on
> the code, tho (Jan, maybe?).

Looking afresh at the small bit I contributed for ns_draw_underwave,
it seems to me that that function does some unnecessary work with
clipping rectangles, so I'd like to offer a slightly simpler version
(revised patch file attached).

(In explanation of this change:  If I understand the NS port's drawing
routine correctly, at the time this function is called the current
clipping path has already been restricted to the glyph string's clip
rects.  Since NSRectClip can only further restrict the current
clipping path, there's no need explicitly to invoke the glyph string's
clip rects at this point.  Indeed, I suspect the NS version could get
away without worrying about clipping at all here, but I'm not as
certain of this latter point.)

[-- Attachment #2: underwave-clip-4.patch --]
[-- Type: application/octet-stream, Size: 31796 bytes --]

=== modified file 'doc/lispref/display.texi'
--- doc/lispref/display.texi	2012-04-04 07:54:02 +0000
+++ doc/lispref/display.texi	2012-04-12 23:56:39 +0000
@@ -2123,10 +2123,32 @@
 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.

=== modified file 'lisp/cus-face.el'
--- lisp/cus-face.el	2012-01-19 07:21:25 +0000
+++ lisp/cus-face.el	2012-04-12 23:56:39 +0000
@@ -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"

=== modified file 'lisp/faces.el'
--- lisp/faces.el	2012-04-09 13:05:48 +0000
+++ lisp/faces.el	2012-04-12 23:56:39 +0000
@@ -613,10 +613,21 @@
 
 `: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'
 

=== modified file 'src/dispextern.h'
--- src/dispextern.h	2012-03-26 05:43:05 +0000
+++ src/dispextern.h	2012-04-12 23:56:39 +0000
@@ -1510,6 +1510,13 @@
   FACE_SUNKEN_BOX
 };
 
+/* Underline type. */
+
+enum face_underline_type
+{
+  FACE_UNDER_LINE,
+  FACE_UNDER_WAVE
+};
 
 /* Structure describing a realized face.
 
@@ -1585,6 +1592,9 @@
      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;

=== modified file 'src/nsterm.m'
--- src/nsterm.m	2012-02-04 15:10:54 +0000
+++ src/nsterm.m	2012-04-13 00:02:50 +0000
@@ -2598,6 +2598,60 @@
   return n;
 }
 
+/* --------------------------------------------------------------------
+   Draw a wavy line under glyph string s. The wave fills wave_height
+   pixels from y.
+
+                    x          wave_length = 3
+                                 --
+                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;
+
+  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);
+  [[NSGraphicsContext currentContext] saveGraphicsState];
+  NSRectClip (waveClip);
+
+  /* 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 +2665,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)

=== modified file 'src/w32term.c'
--- src/w32term.c	2012-04-09 13:05:48 +0000
+++ src/w32term.c	2012-04-12 23:56:39 +0000
@@ -309,6 +309,94 @@
     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 @@
       /* 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;
-                }
-	      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);
+          if (s->face->underline_type == FACE_UNDER_WAVE)
+            {
+              COLORREF color;
+
+              if (s->face->underline_defaulted_p)
+                color = s->gc->foreground;
+              else
+                color = s->face->underline_color;
+
+              w32_draw_underwave (s, color);
+            }
+          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;
+                    }
+                  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.  */

=== modified file 'src/xfaces.c'
--- src/xfaces.c	2012-04-09 13:05:48 +0000
+++ src/xfaces.c	2012-04-12 23:56:39 +0000
@@ -320,6 +320,7 @@
 
 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 @@
   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 @@
 		{
 		  if (EQ (value, Qt)
 		      || NILP (value)
-		      || STRINGP (value))
+		      || STRINGP (value)
+		      || CONSP (value))
 		    to[LFACE_UNDERLINE_INDEX] = value;
 		  else
 		    err = 1;
@@ -2944,15 +2947,54 @@
     }
   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 @@
 
   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 @@
 #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 @@
 
   /* 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 @@
   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");

=== modified file 'src/xterm.c'
--- src/xterm.c	2012-04-09 13:05:48 +0000
+++ src/xterm.c	2012-04-12 23:56:39 +0000
@@ -2665,6 +2665,65 @@
   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.  */
 
@@ -2767,68 +2826,83 @@
     {
       /* 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)
 	{

bug#10736: Add "underwave" face attribute

[Aurélien Aptel]

On Fri, Apr 13, 2012 at 2:49 AM, Alp Aker <alptekin.aker@gmail.com> wrote:
> Looking afresh at the small bit I contributed for ns_draw_underwave,
> it seems to me that that function does some unnecessary work with
> clipping rectangles, so I'd like to offer a slightly simpler version
> (revised patch file attached).

I agree. Most of this is necessary on X11 because the current clipping
region cannot be retrieved. If I just override it for underwaving,
non-visible text can be underwaved. My solution was to compute the
current clipping region again via get_glyph_string_clip_rect() (which
can be empty if the string is not visible) and intersect it with the
wave clipping region. Then draw the wave and set the glyph string
clipping region again.

> (In explanation of this change:  If I understand the NS port's drawing
> routine correctly, at the time this function is called the current
> clipping path has already been restricted to the glyph string's clip
> rects.  Since NSRectClip can only further restrict the current
> clipping path, there's no need explicitly to invoke the glyph string's

This is correct.

> clip rects at this point.  Indeed, I suspect the NS version could get
> away without worrying about clipping at all here, but I'm not as
> certain of this latter point.)

The underwave is computed from the left margin (every wave starts at
x=0) so that two consecutive underwaved strings appear to be
underwaved by one wave.

At first I did not use the clipping region and computed each time the
next line to draw; but for very small lines (1-2 px) the X11 drawline
primitive doesn't work well. Specifically, unneeded pixels were drawn
at both ends of the wave therefore making the juncture between two
waves visible.

My solution was to clip the region and draw a wavelet before/after when needed.

Concerning the NS port, as I understand drawing primitives work on a
non-discrete plane where integer coordinates falls "between" pixels
resulting in two blurred pixels instead of a filled one. Since I can't
experiment on this platform I was wondering if you could try shifting
every coordinate by .5 to see if it looks better.

bug#10736: Add "underwave" face attribute

[Alp Aker]

On Fri, Apr 13, 2012 at 6:05 AM, Aurélien Aptel
<aurelien.aptel@gmail.com> wrote:
> Concerning the NS port, as I understand drawing primitives work on a
> non-discrete plane where integer coordinates falls "between" pixels
> resulting in two blurred pixels instead of a filled one. Since I can't
> experiment on this platform I was wondering if you could try shifting
> every coordinate by .5 to see if it looks better.

It's not a matter of the coordinate system.  The Cocoa line drawing
functions draw each line so that it's centered on its path.  If the
width of the line is an odd number of pixels, anti-aliasing can then
make it look wider than expected.

In my (limited) experience, this effect is only noticeable when
drawing straight vertical or horizontal line segments.  In the present
case, shifting by 0.5 pixels has no discernable affect on the
appearance of the underwave.

bug#10736: Add "underwave" face attribute

[Aurélien Aptel]

Just getting some news.

In case it wasn't clear, the last patch (underwave-clip-4.patch) sent
by Alp Aker is correct and can be applied. Are you waiting for more
people to judge the code or Is there something else I/we can do?

bug#10736: Add "underwave" face attribute

[Stefan Monnier]

> In case it wasn't clear, the last patch (underwave-clip-4.patch) sent
> by Alp Aker is correct and can be applied. Are you waiting for more
> people to judge the code or Is there something else I/we can do?

Alp, Jan, or someone, could you install that code?


        Stefan

bug#10736: Add "underwave" face attribute

[Alp Aker]

On Wed, Jun 6, 2012 at 1:54 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>
> Alp, Jan, or someone, could you install that code?

I'll do so next week if it hasn't been done by then (I'm on the road
at the moment).

bug#10736: Add "underwave" face attribute

[Alp Aker]

On Wed, Jun 6, 2012 at 8:30 AM, Alp Aker <alptekin.aker@gmail.com> wrote:
> On Wed, Jun 6, 2012 at 1:54 AM, Stefan Monnier <monnier@iro.umontreal.ca> wrote:
>>
>> Alp, Jan, or someone, could you install that code?
>
> I'll do so next week if it hasn't been done by then (I'm on the road
> at the moment).

Patch applied as revno 108632.

bug#10736: Add "underwave" face attribute

[Glenn Morris]

I'm not seeing a copyright assignment for this. Unless one was completed
very recently?

bug#10736: Add "underwave" face attribute

[Alp Aker]

 On Sat, Jun 16, 2012 at 9:47 PM, Glenn Morris <rgm@gnu.org> wrote:
>
> I'm not seeing a copyright assignment for this. Unless one was completed
> very recently?

I didn't check, to be honest.  (Since the issue hadn't been raised in
either of the relevant threads and Stefan gave the go ahead to install
the code, I'd assumed the  copyright was in order.)

Aurélien, have you completed an assignment of copyright to the FSF?

bug#10736: Add "underwave" face attribute

[Aurélien Aptel]

On Sun, Jun 17, 2012 at 6:16 PM, Alp Aker <alptekin.aker@gmail.com> wrote:
> Aurélien, have you completed an assignment of copyright to the FSF?

I'm afraid I have not.
Which template should I use from this list [1]?

1: http://git.savannah.gnu.org/cgit/gnulib.git/tree/doc/Copyright

bug#10736: Add "underwave" face attribute

[Glenn Morris]

Aurélien Aptel wrote:

> On Sun, Jun 17, 2012 at 6:16 PM, Alp Aker <alptekin.aker@gmail.com> wrote:
>> Aurélien, have you completed an assignment of copyright to the FSF?
>
> I'm afraid I have not.
> Which template should I use from this list [1]?

I will send you the correct form off-list.
Thanks for the patch and for being willing to assign copyright.

bug#10736: Add "underwave" face attribute

[Jan Djärv]

Hello.

12 apr 2012 kl. 23:57 skrev Stefan Monnier:

>> Looking at the mailing list, it seems like it was agreed that this is
>> useful, and that this interface is correct.  Since we're in Emacs 24.2
>> territory now, should this go in?
> 
> Assuming the code is OK, yes.   I'd like to hear someone's opinion on
> the code, tho (Jan, maybe?).
> 

Looks OK.  In general I don't like the "Get GC values, change some GC value, draw, restore changed GC value" technique. It is better to create more GC:s.  But the new code just mimics the old, so it is nothing particular for this patch.

	Jan D.