Title: [218909] trunk
Revision
218909
Author
mmaxfi...@apple.com
Date
2017-06-28 19:46:11 -0700 (Wed, 28 Jun 2017)

Log Message

[iOS] Cannot italicize or bold text rendered with text styles
https://bugs.webkit.org/show_bug.cgi?id=173634

Reviewed by Darin Adler.

Source/WebCore:

r218616 enabled the new cascade list codepath for "system-ui," but didn't do it for the named
text styles (like "font: -apple-system-tall-body;"). This new codepath is better because it
correctly specifies weights and italics (using kCTFontWeightTrait and kCTFontSlantTrait) instead
of using symbolic traits, and because it correctly handles fonts in the Core Text fallback chain.
This patch migrates the named text styles to this new codepath.

Test: fast/text/ipad/bold-tall-body-text-style.html

* platform/graphics/cocoa/FontDescriptionCocoa.cpp:
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParameters):
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::isHashTableDeletedValue):
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::operator==):
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::hash):
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParametersHash::hash):
(WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParametersHash::equal):
(WebCore::SystemFontDatabase::systemFontCascadeList):
(WebCore::convertArray):
(WebCore::convertArray):
(WebCore::makeNeverDestroyed):
(WebCore::isUIFontTextStyle):
(WebCore::systemFontParameters):
(WebCore::FontCascadeDescription::effectiveFamilyCount):
(WebCore::FontCascadeDescription::effectiveFamilyAt):
(WebCore::SystemFontDatabase::CoreTextCascadeListParametersHash::hash): Deleted.
(WebCore::SystemFontDatabase::CoreTextCascadeListParametersHash::equal): Deleted.
* platform/graphics/ios/FontCacheIOS.mm:
(WebCore::platformFontWithFamilySpecialCase):

LayoutTests:

* fast/text/ipad/bold-tall-body-text-style-expected-mismatch.html: Added.
* fast/text/ipad/bold-tall-body-text-style.html: Added.

Modified Paths

Added Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (218908 => 218909)


--- trunk/LayoutTests/ChangeLog	2017-06-29 01:34:51 UTC (rev 218908)
+++ trunk/LayoutTests/ChangeLog	2017-06-29 02:46:11 UTC (rev 218909)
@@ -1,3 +1,13 @@
+2017-06-27  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [iOS] Cannot italicize or bold text rendered with text styles
+        https://bugs.webkit.org/show_bug.cgi?id=173634
+
+        Reviewed by Darin Adler.
+
+        * fast/text/ipad/bold-tall-body-text-style-expected-mismatch.html: Added.
+        * fast/text/ipad/bold-tall-body-text-style.html: Added.
+
 2017-06-28  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: Instrument active pixel memory used by canvases

Added: trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style-expected-mismatch.html (0 => 218909)


--- trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style-expected-mismatch.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style-expected-mismatch.html	2017-06-29 02:46:11 UTC (rev 218909)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<div style="font: -apple-system-tall-body; text-rendering: optimizeLegibility;"><span style="font-family: '.SFUIText-Light'; display: inline-block; transform-origin: left top; transform: scale(10);">Hello</span></div>
+</body>
+</html>

Added: trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style.html (0 => 218909)


--- trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/ipad/bold-tall-body-text-style.html	2017-06-29 02:46:11 UTC (rev 218909)
@@ -0,0 +1,8 @@
+<!DOCTYPE html>
+<html>
+<head>
+</head>
+<body>
+<div style="font: -apple-system-tall-body;"><b style="display: inline-block; transform-origin: left top; transform: scale(10);">Hello</b></div>
+</body>
+</html>

Modified: trunk/Source/WebCore/ChangeLog (218908 => 218909)


