commit 7880a8f62967bc8cd953538ba057d614e4e76a91
author Ben Wagner <bungeman@google.com> Thu Mar 16 21:42:18 2023 -0400
committer SkCQ <skcq-be@skia-corp.google.com.iam.gserviceaccount.com> Fri Mar 17 18:56:49 2023 +0000
tree dde7813c4f6e95a0878a67ba217e1b50b400cdab
parent 9400df79e8387f914a70a62cfc61fa0aa21e0353

Add tests for SkFontMgr::matchFamilyStyle

Ensure that matchFamilyStyle returns nullptr when it should and doesn't
when it shouldn't.

Update SkFontMgr_Mac to pass the tests.

Change-Id: Ia8faa68f9961095454b8c0e8d792073516b33314
Reviewed-on: https://skia-review.googlesource.com/c/skia/+/657658
Commit-Queue: Ben Wagner <bungeman@google.com>
Reviewed-by: Herb Derby <herb@google.com>

src/ports/SkFontMgr_mac_ct.cpp

diff --git a/src/ports/SkFontMgr_mac_ct.cpp b/src/ports/SkFontMgr_mac_ct.cpp
index 60e530e..f6b3ad6 100644
--- a/src/ports/SkFontMgr_mac_ct.cpp
+++ b/src/ports/SkFontMgr_mac_ct.cpp
@@ -249,10 +249,17 @@
            sqr((a.slant() != b.slant()) * 900);
 }
 
+static SkUniqueCFRef<CFSetRef> name_required() {
+    CFStringRef set_values[] = {kCTFontFamilyNameAttribute};
+    return SkUniqueCFRef<CFSetRef>(CFSetCreate(kCFAllocatorDefault,
+        reinterpret_cast<const void**>(set_values), std::size(set_values),
+        &kCFTypeSetCallBacks));
+}
+
 class SkFontStyleSet_Mac : public SkFontStyleSet {
 public:
     SkFontStyleSet_Mac(CTFontDescriptorRef desc)
-        : fArray(CTFontDescriptorCreateMatchingFontDescriptors(desc, nullptr))
+        : fArray(CTFontDescriptorCreateMatchingFontDescriptors(desc, name_required().get()))
         , fCount(0)
     {
         if (!fArray) {
@@ -433,8 +440,16 @@
 
     SkTypeface* onMatchFamilyStyle(const char familyName[],
                                    const SkFontStyle& style) const override {
-        SkUniqueCFRef<CTFontDescriptorRef> desc = create_descriptor(familyName, style);
-        return create_from_desc(desc.get()).release();
+        SkUniqueCFRef<CTFontDescriptorRef> reqDesc = create_descriptor(familyName, style);
+        if (!familyName) {
+            return create_from_desc(reqDesc.get()).release();
+        }
+        SkUniqueCFRef<CTFontDescriptorRef> resolvedDesc(
+            CTFontDescriptorCreateMatchingFontDescriptor(reqDesc.get(), name_required().get()));
+        if (!resolvedDesc) {
+            return nullptr;
+        }
+        return create_from_desc(resolvedDesc.get()).release();
     }
 
     SkTypeface* onMatchFamilyStyleCharacter(const char familyName[],

tests/FontMgrTest.cpp

diff --git a/tests/FontMgrTest.cpp b/tests/FontMgrTest.cpp
index 2939de9..3121c8c 100644
--- a/tests/FontMgrTest.cpp
+++ b/tests/FontMgrTest.cpp
@@ -114,15 +114,40 @@
             SkString sname;
             SkFontStyle fs;
             set->getStyle(j, &fs, &sname);
-//            REPORTER_ASSERT(reporter, sname.size() > 0);
-
-            sk_sp<SkTypeface> face(set->createTypeface(j));
-//            REPORTER_ASSERT(reporter, face.get());
 
             if (FLAGS_verboseFontMgr) {
                 SkDebugf("\t[%d] %s [%3d %d %d]\n", j, sname.c_str(),
                          fs.weight(), fs.width(), fs.slant());
             }
+
+            sk_sp<SkTypeface> face1(set->createTypeface(j));
+            if (!face1) {
+                REPORTER_ASSERT(reporter, face1.get());
+                continue;
+            }
+            SkString name1;
+            face1->getFamilyName(&name1);
+            SkFontStyle s1 = face1->fontStyle();
+
+            // Note that fs != s1 is fine, though probably rare.
+
+            sk_sp<SkTypeface> face2(fm->matchFamilyStyle(name1.c_str(), s1));
+            if (!face2) {
+                REPORTER_ASSERT(reporter, face2.get());
+                continue;
+            }
+            SkString name2;
+            face2->getFamilyName(&name2);
+
+            REPORTER_ASSERT(reporter, name1 == name2, "%s == %s", name1.c_str(), name2.c_str());
+
+            // TODO: This should work, but Mac matches the wrong font sometimes.
+            if ((false)) {
+                SkFontStyle s2 = face2->fontStyle();
+                REPORTER_ASSERT(reporter, s1 == s2, "%s [%3d %d %d] != %s [%3d %d %d]",
+                                name1.c_str(), s1.weight(), s1.width(), s1.slant(),
+                                name2.c_str(), s2.weight(), s2.width(), s2.slant());
+            }
         }
     }
 }
@@ -133,6 +158,24 @@
     REPORTER_ASSERT(reporter, styleSet);
 }
 
+DEF_TEST(FontMgr_MatchFamilyStyle, reporter) {
+    sk_sp<SkFontMgr> fm(SkFontMgr::RefDefault());
+
+    sk_sp<SkFontStyleSet> styleSet(fm->matchFamily("Non Existing Family Name"));
+    REPORTER_ASSERT(reporter, styleSet);
+    REPORTER_ASSERT(reporter, styleSet->count() == 0);
+
+    using FS = SkFontStyle;
+    sk_sp<SkTypeface> typeface(fm->matchFamilyStyle("Non Existing Family Name", FS::Normal()));
+    REPORTER_ASSERT(reporter, !typeface);
+
+    // TODO: enable after determining if a default font should be required.
+    if ((false)) {
+        sk_sp<SkTypeface> def(fm->matchFamilyStyle(nullptr, FS::Normal()));
+        REPORTER_ASSERT(reporter, def);
+    }
+}
+
 DEF_TEST(FontMgr_MatchStyleCSS3, reporter) {
     static const SkFontStyle invalidFontStyle(101, SkFontStyle::kNormal_Width, SkFontStyle::kUpright_Slant);