SkTypeface::MakeFromName to take SkFontStyle.

src/core/SkTypeface.cpp

 /*
  * Copyright 2011 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 "SkAdvancedTypefaceMetrics.h"
 #include "SkEndian.h"
 #include "SkFontDescriptor.h"
 #include "SkFontMgr.h"
 #include "SkMutex.h"
 #include "SkOTTable_OS_2.h"
 #include "SkOnce.h"
 #include "SkStream.h"
 #include "SkTypeface.h"
 
 SkTypeface::SkTypeface(const SkFontStyle& style, SkFontID fontID, bool isFixedPitch)
     : fUniqueID(fontID), fStyle(style), fIsFixedPitch(isFixedPitch) { }
 
 SkTypeface::~SkTypeface() { }
 
 #ifdef SK_WHITELIST_SERIALIZED_TYPEFACES
 extern void WhitelistSerializeTypeface(const SkTypeface*, SkWStream* );
 #define SK_TYPEFACE_DELEGATE WhitelistSerializeTypeface
 #else
 #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;
 
 ///////////////////////////////////////////////////////////////////////////////
 
 namespace {
 
 class SkEmptyTypeface : public SkTypeface {
 public:
     static SkEmptyTypeface* Create() { return new SkEmptyTypeface; }
@@ skipping 67 matching lines @@
     }
     return face->uniqueID();
 }
 
 bool SkTypeface::Equal(const SkTypeface* facea, const SkTypeface* faceb) {
     return facea == faceb || SkTypeface::UniqueID(facea) == SkTypeface::UniqueID(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);
         if (result) {
             return result;
         }
     }
     if (nullptr == name) {
         return MakeDefault(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) {
         return SkTypeface::MakeDefault(s);
     }
 
     if (family->style() == s) {
         return sk_ref_sp(family);
     }
 
@@ skipping 44 matching lines @@
         return nullptr;
     }
 
     SkFontData* data = desc.detachFontData();
     if (data) {
         sk_sp<SkTypeface> typeface(SkTypeface::MakeFromFontData(data));
         if (typeface) {
             return typeface;
         }
     }
-    return SkTypeface::MakeFromName(desc.getFamilyName(), desc.getStyle());
+
+    return SkTypeface::MakeFromName(desc.getFamilyName(), SkFontStyle::FromOldStyle(desc.getStyle()));
 }
 
 ///////////////////////////////////////////////////////////////////////////////
 
 int SkTypeface::countTables() const {
     return this->onGetTableTags(nullptr);
 }
 
 int SkTypeface::getTableTags(SkFontTableTag tags[]) const {
     return this->onGetTableTags(tags);
@@ skipping 144 matching lines @@
     SkAutoTDelete<SkScalerContext> ctx(this->createScalerContext(noeffects, desc, true));
     if (ctx.get()) {
         SkPaint::FontMetrics fm;
         ctx->getFontMetrics(&fm);
         bounds->set(fm.fXMin * invTextSize, fm.fTop * invTextSize,
                     fm.fXMax * invTextSize, fm.fBottom * invTextSize);
         return true;
     }
     return false;
 }