Subject: bug#8913: 23.2; Raise display property does not work correctly on Mac OS X

From: Alp Aker <[email-masked]>
Date: Wed, 22 Jun 2011 09:48:15 +0000 (UTC)

> The raise display property does not work properly on Mac OS X. [...] On Mac
> OS, this (as far as I can tell) results in the whole line being shifted,
> not just the 5 characters with the property.

I reported this recently as well.  I've since looked into it further and can
offer an explanation of what's going on, followed by a patch.

Raise display specs are actually respected by the NS port for images and
stretch glyphs; they're only problematic in the case of character and
composite character glyphs.  That's because when drawing these types the
relevant function (nsfont_draw) uses the following as the text baseline:

    s->y + font->voffset + (s->height - font->height)/2,

where the 'font' struct is of type nsfont_info.  A comment in the latter's
declaration says that the voffset field is the "mean of ascender/descender
offsets."  However, the only value ever assigned to it (in nsfont_open) is
the ascender height of the font.  In other words, "s->y + font->offset"
back-calculates the baseline by starting from the coordinate for the top of
the glyph and moving down by the ascender height.  Since changes in the
baseline that result from display properties are recorded in s->ybase, the
glyph-drawing function doesn't take account of the altered baseline.  

(This also results in a further bit of anomalous behavior:  When one shrinks
text using a "height" display spec, other ports draw the smaller text on the
normal baseline, but on NS it's drawn above the baseline. This is because
it's using a smaller ascender value for the scaled-down font, and so s->y +
font->voffset doesn't reach all the way to where the baseline should be.)

What you describe as the "whole line being shifted" is the product of the
third term in the above expression, which (I gather) is intended to balance
out vertical positioning of different-sized fonts.  The extra space required
to accomodate raised characters is supposed to be added to the top of the
row, but that third factor causes the extra height to be distributed equally
above and below the line, adding extra space *below* the line where other
terms don't add any.

I asked Adrian Robert about this issue last week.  He said that there wasn't
an NS-specific reason for the different treatment:  the existing code simply
dates from before the display routine took on its present form and there's no
obvious reason not to use the same routine as other terms.

The patch below makes that change (and removes other code that's rendered
superfluous as a result).  

One implementation note:  On NS, the baseline_offset and vertical_centering
fields of a font are always 0; the system font API is supposed to handle the
possibility that a font is for a script with a different notion of
baseline.  Thus we can simply use s->ybase for the text baseline, rather than
the calculation involving those other factors that xterm.c uses.  I've
verified that characters from the relevant Unicode scripts (han, kana,
hangul) display correctly with this simpler implementation, both with Apple's
system fonts and with non-Apple fonts (GNU Unifont, Cyberbit, VLGothic,
WenQuanYi Zen Hei, and a few others).

=== modified file 'src/ChangeLog'
--- src/ChangeLog	2011-06-21 16:47:56 +0000
+++ src/ChangeLog	2011-06-22 09:40:28 +0000
@@ -1,3 +1,10 @@
+2011-06-22  Alp Aker  <[email-masked]>
+
+	* nsfont.m (nsfont_open):  Remove assignment to voffset.  Remove
+	unnecessary uses of hshink, expand, hd, full_height, min_height.
+	(nsfont_draw):  Use s->ybase as baseline for glyph drawing.
+	* nsterm.h (nsfont_info): Remove voffset field.
+
 2011-06-21  Paul Eggert  <[email-masked]>
 
 	Port to Sun C.

=== modified file 'src/nsfont.m'
--- src/nsfont.m	2011-04-16 03:14:08 +0000
+++ src/nsfont.m	2011-06-22 09:23:17 +0000
@@ -804,8 +804,6 @@
   font->props[FONT_FILE_INDEX] = Qnil;
 
   {
-    double expand, hshrink;
-    float full_height, min_height, hd;
     const char *fontName = [[nsfont fontName] UTF8String];
     int len = strlen (fontName);
 
@@ -837,26 +835,16 @@
       [sfont maximumAdvancement].width : ns_char_width (sfont, '0');
 
     brect =  [sfont boundingRectForFont];
-    full_height = brect.size.height;
-    min_height = [sfont ascender] - adjusted_descender;
-    hd = full_height - min_height;
-
-    /* standard height, similar to Carbon. Emacs.app: was 0.5 by default. */
-    expand = 0.0;
-    hshrink = 1.0;
 
     font_info->underpos = 2; /*[sfont underlinePosition] is often clipped out */
     font_info->underwidth = [sfont underlineThickness];
     font_info->size = font->pixel_size;
-    font_info->voffset = lrint (hshrink * [sfont ascender] + expand * hd / 2);
 
     /* max bounds */
-    font_info->max_bounds.ascent =
-      lrint (hshrink * [sfont ascender] + expand * hd/2);
+    font_info->max_bounds.ascent = lrint ([sfont ascender]);
     /* Descender is usually negative.  Use floor to avoid
        clipping descenders. */
-    font_info->max_bounds.descent =
-      -lrint (floor(hshrink* adjusted_descender - expand*hd/2));
+    font_info->max_bounds.descent = -lrint (floor(adjusted_descender));
     font_info->height =
       font_info->max_bounds.ascent + font_info->max_bounds.descent;
     font_info->max_bounds.width = lrint (font_info->width);
@@ -1165,7 +1153,7 @@

   /* set up for character rendering */
-  r.origin.y += font->voffset + (s->height - font->height)/2;
+  r.origin.y = s->ybase;
 
   col = (NS_FACE_FOREGROUND (face) != 0
          ? ns_lookup_indexed_color (NS_FACE_FOREGROUND (face), s->f)

=== modified file 'src/nsterm.h'
--- src/nsterm.h	2011-01-25 04:08:28 +0000
+++ src/nsterm.h	2011-06-22 08:31:34 +0000
@@ -436,7 +436,6 @@
 #endif
   char bold, ital;  /* convenience flags */
   char synthItal;
-  float voffset;  /* mean of ascender/descender offsets */
   XCharStruct max_bounds;
   /* we compute glyph codes and metrics on-demand in blocks of 256 indexed
      by hibyte, lobyte */