--- trunk/Source/WebCore/ChangeLog	2017-06-29 01:34:51 UTC (rev 218908)
+++ trunk/Source/WebCore/ChangeLog	2017-06-29 02:46:11 UTC (rev 218909)
@@ -1,3 +1,38 @@
+2017-06-27  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        [iOS] Cannot italicize or bold text rendered with text styles
+        https://bugs.webkit.org/show_bug.cgi?id=173634
+
+        Reviewed by Darin Adler.
+
+        r218616 enabled the new cascade list codepath for "system-ui," but didn't do it for the named
+        text styles (like "font: -apple-system-tall-body;"). This new codepath is better because it
+        correctly specifies weights and italics (using kCTFontWeightTrait and kCTFontSlantTrait) instead
+        of using symbolic traits, and because it correctly handles fonts in the Core Text fallback chain.
+        This patch migrates the named text styles to this new codepath.
+
+        Test: fast/text/ipad/bold-tall-body-text-style.html
+
+        * platform/graphics/cocoa/FontDescriptionCocoa.cpp:
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParameters):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::isHashTableDeletedValue):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::operator==):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::hash):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParametersHash::hash):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParameters::CoreTextCascadeListParametersHash::equal):
+        (WebCore::SystemFontDatabase::systemFontCascadeList):
+        (WebCore::convertArray):
+        (WebCore::convertArray):
+        (WebCore::makeNeverDestroyed):
+        (WebCore::isUIFontTextStyle):
+        (WebCore::systemFontParameters):
+        (WebCore::FontCascadeDescription::effectiveFamilyCount):
+        (WebCore::FontCascadeDescription::effectiveFamilyAt):
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParametersHash::hash): Deleted.
+        (WebCore::SystemFontDatabase::CoreTextCascadeListParametersHash::equal): Deleted.
+        * platform/graphics/ios/FontCacheIOS.mm:
+        (WebCore::platformFontWithFamilySpecialCase):
+
 2017-06-28  Devin Rousso  <drou...@apple.com>
 
         Web Inspector: Instrument active pixel memory used by canvases

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm (218908 => 218909)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2017-06-29 01:34:51 UTC (rev 218908)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCocoa.mm	2017-06-29 02:46:11 UTC (rev 218909)
@@ -183,7 +183,7 @@
     ascent = ceilf(ascent + adjustment);
     descent = ceilf(descent);
 
-    m_shouldNotBeUsedForArabic = fontFamilyShouldNotBeUsedForArabic(adoptCF(CTFontCopyFamilyName(m_platformData.font())).get());
+    m_shouldNotBeUsedForArabic = fontFamilyShouldNotBeUsedForArabic(familyName.get());
 #endif
 
     CGFloat xHeight = 0;

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp (218908 => 218909)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp	2017-06-29 01:34:51 UTC (rev 218908)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontDescriptionCocoa.cpp	2017-06-29 02:46:11 UTC (rev 218909)
@@ -27,6 +27,7 @@
 #include "FontDescription.h"
 
 #include "CoreTextSPI.h"
+#include "FontCache.h"
 #include "FontFamilySpecificationCoreText.h"
 #include <wtf/HashMap.h>
 #include <wtf/HashTraits.h>
@@ -33,6 +34,10 @@
 #include <wtf/text/AtomicString.h>
 #include <wtf/text/AtomicStringHash.h>
 
+#if PLATFORM(IOS)
+#include "RenderThemeIOS.h"
+#endif
+
 #if USE_PLATFORM_SYSTEM_FALLBACK_LIST
 
 namespace WebCore {
@@ -45,18 +50,19 @@
         }
 
         CoreTextCascadeListParameters(WTF::HashTableDeletedValueType)
-            : locale(WTF::HashTableDeletedValue)
+            : fontName(WTF::HashTableDeletedValue)
         {
         }
 
         bool isHashTableDeletedValue() const
         {
-            return locale.isHashTableDeletedValue();
+            return fontName.isHashTableDeletedValue();
         }
 
         bool operator==(const CoreTextCascadeListParameters& other) const
         {
-            return locale == other.locale
+            return fontName == other.fontName
+                && locale == other.locale
                 && weight == other.weight
                 && size == other.size
                 && italic == other.italic;
@@ -65,6 +71,8 @@
         unsigned hash() const
         {
             IntegerHasher hasher;
+            ASSERT(!fontName.isNull());
+            hasher.add(locale.existingHash());
             hasher.add(locale.isNull() ? 0 : locale.existingHash());
             hasher.add(weight);
             hasher.add(size);
@@ -72,6 +80,19 @@
             return hasher.hash();
         }
 
+        struct CoreTextCascadeListParametersHash : WTF::PairHash<AtomicString, float> {
+            static unsigned hash(const CoreTextCascadeListParameters& parameters)
+            {
+                return parameters.hash();
+            }
+            static bool equal(const CoreTextCascadeListParameters& a, const CoreTextCascadeListParameters& b)
+            {
+                return a == b;
+            }
+            static const bool safeToCompareToEmptyOrDeleted = true;
+        };
+
+        AtomicString fontName;
         AtomicString locale;
         CGFloat weight { 0 };
         float size { 0 };
@@ -84,26 +105,35 @@
         return database.get();
     }
 
