allow both GDI and DW fontmgrs at the same time

src/ports/SkFontHost_win_dw.cpp

Index: src/ports/SkFontHost_win_dw.cpp
diff --git a/src/ports/SkFontHost_win_dw.cpp b/src/ports/SkFontHost_win_dw.cpp
index e1685e63b0750cbc2afe20657d6e5bd2bfc1b9b2..33a88177d15d8ff4231637c42f84d0a9b033c4e0 100644
--- a/src/ports/SkFontHost_win_dw.cpp
+++ b/src/ports/SkFontHost_win_dw.cpp
@@ -704,16 +704,22 @@ static bool FindByDWriteFont(SkTypeface* face, SkTypeface::Style requestedStyle,
            wcscmp(dwFaceFontNameChar.get(), dwFontNameChar.get()) == 0;
 }
 
-SkTypeface* SkCreateTypefaceFromDWriteFont(IDWriteFontFace* fontFace,
+SkTypeface* SkCreateTypefaceFromDWriteFont(SkTypefaceCache* cache,

[bungeman-skia, 2013/08/13 15:53:45]

This is really odd looking, I think what we logically want is not to pass a
cache, but to pass in the SkFontMgr to which we wish to add this guy. Actually,
should this sort of thing be part of the SkFontMgr?

+                                           IDWriteFontFace* fontFace,
                                            IDWriteFont* font,
                                            IDWriteFontFamily* fontFamily,
                                            StreamFontFileLoader* fontFileLoader = NULL,
                                            IDWriteFontCollectionLoader* fontCollectionLoader = NULL) {
-    SkTypeface* face = SkTypefaceCache::FindByProcAndRef(FindByDWriteFont, font);
+    SkTypeface* face = NULL;
+    if (cache) {
+        face = cache->findByProcAndRef(FindByDWriteFont, font);
+    }
     if (NULL == face) {
         face = DWriteFontTypeface::Create(fontFace, font, fontFamily,
                                           fontFileLoader, fontCollectionLoader);
-        SkTypefaceCache::Add(face, get_style(font), fontCollectionLoader != NULL);
+        if (face) {

[bungeman-skia, 2013/08/13 15:53:45]

&& cache

+            cache->add(face, get_style(font), fontCollectionLoader != NULL);
+        }
     }
     return face;
 }
@@ -1220,7 +1226,8 @@ private:
     T* fUnregister;
 };
 
