Fix vertical text scaling on Mac.

src/ports/SkFontHost_mac.cpp

Index: src/ports/SkFontHost_mac.cpp
===================================================================
--- src/ports/SkFontHost_mac.cpp	(revision 9039)
+++ src/ports/SkFontHost_mac.cpp	(working copy)
@@ -251,10 +251,6 @@
     return darwin_version;
 }
 
-static bool isLeopard() {
-    return darwinVersion() == 9;
-}
-
 static bool isSnowLeopard() {
     return darwinVersion() == 10;
 }
@@ -300,11 +296,6 @@
                                   ScalarToCG(matrix[SkMatrix::kMTransY] * sy));
 }
 
-static SkScalar getFontScale(CGFontRef cgFont) {
-    int unitsPerEm = CGFontGetUnitsPerEm(cgFont);
-    return SkScalarInvert(SkIntToScalar(unitsPerEm));
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
 #define BITMAP_INFO_RGB (kCGImageAlphaNoneSkipFirst | kCGBitmapByteOrder32Host)
@@ -530,13 +521,7 @@
                 CTFontDescriptorCreateWithAttributes(cfAttributes));
 
         if (ctFontDesc != NULL) {
-            if (isLeopard()) {
-                // CTFontCreateWithFontDescriptor on Leopard ignores the name
-                AutoCFRelease<CTFontRef> ctNamed(CTFontCreateWithName(cfFontName, 1, NULL));
-                ctFont = CTFontCreateCopyWithAttributes(ctNamed, 1, NULL, ctFontDesc);
-            } else {
-                ctFont = CTFontCreateWithFontDescriptor(ctFontDesc, 0, NULL);
-            }
+            ctFont = CTFontCreateWithFontDescriptor(ctFontDesc, 0, NULL);
         }
     }
 
@@ -648,13 +633,9 @@
     return face;
 }
 
-static void flip(SkMatrix* matrix) {
-    matrix->setSkewX(-matrix->getSkewX());
-    matrix->setSkewY(-matrix->getSkewY());
-}
-
 ///////////////////////////////////////////////////////////////////////////////
 
