Merge r187527 - [Freetype] Always allow font matching for strong aliases

https://bugs.webkit.org/show_bug.cgi?id=147057 Reviewed by Martin Robinson. Source/WebCore: Tests: platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html Treat fonts that are strongly-aliased to each other as if they were identical for the purposes of CSS font fallback. This improves the layout of many web pages by allowing fontconfig to replace fonts with metric-compatible equivalents (e.g. Arial -> Liberation Sans) instead of rejecting the metric-compatible font as unsuitable. * platform/graphics/cairo/RefPtrCairo.cpp: (WTF::refIfNotNull): (WTF::derefIfNotNull): * platform/graphics/cairo/RefPtrCairo.h: * platform/graphics/freetype/FcUniquePtr.h: Added. (WebCore::FcPtrDeleter<FcFontSet>::operator()): (WebCore::FcPtrDeleter<FcLangSet>::operator()): (WebCore::FcPtrDeleter<FcObjectSet>::operator()): * platform/graphics/freetype/FontCacheFreeType.cpp: (WebCore::strengthOfFirstAlias): (WebCore::strongAliasesForFamily): (WebCore::areStronglyAliased): (WebCore::FontCache::createFontPlatformData): Tools: Create family aliases needed for the new layout tests. * WebKitTestRunner/gtk/fonts/fonts.conf: LayoutTests: * platform/gtk/fonts/font-family-fallback-ignores-weak-aliases-expected.html: Added. * platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html: Added. * platform/gtk/fonts/font-family-fallback-respects-strong-aliases-expected.html: Added. * platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html: Added.
WebKit · Aug 5, 2015 · cd10202 · cd10202
1 parent f87722d
commit cd10202
Show file tree

Hide file tree

Showing 12 changed files with 325 additions and 8 deletions.
diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog
@@ -1,3 +1,15 @@
+2015-07-28  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [Freetype] Always allow font matching for strong aliases
+        https://bugs.webkit.org/show_bug.cgi?id=147057
+
+        Reviewed by Martin Robinson.
+
+        * platform/gtk/fonts/font-family-fallback-ignores-weak-aliases-expected.html: Added.
+        * platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html: Added.
+        * platform/gtk/fonts/font-family-fallback-respects-strong-aliases-expected.html: Added.
+        * platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html: Added.
+
 2015-07-28  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Crash happens when calling removeEventListener for an SVG element which has an instance inside a <defs> element of shadow tree

diff --git a/LayoutTests/platform/gtk/fonts/font-family-fallback-ignores-weak-aliases-expected.html b/LayoutTests/platform/gtk/fonts/font-family-fallback-ignores-weak-aliases-expected.html
@@ -0,0 +1,5 @@
+<body style="font-family:serif;">
+This test ensures that if a font is weakly aliased to another, the alias is
+ignored for the purposes of CSS font fallback. This test passes if it is
+displayed in a serif font and fails if it is displayed in FreeMono.
+</body>
diff --git a/LayoutTests/platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html b/LayoutTests/platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html
@@ -0,0 +1,5 @@
+<body style="font-family:FamilyWeakAliasedToFreeMono,serif;">
+This test ensures that if a font is weakly aliased to another, the alias is
+ignored for the purposes of CSS font fallback. This test passes if it is
+displayed in a serif font and fails if it is displayed in FreeMono.
+</body>
diff --git a/LayoutTests/platform/gtk/fonts/font-family-fallback-respects-strong-aliases-expected.html b/LayoutTests/platform/gtk/fonts/font-family-fallback-respects-strong-aliases-expected.html
@@ -0,0 +1,5 @@
+<body style="font-family:FreeMono;">
+This test ensures that if a font is strongly aliased to another, those fonts are
+treated as identical for the purposes of CSS font fallback. This test passes if
+it is displayed in FreeMono and fails if it is displayed in a serif font.
+</body>
diff --git a/LayoutTests/platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html b/LayoutTests/platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html
@@ -0,0 +1,5 @@
+<body style="font-family:FamilyStrongAliasedToFreeMono,serif;">
+This test ensures that if a font is strongly aliased to another, those fonts are
+treated as identical for the purposes of CSS font fallback. This test passes if
+it is displayed in FreeMono and fails if it is displayed in a serif font.
+</body>
diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog
@@ -1,3 +1,32 @@
+2015-07-28  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [Freetype] Always allow font matching for strong aliases
+        https://bugs.webkit.org/show_bug.cgi?id=147057
+
+        Reviewed by Martin Robinson.
+
+        Tests: platform/gtk/fonts/font-family-fallback-ignores-weak-aliases.html
+               platform/gtk/fonts/font-family-fallback-respects-strong-aliases.html
+
+        Treat fonts that are strongly-aliased to each other as if they were identical for the
+        purposes of CSS font fallback. This improves the layout of many web pages by allowing
+        fontconfig to replace fonts with metric-compatible equivalents (e.g. Arial -> Liberation
+        Sans) instead of rejecting the metric-compatible font as unsuitable.
+
+        * platform/graphics/cairo/RefPtrCairo.cpp:
+        (WTF::refIfNotNull):
+        (WTF::derefIfNotNull):
+        * platform/graphics/cairo/RefPtrCairo.h:
+        * platform/graphics/freetype/FcUniquePtr.h: Added.
+        (WebCore::FcPtrDeleter<FcFontSet>::operator()):
+        (WebCore::FcPtrDeleter<FcLangSet>::operator()):
+        (WebCore::FcPtrDeleter<FcObjectSet>::operator()):
+        * platform/graphics/freetype/FontCacheFreeType.cpp:
+        (WebCore::strengthOfFirstAlias):
+        (WebCore::strongAliasesForFamily):
+        (WebCore::areStronglyAliased):
+        (WebCore::FontCache::createFontPlatformData):
+
 2015-07-28  Said Abou-Hallawa  <sabouhallawa@apple.com>
 
         Crash happens when calling removeEventListener for an SVG element which has an instance inside a <defs> element of shadow tree

