<rdar://problem/9110316> REGRESSION: 'ex' unit broken for vertical text
authorhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Mar 2011 22:20:34 +0000 (22:20 +0000)
committerhyatt@apple.com <hyatt@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Wed, 9 Mar 2011 22:20:34 +0000 (22:20 +0000)
Reviewed by Dan Bernstein.

Fall back to the verticalRightOrientation data when obtaining the x-height for vertically oriented
text. That way we use the same metrics as for horizontal.

This fixes regressions in fast/lists and fast/overflow vertical text tests.

* platform/graphics/mac/SimpleFontDataMac.mm:
(WebCore::SimpleFontData::platformInit):
(WebCore::SimpleFontData::platformBoundsForGlyph):

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@80662 268f45cc-cd09-0410-ab3c-d52691b4dbfc

Source/WebCore/ChangeLog
Source/WebCore/platform/graphics/mac/SimpleFontDataMac.mm

index 38ea2f2e56304c4b65293cd25e79363368dd68a5..02d9790e012cdbe5661119e0557ff861174d6a66 100644 (file)
@@ -1,3 +1,18 @@
+2011-03-09  David Hyatt  <hyatt@apple.com>
+
+        Reviewed by Dan Bernstein.
+
+        <rdar://problem/9110316> REGRESSION: 'ex' unit broken for vertical text
+
+        Fall back to the verticalRightOrientation data when obtaining the x-height for vertically oriented
+        text. That way we use the same metrics as for horizontal.
+
+        This fixes regressions in fast/lists and fast/overflow vertical text tests.
+
+        * platform/graphics/mac/SimpleFontDataMac.mm:
+        (WebCore::SimpleFontData::platformInit):
+        (WebCore::SimpleFontData::platformBoundsForGlyph):
+
 2011-03-09  Ryosuke Niwa  <rniwa@webkit.org>
 
         Reviewed by Tony Chang.
index 2f3d1d1ba276593f27ea6c3c2e28c37ecea9efaf..7e1b9d3c43f1f2051281b4cc789c66ad3d771ef3 100644 (file)
@@ -274,29 +274,28 @@ void SimpleFontData::platformInit()
 
     float xHeight;
 
-    // Measure the actual character "x", because AppKit synthesizes X height rather than getting it from the font.
-    // Unfortunately, NSFont will round this for us so we don't quite get the right value.
-    GlyphPage* glyphPageZero = GlyphPageTreeNode::getRootChild(this, 0)->page();
-    NSGlyph xGlyph = glyphPageZero ? glyphPageZero->glyphDataForCharacter('x').glyph : 0;
-    if (xGlyph) {
-        CGRect xBox = platformBoundsForGlyph(xGlyph);
-        // Use the maximum of either width or height because "x" is nearly square
-        // and web pages that foolishly use this metric for width will be laid out
-        // poorly if we return an accurate height. Classic case is Times 13 point,
-        // which has an "x" that is 7x6 pixels.
-        xHeight = static_cast<float>(max(CGRectGetMaxX(xBox), -CGRectGetMinY(xBox)));
-    } else {
-#ifndef BUILDING_ON_TIGER
-        xHeight = static_cast<float>(CGFontGetXHeight(m_platformData.cgFont())) / unitsPerEm;
-#else
-        xHeight = m_platformData.font() ? [m_platformData.font() xHeight] : 0;
-#endif
-        // CGFontGetXHeight() returns a wrong value for "Apple Symbols" font (a float close to 0, but not strictly 0).
-        // The following code makes a guess for xHeight in that case.
-        // The int cast is a workaround for the "almost" zero value returned by CGFontGetXHeight().
-        if (!static_cast<int>(xHeight) && ascent)
-            xHeight = 2 * ascent / 3;
-    }
+    // Measure the actual character "x", since it's possible for it to extend below the baseline, and we need the
+    // reported x-height to only include the portion of the glyph that is above the baseline.
+    if (platformData().orientation() == Horizontal) {
+        GlyphPage* glyphPageZero = GlyphPageTreeNode::getRootChild(this, 0)->page();
+        NSGlyph xGlyph = glyphPageZero ? glyphPageZero->glyphDataForCharacter('x').glyph : 0;
+        if (xGlyph) {
+            CGRect xBox = platformBoundsForGlyph(xGlyph);
+            // Use the maximum of either width or height because "x" is nearly square
+            // and web pages that foolishly use this metric for width will be laid out
+            // poorly if we return an accurate height. Classic case is Times 13 point,
+            // which has an "x" that is 7x6 pixels.
+            xHeight = max(CGRectGetMaxX(xBox), -CGRectGetMinY(xBox));
+        } else {
+            xHeight = static_cast<float>(CGFontGetXHeight(m_platformData.cgFont())) / unitsPerEm;
+            // CGFontGetXHeight() returns a wrong value for "Apple Symbols" font (a float close to 0, but not strictly 0).
+            // The following code makes a guess for xHeight in that case.
+            // The int cast is a workaround for the "almost" zero value returned by CGFontGetXHeight().
+            if (!static_cast<int>(xHeight) && ascent)
+                xHeight = 2 * ascent / 3;
+        }
+    } else
+        xHeight = verticalRightOrientationFontData()->fontMetrics().xHeight();
 
     m_fontMetrics.setUnitsPerEm(unitsPerEm);
     m_fontMetrics.setAscent(ascent);
@@ -469,16 +468,8 @@ void SimpleFontData::determinePitch()
 FloatRect SimpleFontData::platformBoundsForGlyph(Glyph glyph) const
 {
     FloatRect boundingBox;
-#ifndef BUILDING_ON_TIGER
     boundingBox = CTFontGetBoundingRectsForGlyphs(m_platformData.ctFont(), platformData().orientation() == Vertical ? kCTFontVerticalOrientation : kCTFontHorizontalOrientation, &glyph, 0, 1);
     boundingBox.setY(-boundingBox.maxY());
-#else
-    // FIXME: Custom fonts don't have NSFonts, so this function doesn't compute correct bounds for these on Tiger.
-    if (!m_platformData.font())
-        return boundingBox;
-    boundingBox = [m_platformData.font() boundingRectForGlyph:glyph];
-    boundingBox.setY(-boundingBox.maxY());
-#endif
     if (m_syntheticBoldOffset)
         boundingBox.setWidth(boundingBox.width() + m_syntheticBoldOffset);