Fix typeface ids on Mac.

The current typeface ids with CoreText rely on ATS (which causes
memory use issues) or font data (which is unreliable, and will not
work with axes and synthetics). This changes the code so that stream
fonts never end up in the cache and always get a unique id, and
system fonts will be given an id as they are used.

Review URL: https://codereview.chromium.org/1125763002

diff --git a/src/core/SkTypefaceCache.cpp b/src/core/SkTypefaceCache.cpp
index 8adffe6a3a5cc6fa7c90ea8139cc1ab2d1537d3e..f253b60523b40ad9e79ffd4195d12285abe1e998 100644 (file)
--- a/src/core/SkTypefaceCache.cpp
+++ b/src/core/SkTypefaceCache.cpp
@@ -34,18 +34,6 @@ void SkTypefaceCache::add(SkTypeface* face, const SkFontStyle& requestedStyle) {
     rec->fRequestedStyle = requestedStyle;
 }
 
-SkTypeface* SkTypefaceCache::findByID(SkFontID fontID) const {
-    const Rec* curr = fArray.begin();
-    const Rec* stop = fArray.end();
-    while (curr < stop) {
-        if (curr->fFace->uniqueID() == fontID) {
-            return curr->fFace;
-        }
-        curr += 1;
-    }
-    return NULL;
-}
-
 SkTypeface* SkTypefaceCache::findByProcAndRef(FindProc proc, void* ctx) const {
     const Rec* curr = fArray.begin();
     const Rec* stop = fArray.end();
@@ -100,11 +88,6 @@ void SkTypefaceCache::Add(SkTypeface* face, const SkFontStyle& requestedStyle) {
     Get().add(face, requestedStyle);
 }
 
-SkTypeface* SkTypefaceCache::FindByID(SkFontID fontID) {
-    SkAutoMutexAcquire ama(gMutex);
-    return Get().findByID(fontID);
-}
-
 SkTypeface* SkTypefaceCache::FindByProcAndRef(FindProc proc, void* ctx) {
     SkAutoMutexAcquire ama(gMutex);
     SkTypeface* typeface = Get().findByProcAndRef(proc, ctx);

diff --git a/src/core/SkTypefaceCache.h b/src/core/SkTypefaceCache.h
index c6b433deadc6245a291331a9d690129d877d396c..792958679c45a926bb4076f14b5edd9f79191d13 100644 (file)
--- a/src/core/SkTypefaceCache.h
+++ b/src/core/SkTypefaceCache.h
@@ -41,14 +41,6 @@ public:
      */
     void add(SkTypeface*, const SkFontStyle& requested);
 
-    /**
-     *  Search the cache for a typeface with the specified fontID (uniqueID).
-     *  If one is found, return it (its reference count is unmodified). If none
-     *  is found, return NULL. The reference count is unmodified as it is
-     *  assumed that the stack will contain a ref to the typeface.
-     */
-    SkTypeface* findByID(SkFontID findID) const;
-
     /**
      *  Iterate through the cache, calling proc(typeface, ctx) with each
      *  typeface. If proc returns true, then we return that typeface (this
@@ -73,7 +65,6 @@ public:
     // These are static wrappers around a global instance of a cache.
 
     static void Add(SkTypeface*, const SkFontStyle& requested);
-    static SkTypeface* FindByID(SkFontID fontID);
     static SkTypeface* FindByProcAndRef(FindProc proc, void* ctx);
     static void PurgeAll();
 

diff --git a/src/ports/SkFontHost_mac.cpp b/src/ports/SkFontHost_mac.cpp
index 5eaf7df6036b281fe08560d07baeef9e418fa7fe..d97b09c6516cb8df62b0681fbb3113f17c803bad 100755 (executable)
--- a/src/ports/SkFontHost_mac.cpp
+++ b/src/ports/SkFontHost_mac.cpp
@@ -92,6 +92,12 @@ public:
         }
     }
 
+    CFRef detach() {
+        CFRef self = fCFRef;
+        fCFRef = NULL;
+        return self;
+    }
+
     operator CFRef() const { return fCFRef; }
     CFRef get() const { return fCFRef; }
 
@@ -408,34 +414,6 @@ static SkTypeface::Style computeStyleBits(CTFontRef font, bool* isFixedPitch) {
     return (SkTypeface::Style)style;
 }
 
-static SkFontID CTFontRef_to_SkFontID(CTFontRef fontRef) {
-    SkFontID id = 0;
-// CTFontGetPlatformFont and ATSFontRef are not supported on iOS, so we have to
-// bracket this to be Mac only.
-#ifdef SK_BUILD_FOR_MAC
-    ATSFontRef ats = CTFontGetPlatformFont(fontRef, NULL);
-    id = (SkFontID)ats;
-    if (id != 0) {
-        id &= 0x3FFFFFFF; // make top two bits 00
-        return id;
-    }
-#endif
-    // CTFontGetPlatformFont returns NULL if the font is local
-    // (e.g., was created by a CSS3 @font-face rule).
-    AutoCFRelease<CGFontRef> cgFont(CTFontCopyGraphicsFont(fontRef, NULL));
-    AutoCGTable<SkOTTableHead> headTable(cgFont);
-    if (headTable.fData) {
-        id = (SkFontID) headTable->checksumAdjustment;
-        id = (id & 0x3FFFFFFF) | 0x40000000; // make top two bits 01
-    }
-    // well-formed fonts have checksums, but as a last resort, use the pointer.
-    if (id == 0) {
-        id = (SkFontID) (uintptr_t) fontRef;
-        id = (id & 0x3FFFFFFF) | 0x80000000; // make top two bits 10
-    }
-    return id;
-}
-
 #define WEIGHT_THRESHOLD    ((SkFontStyle::kNormal_Weight + SkFontStyle::kBold_Weight)/2)
 
 // kCTFontColorGlyphsTrait was added in the Mac 10.7 and iPhone 4.3 SDKs.
@@ -448,9 +426,9 @@ static const uint32_t SkCTFontColorGlyphsTrait = (1 << 13);
 
 class SkTypeface_Mac : public SkTypeface {
 public:
-    SkTypeface_Mac(const SkFontStyle& fs, SkFontID fontID, bool isFixedPitch,
+    SkTypeface_Mac(const SkFontStyle& fs, bool isFixedPitch,
                    CTFontRef fontRef, const char requestedName[], bool isLocalStream)
-        : SkTypeface(fs, fontID, isFixedPitch)
+        : SkTypeface(fs, SkTypefaceCache::NewFontID(), isFixedPitch)
         , fRequestedName(requestedName)
         , fFontRef(fontRef) // caller has already called CFRetain for us
         , fHasColorGlyphs(SkToBool(CTFontGetSymbolicTraits(fFontRef) & SkCTFontColorGlyphsTrait))
@@ -487,18 +465,24 @@ private:
     typedef SkTypeface INHERITED;
 };
 
+/** Creates a typeface without searching the cache. Takes ownership of the CTFontRef. */
 static SkTypeface* NewFromFontRef(CTFontRef fontRef, const char name[], bool isLocalStream) {
     SkASSERT(fontRef);
     bool isFixedPitch;
     SkFontStyle style = SkFontStyle(computeStyleBits(fontRef, &isFixedPitch));
-    SkFontID fontID = CTFontRef_to_SkFontID(fontRef);
 
-    return new SkTypeface_Mac(style, fontID, isFixedPitch, fontRef, name, isLocalStream);
+    return new SkTypeface_Mac(style, isFixedPitch, fontRef, name, isLocalStream);
 }
 
-static SkTypeface* NewFromName(const char familyName[], const SkFontStyle& theStyle) {
-    CTFontRef ctFont = NULL;
+static bool find_by_CTFontRef(SkTypeface* cached, const SkFontStyle&, void* context) {
+    CTFontRef self = (CTFontRef)context;
+    CTFontRef other = ((SkTypeface_Mac*)cached)->fFontRef;
 
+    return CFEqual(self, other);
+}
+
+/** Creates a typeface from a name, searching the cache. */
+static SkTypeface* NewFromName(const char familyName[], const SkFontStyle& theStyle) {
     CTFontSymbolicTraits ctFontTraits = 0;
     if (theStyle.weight() >= SkFontStyle::kBold_Weight) {
         ctFontTraits |= kCTFontBoldTrait;
@@ -525,22 +509,32 @@ static SkTypeface* NewFromName(const char familyName[], const SkFontStyle& theSt
                                       &kCFTypeDictionaryKeyCallBacks,
                                       &kCFTypeDictionaryValueCallBacks));
 
-    // Create the font
-    if (cfFontName != NULL && cfFontTraits != NULL && cfAttributes != NULL && cfTraits != NULL) {
-        CFDictionaryAddValue(cfTraits, kCTFontSymbolicTrait, cfFontTraits);
+    if (!cfFontName || !cfFontTraits || !cfAttributes || !cfTraits) {
+        return NULL;
+    }
 
-        CFDictionaryAddValue(cfAttributes, kCTFontFamilyNameAttribute, cfFontName);
-        CFDictionaryAddValue(cfAttributes, kCTFontTraitsAttribute, cfTraits);
+    CFDictionaryAddValue(cfTraits, kCTFontSymbolicTrait, cfFontTraits);
 
-        AutoCFRelease<CTFontDescriptorRef> ctFontDesc(
-                CTFontDescriptorCreateWithAttributes(cfAttributes));
+    CFDictionaryAddValue(cfAttributes, kCTFontFamilyNameAttribute, cfFontName);
+    CFDictionaryAddValue(cfAttributes, kCTFontTraitsAttribute, cfTraits);
 
-        if (ctFontDesc != NULL) {
-            ctFont = CTFontCreateWithFontDescriptor(ctFontDesc, 0, NULL);
-        }
+    AutoCFRelease<CTFontDescriptorRef> ctFontDesc(
+            CTFontDescriptorCreateWithAttributes(cfAttributes));
+    if (!ctFontDesc) {
+        return NULL;
     }
 
-    return ctFont ? NewFromFontRef(ctFont, familyName, false) : NULL;
+    AutoCFRelease<CTFontRef> ctFont(CTFontCreateWithFontDescriptor(ctFontDesc, 0, NULL));
+    if (!ctFont) {
+        return NULL;
+    }
+
+    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_CTFontRef, (void*)ctFont.get());
+    if (!face) {
+        face = NewFromFontRef(ctFont.detach(), NULL, false);
+        SkTypefaceCache::Add(face, face->fontStyle());
+    }
+    return face;
 }
 
 SK_DECLARE_STATIC_MUTEX(gGetDefaultFaceMutex);
@@ -568,19 +562,12 @@ CTFontRef SkTypeface_GetCTFontRef(const SkTypeface* face) {
  *  not found, returns a new entry (after adding it to the cache).
  */
 SkTypeface* SkCreateTypefaceFromCTFont(CTFontRef fontRef) {
-    SkFontID fontID = CTFontRef_to_SkFontID(fontRef);
-    SkTypeface* face = SkTypefaceCache::FindByID(fontID);
-    if (face) {
-        face->ref();
-    } else {
+    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_CTFontRef, (void*)fontRef);
+    if (!face) {
+        CFRetain(fontRef);
         face = NewFromFontRef(fontRef, NULL, false);
         SkTypefaceCache::Add(face, face->fontStyle());
-        // NewFromFontRef doesn't retain the parameter, but the typeface it
-        // creates does release it in its destructor, so we balance that with
-        // a retain call here.
-        CFRetain(fontRef);
     }
-    SkASSERT(face->getRefCnt() > 1);
     return face;
 }
 
@@ -2083,24 +2070,16 @@ static SkTypeface* createFromDesc(CFStringRef cfFamilyName, CTFontDescriptorRef
         return face;
     }
 
-    AutoCFRelease<CFDictionaryRef> fontFamilyNameDictionary(
-        CFDictionaryCreate(kCFAllocatorDefault,
-                           (const void**)&kCTFontFamilyNameAttribute, (const void**)&cfFamilyName,
-                           1, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
-    AutoCFRelease<CTFontDescriptorRef> fontDescriptor(
-        CTFontDescriptorCreateWithAttributes(fontFamilyNameDictionary));
-    AutoCFRelease<CTFontRef> ctNamed(CTFontCreateWithFontDescriptor(fontDescriptor, 0, NULL));
-    CTFontRef ctFont = CTFontCreateCopyWithAttributes(ctNamed, 1, NULL, desc);
-    if (NULL == ctFont) {
+    AutoCFRelease<CTFontRef> ctFont(CTFontCreateWithFontDescriptor(desc, 0, NULL));
+    if (!ctFont) {
         return NULL;
     }
 
     bool isFixedPitch;
     (void)computeStyleBits(ctFont, &isFixedPitch);
-    SkFontID fontID = CTFontRef_to_SkFontID(ctFont);
 
-    face = SkNEW_ARGS(SkTypeface_Mac, (cacheRequest.fStyle, fontID, isFixedPitch,
-                                       ctFont, skFamilyName.c_str(), false));
+    face = SkNEW_ARGS(SkTypeface_Mac, (cacheRequest.fStyle, isFixedPitch,
+                                       ctFont.detach(), skFamilyName.c_str(), false));
     SkTypefaceCache::Add(face, face->fontStyle());
     return face;
 }