diff --git a/Source/WebCore/platform/graphics/cairo/RefPtrCairo.cpp b/Source/WebCore/platform/graphics/cairo/RefPtrCairo.cpp
@@ -115,6 +115,17 @@ template<> void derefIfNotNull(FcPattern* ptr)
         FcPatternDestroy(ptr);
 }
 
+template<> void refIfNotNull(FcConfig* ptr)
+{
+    if (LIKELY(ptr != nullptr))
+        FcConfigReference(ptr);
+}
+
+template<> void derefIfNotNull(FcConfig* ptr)
+{
+    if (LIKELY(ptr != nullptr))
+        FcConfigDestroy(ptr);
+}
 #endif
 
 } // namespace WTF

diff --git a/Source/WebCore/platform/graphics/cairo/RefPtrCairo.h b/Source/WebCore/platform/graphics/cairo/RefPtrCairo.h
@@ -33,6 +33,7 @@ typedef struct _cairo_region cairo_region_t;
 
 #if USE(FREETYPE)
 typedef struct _FcPattern FcPattern;
+typedef struct _FcConfig FcConfig;
 #endif
 
 namespace WTF {
@@ -58,6 +59,9 @@ template<> void derefIfNotNull(cairo_region_t*);
 #if USE(FREETYPE)
 template<> void refIfNotNull(FcPattern* ptr);
 template<> void derefIfNotNull(FcPattern* ptr);
+
+template<> void refIfNotNull(FcConfig* ptr);
+template<> void derefIfNotNull(FcConfig* ptr);
 #endif
 
 } // namespace WTF

