bug#8863: [PATCH] bug#8863: 23.3; Strikethrough won't display on MacOS

[Alp Aker]

Dave Abrahams wrote:

> Thanks for your work on it.  Will you apply this to the 23.3 branch as
> well as trunk?

I'm not a committer, so that will be up to whoever decides to apply
the patch.  I should note the code below has only been tested on
24.0.50; I suspect it should work as it on 23.3, but haven't tried it.
 (About the anti-aliasing issue I mentioned before:  Whether it's
noticeable depends on factors like font metrics and the degree of
contrast between background and foreground.  I've done everything with
exact pixel drawing, so it shouldn't be an issue.)

A quick implementation note:  This patch implements strike-through and
overline face attributes on NS. It also implements underlining for
stretch and image glyphs (the existing implementation does underlining
only for char and composite char glyphs).  It also adds support for
the user options underline-minimum-offset,
x-use-underline-position-properties, and x-underline-at-descent-line,
which are currently ignored on NextStep.

Sorry for the delay in posting this.  This patch is large enough that
I need a copyright assignment, which has taken a few weeks.  The
signed papers should have arrived as FSF today, so hopefully the way
will soon be clear to incorporate this code.

2011-07-26  Alp Aker  <alp.tekin.aker@gmail.com>

        Implement strike-through and overline face attributes on
        NextStep.  Extend implemention of underlining to include stretch
        and image glyphs.  Add support for user options
        underline-minimum-offset, x-use-underline-position-properties, and
        x-underline-at-descent-line.
        * nsfont.m (nsfont_open): Use underline position provided by font,
        instead of hard-coded value of 2.
        (nsfont_draw): Remove underlining code.  Call
        ns_draw_text_decoration instead.
        * nsterm.h: Add declaration for ns_draw_text_decoration.
        * nsterm.m (ns_draw_text_decoration): New function for drawing
        underline, overline, and strike-through.
        (ns_dumpglyphs_image, ns_dumpglyphs_stretch): Add call to
        ns_draw_text_decoration. Change treatment of cursor drawing to
        accomodate underlining, etc.

=== modified file 'src/nsfont.m'
--- src/nsfont.m        2011-04-16 03:14:08 +0000
+++ src/nsfont.m        2011-07-25 01:29:36 +0000
@@ -845,7 +845,7 @@
     expand = 0.0;
     hshrink = 1.0;

-    font_info->underpos = 2; /*[sfont underlinePosition] is often
clipped out */
+    font_info->underpos = [sfont underlinePosition];
     font_info->underwidth = [sfont underlineThickness];
     font_info->size = font->pixel_size;
     font_info->voffset = lrint (hshrink * [sfont ascender] + expand * hd / 2);
@@ -1196,20 +1196,7 @@
 /*[context GSSetTextDrawingMode: GSTextFill]; /// not implemented yet */
       }

-    /* do underline */
-    if (face->underline_p)
-      {
-        if (face->underline_color != 0)
-          [ns_lookup_indexed_color (face->underline_color, s->f) set];
-        else
-          [col set];
-        DPSmoveto (context, r.origin.x, r.origin.y + font->underpos);
-        DPSlineto (context, r.origin.x+r.size.width,
r.origin.y+font->underpos);
-        if (face->underline_color != 0)
-          [col set];
-      }
-    else
-      [col set];
+    [col set];

     /* draw with DPSxshow () */
     DPSmoveto (context, r.origin.x, r.origin.y);
@@ -1255,23 +1242,7 @@
         CGContextSetTextDrawingMode (gcontext, kCGTextFill);
       }

-    if (face->underline_p)
-      {
-        if (face->underline_color != 0)
-          [ns_lookup_indexed_color (face->underline_color, s->f) set];
-        else
-          [col set];
-        CGContextBeginPath (gcontext);
-        CGContextMoveToPoint (gcontext,
-                              r.origin.x, r.origin.y + font->underpos);
-        CGContextAddLineToPoint (gcontext, r.origin.x + r.size.width,
-                                r.origin.y + font->underpos);
-        CGContextStrokePath (gcontext);
-        if (face->underline_color != 0)
-          [col set];
-      }
-    else
-      [col set];
+    [col set];

     CGContextSetTextPosition (gcontext, r.origin.x, r.origin.y);
     CGContextShowGlyphsWithAdvances (gcontext, s->char2b + s->cmp_from,
@@ -1287,6 +1258,10 @@
     CGContextRestoreGState (gcontext);
   }
 #endif  /* NS_IMPL_COCOA */
