Title: [210054] trunk
Revision
210054
Author
mmaxfi...@apple.com
Date
2016-12-20 20:44:01 -0800 (Tue, 20 Dec 2016)

Log Message

Skia lighter wght variation looks bolder than regular
https://bugs.webkit.org/show_bug.cgi?id=165948

Reviewed by Antti Koivisto.

Source/WebCore:

Test: fast/text/variations/default-value.html

This patch inspects the font's information regarding variations. It uses this information
to work around a bug in CoreText where default variation values were not getting applied.
This workaround is placed behind a version check and the macro name
"WORKAROUND_CORETEXT_VARIATIONS_DEFAULT_VALUE_BUG" so we know to delete it whenever
possible. It also uses the minimum and maximum supported values for the axis to clamp our
variation values to the closest supported point, which is in line with a recent edit to
the fonts spec:
https://github.com/w3c/csswg-drafts/commit/52b802ac38619286a30662dceb71b8a29fa72f42
This clamping behavior also revealed another bug in CoreText, which was worked around
behind another version check and macro name WORKAROUND_CORETEXT_VARIATIONS_EXTENTS_BUG so
we know to delete it whenever possible.

* platform/graphics/cocoa/FontCacheCoreText.cpp:
(WebCore::defaultVariationValues):
(WebCore::preparePlatformFont):

LayoutTests:

* fast/text/variations/default-value-expected.html: Added.
* fast/text/variations/default-value.html: Added.
* fast/text/variations/outofbounds-expected-mismatch.html: Renamed from LayoutTests/fast/text/variations/outofbounds-expected.html.
* fast/text/variations/outofbounds.html:
* platform/ios-simulator/TestExpectations:

Modified Paths

Added Paths

Removed Paths

Diff

Modified: trunk/LayoutTests/ChangeLog (210053 => 210054)


--- trunk/LayoutTests/ChangeLog	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/LayoutTests/ChangeLog	2016-12-21 04:44:01 UTC (rev 210054)
@@ -1,3 +1,16 @@
+2016-12-20  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        Skia lighter wght variation looks bolder than regular
+        https://bugs.webkit.org/show_bug.cgi?id=165948
+
+        Reviewed by Antti Koivisto.
+
+        * fast/text/variations/default-value-expected.html: Added.
+        * fast/text/variations/default-value.html: Added.
+        * fast/text/variations/outofbounds-expected-mismatch.html: Renamed from LayoutTests/fast/text/variations/outofbounds-expected.html.
+        * fast/text/variations/outofbounds.html:
+        * platform/ios-simulator/TestExpectations:
+
 2016-12-20  Keith Miller  <keith_mil...@apple.com>
 
         Add support for global

Added: trunk/LayoutTests/fast/text/variations/default-value-expected.html (0 => 210054)


--- trunk/LayoutTests/fast/text/variations/default-value-expected.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/variations/default-value-expected.html	2016-12-21 04:44:01 UTC (rev 210054)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+   window.internals.settings.setVariationFontsEnabled(true);
+</script>
+</head>
+<body>
+This test makes sure that default values of variations get applied correctly. The test fails if the text below isn't thin.
+<div style="font: 70px 'Skia'; font-variation-settings: 'wght' 1.000001;">hamburgefonstiv</div>
+</body>
+<html>

Added: trunk/LayoutTests/fast/text/variations/default-value.html (0 => 210054)


--- trunk/LayoutTests/fast/text/variations/default-value.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/variations/default-value.html	2016-12-21 04:44:01 UTC (rev 210054)
@@ -0,0 +1,13 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+   window.internals.settings.setVariationFontsEnabled(true);
+</script>
+</head>
+<body>
+This test makes sure that default values of variations get applied correctly. The test fails if the text below isn't thin.
+<div style="font: 70px 'Skia'; font-variation-settings: 'wght' 1.0;">hamburgefonstiv</div>
+</body>
+<html>

Copied: trunk/LayoutTests/fast/text/variations/outofbounds-expected-mismatch.html (from rev 210053, trunk/LayoutTests/fast/text/variations/outofbounds-expected.html) (0 => 210054)


--- trunk/LayoutTests/fast/text/variations/outofbounds-expected-mismatch.html	                        (rev 0)
+++ trunk/LayoutTests/fast/text/variations/outofbounds-expected-mismatch.html	2016-12-21 04:44:01 UTC (rev 210054)
@@ -0,0 +1,12 @@
+<!DOCTYPE html>
+<html>
+<head>
+<script>
+if (window.internals)
+   window.internals.settings.setVariationFontsEnabled(true);
+</script>
+</head>
+<body>
+<div style="font-family: 'Skia';">Test passes if this text is bold.</div>
+</body>
+</html>
\ No newline at end of file

Deleted: trunk/LayoutTests/fast/text/variations/outofbounds-expected.html (210053 => 210054)