diff --git a/Source/WebCore/platform/graphics/freetype/FcUniquePtr.h b/Source/WebCore/platform/graphics/freetype/FcUniquePtr.h
@@ -0,0 +1,69 @@
+/*
+ * Copyright (C) 2015 Igalia S.L
+ *
+ * Redistribution and use in source and binary forms, with or without
+ * modification, are permitted provided that the following conditions
+ * are met:
+ *
+ * 1.  Redistributions of source code must retain the above copyright
+ *     notice, this list of conditions and the following disclaimer.
+ * 2.  Redistributions in binary form must reproduce the above copyright
+ *     notice, this list of conditions and the following disclaimer in the
+ *     documentation and/or other materials provided with the distribution.
+ *
+ * THIS SOFTWARE IS PROVIDED BY APPLE AND ITS CONTRIBUTORS "AS IS" AND ANY
+ * EXPRESS OR IMPLIED WARRANTIES, INCLUDING, BUT NOT LIMITED TO, THE IMPLIED
+ * WARRANTIES OF MERCHANTABILITY AND FITNESS FOR A PARTICULAR PURPOSE ARE
+ * DISCLAIMED. IN NO EVENT SHALL APPLE OR ITS CONTRIBUTORS BE LIABLE FOR ANY
+ * DIRECT, INDIRECT, INCIDENTAL, SPECIAL, EXEMPLARY, OR CONSEQUENTIAL DAMAGES
+ * (INCLUDING, BUT NOT LIMITED TO, PROCUREMENT OF SUBSTITUTE GOODS OR SERVICES;
+ * LOSS OF USE, DATA, OR PROFITS; OR BUSINESS INTERRUPTION) HOWEVER CAUSED AND
+ * ON ANY THEORY OF LIABILITY, WHETHER IN CONTRACT, STRICT LIABILITY, OR TORT
+ * (INCLUDING NEGLIGENCE OR OTHERWISE) ARISING IN ANY WAY OUT OF THE USE OF
+ * THIS SOFTWARE, EVEN IF ADVISED OF THE POSSIBILITY OF SUCH DAMAGE.
+ */
+
+#ifndef FcUniquePtr_h
+#define FcUniquePtr_h
+
+#if USE(FREETYPE)
+
+#include <fontconfig/fontconfig.h>
+#include <memory>
+
+namespace WebCore {
+
+template<typename T>
+struct FcPtrDeleter {
+    void operator()(T* ptr) const = delete;
+};
+
+template<typename T>
+using FcUniquePtr = std::unique_ptr<T, FcPtrDeleter<T>>;
+
+template<> struct FcPtrDeleter<FcFontSet> {
+    void operator()(FcFontSet* ptr) const
+    {
+        FcFontSetDestroy(ptr);
+    }
+};
+
+template<> struct FcPtrDeleter<FcLangSet> {
+    void operator()(FcLangSet* ptr) const
+    {
+        FcLangSetDestroy(ptr);
+    }
+};
+
+template<> struct FcPtrDeleter<FcObjectSet> {
+    void operator()(FcObjectSet* ptr) const
+    {
+        FcObjectSetDestroy(ptr);
+    }
+};
+
+} // namespace WebCore
+
+#endif // USE(FREETYPE)
+
+#endif // FcUniquePtr_h
diff --git a/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp b/Source/WebCore/platform/graphics/freetype/FontCacheFreeType.cpp
@@ -22,6 +22,7 @@
 #include "config.h"
 #include "FontCache.h"
 
+#include "FcUniquePtr.h"
 #include "Font.h"
 #include "OwnPtrCairo.h"
 #include "RefPtrCairo.h"
@@ -158,6 +159,141 @@ int fontWeightToFontconfigWeight(FontWeight weight)
     }
 }
 
