Use only one CGFont/CTFont per scaler context on Mac.

src/ports/SkFontHost_mac.cpp

 
 /*
  * Copyright 2006 The Android Open Source Project
  *
  * Use of this source code is governed by a BSD-style license that can be
  * found in the LICENSE file.
  */
 
 #include "SkTypes.h"  // Keep this before any #ifdef ...
 
@@ skipping 666 matching lines @@
     bool generateBBoxes();
 
     /** Converts from FUnits (em space, y up) to SkGlyph units (pixels, y down).
      *
      *  Used on Snow Leopard to correct CTFontGetVerticalTranslationsForGlyphs.
      *  Used on Lion to correct CTFontGetBoundingRectsForGlyphs.
      */
     SkMatrix fFUnitMatrix;
 
     Offscreen fOffscreen;
-    AutoCFRelease<CTFontRef> fCTFont;
-    CGAffineTransform fTransform;
-    CGAffineTransform fInvTransform;
 
     /** Unrotated variant of fCTFont.
      *
      *  In 10.10.1 CTFontGetAdvancesForGlyphs applies the font transform to the width of the
      *  advances, but always sets the height to 0. This font is used to get the advances of the
      *  unrotated glyph, and then the rotation is applied separately.
      *
      *  CT vertical metrics are pre-rotated (in em space, before transform) 90deg clock-wise.
      *  This makes kCTFontDefaultOrientation dangerous, because the metrics from
      *  kCTFontHorizontalOrientation are in a different space from kCTFontVerticalOrientation.
      *  With kCTFontVerticalOrientation the advances must be unrotated.
+     *
+     *  Sometimes, creating a copy of a CTFont with the same size but different trasform will select
+     *  different underlying font data. As a result, avoid ever creating more than one CTFont per
+     *  SkScalerContext to ensure that only one CTFont is used.
+     *
+     *  As a result of the above (and other constraints) this font contains the size, but not the
+     *  transform. The transform must always be applied separately.
      */
-    AutoCFRelease<CTFontRef> fCTUnrotatedFont;
+    AutoCFRelease<CTFontRef> fCTFont;
+
+    /** The transform without the font size. */
+    CGAffineTransform fTransform;
+    CGAffineTransform fInvTransform;
 
     AutoCFRelease<CGFontRef> fCGFont;
     SkAutoTMalloc<GlyphRect> fFBoundingBoxes;
     uint16_t fFBoundingBoxesGlyphOffset;
     uint16_t fGlyphCount;
     bool fGeneratedFBoundingBoxes;
     const bool fDoSubPosition;
     const bool fVertical;
 
     friend class Offscreen;
@@ skipping 44 matching lines @@
     SkVector scale;
     SkMatrix skTransform;
     fRec.computeMatrices(SkScalerContextRec::kVertical_PreMatrixScale, &scale, &skTransform,
                          nullptr, nullptr, &fFUnitMatrix);
     fTransform = MatrixToCGAffineTransform(skTransform);
     fInvTransform = CGAffineTransformInvert(fTransform);
 
     // The transform contains everything except the requested text size.
     // Some properties, like 'trak', are based on the text size (before applying the matrix).
     CGFloat textSize = ScalarToCG(scale.y());
-
-    fCTFont.reset(ctfont_create_exact_copy(ctFont, textSize, &fTransform));
+    fCTFont.reset(ctfont_create_exact_copy(ctFont, textSize, nullptr));
     fCGFont.reset(CTFontCopyGraphicsFont(fCTFont, nullptr));