--- trunk/LayoutTests/fast/text/variations/outofbounds-expected.html	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/LayoutTests/fast/text/variations/outofbounds-expected.html	2016-12-21 04:44:01 UTC (rev 210054)
@@ -1,12 +0,0 @@
-<!DOCTYPE html>
-<html>
-<head>
-<script>
-if (window.internals)
-   window.internals.settings.setVariationFontsEnabled(true);
-</script>
-</head>
-<body>
-<div style="font-family: 'Skia';">Test passes if this text is not bold.</div>
-</body>
-</html>
\ No newline at end of file

Modified: trunk/LayoutTests/fast/text/variations/outofbounds.html (210053 => 210054)


--- trunk/LayoutTests/fast/text/variations/outofbounds.html	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/LayoutTests/fast/text/variations/outofbounds.html	2016-12-21 04:44:01 UTC (rev 210054)
@@ -7,6 +7,6 @@
 </script>
 </head>
 <body>
-<div style="font-family: 'Skia'; font-variation-settings: 'wght' 27;">Test passes if this text is not bold.</div>
+<div style="font-family: 'Skia'; font-variation-settings: 'wght' 27;">Test passes if this text is bold.</div>
 </body>
 </html>
\ No newline at end of file

Modified: trunk/LayoutTests/platform/ios-simulator/TestExpectations (210053 => 210054)


--- trunk/LayoutTests/platform/ios-simulator/TestExpectations	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/LayoutTests/platform/ios-simulator/TestExpectations	2016-12-21 04:44:01 UTC (rev 210054)
@@ -2713,6 +2713,8 @@
 
 # This variation font test requires Skia which isn't available on iOS.
 webkit.org/b/163093 fast/text/variations/advances.html [ Failure ]
+webkit.org/b/163093 fast/text/variations/outofbounds.html [ ImageOnlyFailure ]
+webkit.org/b/163093 fast/text/variations/default-value.html [ ImageOnlyFailure ]
 
 webkit.org/b/162647 http/tests/xmlhttprequest/onabort-response-getters.html [ Pass Failure ]
 

Modified: trunk/Source/WebCore/ChangeLog (210053 => 210054)


--- trunk/Source/WebCore/ChangeLog	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/Source/WebCore/ChangeLog	2016-12-21 04:44:01 UTC (rev 210054)
@@ -1,3 +1,28 @@
+2016-12-20  Myles C. Maxfield  <mmaxfi...@apple.com>
+
+        Skia lighter wght variation looks bolder than regular
+        https://bugs.webkit.org/show_bug.cgi?id=165948
+
+        Reviewed by Antti Koivisto.
+
+        Test: fast/text/variations/default-value.html
+
+        This patch inspects the font's information regarding variations. It uses this information
+        to work around a bug in CoreText where default variation values were not getting applied.
+        This workaround is placed behind a version check and the macro name
+        "WORKAROUND_CORETEXT_VARIATIONS_DEFAULT_VALUE_BUG" so we know to delete it whenever
+        possible. It also uses the minimum and maximum supported values for the axis to clamp our
+        variation values to the closest supported point, which is in line with a recent edit to
+        the fonts spec:
+        https://github.com/w3c/csswg-drafts/commit/52b802ac38619286a30662dceb71b8a29fa72f42
+        This clamping behavior also revealed another bug in CoreText, which was worked around
+        behind another version check and macro name WORKAROUND_CORETEXT_VARIATIONS_EXTENTS_BUG so
+        we know to delete it whenever possible.
+
+        * platform/graphics/cocoa/FontCacheCoreText.cpp:
+        (WebCore::defaultVariationValues):
+        (WebCore::preparePlatformFont):
+
 2016-12-20  Tim Horton  <timothy_hor...@apple.com>
 
         Remove a duplicate reference to ScrollingMomentumCalculatorMac.h in the Xcode project

Modified: trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp (210053 => 210054)


--- trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp	2016-12-21 04:29:44 UTC (rev 210053)
+++ trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp	2016-12-21 04:44:01 UTC (rev 210054)
@@ -363,6 +363,66 @@
     return result;
 }
 
