SkTypeface::MakeFromName to take SkFontStyle.

src/core/SkTypeface.cpp

Index: src/core/SkTypeface.cpp
diff --git a/src/core/SkTypeface.cpp b/src/core/SkTypeface.cpp
index 3c15878f737bc5c10a1d38b9d0377487300e8e38..0382057fdb20fe019e7c12ab2c1ecf43cafdecee 100644
--- a/src/core/SkTypeface.cpp
+++ b/src/core/SkTypeface.cpp
@@ -27,7 +27,11 @@ extern void WhitelistSerializeTypeface(const SkTypeface*, SkWStream* );
 #define SK_TYPEFACE_DELEGATE nullptr
 #endif
 
-sk_sp<SkTypeface> (*gCreateTypefaceDelegate)(const char [], SkTypeface::Style ) = nullptr;
+#ifndef SK_DONT_USE_LEGACY_TYPEFACE_MAKE_FROM_NAME
+sk_sp<SkTypeface>(*gCreateTypefaceDelegate)(const char[], SkTypeface::Style style) = nullptr;
+#endif
+sk_sp<SkTypeface> (*gNewCreateTypefaceDelegate)(const char[], SkFontStyle) = nullptr;

[bungeman-skia, 2016/05/24 14:49:26]

Only Skia uses this gCreateTypefaceDelegate, and only for testing. It would be
better to make sure that Skia is updated and just update the existing type. If
Skia is no longer using SkTypeface::MakeFromName(const char[], Style), then the
'if (gCreateTypefaceDelegate) {' block in that call can just be removed. This
avoids the awkward 'New' in the name which will, of course, soon not be new. I
think the only user that would need to be updated is dm/DM.cpp. You'll need to
do that anyway to get dm to compile.

[Mikus, 2016/05/24 15:22:26]

Done. Seems to me that the 'new' version is not needed at all.

+
 void (*gSerializeTypefaceDelegate)(const SkTypeface*, SkWStream* ) = SK_TYPEFACE_DELEGATE;
 sk_sp<SkTypeface> (*gDeserializeTypefaceDelegate)(SkStream* ) = nullptr;
 
@@ -115,6 +119,7 @@ bool SkTypeface::Equal(const SkTypeface* facea, const SkTypeface* faceb) {
 
 ///////////////////////////////////////////////////////////////////////////////
 
+#ifndef SK_DONT_USE_LEGACY_TYPEFACE_MAKE_FROM_NAME
 sk_sp<SkTypeface> SkTypeface::MakeFromName(const char name[], Style style) {
     if (gCreateTypefaceDelegate) {
         sk_sp<SkTypeface> result = (*gCreateTypefaceDelegate)(name, style);
@@ -128,6 +133,23 @@ sk_sp<SkTypeface> SkTypeface::MakeFromName(const char name[], Style style) {
     SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
     return sk_sp<SkTypeface>(fm->legacyCreateTypeface(name, SkFontStyle::FromOldStyle(style)));
 }
+#endif
+
+sk_sp<SkTypeface> SkTypeface::MakeFromName(const char name[],
+                                           SkFontStyle fontStyle) {
+    if (gCreateTypefaceDelegate) {
+        sk_sp<SkTypeface> result = (*gNewCreateTypefaceDelegate)(name, fontStyle);
+        if (result) {
+            return result;
+        }
+    }
+    if (nullptr == name) {

[bungeman-skia, 2016/05/24 14:49:26]

MakeDefault is just for caching common requests, and calling this way simply
gives incorrect results. At the very least the test here would need to be

if (nullptr == name &&
     (fontStyle.slant() == SkFontStyle::kItalic_Slant ||
      fontStyle.slant() == SkFontStyle::kNormal_Slant) &&
     (fontStyle.weight() == skFontStyle::kBold_Weight ||
      fontStyle.weight() == skFontStyle::kNormal_Weight)
{
    ...
}

otherwise calling this with nullptr as 'name' may give the wrong result.

[Mikus, 2016/05/24 15:22:26]

Done.

+        return MakeDefault(static_cast<Style>((fontStyle.slant() == SkFontStyle::kItalic_Slant ? kItalic : kNormal) |

[bungeman-skia, 2016/05/24 14:49:26]

nit (here and a few other places): Skia allows 100 character long lines, and
these are a bit long.

[Mikus, 2016/05/24 15:22:26]

Done.

+                                              (fontStyle.weight() >= SkFontStyle::kBold_Weight ? kBold : kNormal)));
+    }
+    SkAutoTUnref<SkFontMgr> fm(SkFontMgr::RefDefault());
+    return sk_sp<SkTypeface>(fm->legacyCreateTypeface(name, fontStyle));
+}
 
 sk_sp<SkTypeface> SkTypeface::MakeFromTypeface(SkTypeface* family, Style s) {
     if (!family) {
@@ -192,7 +214,8 @@ sk_sp<SkTypeface> SkTypeface::MakeDeserialize(SkStream* stream) {
             return typeface;
         }
     }
-    return SkTypeface::MakeFromName(desc.getFamilyName(), desc.getStyle());
+
+    return SkTypeface::MakeFromName(desc.getFamilyName(), SkFontStyle::FromOldStyle(desc.getStyle()));
 }
 
 ///////////////////////////////////////////////////////////////////////////////