-    Vector<RetainPtr<CTFontDescriptorRef>> systemFontCascadeList(const CoreTextCascadeListParameters& parameters)
+    enum class ClientUse { ForSystemUI, ForTextStyle };
+
+    Vector<RetainPtr<CTFontDescriptorRef>> systemFontCascadeList(const CoreTextCascadeListParameters& parameters, ClientUse clientUse)
     {
-        auto createSystemFont = [&] {
+        ASSERT(!parameters.fontName.isNull());
+        return m_systemFontCache.ensure(parameters, [&] {
             auto localeString = parameters.locale.string().createCFString();
-            return std::make_pair(localeString, adoptCF(CTFontCreateUIFontForLanguage(kCTFontUIFontSystem, parameters.size, localeString.get())));
-        };
-
-        // Avoid adding an empty value to the hash map
-        if (!parameters.size) {
-            auto systemFont = createSystemFont().second;
-            auto descriptor = adoptCF(CTFontCopyFontDescriptor(systemFont.get()));
-            return { removeCascadeList(descriptor.get()) };
-        }
-
-        return m_systemFontCache.ensure(parameters, [&] {
-            RetainPtr<CFStringRef> localeString;
             RetainPtr<CTFontRef> systemFont;
-            std::tie(localeString, systemFont) = createSystemFont();
-            systemFont = applyWeightAndItalics(systemFont.get(), parameters.weight, parameters.italic, parameters.size);
-            return computeCascadeList(systemFont.get(), localeString.get());
+            if (clientUse == ClientUse::ForSystemUI) {
+                systemFont = adoptCF(CTFontCreateUIFontForLanguage(kCTFontUIFontSystem, parameters.size, localeString.get()));
+                ASSERT(systemFont);
+                // FIXME: Use applyWeightAndItalics() in both cases once <rdar://problem/33046041> is fixed.
+                systemFont = applyWeightAndItalics(systemFont.get(), parameters.weight, parameters.italic, parameters.size);
+            } else {
+#if PLATFORM(IOS)
+                ASSERT(clientUse == ClientUse::ForTextStyle);
+                auto fontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyle(parameters.fontName.string().createCFString().get(), RenderThemeIOS::contentSizeCategory(), nullptr));
+                CTFontSymbolicTraits traits = (parameters.weight >= kCTFontWeightSemibold ? kCTFontTraitBold : 0) | (parameters.italic ? kCTFontTraitItalic : 0);
+                if (traits)
+                    fontDescriptor = adoptCF(CTFontDescriptorCreateCopyWithSymbolicTraits(fontDescriptor.get(), traits, traits));
+                systemFont = adoptCF(CTFontCreateWithFontDescriptor(fontDescriptor.get(), parameters.size, nullptr));
+#else
+                ASSERT_NOT_REACHED();
+#endif
+            }
+            ASSERT(systemFont);
+            auto result = computeCascadeList(systemFont.get(), localeString.get());
+            ASSERT(!result.isEmpty());
+            return result;
         }).iterator->value;
     }
 
@@ -157,19 +187,7 @@
         return result;
     }
 
-    struct CoreTextCascadeListParametersHash : WTF::PairHash<AtomicString, float> {
-        static unsigned hash(const CoreTextCascadeListParameters& parameters)
-        {
-            return parameters.hash();
-        }
-        static bool equal(const CoreTextCascadeListParameters& a, const CoreTextCascadeListParameters& b)
-        {
-            return a == b;
-        }
-        static const bool safeToCompareToEmptyOrDeleted = true;
-    };
-
-    HashMap<CoreTextCascadeListParameters, Vector<RetainPtr<CTFontDescriptorRef>>, CoreTextCascadeListParametersHash, SimpleClassHashTraits<CoreTextCascadeListParameters>> m_systemFontCache;
+    HashMap<CoreTextCascadeListParameters, Vector<RetainPtr<CTFontDescriptorRef>>, CoreTextCascadeListParameters::CoreTextCascadeListParametersHash, SimpleClassHashTraits<CoreTextCascadeListParameters>> m_systemFontCache;
 };
 
 static inline bool isSystemFontString(const AtomicString& string)