+#if ENABLE(VARIATION_FONTS)
+struct VariationDefaults {
+    float defaultValue;
+    float minimumValue;
+    float maximumValue;
+};
+
+typedef HashMap<FontTag, VariationDefaults, FourCharacterTagHash, FourCharacterTagHashTraits> VariationDefaultsMap;
+
+static VariationDefaultsMap defaultVariationValues(CTFontRef font)
+{
+    VariationDefaultsMap result;
+    auto axes = adoptCF(CTFontCopyVariationAxes(font));
+    if (!axes)
+        return result;
+    auto size = CFArrayGetCount(axes.get());
+    for (CFIndex i = 0; i < size; ++i) {
+        CFDictionaryRef axis = static_cast<CFDictionaryRef>(CFArrayGetValueAtIndex(axes.get(), i));
+        CFNumberRef axisIdentifier = static_cast<CFNumberRef>(CFDictionaryGetValue(axis, kCTFontVariationAxisIdentifierKey));
+        CFNumberRef defaultValue = static_cast<CFNumberRef>(CFDictionaryGetValue(axis, kCTFontVariationAxisDefaultValueKey));
+        CFNumberRef minimumValue = static_cast<CFNumberRef>(CFDictionaryGetValue(axis, kCTFontVariationAxisMinimumValueKey));
+        CFNumberRef maximumValue = static_cast<CFNumberRef>(CFDictionaryGetValue(axis, kCTFontVariationAxisMaximumValueKey));
+        int64_t rawAxisIdentifier = 0;
+        Boolean success = CFNumberGetValue(axisIdentifier, kCFNumberSInt64Type, &rawAxisIdentifier);
+        ASSERT_UNUSED(success, success);
+        float rawDefaultValue = 0;
+        float rawMinimumValue = 0;
+        float rawMaximumValue = 0;
+        CFNumberGetValue(defaultValue, kCFNumberFloatType, &rawDefaultValue);
+        CFNumberGetValue(minimumValue, kCFNumberFloatType, &rawMinimumValue);
+        CFNumberGetValue(maximumValue, kCFNumberFloatType, &rawMaximumValue);
+
+        // FIXME: Remove when <rdar://problem/28893836> is fixed
+#define WORKAROUND_CORETEXT_VARIATIONS_EXTENTS_BUG (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101300) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000)
+#if WORKAROUND_CORETEXT_VARIATIONS_EXTENTS_BUG
+        float epsilon = 0.001;
+        rawMinimumValue += epsilon;
+        rawMaximumValue -= epsilon;
+#endif
+#undef WORKAROUND_CORETEXT_VARIATIONS_EXTENTS_BUG
+
+        if (rawMinimumValue > rawMaximumValue)
+            std::swap(rawMinimumValue, rawMaximumValue);
+
+        auto b1 = rawAxisIdentifier >> 24;
+        auto b2 = (rawAxisIdentifier & 0xFF0000) >> 16;
+        auto b3 = (rawAxisIdentifier & 0xFF00) >> 8;
+        auto b4 = rawAxisIdentifier & 0xFF;
+        ASSERT(b1 >= 0 && b1 <= std::numeric_limits<char>::max());
+        ASSERT(b2 >= 0 && b2 <= std::numeric_limits<char>::max());
+        ASSERT(b3 >= 0 && b3 <= std::numeric_limits<char>::max());
+        ASSERT(b4 >= 0 && b4 <= std::numeric_limits<char>::max());
+        FontTag resultKey = {{ static_cast<char>(b1), static_cast<char>(b2), static_cast<char>(b3), static_cast<char>(b4) }};
+        VariationDefaults resultValues = { rawDefaultValue, rawMinimumValue, rawMaximumValue };
+        result.set(resultKey, resultValues);
+    }
+    return result;
+}
+#endif
+
 RetainPtr<CTFontRef> preparePlatformFont(CTFontRef originalFont, TextRenderingMode textRenderingMode, const FontFeatureSettings* fontFaceFeatures, const FontVariantSettings* fontFaceVariantSettings, const FontFeatureSettings& features, const FontVariantSettings& variantSettings, const FontVariationSettings& variations)
 {
     if (!originalFont || (!features.size() && variations.isEmpty() && (textRenderingMode == AutoTextRendering) && variantSettings.isAllNormal()
@@ -402,11 +462,27 @@
         featuresToBeApplied.set(newFeature.tag(), newFeature.value());
 
 #if ENABLE(VARIATION_FONTS)
+    auto defaultValues = defaultVariationValues(originalFont);
+
     VariationsMap variationsToBeApplied;
-    for (auto& newVariation : variations)
-        variationsToBeApplied.set(newVariation.tag(), newVariation.value());
+    for (auto& newVariation : variations) {
+        auto iterator = defaultValues.find(newVariation.tag());
+        if (iterator != defaultValues.end()) {
+            float valueToApply = std::max(std::min(newVariation.value(), iterator->value.maximumValue), iterator->value.minimumValue);
+
+            // FIXME: Remove when <rdar://problem/28707822> is fixed
+#define WORKAROUND_CORETEXT_VARIATIONS_DEFAULT_VALUE_BUG (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED < 101300) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED < 110000)
+#if WORKAROUND_CORETEXT_VARIATIONS_DEFAULT_VALUE_BUG
+            if (valueToApply == iterator->value.defaultValue)
+                valueToApply += 0.0001;
 #endif
+#undef WORKAROUND_CORETEXT_VARIATIONS_DEFAULT_VALUE_BUG
 
+            variationsToBeApplied.set(newVariation.tag(), valueToApply);
+        }
+    }
+#endif
+
     RetainPtr<CFMutableDictionaryRef> attributes = adoptCF(CFDictionaryCreateMutable(kCFAllocatorDefault, 2, &kCFTypeDictionaryKeyCallBacks, &kCFTypeDictionaryValueCallBacks));
     if (!featuresToBeApplied.isEmpty()) {
         RetainPtr<CFMutableArrayRef> featureArray = adoptCF(CFArrayCreateMutable(kCFAllocatorDefault, features.size(), &kCFTypeArrayCallBacks));
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to