Changeset 80662 in webkit

Timestamp:

Mar 9, 2011 2:20:34 PM (13 years ago)

Author:

hyatt@apple.com

Message:

<rdar://problem/9110316> REGRESSION: 'ex' unit broken for vertical text

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

Location:

trunk/Source/WebCore

Files:

• ChangeLog (modified) (1 diff)
• platform/graphics/mac/SimpleFontDataMac.mm (modified) (2 diffs)

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+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>
 

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

@@ -275 +275 @@
     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);
@@ -470 +469 @@
 {
     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);