@@ -180,8 +198,54 @@
         || equalLettersIgnoringASCIICase(string, "system-ui");
 }
 
-static inline SystemFontDatabase::CoreTextCascadeListParameters systemFontParameters(const FontCascadeDescription& description)
+#if PLATFORM(IOS)
+
+template<typename T, typename U, std::size_t size, std::size_t... indices> std::array<T, size> convertArray(U (&array)[size], std::index_sequence<indices...>)
 {
+    return { { array[indices]... } };
+}
+
+template<typename T, typename U, std::size_t size> inline std::array<T, size> convertArray(U (&array)[size])
+{
+    return convertArray<T>(array, std::make_index_sequence<size> { });
+}
+
+template<typename T> inline NeverDestroyed<T> makeNeverDestroyed(T&& argument)
+{
+    return WTFMove(argument);
+}
+
+static inline bool isUIFontTextStyle(const AtomicString& string)
+{
+    static const CFStringRef styles[] = {
+        kCTUIFontTextStyleHeadline,
+        kCTUIFontTextStyleBody,
+        kCTUIFontTextStyleTitle1,
+        kCTUIFontTextStyleTitle2,
+        kCTUIFontTextStyleTitle3,
+        kCTUIFontTextStyleSubhead,
+        kCTUIFontTextStyleFootnote,
+        kCTUIFontTextStyleCaption1,
+        kCTUIFontTextStyleCaption2,
+        kCTUIFontTextStyleShortHeadline,
+        kCTUIFontTextStyleShortBody,
+        kCTUIFontTextStyleShortSubhead,
+        kCTUIFontTextStyleShortFootnote,
+        kCTUIFontTextStyleShortCaption1,
+        kCTUIFontTextStyleTallBody,
+#if __IPHONE_OS_VERSION_MIN_REQUIRED >= 110000
+        kCTUIFontTextStyleTitle0,
+        kCTUIFontTextStyleTitle4,
+#endif
+    };
+    
+    static auto strings { makeNeverDestroyed(convertArray<AtomicString>(styles)) };
+    return std::find(strings.get().begin(), strings.get().end(), string) != strings.get().end();
+}
+#endif
+
+static inline SystemFontDatabase::CoreTextCascadeListParameters systemFontParameters(const FontCascadeDescription& description, const AtomicString& familyName, SystemFontDatabase::ClientUse clientUse)
+{
     SystemFontDatabase::CoreTextCascadeListParameters result;
     result.locale = description.locale();
     result.size = description.computedSize();
@@ -188,6 +252,9 @@
     result.italic = isItalic(description.italic());
 
     auto weight = description.weight();
+    if (FontCache::singleton().shouldMockBoldSystemFontForAccessibility())
+        weight = weight + FontSelectionValue(200);
+
     if (weight < FontSelectionValue(150))
         result.weight = kCTFontWeightUltraLight;
     else if (weight < FontSelectionValue(250))
@@ -207,6 +274,14 @@
     else
         result.weight = kCTFontWeightBlack;
 
+    if (clientUse == SystemFontDatabase::ClientUse::ForSystemUI) {
+        static NeverDestroyed<AtomicString> systemUI = AtomicString("system-ui", AtomicString::ConstructFromLiteral);
+        result.fontName = systemUI.get();
+    } else {
+        ASSERT(clientUse == SystemFontDatabase::ClientUse::ForTextStyle);
+        result.fontName = familyName;
+    }
+
     return result;
 }
 
@@ -215,6 +290,11 @@
     SystemFontDatabase::singleton().clear();
 }
 