+/** GlyphRect is in upside-down FUnits (em space, y down). */
 struct GlyphRect {
     int16_t fMinX;
     int16_t fMinY;
@@ -665,9 +646,8 @@
 class SkScalerContext_Mac : public SkScalerContext {
 public:
     SkScalerContext_Mac(SkTypeface_Mac*, const SkDescriptor*);
-    virtual ~SkScalerContext_Mac();
+    virtual ~SkScalerContext_Mac() { };
 
-
 protected:
     unsigned generateGlyphCount(void) SK_OVERRIDE;
     uint16_t generateCharToGlyph(SkUnichar uni) SK_OVERRIDE;
@@ -683,21 +663,26 @@
     void getVerticalOffset(CGGlyph glyphID, SkIPoint* offset) const;
     bool generateBBoxes();
 
-    CGAffineTransform fTransform;
-    SkMatrix fUnitMatrix; // without font size
-    SkMatrix fVerticalMatrix; // unit rotated
-    SkMatrix fMatrix; // with font size
-    SkMatrix fFBoundingBoxesMatrix; // lion-specific fix
+    /**
+     *  Converts from FUnits (em space) to SkGlyph units (pixels) without flipping y.
+     *
+     *  Used on SnowLeopard to correct the implementation of CTFontGetVerticalTranslationsForGlyphs.
+     *  (Matrix from FUnits (em space, y up) to CG units (pixels, y up).)
+     *
+     *  Used on Lion to correct the implementation of CTFontGetBoundingRectsForGlyphs.
+     *  (Matrix from upside-down FUnits (em space, y down) to SkGlyph units (pixels, y down).)
+     */
+    SkMatrix fFUnitMatrix;
     Offscreen fOffscreen;
     AutoCFRelease<CTFontRef> fCTFont;
-    AutoCFRelease<CTFontRef> fCTVerticalFont; // for vertical advance
+    AutoCFRelease<CTFontRef> fCTVerticalFont;
     AutoCFRelease<CGFontRef> fCGFont;
-    GlyphRect* fFBoundingBoxes;
+    SkAutoTMalloc<GlyphRect> fFBoundingBoxes;
     uint16_t fFBoundingBoxesGlyphOffset;
     uint16_t fGlyphCount;
     bool fGeneratedFBoundingBoxes;
-    bool fDoSubPosition;
-    bool fVertical;
+    const bool fDoSubPosition;
+    const bool fVertical;
 
     friend class Offscreen;
 
@@ -707,39 +692,21 @@
 SkScalerContext_Mac::SkScalerContext_Mac(SkTypeface_Mac* typeface,
                                          const SkDescriptor* desc)
         : INHERITED(typeface, desc)
-        , fFBoundingBoxes(NULL)
+        , fFBoundingBoxes()
         , fFBoundingBoxesGlyphOffset(0)
         , fGeneratedFBoundingBoxes(false)
+        , fDoSubPosition(SkToBool(fRec.fFlags & kSubpixelPositioning_Flag))
+        , fVertical(SkToBool(fRec.fFlags & kVertical_Flag))
+
 {
     CTFontRef ctFont = typeface->fFontRef.get();
     CFIndex numGlyphs = CTFontGetGlyphCount(ctFont);
-
-    // Get the state we need
-    fRec.getSingleMatrix(&fMatrix);
-    fUnitMatrix = fMatrix;
-
-    // extract the font size out of the matrix, but leave the skewing for italic
-    SkScalar reciprocal = SkScalarInvert(fRec.fTextSize);
-    fUnitMatrix.preScale(reciprocal, reciprocal);
-
     SkASSERT(numGlyphs >= 1 && numGlyphs <= 0xFFFF);
-
-    fTransform = MatrixToCGAffineTransform(fMatrix);
-
-    CGAffineTransform transform;
-    CGFloat unitFontSize;
-    if (isLeopard()) {
-        // passing 1 for pointSize to Leopard sets the font size to 1 pt.
-        // pass the CoreText size explicitly
-        transform = MatrixToCGAffineTransform(fUnitMatrix);
-        unitFontSize = SkScalarToFloat(fRec.fTextSize);
-    } else {
-        // since our matrix includes everything, we pass 1 for pointSize
-        transform = fTransform;
-        unitFontSize = 1;
-    }
-    flip(&fUnitMatrix); // flip to fix up bounds later
-    fVertical = SkToBool(fRec.fFlags & kVertical_Flag);
+    fGlyphCount = SkToU16(numGlyphs);
+    
+    fRec.getSingleMatrix(&fFUnitMatrix);
+    CGAffineTransform transform = MatrixToCGAffineTransform(fFUnitMatrix);
+    
     AutoCFRelease<CTFontDescriptorRef> ctFontDesc;
     if (fVertical) {
         AutoCFRelease<CFMutableDictionaryRef> cfAttributes(CFDictionaryCreateMutable(
@@ -754,29 +721,18 @@
             ctFontDesc = CTFontDescriptorCreateWithAttributes(cfAttributes);
         }
     }
-    fCTFont = CTFontCreateCopyWithAttributes(ctFont, unitFontSize, &transform, ctFontDesc);
+    // Since our matrix includes everything, we pass 1 for size.
+    fCTFont = CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, ctFontDesc);
     fCGFont = CTFontCopyGraphicsFont(fCTFont, NULL);
     if (fVertical) {
         CGAffineTransform rotateLeft = CGAffineTransformMake(0, -1, 1, 0, 0, 0);
         transform = CGAffineTransformConcat(rotateLeft, transform);
-        fCTVerticalFont = CTFontCreateCopyWithAttributes(ctFont, unitFontSize, &transform, NULL);
-        fVerticalMatrix = fUnitMatrix;
-        if (isSnowLeopard()) {
-            SkScalar scale = SkScalarMul(fRec.fTextSize, getFontScale(fCGFont));
-            fVerticalMatrix.preScale(scale, scale);
-        } else {
-            fVerticalMatrix.preRotate(SkIntToScalar(90));
-        }
-        fVerticalMatrix.postScale(SK_Scalar1, -SK_Scalar1);
+        fCTVerticalFont = CTFontCreateCopyWithAttributes(ctFont, 1.f, &transform, NULL);
     }
-    fGlyphCount = SkToU16(numGlyphs);
-    fDoSubPosition = SkToBool(fRec.fFlags & kSubpixelPositioning_Flag);
+    SkScalar emPerFUnit = SkScalarInvert(SkIntToScalar(CGFontGetUnitsPerEm(fCGFont)));
+    fFUnitMatrix.preScale(emPerFUnit, emPerFUnit);
 }
 
-SkScalerContext_Mac::~SkScalerContext_Mac() {
-    delete[] fFBoundingBoxes;
-}
-
 CGRGBPixel* Offscreen::getCG(const SkScalerContext_Mac& context, const SkGlyph& glyph,
                              CGGlyph glyphID, size_t* rowBytesPtr,
                              bool generateA8FromLCD) {
@@ -823,8 +779,8 @@
 
         CGContextSetTextDrawingMode(fCG, kCGTextFill);
         CGContextSetFont(fCG, context.fCGFont);
-        CGContextSetFontSize(fCG, 1);
-        CGContextSetTextMatrix(fCG, context.fTransform);
+        CGContextSetFontSize(fCG, 1.0f /*CTFontGetSize(context.fCTFont)*/);
+        CGContextSetTextMatrix(fCG, CTFontGetMatrix(context.fCTFont));
 
         CGContextSetAllowsFontSubpixelPositioning(fCG, context.fDoSubPosition);
         CGContextSetShouldSubpixelPositionFonts(fCG, context.fDoSubPosition);
@@ -852,7 +808,7 @@
     image += (fSize.fHeight - glyph.fHeight) * fSize.fWidth;
 
     // erase to black
-    sk_memset_rect32(image, 0, glyph.fWidth, glyph.fHeight, rowBytes);
+    sk_memset_rect32(image, 0x00333333, glyph.fWidth, glyph.fHeight, rowBytes);

[bungeman-skia, 2013/05/10 17:08:03]

This is here just to make the bbox visible. Will be removed.

 
     float subX = 0;
     float subY = 0;
@@ -860,12 +816,16 @@
         subX = SkFixedToFloat(glyph.getSubXFixed());
         subY = SkFixedToFloat(glyph.getSubYFixed());
     }
+    
+    // CGContextShowGlyphsAtPoint always draws using the horizontal baseline origin.
+    // Get the offset from the vertical origin to the horizontal origin.
     if (context.fVertical) {
         SkIPoint offset;
         context.getVerticalOffset(glyphID, &offset);
         subX += offset.fX;
         subY += offset.fY;
     }
+    
     CGContextShowGlyphsAtPoint(fCG, -glyph.fLeft + subX,
                                glyph.fTop + glyph.fHeight - subY,
                                &glyphID, 1);
@@ -876,21 +836,21 @@
 }
 
 void SkScalerContext_Mac::getVerticalOffset(CGGlyph glyphID, SkIPoint* offset) const {
-    CGSize vertOffset;
-    CTFontGetVerticalTranslationsForGlyphs(fCTVerticalFont, &glyphID, &vertOffset, 1);
-    const SkPoint trans = {CGToScalar(vertOffset.width),
-                           CGToScalar(vertOffset.height)};
-    SkPoint floatOffset;
-    fVerticalMatrix.mapPoints(&floatOffset, &trans, 1);
-    if (!isSnowLeopard()) {
-    // SnowLeopard fails to apply the font's matrix to the vertical metrics,
-    // but Lion and Leopard do. The unit matrix describes the font's matrix at
-    // point size 1. There may be some way to avoid mapping here by setting up
-    // fVerticalMatrix differently, but this works for now.
-        fUnitMatrix.mapPoints(&floatOffset, 1);
+    CGSize cgVertOffset;
+    CTFontGetVerticalTranslationsForGlyphs(fCTFont, &glyphID, &cgVertOffset, 1);
+
+    // While this is an SkPoint, it is in CG space (y up).
+    SkPoint skVertOffset = { CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
+    
+    // SnowLeopard returns cgVertOffset in completely un-transformed FUnits (em space, y up).
+    // Lion and Leopard return cgVertOffset in transformed font units (pixels, y up).
+    if (isSnowLeopard()) {
+        // From FUnits (em space, y up) to CG units (pixels, y up).
+        fFUnitMatrix.mapPoints(&skVertOffset, 1);
     }
-    offset->fX = SkScalarRound(floatOffset.fX);
-    offset->fY = SkScalarRound(floatOffset.fY);
+    
+    offset->fX = SkScalarRound(skVertOffset.fX);
+    offset->fY = SkScalarRound(-skVertOffset.fY);
 }
 
 uint16_t SkScalerContext_Mac::getFBoundingBoxesGlyphOffset() {
@@ -938,7 +898,7 @@
     }
 
     uint16_t entries = fGlyphCount - fFBoundingBoxesGlyphOffset;
-    fFBoundingBoxes = new GlyphRect[entries];
+    fFBoundingBoxes.reset(entries);
 
     SkOTTableHead::IndexToLocFormat locaFormat = headTable->indexToLocFormat;
     SkOTTableGlyph::Iterator glyphDataIter(*glyfTable.fData, *locaTable.fData, locaFormat);
@@ -947,14 +907,10 @@
         const SkOTTableGlyphData* glyphData = glyphDataIter.next();
         GlyphRect& rect = fFBoundingBoxes[boundingBoxesIndex];
         rect.fMinX = SkEndian_SwapBE16(glyphData->xMin);
-        rect.fMinY = SkEndian_SwapBE16(glyphData->yMin);
+        rect.fMinY = -SkEndian_SwapBE16(glyphData->yMin);
         rect.fMaxX = SkEndian_SwapBE16(glyphData->xMax);
-        rect.fMaxY = SkEndian_SwapBE16(glyphData->yMax);
+        rect.fMaxY = -SkEndian_SwapBE16(glyphData->yMax);
     }
-    fFBoundingBoxesMatrix = fMatrix;
-    flip(&fFBoundingBoxesMatrix);
-    SkScalar fontScale = getFontScale(fCGFont);
-    fFBoundingBoxesMatrix.preScale(fontScale, fontScale);
     return true;
 }
 
@@ -985,13 +941,11 @@
 }
 
 void SkScalerContext_Mac::generateMetrics(SkGlyph* glyph) {
+    const CGGlyph cgGlyph = (CGGlyph) glyph->getGlyphID(fBaseGlyphCount);
+    
+    // The following block produces advance and bounds in CG units (pixels, y up).
     CGSize advance;
     CGRect bounds;
-    CGGlyph cgGlyph;
-
-    // Get the state we need
-    cgGlyph = (CGGlyph) glyph->getGlyphID(fBaseGlyphCount);
-
     if (fVertical) {
         if (!isSnowLeopard()) {
             // Lion and Leopard respect the vertical font metrics.
@@ -1000,17 +954,20 @@
         } else {
             // Snow Leopard and earlier respect the vertical font metrics for
             // advances, but not bounds, so use the default box and adjust it below.
-            CTFontGetBoundingRectsForGlyphs(fCTFont, kCTFontDefaultOrientation,
+            CTFontGetBoundingRectsForGlyphs(fCTFont, kCTFontHorizontalOrientation,
                                             &cgGlyph, &bounds, 1);
         }
         CTFontGetAdvancesForGlyphs(fCTVerticalFont, kCTFontVerticalOrientation,
                                    &cgGlyph, &advance, 1);
     } else {
-        CTFontGetBoundingRectsForGlyphs(fCTFont, kCTFontDefaultOrientation,
+        CTFontGetBoundingRectsForGlyphs(fCTFont, kCTFontHorizontalOrientation,
                                         &cgGlyph, &bounds, 1);
-        CTFontGetAdvancesForGlyphs(fCTFont, kCTFontDefaultOrientation,
+        CTFontGetAdvancesForGlyphs(fCTFont, kCTFontHorizontalOrientation,
                                    &cgGlyph, &advance, 1);
     }
+    // Convert advance and bounds to SkGlyph units (pixels, y down).
+    bounds.origin.y = -bounds.origin.y - bounds.size.height;
+    advance.height = -advance.height;
 
     // BUG?
     // 0x200B (zero-advance space) seems to return a huge (garbage) bounds, when
@@ -1025,34 +982,14 @@
     }
 
     glyph->zeroMetrics();
-    glyph->fAdvanceX =  SkFloatToFixed_Check(advance.width);
-    glyph->fAdvanceY = -SkFloatToFixed_Check(advance.height);
+    glyph->fAdvanceX = SkFloatToFixed_Check(advance.width);
+    glyph->fAdvanceY = SkFloatToFixed_Check(advance.height);
 
     if (CGRectIsEmpty_inline(bounds)) {
         return;
     }
 
-    if (isLeopard() && !fVertical) {
-        // Leopard does not consider the matrix skew in its bounds.
-        // Run the bounding rectangle through the skew matrix to determine
-        // the true bounds. However, this doesn't work if the font is vertical.
-        // FIXME (Leopard): If the font has synthetic italic (e.g., matrix skew)
-        // and the font is vertical, the bounds need to be recomputed.
-        SkRect glyphBounds = SkRect::MakeXYWH(
-                bounds.origin.x, bounds.origin.y,
-                bounds.size.width, bounds.size.height);
-        fUnitMatrix.mapRect(&glyphBounds);
-        bounds.origin.x = glyphBounds.fLeft;
-        bounds.origin.y = glyphBounds.fTop;
-        bounds.size.width = glyphBounds.width();
-        bounds.size.height = glyphBounds.height();
-    }
-    // Adjust the bounds
-    //
-    // CTFontGetBoundingRectsForGlyphs ignores the font transform, so we need
-    // to transform the bounding box ourselves.
-    //
-    // The bounds are also expanded by 1 pixel, to give CG room for anti-aliasing.
+    // Expand the bounds by 1 pixel, to give CG room for anti-aliasing.
     CGRectInset_inline(&bounds, -1, -1);
 
     // Get the metrics
@@ -1060,10 +997,10 @@
     if (isLion() || isMountainLion()) {
         if (cgGlyph < fGlyphCount && cgGlyph >= getFBoundingBoxesGlyphOffset() && generateBBoxes()){
             lionAdjustedMetrics = true;
-            SkRect adjust;
             const GlyphRect& gRect = fFBoundingBoxes[cgGlyph - fFBoundingBoxesGlyphOffset];
-            adjust.set(gRect.fMinX, gRect.fMinY, gRect.fMaxX, gRect.fMaxY);
-            fFBoundingBoxesMatrix.mapRect(&adjust);
+            SkRect adjust = SkRect::MakeLTRB(gRect.fMinX, gRect.fMinY, gRect.fMaxX, gRect.fMaxY);
+            // From upside-down FUnits (em space, y down) to SkGlyph units (pixels, y down).
+            fFUnitMatrix.mapRect(&adjust);
             bounds.origin.x = SkScalarToFloat(adjust.fLeft) - 1;
             bounds.origin.y = SkScalarToFloat(adjust.fTop) - 1;
         }
@@ -1074,12 +1011,12 @@
         glyph->fWidth = SkToU16(sk_float_round2int(bounds.size.width));
         glyph->fHeight = SkToU16(sk_float_round2int(bounds.size.height));
     }
-    glyph->fTop = SkToS16(-sk_float_round2int(CGRectGetMaxY_inline(bounds)));
-    glyph->fLeft = SkToS16(sk_float_round2int(CGRectGetMinX_inline(bounds)));
-    SkIPoint offset;
+    glyph->fTop = SkToS16(sk_float_round2int(bounds.origin.y));
+    glyph->fLeft = SkToS16(sk_float_round2int(bounds.origin.x));
     if (fVertical && (isSnowLeopard() || lionAdjustedMetrics)) {
         // SnowLeopard doesn't respect vertical metrics, so compute them manually.
         // Also compute them for Lion when the metrics were computed by hand.
+        SkIPoint offset;
         getVerticalOffset(cgGlyph, &offset);
         glyph->fLeft += offset.fX;
         glyph->fTop += offset.fY;