Test CTFonts for equality, not just name and style.

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 ...
 #if defined(SK_BUILD_FOR_MAC) || defined(SK_BUILD_FOR_IOS)
 
@@ skipping 540 matching lines @@
     if (!ctFontDesc) {
         return nullptr;
     }
 
     AutoCFRelease<CTFontRef> ctFont(CTFontCreateWithFontDescriptor(ctFontDesc, 0, nullptr));
     if (!ctFont) {
         return nullptr;
     }
 
     SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_CTFontRef, (void*)ctFont.get());
-    if (!face) {
-        face = NewFromFontRef(ctFont.release(), nullptr, nullptr, false);
-        SkTypefaceCache::Add(face, face->fontStyle());
+    if (face) {
+        return face;
     }
+    face = NewFromFontRef(ctFont.release(), nullptr, nullptr, false);
+    SkTypefaceCache::Add(face, face->fontStyle());
     return face;
 }
 
 SK_DECLARE_STATIC_MUTEX(gGetDefaultFaceMutex);
 static SkTypeface* GetDefaultFace() {
     SkAutoMutexAcquire ma(gGetDefaultFaceMutex);
 
     static SkTypeface* gDefaultFace;
 
     if (nullptr == gDefaultFace) {
         gDefaultFace = NewFromName(FONT_DEFAULT_NAME, SkFontStyle());
-        SkTypefaceCache::Add(gDefaultFace, SkFontStyle());
     }
     return gDefaultFace;
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 extern CTFontRef SkTypeface_GetCTFontRef(const SkTypeface* face);
 CTFontRef SkTypeface_GetCTFontRef(const SkTypeface* face) {
     const SkTypeface_Mac* macface = (const SkTypeface_Mac*)face;
     return macface ? macface->fFontRef.get() : nullptr;
 }
 
 /*  This function is visible on the outside. It first searches the cache, and if
  *  not found, returns a new entry (after adding it to the cache).
  */
 SkTypeface* SkCreateTypefaceFromCTFont(CTFontRef fontRef, CFTypeRef resourceRef) {
     SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_CTFontRef, (void*)fontRef);
-    if (!face) {
-        CFRetain(fontRef);
-        if (resourceRef) {
-            CFRetain(resourceRef);
-        }
-        face = NewFromFontRef(fontRef, resourceRef, nullptr, false);
-        SkTypefaceCache::Add(face, face->fontStyle());
+    if (face) {
+        return face;
     }
+    CFRetain(fontRef);
+    if (resourceRef) {
+        CFRetain(resourceRef);
+    }
+    face = NewFromFontRef(fontRef, resourceRef, nullptr, false);
+    SkTypefaceCache::Add(face, face->fontStyle());
     return face;
 }
 
 struct NameStyle {
     const char* fName;
     SkFontStyle fStyle;
 };
 
 static bool find_by_NameStyle(SkTypeface* cachedFace, const SkFontStyle& cachedStyle, void* ctx) {
     const SkTypeface_Mac* cachedMacFace = static_cast<SkTypeface_Mac*>(cachedFace);
@@ skipping 1561 matching lines @@
 }
 
 // We normalize each axis (weight, width, italic) to be base-900
 static int compute_metric(const SkFontStyle& a, const SkFontStyle& b) {
     return sqr(a.weight() - b.weight()) +
            sqr((a.width() - b.width()) * 100) +
            sqr((a.isItalic() != b.isItalic()) * 900);
 }
 
 static SkTypeface* createFromDesc(CFStringRef cfFamilyName, CTFontDescriptorRef desc) {
-    NameStyle cacheRequest;
     SkString skFamilyName;
     CFStringToSkString(cfFamilyName, &skFamilyName);
-    cacheRequest.fName = skFamilyName.c_str();
-    cacheRequest.fStyle = fontstyle_from_descriptor(desc);
-
-    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_NameStyle, &cacheRequest);
-    if (face) {
-        return face;
-    }
 
     AutoCFRelease<CTFontRef> ctFont(CTFontCreateWithFontDescriptor(desc, 0, nullptr));
     if (!ctFont) {
         return nullptr;
     }
 
-    bool isFixedPitch;
-    (void)computeStyleBits(ctFont, &isFixedPitch);
+    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_CTFontRef, (void*)ctFont.get());
+    if (face) {
+        return face;
+    }
 
-    face = new SkTypeface_Mac(ctFont.release(), nullptr, cacheRequest.fStyle, isFixedPitch,
-                              skFamilyName.c_str(), false);
+    face = NewFromFontRef(ctFont.release(), nullptr, skFamilyName.c_str(), false);
     SkTypefaceCache::Add(face, face->fontStyle());
     return face;
 }
 
 class SkFontStyleSet_Mac : public SkFontStyleSet {
 public:
     SkFontStyleSet_Mac(CFStringRef familyName, CTFontDescriptorRef desc)
         : fArray(CTFontDescriptorCreateMatchingFontDescriptors(desc, nullptr))
         , fFamilyName(familyName)
         , fCount(0) {
@@ skipping 390 matching lines @@
         SkFontStyle style = SkFontStyle((SkTypeface::Style)styleBits);
         if (familyName) {
             familyName = map_css_names(familyName);
         }
 
         if (!familyName || !*familyName) {
             familyName = FONT_DEFAULT_NAME;
         }
 
         NameStyle cacheRequest = { familyName, style };
         SkTypeface* face = SkTypefaceCache::FindByProcAndRef(find_by_NameStyle, &cacheRequest);

[bungeman-skia, 2016/04/05 22:03:26]

Leaving this find_by_NameStyle here is trouble, which shows up in some gms. Some
gms like typefacestyles_kerningMac end up with some of the lines looking
different. Now that we're doing the other lookups correctly, there are a lot
more typefaces in the cache after running the font iteration gms and tests, so
it is more likely that the wrong one is going to be a cache hit. This is
normally observed as the fist line of typefacestyles_kerningMac being the
'wrong' font, it has the right name and style value, but it is not the one which
would have been selected had we done the lookup.

+        if (face) {
+            return face;
+        }
 
-        if (nullptr == face) {
-            face = NewFromName(familyName, style);
-            if (face) {
-                SkTypefaceCache::Add(face, style);
-            } else {
-                face = GetDefaultFace();
-                face->ref();
-            }
+        face = NewFromName(familyName, style);
+        if (face) {
+            return face;
         }
-        return face;
+
+        return SkSafeRef(GetDefaultFace());
     }
 };
 
 ///////////////////////////////////////////////////////////////////////////////
 
 SkFontMgr* SkFontMgr::Factory() { return new SkFontMgr_Mac; }
 
 #endif//defined(SK_BUILD_FOR_MAC) || defined(SK_BUILD_FOR_IOS)