+static inline Vector<RetainPtr<CTFontDescriptorRef>> systemFontCascadeList(const FontCascadeDescription& description, const AtomicString& cssFamily, SystemFontDatabase::ClientUse clientUse)
+{
+    return SystemFontDatabase::singleton().systemFontCascadeList(systemFontParameters(description, cssFamily, clientUse), clientUse);
+}
+
 unsigned FontCascadeDescription::effectiveFamilyCount() const
 {
     // FIXME: Move all the other system font keywords from platformFontWithFamilySpecialCase() to here.
@@ -222,7 +302,11 @@
     for (unsigned i = 0; i < familyCount(); ++i) {
         const auto& cssFamily = familyAt(i);
         if (isSystemFontString(cssFamily))
-            result += SystemFontDatabase::singleton().systemFontCascadeList(systemFontParameters(*this)).size();
+            result += systemFontCascadeList(*this, cssFamily, SystemFontDatabase::ClientUse::ForSystemUI).size();
+#if PLATFORM(IOS)
+        else if (isUIFontTextStyle(cssFamily))
+            result += systemFontCascadeList(*this, cssFamily, SystemFontDatabase::ClientUse::ForTextStyle).size();
+#endif
         else
             ++result;
     }
@@ -234,11 +318,20 @@
     for (unsigned i = 0; i < familyCount(); ++i) {
         const auto& cssFamily = familyAt(i);
         if (isSystemFontString(cssFamily)) {
-            auto cascadeList = SystemFontDatabase::singleton().systemFontCascadeList(systemFontParameters(*this));
+            auto cascadeList = systemFontCascadeList(*this, cssFamily, SystemFontDatabase::ClientUse::ForSystemUI);
             if (index < cascadeList.size())
                 return FontFamilySpecification(cascadeList[index].get());
             index -= cascadeList.size();
-        } else if (!index)
+        }
+#if PLATFORM(IOS)
+        else if (isUIFontTextStyle(cssFamily)) {
+            auto cascadeList = systemFontCascadeList(*this, cssFamily, SystemFontDatabase::ClientUse::ForTextStyle);
+            if (index < cascadeList.size())
+                return FontFamilySpecification(cascadeList[index].get());
+            index -= cascadeList.size();
+        }
+#endif
+        else if (!index)
             return cssFamily;
         else
             --index;
@@ -250,3 +343,4 @@
 }
 
 #endif
+

Modified: trunk/Source/WebCore/platform/graphics/ios/FontCacheIOS.mm (218908 => 218909)


--- trunk/Source/WebCore/platform/graphics/ios/FontCacheIOS.mm	2017-06-29 01:34:51 UTC (rev 218908)
+++ trunk/Source/WebCore/platform/graphics/ios/FontCacheIOS.mm	2017-06-29 02:46:11 UTC (rev 218909)
@@ -148,6 +148,7 @@
 RetainPtr<CTFontRef> platformFontWithFamilySpecialCase(const AtomicString& family, FontSelectionRequest request, float size)
 {
     if (family.startsWith("UICTFontTextStyle")) {
+#if !USE_PLATFORM_SYSTEM_FALLBACK_LIST
         CTFontSymbolicTraits traits = (isFontWeightBold(request.weight) || FontCache::singleton().shouldMockBoldSystemFontForAccessibility() ? kCTFontTraitBold : 0) | (isItalic(request.slope) ? kCTFontTraitItalic : 0);
         RetainPtr<CFStringRef> familyNameStr = family.string().createCFString();
         RetainPtr<CTFontDescriptorRef> fontDescriptor = adoptCF(CTFontDescriptorCreateWithTextStyle(familyNameStr.get(), RenderThemeIOS::contentSizeCategory(), nullptr));
@@ -155,6 +156,10 @@
             fontDescriptor = adoptCF(CTFontDescriptorCreateCopyWithSymbolicTraits(fontDescriptor.get(), traits, traits));
 
         return adoptCF(CTFontCreateWithFontDescriptor(fontDescriptor.get(), size, nullptr));
+#else
+        UNUSED_PARAM(request);
+        ASSERT_NOT_REACHED();
+#endif
     }
 
     if (equalLettersIgnoringASCIICase(family, "-webkit-system-font") || equalLettersIgnoringASCIICase(family, "-apple-system") || equalLettersIgnoringASCIICase(family, "-apple-system-font") || equalLettersIgnoringASCIICase(family, "system-ui")) {
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to