+
+  /* Draw underline, overline, strike-through. */
+  ns_draw_text_decoration (s, face, col, r.size.width, r.origin.x);
+
   return to-from;
 }


=== modified file 'src/nsterm.h'
--- src/nsterm.h        2011-07-08 10:04:23 +0000
+++ src/nsterm.h        2011-07-25 01:29:36 +0000
@@ -825,6 +825,13 @@
                                        float r, float g, float b, float a);
 extern NSPoint last_mouse_motion_position;

+/* From nsterm.m, needed in nsfont.m. */
+#ifdef __OBJC__
+extern void
+ns_draw_text_decoration (struct glyph_string *s, struct face *face,
+                         NSColor *defaultCol, CGFloat width, CGFloat x);
+#endif
+
 #ifdef NS_IMPL_GNUSTEP
 extern char gnustep_base_version[];  /* version tracking */
 #endif

=== modified file 'src/nsterm.m'
--- src/nsterm.m        2011-07-23 08:33:06 +0000
+++ src/nsterm.m        2011-07-25 06:33:54 +0000
@@ -263,8 +263,6 @@
 static void ns_judge_scroll_bars (struct frame *f);
 void x_set_frame_alpha (struct frame *f);

-/* FIXME: figure out what to do with underline_minimum_offset. */
-

 /* ==========================================================================

@@ -2597,6 +2595,107 @@
   return n;
 }

+void
+ns_draw_text_decoration (struct glyph_string *s, struct face *face,
+                         NSColor *defaultCol, CGFloat width, CGFloat x)
+/* --------------------------------------------------------------------------
+   Draw underline, overline, and strike-through on glyph string s.
+   --------------------------------------------------------------------------
*/
+{
+  if (s->for_overlaps)
+    return;
+
+  /* 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);
+    }
+
+  /* Do overline. We follow other terms in using a thickness of 1
+     and ignoring overline_margin. */
+  if (face->overline_p)
+    {
+      NSRect r;
+      r = NSMakeRect (x, s->y, width, 1);
+
+      if (face->overline_color_defaulted_p)
+        [defaultCol set];
+      else
+        [ns_lookup_indexed_color (face->overline_color, s->f) set];
+      NSRectFill (r);
+    }
+
+  /* Do strike-through.  We follow other terms for thickness and
+     vertical position.*/
+  if (face->strike_through_p)
+    {
+      NSRect r;
+      unsigned long dy;
+
+      dy = lrint ((s->height - 1) / 2);
+      r = NSMakeRect (x, s->y + dy, width, 1);
+
+      if (face->strike_through_color_defaulted_p)
+        [defaultCol set];
+      else
+        [ns_lookup_indexed_color (face->strike_through_color, s->f) set];
+      NSRectFill (r);
+    }
+}

 static void
 ns_draw_box (NSRect r, float thickness, NSColor *col, char left_p,
char right_p)
@@ -2854,6 +2953,7 @@
   char raised_p;
   NSRect br;
   struct face *face;
+  NSColor *tdCol;

   NSTRACE (ns_dumpglyphs_image);

@@ -2882,10 +2982,7 @@
   else
     face = FACE_FROM_ID (s->f, s->first_glyph->face_id);

-  if (s->hl == DRAW_CURSOR)
-      [FRAME_CURSOR_COLOR (s->f) set];
-  else
-    [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f) set];
+  [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f) set];

   if (bg_height > s->slice.height || s->img->hmargin || s->img->vmargin
       || s->img->mask || s->img->pixmap == 0 || s->width !=
s->background_width)
@@ -2923,6 +3020,27 @@
     [img compositeToPoint: NSMakePoint (x, y + s->slice.height)
                 operation: NSCompositeSourceOver];