-    fCTUnrotatedFont.reset(ctfont_create_exact_copy(ctFont, textSize, nullptr));
 
     // The fUnitMatrix includes the text size (and em) as it is used to scale the raw font data.
     SkScalar emPerFUnit = SkScalarInvert(SkIntToScalar(CGFontGetUnitsPerEm(fCGFont)));
     fFUnitMatrix.preScale(emPerFUnit, -emPerFUnit);
 }
 
 /** This is an implementation of CTFontDrawGlyphs for 10.6; it was introduced in 10.7. */
 static void legacy_CTFontDrawGlyphs(CTFontRef, const CGGlyph glyphs[], const CGPoint points[],
                                     size_t count, CGContextRef cg) {
     CGContextShowGlyphsAtPositions(cg, glyphs, points, count);
@@ skipping 126 matching lines @@
     CGPoint point = CGPointMake(-glyph.fLeft + subX, glyph.fTop + glyph.fHeight - subY);
     // Prior to 10.10, CTFontDrawGlyphs acted like CGContextShowGlyphsAtPositions and took
     // 'positions' which are in text space. The glyph location (in device space) must be
     // mapped into text space, so that CG can convert it back into device space.
     // In 10.10.1, this is handled directly in CTFontDrawGlyphs.
     //
     // However, in 10.10.2 color glyphs no longer rotate based on the font transform.
     // So always make the font transform identity and place the transform on the context.
     point = CGPointApplyAffineTransform(point, context.fInvTransform);
 
-    ctFontDrawGlyphs(context.fCTUnrotatedFont, &glyphID, &point, 1, fCG);
+    ctFontDrawGlyphs(context.fCTFont, &glyphID, &point, 1, fCG);
 
     SkASSERT(rowBytesPtr);
     *rowBytesPtr = rowBytes;
     return image;
 }
 
 void SkScalerContext_Mac::getVerticalOffset(CGGlyph glyphID, SkPoint* offset) const {
     // Snow Leopard returns cgVertOffset in completely un-transformed FUnits (em space, y up).
     // Lion and Leopard return cgVertOffset in CG units (pixels, y up).
     CGSize cgVertOffset;
     CTFontGetVerticalTranslationsForGlyphs(fCTFont, &glyphID, &cgVertOffset, 1);
-
-    SkPoint skVertOffset = { CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
     if (isSnowLeopard()) {
+        SkPoint skVertOffset = { CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
+        skVertOffset = { CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };

[reed1, 2015/10/29 15:44:23]

are these two lines the same?

[bungeman-skia, 2015/10/29 15:52:13]

Done.

         // From FUnits (em space, y up) to SkGlyph units (pixels, y down).
         fFUnitMatrix.mapPoints(&skVertOffset, 1);
-    } else {
-        // From CG units (pixels, y up) to SkGlyph units (pixels, y down).
-        skVertOffset.fY = -skVertOffset.fY;
+        *offset = skVertOffset;
+        return;
     }
-
+    cgVertOffset = CGSizeApplyAffineTransform(cgVertOffset, fTransform);
+    SkPoint skVertOffset = { CGToScalar(cgVertOffset.width), CGToScalar(cgVertOffset.height) };
+    // From CG units (pixels, y up) to SkGlyph units (pixels, y down).
+    skVertOffset.fY = -skVertOffset.fY;
     *offset = skVertOffset;
 }
 
 uint16_t SkScalerContext_Mac::getFBoundingBoxesGlyphOffset() {
     if (fFBoundingBoxesGlyphOffset) {
         return fFBoundingBoxesGlyphOffset;
     }
     fFBoundingBoxesGlyphOffset = fGlyphCount; // fallback for all fonts
     AutoCGTable<SkOTTableHorizontalHeader> hheaTable(fCGFont);
     if (hheaTable.fData) {
@@ skipping 68 matching lines @@
 
 void SkScalerContext_Mac::generateMetrics(SkGlyph* glyph) {
     AUTO_CG_LOCK();
 
     const CGGlyph cgGlyph = (CGGlyph) glyph->getGlyphID();
     glyph->zeroMetrics();
 
     // The following block produces cgAdvance in CG units (pixels, y up).
     CGSize cgAdvance;
     if (fVertical) {
-        CTFontGetAdvancesForGlyphs(fCTUnrotatedFont, kCTFontVerticalOrientation,
+        CTFontGetAdvancesForGlyphs(fCTFont, kCTFontVerticalOrientation,
                                    &cgGlyph, &cgAdvance, 1);
         // Vertical advances are returned as widths instead of heights.
         SkTSwap(cgAdvance.height, cgAdvance.width);
         cgAdvance.height = -cgAdvance.height;
     } else {
-        CTFontGetAdvancesForGlyphs(fCTUnrotatedFont, kCTFontHorizontalOrientation,
+        CTFontGetAdvancesForGlyphs(fCTFont, kCTFontHorizontalOrientation,
                                    &cgGlyph, &cgAdvance, 1);
     }
-    cgAdvance = CGSizeApplyAffineTransform(cgAdvance, CTFontGetMatrix(fCTFont));
+    cgAdvance = CGSizeApplyAffineTransform(cgAdvance, fTransform);
     glyph->fAdvanceX =  SkFloatToFixed_Check(cgAdvance.width);
     glyph->fAdvanceY = -SkFloatToFixed_Check(cgAdvance.height);
 
     // The following produces skBounds in SkGlyph units (pixels, y down),
     // or returns early if skBounds would be empty.
     SkRect skBounds;
 
     // On Mountain Lion, CTFontGetBoundingRectsForGlyphs with kCTFontVerticalOrientation and
     // CTFontGetVerticalTranslationsForGlyphs do not agree when using OTF CFF fonts.
     // For TTF fonts these two do agree and we can use CTFontGetBoundingRectsForGlyphs to get
@@ skipping 24 matching lines @@
         }
         skBounds = SkRect::MakeLTRB(gRect.fMinX, gRect.fMinY, gRect.fMaxX, gRect.fMaxY);
         // From FUnits (em space, y up) to SkGlyph units (pixels, y down).
         fFUnitMatrix.mapRect(&skBounds);
 
     } else {
         // CTFontGetBoundingRectsForGlyphs produces cgBounds in CG units (pixels, y up).
         CGRect cgBounds;
         CTFontGetBoundingRectsForGlyphs(fCTFont, kCTFontHorizontalOrientation,
                                         &cgGlyph, &cgBounds, 1);
+        cgBounds = CGRectApplyAffineTransform(cgBounds, fTransform);
 
         // BUG?
         // 0x200B (zero-advance space) seems to return a huge (garbage) bounds, when
         // it should be empty. So, if we see a zero-advance, we check if it has an
         // empty path or not, and if so, we jam the bounds to 0. Hopefully a zero-advance
         // is rare, so we won't incur a big performance cost for this extra check.
         if (0 == cgAdvance.width && 0 == cgAdvance.height) {
             AutoCFRelease<CGPathRef> path(CTFontCreatePathForGlyph(fCTFont, cgGlyph, nullptr));
             if (nullptr == path || CGPathIsEmpty(path)) {
                 return;
@@ skipping 238 matching lines @@
 /*
  *  Our subpixel resolution is only 2 bits in each direction, so a scale of 4
  *  seems sufficient, and possibly even correct, to allow the hinted outline
  *  to be subpixel positioned.
  */
 #define kScaleForSubPixelPositionHinting (4.0f)
 
 void SkScalerContext_Mac::generatePath(const SkGlyph& glyph, SkPath* path) {
     AUTO_CG_LOCK();
 
-    CTFontRef font = fCTFont;
     SkScalar scaleX = SK_Scalar1;
     SkScalar scaleY = SK_Scalar1;
 
+    CGAffineTransform xform = fTransform;
     /*
      *  For subpixel positioning, we want to return an unhinted outline, so it
      *  can be positioned nicely at fractional offsets. However, we special-case
      *  if the baseline of the (horizontal) text is axis-aligned. In those cases
      *  we want to retain hinting in the direction orthogonal to the baseline.
      *  e.g. for horizontal baseline, we want to retain hinting in Y.
      *  The way we remove hinting is to scale the font by some value (4) in that
      *  direction, ask for the path, and then scale the path back down.
      */
     if (fDoSubPosition) {
         SkMatrix m;
         fRec.getSingleMatrix(&m);
 
         // start out by assuming that we want no hining in X and Y
         scaleX = scaleY = kScaleForSubPixelPositionHinting;
         // now see if we need to restore hinting for axis-aligned baselines
         switch (SkComputeAxisAlignmentForHText(m)) {
             case kX_SkAxisAlignment:
                 scaleY = SK_Scalar1; // want hinting in the Y direction
                 break;
             case kY_SkAxisAlignment:
                 scaleX = SK_Scalar1; // want hinting in the X direction
                 break;
             default:
                 break;
         }
 
-        CGAffineTransform xform = MatrixToCGAffineTransform(m, scaleX, scaleY);
-        font = ctfont_create_exact_copy(fCTFont, 1, &xform);
+        CGAffineTransform scale(CGAffineTransformMakeScale(ScalarToCG(scaleX), ScalarToCG(scaleY)));

[mtklein, 2015/10/05 21:58:12]

I feel like this diff is confusing, but I'm not sure the new code is confusing. 
I think the old code is confusing.  I maybe be generally confused.  Might help
to diagram what matrices are concatted here.

+        xform = CGAffineTransformConcat(fTransform, scale);
     }
 
     CGGlyph cgGlyph = (CGGlyph)glyph.getGlyphID();
-    AutoCFRelease<CGPathRef> cgPath(CTFontCreatePathForGlyph(font, cgGlyph, nullptr));
+    AutoCFRelease<CGPathRef> cgPath(CTFontCreatePathForGlyph(fCTFont, cgGlyph, &xform));
 
     path->reset();
     if (cgPath != nullptr) {
         CGPathApply(cgPath, path, SkScalerContext_Mac::CTPathElement);
     }
 
     if (fDoSubPosition) {
         SkMatrix m;
         m.setScale(SkScalarInvert(scaleX), SkScalarInvert(scaleY));
         path->transform(m);
-        // balance the call to ctfont_create_exact_copy
-        CFSafeRelease(font);
     }
     if (fVertical) {
         SkPoint offset;
         getVerticalOffset(cgGlyph, &offset);
         path->offset(offset.fX, offset.fY);
     }
 }
 
 void SkScalerContext_Mac::generateFontMetrics(SkPaint::FontMetrics* metrics) {
     if (nullptr == metrics) {
@@ skipping 1080 matching lines @@
                 face->ref();
             }
         }
         return face;
     }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkFontMgr* SkFontMgr::Factory() { return new SkFontMgr_Mac; }