-static SkTypeface* create_from_stream(SkStream* stream, int ttcIndex) {
+static SkTypeface* create_from_stream(SkStream* stream, int ttcIndex,

[bungeman-skia, 2013/08/13 15:53:45]

I've been debating this for a while, but should this use the cache at all? If
the underlying font is a "local font" (provided by the OS/FontMrg) it makes
sense to cache those typefaces. However, does it make sense to try to cache
typefaces when the user says, "hey, here is some data, try to make a typeface
for me" (external font). If we already have a font of the same name somewhere
(either a "local font" or another "external font") I'm not sure we should be
returning the other one (unless we actually go and verify that the cached one is
byte identical).

+                                      SkTypefaceCache* cache) {
     IDWriteFactory* factory;
     HRN(get_dwrite_factory(&factory));
 
@@ -1258,7 +1265,7 @@ static SkTypeface* create_from_stream(SkStream* stream, int ttcIndex) {
 
             UINT32 faceIndex = fontFace->GetIndex();
             if (faceIndex == ttcIndex) {
-                return SkCreateTypefaceFromDWriteFont(fontFace.get(), font.get(), fontFamily.get(),
+                return SkCreateTypefaceFromDWriteFont(cache, fontFace.get(), font.get(), fontFamily.get(),
                                                       autoUnregisterFontFileLoader.detatch(),
                                                       autoUnregisterFontCollectionLoader.detatch());
             }
@@ -1596,7 +1603,8 @@ SkAdvancedTypefaceMetrics* DWriteFontTypeface::onGetAdvancedTypefaceMetrics(
 
 static SkTypeface* create_typeface(const SkTypeface* familyFace,
                                    const char familyName[],
-                                   unsigned style) {
+                                   unsigned style,
+                                   SkTypefaceCache* cache) {
     HRESULT hr;
     SkTScopedComPtr<IDWriteFontFamily> fontFamily;
     if (familyFace) {
@@ -1627,11 +1635,13 @@ static SkTypeface* create_typeface(const SkTypeface* familyFace,
     SkTScopedComPtr<IDWriteFontFace> fontFace;
     hr = font->CreateFontFace(&fontFace);
 
-    return SkCreateTypefaceFromDWriteFont(fontFace.get(), font.get(), fontFamily.get());
+    return SkCreateTypefaceFromDWriteFont(cache, fontFace.get(), font.get(), fontFamily.get());
 }
 
 SkTypeface* DWriteFontTypeface::onRefMatchingStyle(Style style) const {
-    return create_typeface(this, NULL, style);
+    SkTypefaceCache* cache = NULL;
+    // todo: should each typeface have a ref to its fontmgr/cache?
+    return create_typeface(this, NULL, style, cache);
 }
 
 ///////////////////////////////////////////////////////////////////////////////
@@ -1664,56 +1674,18 @@ class SkFontMgr_DirectWrite;
 
 class SkFontStyleSet_DirectWrite : public SkFontStyleSet {
 public:
-    SkFontStyleSet_DirectWrite(const SkFontMgr_DirectWrite* fontMgr, IDWriteFontFamily* fontFamily)
+    SkFontStyleSet_DirectWrite(SkFontMgr_DirectWrite* fontMgr, IDWriteFontFamily* fontFamily)
         : fFontMgr(SkRef(fontMgr))
         , fFontFamily(SkRefComPtr(fontFamily))
     { }
 
-    virtual int count() SK_OVERRIDE {
-        return fFontFamily->GetFontCount();
-    }
-
-    virtual void getStyle(int index, SkFontStyle* fs, SkString* styleName);
-
-    virtual SkTypeface* createTypeface(int index) SK_OVERRIDE {
-        SkTScopedComPtr<IDWriteFont> font;
-        HRNM(fFontFamily->GetFont(index, &font), "Could not get font.");
-
-        SkTScopedComPtr<IDWriteFontFace> fontFace;
-        HRNM(font->CreateFontFace(&fontFace), "Could not create font face.");
-
-        return SkCreateTypefaceFromDWriteFont(fontFace.get(), font.get(), fFontFamily.get());
-    }
-
-    virtual SkTypeface* matchStyle(const SkFontStyle& pattern) SK_OVERRIDE {
-        DWRITE_FONT_STYLE slant;
-        switch (pattern.slant()) {
-        case SkFontStyle::kUpright_Slant:
-            slant = DWRITE_FONT_STYLE_NORMAL;
-            break;
-        case SkFontStyle::kItalic_Slant:
-            slant = DWRITE_FONT_STYLE_ITALIC;
-            break;
-        default:
-            SkASSERT(false);
-        }
-
-        DWRITE_FONT_WEIGHT weight = (DWRITE_FONT_WEIGHT)pattern.weight();
-        DWRITE_FONT_STRETCH width = (DWRITE_FONT_STRETCH)pattern.width();
-
-        SkTScopedComPtr<IDWriteFont> font;
-        // TODO: perhaps use GetMatchingFonts and get the least simulated?
-        HRNM(fFontFamily->GetFirstMatchingFont(weight, width, slant, &font),
-             "Could not match font in family.");
-
-        SkTScopedComPtr<IDWriteFontFace> fontFace;
-        HRNM(font->CreateFontFace(&fontFace), "Could not create font face.");
-
-        return SkCreateTypefaceFromDWriteFont(fontFace.get(), font.get(), fFontFamily.get());
-    }
+    virtual int count() SK_OVERRIDE;
+    virtual void getStyle(int index, SkFontStyle* fs, SkString* styleName) SK_OVERRIDE;
+    virtual SkTypeface* createTypeface(int index) SK_OVERRIDE;
+    virtual SkTypeface* matchStyle(const SkFontStyle& pattern) SK_OVERRIDE;
 
 private:
-    SkAutoTUnref<const SkFontMgr_DirectWrite> fFontMgr;
+    SkAutoTUnref<SkFontMgr_DirectWrite> fFontMgr;
     SkTScopedComPtr<IDWriteFontFamily> fFontFamily;
 };
 
@@ -1728,10 +1700,13 @@ public:
         memcpy(fLocaleName.get(), localeName, localeNameLength * sizeof(WCHAR));
     }
 
+    SkTypefaceCache* getTypefaceCache() { return &fTFCache; }
+
 private:
     friend class SkFontStyleSet_DirectWrite;
     SkTScopedComPtr<IDWriteFontCollection> fFontCollection;
     SkSMallocWCHAR fLocaleName;
+    SkTypefaceCache fTFCache;
 
 protected:
     virtual int onCountFamilies() SK_OVERRIDE {
@@ -1781,7 +1756,7 @@ protected:
         return sset.matchStyle(fontstyle);
     }
     virtual SkTypeface* onCreateFromStream(SkStream* stream, int ttcIndex) SK_OVERRIDE {
-        return create_from_stream(stream, ttcIndex);
+        return create_from_stream(stream, ttcIndex, this->getTypefaceCache());
     }
     virtual SkTypeface* onCreateFromData(SkData* data, int ttcIndex) SK_OVERRIDE {
         SkAutoTUnref<SkStream> stream(SkNEW_ARGS(SkMemoryStream, (data)));
@@ -1794,10 +1769,27 @@ protected:
 
     virtual SkTypeface* onLegacyCreateTypeface(const char familyName[],
                                                unsigned styleBits) SK_OVERRIDE {
-        return create_typeface(NULL, familyName, styleBits);
+        return create_typeface(NULL, familyName, styleBits, this->getTypefaceCache());
     }
 };
 
+///////////////////////////////////////////////////////////////////////////////
+
+int SkFontStyleSet_DirectWrite::count() {
+    return fFontFamily->GetFontCount();
+}
+
+SkTypeface* SkFontStyleSet_DirectWrite::createTypeface(int index) {
+    SkTScopedComPtr<IDWriteFont> font;
+    HRNM(fFontFamily->GetFont(index, &font), "Could not get font.");
+
+    SkTScopedComPtr<IDWriteFontFace> fontFace;
+    HRNM(font->CreateFontFace(&fontFace), "Could not create font face.");
+
+    return SkCreateTypefaceFromDWriteFont(fFontMgr->getTypefaceCache(),
+                                            fontFace.get(), font.get(), fFontFamily.get());
+}
+
 void SkFontStyleSet_DirectWrite::getStyle(int index, SkFontStyle* fs, SkString* styleName) {
     SkTScopedComPtr<IDWriteFont> font;
     HRVM(fFontFamily->GetFont(index, &font), "Could not get font.");
@@ -1826,6 +1818,34 @@ void SkFontStyleSet_DirectWrite::getStyle(int index, SkFontStyle* fs, SkString*
     }
 }
 
+SkTypeface* SkFontStyleSet_DirectWrite::matchStyle(const SkFontStyle& pattern) {
+    DWRITE_FONT_STYLE slant;
+    switch (pattern.slant()) {
+    case SkFontStyle::kUpright_Slant:
+        slant = DWRITE_FONT_STYLE_NORMAL;
+        break;
+    case SkFontStyle::kItalic_Slant:
+        slant = DWRITE_FONT_STYLE_ITALIC;
+        break;
+    default:
+        SkASSERT(false);
+    }
+
+    DWRITE_FONT_WEIGHT weight = (DWRITE_FONT_WEIGHT)pattern.weight();
+    DWRITE_FONT_STRETCH width = (DWRITE_FONT_STRETCH)pattern.width();
+
+    SkTScopedComPtr<IDWriteFont> font;
+    // TODO: perhaps use GetMatchingFonts and get the least simulated?
+    HRNM(fFontFamily->GetFirstMatchingFont(weight, width, slant, &font),
+            "Could not match font in family.");
+
+    SkTScopedComPtr<IDWriteFontFace> fontFace;
+    HRNM(font->CreateFontFace(&fontFace), "Could not create font face.");
+
+    return SkCreateTypefaceFromDWriteFont(fFontMgr->getTypefaceCache(),
+                                            fontFace.get(), font.get(), fFontFamily.get());
+}
+
 ///////////////////////////////////////////////////////////////////////////////
 
 #ifndef SK_FONTHOST_USES_FONTMGR
@@ -1842,12 +1862,13 @@ SkTypeface* SkFontHost::CreateTypefaceFromFile(const char path[]) {
 }
 
 SkTypeface* SkFontHost::CreateTypefaceFromStream(SkStream* stream) {
-    return create_from_stream(stream, 0);
+    return create_from_stream(stream, 0, NULL);
 }
 
 #endif
 
-SkFontMgr* SkFontMgr::Factory() {
+extern SkFontMgr* SkFontMgr_New_DirectWrite();
+SkFontMgr* SkFontMgr_New_DirectWrite() {
     IDWriteFactory* factory;
     HRNM(get_dwrite_factory(&factory), "Could not get factory.");
 
@@ -1864,3 +1885,4 @@ SkFontMgr* SkFontMgr::Factory() {
 
     return SkNEW_ARGS(SkFontMgr_DirectWrite, (sysFontCollection.get(), localeName, localeNameLen));
 }
+