+  if (s->hl == DRAW_CURSOR)
+    {
+    [FRAME_CURSOR_COLOR (s->f) set];
+    if (s->w->phys_cursor_type == FILLED_BOX_CURSOR)
+      tdCol = ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f);
+    else
+      /* Currently on NS img->mask is always 0. Since
+         get_window_cursor_type specifies a hollow box cursor when on
+         a non-masked image we never reach this clause. But we put it
+         in in antipication of better support for image masks on
+         NS. */
+      tdCol = ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f);
+    }
+  else
+    {
+      tdCol = ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f);
+    }
+
+  /* Draw underline, overline, strike-through. */
+  ns_draw_text_decoration (s, face, tdCol, br.size.width, br.origin.x);
+
   /* Draw relief, if requested */
   if (s->img->relief || s->hl ==DRAW_IMAGE_RAISED || s->hl ==DRAW_IMAGE_SUNKEN)
     {
@@ -2967,12 +3085,27 @@
   NSRect r[2];
   int n, i;
   struct face *face;
+  NSColor *fgCol, *bgCol;

   if (!s->background_filled_p)
     {
       n = ns_get_glyph_string_clip_rect (s, r);
       *r = NSMakeRect (s->x, s->y, s->background_width, s->height);

+      ns_focus (s->f, r, n);
+
+      if (s->hl == DRAW_MOUSE_FACE)
+       {
+         face = FACE_FROM_ID (s->f, MOUSE_HL_INFO (s->f)->mouse_face_face_id);
+         if (!face)
+           face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
+       }
+      else
+       face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
+
+      bgCol = ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f);
+      fgCol = ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f);
+
       for (i=0; i<n; i++)
         {
           if (!s->row->full_width_p)
@@ -2998,30 +3131,37 @@
                                       FRAME_PIXEL_WIDTH (s->f));
             }

+          [bgCol set];
+
           /* NOTE: under NS this is NOT used to draw cursors, but we must avoid
              overwriting cursor (usually when cursor on a tab) */
           if (s->hl == DRAW_CURSOR)
             {
-              r[i].origin.x += s->width;
-              r[i].size.width -= s->width;
-            }
+              CGFloat x, width;
+
+              x = r[i].origin.x;
+              width = s->w->phys_cursor_width;
+              r[i].size.width -= width;
+              r[i].origin.x += width;
+
+              NSRectFill (r[i]);
+
+              /* Draw overlining, etc. on the cursor. */
+              if (s->w->phys_cursor_type == FILLED_BOX_CURSOR)
+                ns_draw_text_decoration (s, face, bgCol, width, x);
+              else
+                ns_draw_text_decoration (s, face, fgCol, width, x);
+            }
+          else
+            {
+              NSRectFill (r[i]);
+            }
+
+          /* Draw overlining, etc. on the stretch glyph (or the part
+             of the stretch glyph after the cursor). */
+          ns_draw_text_decoration (s, face, fgCol, r[i].size.width,
+                                   r[i].origin.x);
         }
-
-      ns_focus (s->f, r, n);
-
-      if (s->hl == DRAW_MOUSE_FACE)
-       {
-         face = FACE_FROM_ID (s->f, MOUSE_HL_INFO (s->f)->mouse_face_face_id);
-         if (!face)
-           face = FACE_FROM_ID (s->f, MOUSE_FACE_ID);
-       }
-      else
-       face = FACE_FROM_ID (s->f, s->first_glyph->face_id);
-
-      [ns_lookup_indexed_color (NS_FACE_BACKGROUND (face), s->f) set];
-
-      NSRectFill (r[0]);
-      NSRectFill (r[1]);
       ns_unfocus (s->f);
       s->background_filled_p = 1;
     }
@@ -6556,23 +6696,17 @@
   Vx_toolkit_scroll_bars = Qnil;
 #endif

-  /* these are unsupported but we need the declarations to avoid whining
-     messages from cus-start.el */
   DEFVAR_BOOL ("x-use-underline-position-properties",
               x_use_underline_position_properties,
-     doc: /* NOT SUPPORTED UNDER NS.
-*Non-nil means make use of UNDERLINE_POSITION font properties.
+     doc: /*Non-nil means make use of UNDERLINE_POSITION font properties.
 A value of nil means ignore them.  If you encounter fonts with bogus
 UNDERLINE_POSITION font properties, for example 7x13 on XFree prior
-to 4.1, set this to nil.
-
-NOTE: Not supported on Mac yet.  */);
+to 4.1, set this to nil. */);
   x_use_underline_position_properties = 0;

   DEFVAR_BOOL ("x-underline-at-descent-line",
               x_underline_at_descent_line,
-     doc: /* NOT SUPPORTED UNDER NS.
-*Non-nil means to draw the underline at the same place as the descent line.
+     doc: /* Non-nil means to draw the underline at the same place as
the descent line.
 A value of nil means to draw the underline according to the value of the
 variable `x-use-underline-position-properties', which is usually at the
 baseline level.  The default value is nil.  */);