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;
+sk_sp<SkTypeface> (*gCreateTypefaceDelegate)(const char[], SkTypeface::Style) = nullptr;
+
 void (*gSerializeTypefaceDelegate)(const SkTypeface*, SkWStream* ) = SK_TYPEFACE_DELEGATE;
 sk_sp<SkTypeface> (*gDeserializeTypefaceDelegate)(SkStream* ) = nullptr;
 
 ///////////////////////////////////////////////////////////////////////////////
 
 namespace {
 
+bool UsesOnlyItalicOrBoldStyling(SkFontStyle style) {

[bungeman-skia, 2016/05/24 15:29:11]

I would prefer not to give this a name and staying power, it's a hack that
should be used in one place. Indeed, I would probably prefer it not exist at all
because I don't think the MakeDefault call this is protecting is even needed.

[Mikus, 2016/05/25 10:57:25]

Done.

+  return (style.slant() == SkFontStyle::kItalic_Slant ||
+          style.slant() == SkFontStyle::kUpright_Slant) &&
+         (style.weight() == SkFontStyle::kBold_Weight ||
+          style.weight() == SkFontStyle::kNormal_Weight);
+}
+
+SkTypeface::Style ToSkTypefaceStyle(SkFontStyle style) {
+  return static_cast<SkTypeface::Style>(
+      (style.slant() == SkFontStyle::kItalic_Slant ? SkTypeface::kItalic : SkTypeface::kNormal) |
+      (style.weight() >= SkFontStyle::kBold_Weight ? SkTypeface::kBold : SkTypeface::kNormal));
+}
+
 class SkEmptyTypeface : public SkTypeface {
 public:
     static SkEmptyTypeface* Create() { return new SkEmptyTypeface; }
 protected:
     SkEmptyTypeface() : SkTypeface(SkFontStyle(), 0, true) { }
 
     SkStreamAsset* onOpenStream(int* ttcIndex) const override { return nullptr; }
     SkScalerContext* onCreateScalerContext(const SkScalerContextEffects&,
                                            const SkDescriptor*) const override {
         return nullptr;
@@ skipping 60 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

[bungeman-skia, 2016/05/24 17:45:44]

Usually we would add a define to Chromium to keep the old signature around. I've
created https://codereview.chromium.org/2008003002/ to add
SK_SUPPORT_LEGACY_TYPEFACE_MAKE_FROM_NAME to do this instead so that the logic
has fewer negations in it and so that it's obvious for which users this still
exists.

[Mikus, 2016/05/25 10:57:25]

Done.

 sk_sp<SkTypeface> SkTypeface::MakeFromName(const char name[], Style style) {
     if (gCreateTypefaceDelegate) {

[bungeman-skia, 2016/05/24 17:45:44]

Since the intent is to make this go away in the end, it would probably be better
to make the entire implementation of this method

SkTypeface::MakeFromName(name, SkFontStyle::FromOldStyle(style);

[Mikus, 2016/05/25 10:57:25]

Done.

         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 && UsesOnlyItalicOrBoldStyling(fontStyle)) {

[bungeman-skia, 2016/05/24 15:29:11]

Far from adding to this hack, we need to make it easier to remove in the future.
You should just move gCreateTypefaceDelegate to the SkFontStyle signature and
update the one single user in dm/DM.cpp.

[Mikus, 2016/05/25 10:57:25]

Done.

+        sk_sp<SkTypeface> result = (*gCreateTypefaceDelegate)(name, ToSkTypefaceStyle(fontStyle));
+        if (result) {
+            return result;
+        }
+    }
+    if (nullptr == name && UsesOnlyItalicOrBoldStyling(fontStyle)) {
+        return MakeDefault(ToSkTypefaceStyle(fontStyle));
+    }
+    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;
 }