Avoid CTFontCreateCopyWithAttributes.

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 264 matching lines @@
 
 static SkScalar CGToScalar(CGFloat cgFloat) {
     if (sizeof(CGFloat) == sizeof(float)) {
         return cgFloat;
     } else {
         SkASSERT(sizeof(CGFloat) == sizeof(double));
         return SkDoubleToScalar(cgFloat);
     }
 }
 
-static CGAffineTransform MatrixToCGAffineTransform(const SkMatrix& matrix,
-                                                   SkScalar sx = SK_Scalar1,
-                                                   SkScalar sy = SK_Scalar1) {
-    return CGAffineTransformMake( ScalarToCG(matrix[SkMatrix::kMScaleX] * sx),
-                                 -ScalarToCG(matrix[SkMatrix::kMSkewY]  * sy),
-                                 -ScalarToCG(matrix[SkMatrix::kMSkewX]  * sx),
-                                  ScalarToCG(matrix[SkMatrix::kMScaleY] * sy),
-                                  ScalarToCG(matrix[SkMatrix::kMTransX] * sx),
-                                  ScalarToCG(matrix[SkMatrix::kMTransY] * sy));
+static CGAffineTransform MatrixToCGAffineTransform(const SkMatrix& matrix) {
+    return CGAffineTransformMake( ScalarToCG(matrix[SkMatrix::kMScaleX]),
+                                 -ScalarToCG(matrix[SkMatrix::kMSkewY]),
+                                 -ScalarToCG(matrix[SkMatrix::kMSkewX]),
+                                  ScalarToCG(matrix[SkMatrix::kMScaleY]),
+                                  ScalarToCG(matrix[SkMatrix::kMTransX]),
+                                  ScalarToCG(matrix[SkMatrix::kMTransY]));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 #define BITMAP_INFO_RGB (kCGImageAlphaNoneSkipFirst | kCGBitmapByteOrder32Host)
 
 /**
  * There does not appear to be a publicly accessable API for determining if lcd
  * font smoothing will be applied if we request it. The main issue is that if
  * smoothing is applied a gamma of 2.0 will be used, if not a gamma of 1.0.
@@ skipping 403 matching lines @@
     uint16_t fGlyphCount;
     bool fGeneratedFBoundingBoxes;
     const bool fDoSubPosition;
     const bool fVertical;
 
     friend class Offscreen;
 
     typedef SkScalerContext INHERITED;
 };
 
+// CTFontCreateCopyWithAttributes or CTFontCreateCopyWithSymbolicTraits cannot be used on 10.10

[Mark Mentovai, 2015/09/19 03:18:41]

The comment says 10.10 but the BUG= is about 10.11. Is this right?

[bungeman-skia, 2015/09/19 06:25:47]

After letting this run through the bots and seeing the massive difference on
10.10 vs previous versions, it sure looks like 10.10 has at least similar
issues.

+// as they appear to be buggy with respect to the default font. It is not possible to use
+// descriptors with CTFontCreateWithFontDescriptor, since that does not work with non-system
+// fonts. As a result, create the strike specific CTFonts from the underlying CGFont.
+static CTFontRef ctfont_create_exact_copy(CTFontRef baseFont, CGFloat textSize,
+                                          const CGAffineTransform* transform, bool setVertical)
+{
+    AutoCFRelease<CTFontDescriptorRef> baseDescriptor;
+    AutoCFRelease<CGFontRef> baseCGFont(CTFontCopyGraphicsFont(baseFont, &baseDescriptor));
+
+    // Make a mutable copy of baseDescriptor attributes.
+    AutoCFRelease<CFMutableDictionaryRef> newAttributes([](CTFontDescriptorRef descriptor) ->
+    CFMutableDictionaryRef {
+        if (nullptr == descriptor) {
+            return CFDictionaryCreateMutable(kCFAllocatorDefault, 0,
+                                             &kCFTypeDictionaryKeyCallBacks,
+                                             &kCFTypeDictionaryValueCallBacks);
+        }
+        AutoCFRelease<CFDictionaryRef> attributes(CTFontDescriptorCopyAttributes(descriptor));
+        return CFDictionaryCreateMutableCopy(kCFAllocatorDefault, 0, attributes);
+    }(baseDescriptor));
+
+    // Set the text size in attributes.
+    AutoCFRelease<CFNumberRef> cfTextSize(
+        CFNumberCreate(kCFAllocatorDefault, kCFNumberCGFloatType, &textSize));
+    CFDictionarySetValue(newAttributes, kCTFontSizeAttribute, cfTextSize);
+
+    // Set the transform in attributes.
+    if (nullptr == transform) {
+        CFDictionaryRemoveValue(newAttributes, kCTFontMatrixAttribute);
+    } else {
+        AutoCFRelease<CFDataRef> cfMatrixData(CFDataCreate(
+            kCFAllocatorDefault, reinterpret_cast<const UInt8*>(transform), sizeof(*transform)));
+        CFDictionarySetValue(newAttributes, kCTFontMatrixAttribute, cfMatrixData);
+    }
+
+    // Set vertical orientation to attributes if requested.
+    if (setVertical) {
+        CTFontOrientation ctOrientation = kCTFontVerticalOrientation;
+        AutoCFRelease<CFNumberRef> cfVertical(
+            CFNumberCreate(kCFAllocatorDefault, kCFNumberSInt32Type, &ctOrientation));
+        CFDictionarySetValue(newAttributes, kCTFontOrientationAttribute, cfVertical);
+    }
+
+    // Create the new CTFont from the baseCGFont.
+    AutoCFRelease<CTFontDescriptorRef> newDescriptor(
+        CTFontDescriptorCreateWithAttributes(newAttributes));
+    return CTFontCreateWithGraphicsFont(baseCGFont, textSize, transform, newDescriptor);
+
+}
+
 SkScalerContext_Mac::SkScalerContext_Mac(SkTypeface_Mac* typeface,
                                          const SkDescriptor* desc)
         : INHERITED(typeface, desc)
         , fFBoundingBoxes()
         , fFBoundingBoxesGlyphOffset(0)
         , fGeneratedFBoundingBoxes(false)
         , fDoSubPosition(SkToBool(fRec.fFlags & kSubpixelPositioning_Flag))
         , fVertical(SkToBool(fRec.fFlags & kVertical_Flag))
 
 {
     AUTO_CG_LOCK();
 
     CTFontRef ctFont = typeface->fFontRef.get();
     CFIndex numGlyphs = CTFontGetGlyphCount(ctFont);
     SkASSERT(numGlyphs >= 1 && numGlyphs <= 0xFFFF);
     fGlyphCount = SkToU16(numGlyphs);
 
     // CT on (at least) 10.9 will size color glyphs down from the requested size, but not up.
     // As a result, it is necessary to know the actual device size and request that.
     SkVector scale;
     SkMatrix skTransform;
     fRec.computeMatrices(SkScalerContextRec::kVertical_PreMatrixScale, &scale, &skTransform,
                          nullptr, nullptr, &fFUnitMatrix);
     fTransform = MatrixToCGAffineTransform(skTransform);
     fInvTransform = CGAffineTransformInvert(fTransform);
 
-    AutoCFRelease<CTFontDescriptorRef> ctFontDesc;
-    if (fVertical) {
-        // Setting the vertical orientation here is required for vertical metrics on some versions.
-        AutoCFRelease<CFMutableDictionaryRef> cfAttributes(CFDictionaryCreateMutable(
-                kCFAllocatorDefault, 0,
-                &kCFTypeDictionaryKeyCallBacks,
-                &kCFTypeDictionaryValueCallBacks));
-        if (cfAttributes) {
-            CTFontOrientation ctOrientation = kCTFontVerticalOrientation;
-            AutoCFRelease<CFNumberRef> cfVertical(CFNumberCreate(
-                    kCFAllocatorDefault, kCFNumberSInt32Type, &ctOrientation));
-            CFDictionaryAddValue(cfAttributes, kCTFontOrientationAttribute, cfVertical);
-            ctFontDesc.reset(CTFontDescriptorCreateWithAttributes(cfAttributes));
-        }
-    }
-
     // 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(CTFontCreateCopyWithAttributes(ctFont, textSize, &fTransform, ctFontDesc));
+    fCTFont.reset(ctfont_create_exact_copy(ctFont, textSize, &fTransform, fVertical));
     fCGFont.reset(CTFontCopyGraphicsFont(fCTFont, nullptr));
-    fCTUnrotatedFont.reset(CTFontCreateCopyWithAttributes(ctFont, textSize,
-                                                          &CGAffineTransformIdentity, nullptr));
+    fCTUnrotatedFont.reset(ctfont_create_exact_copy(ctFont, textSize, nullptr, false));
 
     // 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 561 matching lines @@
 
     /*
      *  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 (fRec.fFlags & SkScalerContext::kSubpixelPositioning_Flag) {

[Mark Mentovai, 2015/09/19 03:18:41]

Can this be if (fDoSubPosition)? It’s harder to reason about whether the “need
to release” font on line 1403 is properly balanced by the CFSafeRelease() on
line 1420 when the conditions that guard them are different.

[bungeman-skia, 2015/09/19 06:25:47]

Yes, that should be changed, so why not change it now? Done.

         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);
-        // need to release font when we're done
-        font = CTFontCreateCopyWithAttributes(fCTFont, 1, &xform, nullptr);
+        CGFloat textSize = CTFontGetSize(fCTFont);
+        CGAffineTransform baseTransform = CTFontGetMatrix(fCTFont);
+        CGAffineTransform newTransform =
+            CGAffineTransformScale(baseTransform, ScalarToCG(scaleX), ScalarToCG(scaleY));
+
+        // need to release this font when we're done
+        font = ctfont_create_exact_copy(fCTFont, textSize, &newTransform, false);
     }
 
     CGGlyph cgGlyph = (CGGlyph)glyph.getGlyphID();
     AutoCFRelease<CGPathRef> cgPath(CTFontCreatePathForGlyph(font, cgGlyph, nullptr));
 
     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 CTFontCreateCopyWithAttributes
+        // 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) {
@@ skipping 160 matching lines @@
 }
 
 SkAdvancedTypefaceMetrics* SkTypeface_Mac::onGetAdvancedTypefaceMetrics(
         PerGlyphInfo perGlyphInfo,
         const uint32_t* glyphIDs,
         uint32_t glyphIDsCount) const {
 
     AUTO_CG_LOCK();
 
     CTFontRef originalCTFont = fFontRef.get();
-    AutoCFRelease<CTFontRef> ctFont(CTFontCreateCopyWithAttributes(
-            originalCTFont, CTFontGetUnitsPerEm(originalCTFont), nullptr, nullptr));
+    AutoCFRelease<CTFontRef> ctFont(ctfont_create_exact_copy(
+            originalCTFont, CTFontGetUnitsPerEm(originalCTFont), nullptr, false));
+
     SkAdvancedTypefaceMetrics* info = new SkAdvancedTypefaceMetrics;
 
     {
         AutoCFRelease<CFStringRef> fontName(CTFontCopyPostScriptName(ctFont));
         if (fontName.get()) {
             CFStringToSkString(fontName, &info->fFontName);
         }
     }
 
     CFIndex glyphCount = CTFontGetGlyphCount(ctFont);
@@ skipping 898 matching lines @@
                 face->ref();
             }
         }
         return face;
     }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkFontMgr* SkFontMgr::Factory() { return new SkFontMgr_Mac; }