+// This is based on Chromium BSD code from Skia (src/ports/SkFontMgr_fontconfig.cpp). It is a
+// hack for lack of API in Fontconfig: https://bugs.freedesktop.org/show_bug.cgi?id=19375
+// FIXME: This is horrible. It should be deleted once Fontconfig can do this itself.
+enum class AliasStrength {
+    Weak,
+    Strong,
+    Done
+};
+
+static AliasStrength strengthOfFirstAlias(const FcPattern& original)
+{
+    // Ideally there would exist a call like
+    // FcResult FcPatternIsWeak(pattern, object, id, FcBool* isWeak);
+    //
+    // However, there is no such call and as of Fc 2.11.0 even FcPatternEquals ignores the weak bit.
+    // Currently, the only reliable way of finding the weak bit is by its effect on matching.
+    // The weak bit only affects the matching of FC_FAMILY and FC_POSTSCRIPT_NAME object values.
+    // A element with the weak bit is scored after FC_LANG, without the weak bit is scored before.
+    // Note that the weak bit is stored on the element, not on the value it holds.
+    FcValue value;
+    FcResult result = FcPatternGet(&original, FC_FAMILY, 0, &value);
+    if (result != FcResultMatch)
+        return AliasStrength::Done;
+
+    RefPtr<FcPattern> pattern = adoptRef(FcPatternDuplicate(&original));
+    FcBool hasMultipleFamilies = true;
+    while (hasMultipleFamilies)
+        hasMultipleFamilies = FcPatternRemove(pattern.get(), FC_FAMILY, 1);
+
+    // Create a font set with two patterns.
+    // 1. the same FC_FAMILY as pattern and a lang object with only 'nomatchlang'.
+    // 2. a different FC_FAMILY from pattern and a lang object with only 'matchlang'.
+    FcUniquePtr<FcFontSet> fontSet(FcFontSetCreate());
+
+    FcUniquePtr<FcLangSet> strongLangSet(FcLangSetCreate());
+    FcLangSetAdd(strongLangSet.get(), reinterpret_cast<const FcChar8*>("nomatchlang"));
+    RefPtr<FcPattern> strong = adoptRef(FcPatternDuplicate(pattern.get()));
+    FcPatternAddLangSet(strong.get(), FC_LANG, strongLangSet.get());
+
+    FcUniquePtr<FcLangSet> weakLangSet(FcLangSetCreate());
+    FcLangSetAdd(weakLangSet.get(), reinterpret_cast<const FcChar8*>("matchlang"));
+    RefPtr<FcPattern> weak(FcPatternCreate());
+    FcPatternAddString(weak.get(), FC_FAMILY, reinterpret_cast<const FcChar8*>("nomatchstring"));
+    FcPatternAddLangSet(weak.get(), FC_LANG, weakLangSet.get());
+
+    FcFontSetAdd(fontSet.get(), strong.leakRef());
+    FcFontSetAdd(fontSet.get(), weak.leakRef());
+
+    // Add 'matchlang' to the copy of the pattern.
+    FcPatternAddLangSet(pattern.get(), FC_LANG, weakLangSet.get());
+
+    // Run a match against the copy of the pattern.
+    // If the first element was weak, then we should match the pattern with 'matchlang'.
+    // If the first element was strong, then we should match the pattern with 'nomatchlang'.
+
+    // Note that this config is only used for FcFontRenderPrepare, which we don't even want.
+    // However, there appears to be no way to match/sort without it.
+    RefPtr<FcConfig> config = adoptRef(FcConfigCreate());
+    FcFontSet* fontSets[1] = { fontSet.get() };
+    RefPtr<FcPattern> match = adoptRef(FcFontSetMatch(config.get(), fontSets, 1, pattern.get(), &result));
+
+    FcLangSet* matchLangSet;
+    FcPatternGetLangSet(match.get(), FC_LANG, 0, &matchLangSet);
+    return FcLangEqual == FcLangSetHasLang(matchLangSet, reinterpret_cast<const FcChar8*>("matchlang"))
+        ? AliasStrength::Weak : AliasStrength::Strong;
+}
+
+static Vector<String> strongAliasesForFamily(const String& family)
+{
+    RefPtr<FcPattern> pattern = adoptRef(FcPatternCreate());
+    if (!FcPatternAddString(pattern.get(), FC_FAMILY, reinterpret_cast<const FcChar8*>(family.utf8().data())))
+        return Vector<String>();
+
+    FcConfigSubstitute(nullptr, pattern.get(), FcMatchPattern);
+    FcDefaultSubstitute(pattern.get());
+
+    FcUniquePtr<FcObjectSet> familiesOnly(FcObjectSetBuild(FC_FAMILY, nullptr));
+    RefPtr<FcPattern> minimal = adoptRef(FcPatternFilter(pattern.get(), familiesOnly.get()));
+
+    // We really want to match strong (preferred) and same (acceptable) only here.
+    // If a family name was specified, assume that any weak matches after the last strong match
+    // are weak (default) and ignore them.
+    // The reason for is that after substitution the pattern for 'sans-serif' looks like
+    // "wwwwwwwwwwwwwwswww" where there are many weak but preferred names, followed by defaults.
+    // So it is possible to have weakly matching but preferred names.
+    // In aliases, bindings are weak by default, so this is easy and common.
+    // If no family name was specified, we'll probably only get weak matches, but that's ok.
+    int lastStrongId = -1;
+    int numIds = 0;
+    for (int id = 0; ; ++id) {
+        AliasStrength result = strengthOfFirstAlias(*minimal);
+        if (result == AliasStrength::Done) {
+            numIds = id;
+            break;
+        }
+        if (result == AliasStrength::Strong)
+            lastStrongId = id;
+        if (!FcPatternRemove(minimal.get(), FC_FAMILY, 0))
+            return Vector<String>();
+    }
+
+    // If they were all weak, then leave the pattern alone.
+    if (lastStrongId < 0)
+        return Vector<String>();
+
+    // Remove everything after the last strong.
+    for (int id = lastStrongId + 1; id < numIds; ++id) {
+        if (!FcPatternRemove(pattern.get(), FC_FAMILY, lastStrongId + 1)) {
+            ASSERT_NOT_REACHED();
+            return Vector<String>();
+        }
+    }
+
+    // Take the resulting pattern and remove everything but the families.
+    minimal = adoptRef(FcPatternFilter(pattern.get(), familiesOnly.get()));
+    // Convert the pattern to a string, and cut out the non-family junk that gets added to the end.
+    char* patternChars = reinterpret_cast<char*>(FcPatternFormat(pattern.get(), reinterpret_cast<const FcChar8*>("%{family}")));
+    String patternString = String::fromUTF8(patternChars);
+    free(patternChars);
+
+    Vector<String> results;
+    patternString.split(',', results);
+    return results;
+}
+
+static bool areStronglyAliased(const String& familyA, const String& familyB)
+{
+    for (auto& family : strongAliasesForFamily(familyA)) {
+        if (family == familyB)
+            return true;
+    }
+    return false;
+}
+
+
 std::unique_ptr<FontPlatformData> FontCache::createFontPlatformData(const FontDescription& fontDescription, const AtomicString& family)
 {
     // The CSS font matching algorithm (http://www.w3.org/TR/css3-fonts/#font-matching-algorithm)
@@ -179,10 +315,17 @@ std::unique_ptr<FontPlatformData> FontCache::createFontPlatformData(const FontDe
         return nullptr;
 
     // The strategy is originally from Skia (src/ports/SkFontHost_fontconfig.cpp):
-
-    // Allow Fontconfig to do pre-match substitution. Unless we are accessing a "fallback"
-    // family like "sans," this is the only time we allow Fontconfig to substitute one
-    // family name for another (i.e. if the fonts are aliased to each other).
+    //
+    // We do not normally allow fontconfig to substitute one font family for another, since this
+    // would break CSS font family fallback: the website should be in control of fallback. During
+    // normal font matching, the only font family substitution permitted is for generic families
+    // (sans, serif, monospace) or for strongly-aliased fonts (which are to be treated as
+    // effectively identical). This is because the font matching step is designed to always find a
+    // match for the font, which we don't want.
+    //
+    // Fontconfig is used in two stages: (1) configuration and (2) matching. During the
+    // configuration step, before any matching occurs, we allow arbitrary family substitutions,
+    // since this is an exact matter of respecting the user's font configuration.
     FcConfigSubstitute(0, pattern.get(), FcMatchPattern);
     FcDefaultSubstitute(pattern.get());
 
@@ -199,13 +342,15 @@ std::unique_ptr<FontPlatformData> FontCache::createFontPlatformData(const FontDe
     FcPatternGetString(resultPattern.get(), FC_FAMILY, 0, &fontConfigFamilyNameAfterMatching);
     String familyNameAfterMatching = String::fromUTF8(reinterpret_cast<char*>(fontConfigFamilyNameAfterMatching));
 
-    // If Fontconfig gave use a different font family than the one we requested, we should ignore it
-    // and allow WebCore to give us the next font on the CSS fallback list. The only exception is if
-    // this family name is a commonly used generic family.
+    // If Fontconfig gave us a different font family than the one we requested, we should ignore it
+    // and allow WebCore to give us the next font on the CSS fallback list. The exceptions are if
+    // this family name is a commonly-used generic family, or if the families are strongly-aliased.
+    // Checking for a strong alias comes last, since it is slow.
     if (!equalIgnoringCase(familyNameAfterConfiguration, familyNameAfterMatching)
         && !(equalIgnoringCase(familyNameString, "sans") || equalIgnoringCase(familyNameString, "sans-serif")
           || equalIgnoringCase(familyNameString, "serif") || equalIgnoringCase(familyNameString, "monospace")
-          || equalIgnoringCase(familyNameString, "fantasy") || equalIgnoringCase(familyNameString, "cursive")))
+          || equalIgnoringCase(familyNameString, "fantasy") || equalIgnoringCase(familyNameString, "cursive"))
+        && !areStronglyAliased(familyNameAfterConfiguration, familyNameAfterMatching))
         return nullptr;
 
     // Verify that this font has an encoding compatible with Fontconfig. Fontconfig currently

diff --git a/Tools/ChangeLog b/Tools/ChangeLog
@@ -1,3 +1,14 @@
+2015-07-28  Michael Catanzaro  <mcatanzaro@igalia.com>
+
+        [Freetype] Always allow font matching for strong aliases
+        https://bugs.webkit.org/show_bug.cgi?id=147057
+
+        Reviewed by Martin Robinson.
+
+        Create family aliases needed for the new layout tests.
+
+        * WebKitTestRunner/gtk/fonts/fonts.conf:
+
 2015-07-23  Carlos Garcia Campos  <cgarcia@igalia.com>
 
         Unregistering and re-registering a user message handler does not work