Changeset 245672 in webkit


Ignore:
Timestamp:
May 22, 2019 9:18:43 PM (5 years ago)
Author:
mmaxfield@apple.com
Message:

font-optical-sizing applies the wrong variation value
​https://bugs.webkit.org/show_bug.cgi?id=197528
<rdar://problem/50152854>

Reviewed by Antti Koivisto.

Source/WebCore:

The OpenType spec says in
​https://docs.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_opsz

"Scale interpretation: Values can be interpreted as text size, in points."

It turns out that it means "typographic point size" which is equal to CSS pixels, not
CSS points.

There are two parts of the font that are sensitive to optical sizing: variation values and
the trak table. We don't want to set the variation value directly because then the trak table
won't be affected. Instead, we can use kCTFontOpticalSizeAttribute to set both of them together.
We will only do this when the CSS says text-rendering:optimizeLegibility or when the font has
an opsz axis but no STAT table. Otherwise, we won't do anything special, which lets CoreText
handle the default behavior for us. This gives us the same default behavior as the rest of the
system.

Tests: fast/text/variations/optical-sizing-trak-2.html

fast/text/variations/optical-sizing-trak.html
fast/text/variations/optical-sizing-units-2.html
fast/text/variations/optical-sizing-units.html

  • platform/graphics/cocoa/FontCacheCoreText.cpp:

(WebCore::FontType::FontType):
(WebCore::preparePlatformFont):
(WebCore::fontWithFamily):
(WebCore::FontCache::systemFallbackForCharacters):

  • platform/graphics/cocoa/FontCacheCoreText.h:
  • platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp:

(WebCore::FontFamilySpecificationCoreText::fontRanges const):

  • platform/graphics/mac/FontCustomPlatformData.cpp:

(WebCore::FontCustomPlatformData::fontPlatformData):

Source/WTF:

  • wtf/Platform.h:

LayoutTests:

  • fast/text/variations/optical-sizing-trak-2-expected-mismatch.html: Added.
  • fast/text/variations/optical-sizing-trak-2.html: Added.
  • fast/text/variations/optical-sizing-trak-expected.html: Added.
  • fast/text/variations/optical-sizing-trak.html: Added.
  • fast/text/variations/optical-sizing-units-2-expected-mismatch.html: Added.
  • fast/text/variations/optical-sizing-units-2.html: Added.
  • fast/text/variations/optical-sizing-units-expected.html: Added.
  • fast/text/variations/optical-sizing-units.html: Added.
  • fast/text/variations/resources/Amstelvar/Amstelvar-Roman-VF104.ttf: Added.

This font havariations/s been approved by the lawyers to add for layout tests.

  • fast/text/variations/resources/Amstelvar/COPYRIGHT.md: Added.
  • fast/text/variations/resources/Amstelvar/OFL.txt: Added.
  • platform/win/TestExpectations:
Location:
trunk
Files:
12 added
9 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r245664 r245672  
     12019-05-22  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        font-optical-sizing applies the wrong variation value
     4        https://bugs.webkit.org/show_bug.cgi?id=197528
     5        <rdar://problem/50152854>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        * fast/text/variations/optical-sizing-trak-2-expected-mismatch.html: Added.
     10        * fast/text/variations/optical-sizing-trak-2.html: Added.
     11        * fast/text/variations/optical-sizing-trak-expected.html: Added.
     12        * fast/text/variations/optical-sizing-trak.html: Added.
     13        * fast/text/variations/optical-sizing-units-2-expected-mismatch.html: Added.
     14        * fast/text/variations/optical-sizing-units-2.html: Added.
     15        * fast/text/variations/optical-sizing-units-expected.html: Added.
     16        * fast/text/variations/optical-sizing-units.html: Added.
     17        * fast/text/variations/resources/Amstelvar/Amstelvar-Roman-VF104.ttf: Added.
     18        This font havariations/s been approved by the lawyers to add for layout tests.
     19        * fast/text/variations/resources/Amstelvar/COPYRIGHT.md: Added.
     20        * fast/text/variations/resources/Amstelvar/OFL.txt: Added.
     21        * platform/win/TestExpectations:
     22
    1232019-05-22  Antti Koivisto  <antti@apple.com>
    224

  • trunk/LayoutTests/platform/win/TestExpectations

    r245647 r245672  
    32513251fast/text/text-combine-shrink-on-color-change.html [ Failure ]
    32523252fast/text/trak-optimizeLegibility.html [ Failure ]
    3253 fast/text/variations/advances.html [ Failure ]
    3254 fast/text/variations/getComputedStyle.html [ Failure ]
     3253fast/text/variations [ Failure Pass ImageOnlyFailure ]
    32553254fast/text/whitespace/023.html [ Failure ]
    32563255fast/transforms/bounding-rect-zoom.html [ Failure ]
    … …  
    33813380fast/text/emoji-single-parent-family-2.html [ ImageOnlyFailure ]
    33823381fast/text/emoji-single-parent-family.html [ ImageOnlyFailure ]
    3383 fast/text/variations/exist.html [ ImageOnlyFailure ]
    3384 fast/text/variations/outofbounds.html [ ImageOnlyFailure ]
    33853382http/tests/security/http-0.9/image-on-HTTP-0.9-default-port-page-allowed-ref-test.html [ ImageOnlyFailure ]
    33863383mathml/presentation/non-bmp-operators-stretching.html [ ImageOnlyFailure ]
    … …  
    35573554fast/text/softbank-emoji.html [ Failure ]
    35583555fast/text/system-font-fallback-emoji.html [ Failure ]
    3559 fast/text/variations/font-loading-api-parse-ranges.html [ Failure ]
    3560 fast/text/variations/optical-sizing.html [ Failure ]
    35613556fast/text/web-font-load-invisible-during-loading.html [ Failure ]
    35623557fast/url/standard-url.html [ Failure ]
    … …  
    36243619fast/text/multiple-codeunit-vertical-upright.html [ ImageOnlyFailure ]
    36253620fast/text/simple-line-layout-simple-text-but-complex-font-path.html [ ImageOnlyFailure ]
    3626 fast/text/variations/font-face-format.html [ ImageOnlyFailure ]
    3627 fast/text/variations/font-face-format-woff2.html [ ImageOnlyFailure ]
    36283621imported/blink/scrollbars/avoid-double-scrollbars-when-html-element-is-not-the-renderview.html [ ImageOnlyFailure ]
    36293622imported/w3c/i18n/bidi/bidi-plaintext-011.html [ ImageOnlyFailure ]
    … …  
    36933686http/wpt/fetch/response-status-text.html [ Failure ]
    36943687js/dom/builtin-getter-name.html [ Failure ]
    3695 fast/text/variations/font-selection-font-weight.html [ ImageOnlyFailure ]
    36963688fast/forms/file/entries-api/webkitdirectory-open-panel.html [ Skip ]
    36973689
    … …  
    37413733# Animated image throttling behaves differently on WK1.
    37423734svg/animations/animated-svg-image-outside-viewport-paused.html [ Skip ]
    3743 
    3744 # This test requires Skia, which isn't available on Windows.
    3745 webkit.org/b/174079 fast/text/variations/skia-postscript-name.html [ ImageOnlyFailure ]
    37463735
    37473736# Beacon is not supported on WK1.

  • trunk/Source/WTF/ChangeLog

    r245647 r245672  
     12019-05-22  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        font-optical-sizing applies the wrong variation value
     4        https://bugs.webkit.org/show_bug.cgi?id=197528
     5        <rdar://problem/50152854>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        * wtf/Platform.h:
     10
    1112019-05-22  Ryan Haddad  <ryanhaddad@apple.com>
    212

  • trunk/Source/WTF/wtf/Platform.h

    r245647 r245672  
    15561556#endif
    15571557
     1558#if (PLATFORM(MAC) && __MAC_OS_X_VERSION_MIN_REQUIRED >= 101400) || (PLATFORM(IOS) && __IPHONE_OS_VERSION_MIN_REQUIRED >= 120000) || (PLATFORM(WATCHOS) && __WATCH_OS_VERSION_MIN_REQUIRED >= 50000) || (PLATFORM(APPLETV) && __TV_OS_VERSION_MIN_REQUIRED >= 120000)
     1559#define HAVE_CORETEXT_AUTO_OPTICAL_SIZING 1
     1560#endif
     1561

  • trunk/Source/WebCore/ChangeLog

    r245664 r245672  
     12019-05-22  Myles C. Maxfield  <mmaxfield@apple.com>
     2
     3        font-optical-sizing applies the wrong variation value
     4        https://bugs.webkit.org/show_bug.cgi?id=197528
     5        <rdar://problem/50152854>
     6
     7        Reviewed by Antti Koivisto.
     8
     9        The OpenType spec says in
     10        https://docs.microsoft.com/en-us/typography/opentype/spec/dvaraxistag_opsz
     11
     12        "Scale interpretation: Values can be interpreted as text size, in points."
     13
     14        It turns out that it means "typographic point size" which is equal to CSS pixels, not
     15        CSS points.
     16
     17        There are two parts of the font that are sensitive to optical sizing: variation values and
     18        the trak table. We don't want to set the variation value directly because then the trak table
     19        won't be affected. Instead, we can use kCTFontOpticalSizeAttribute to set both of them together.
     20        We will only do this when the CSS says text-rendering:optimizeLegibility or when the font has
     21        an opsz axis but no STAT table. Otherwise, we won't do anything special, which lets CoreText
     22        handle the default behavior for us. This gives us the same default behavior as the rest of the
     23        system.
     24
     25        Tests: fast/text/variations/optical-sizing-trak-2.html
     26               fast/text/variations/optical-sizing-trak.html
     27               fast/text/variations/optical-sizing-units-2.html
     28               fast/text/variations/optical-sizing-units.html
     29
     30        * platform/graphics/cocoa/FontCacheCoreText.cpp:
     31        (WebCore::FontType::FontType):
     32        (WebCore::preparePlatformFont):
     33        (WebCore::fontWithFamily):
     34        (WebCore::FontCache::systemFallbackForCharacters):
     35        * platform/graphics/cocoa/FontCacheCoreText.h:
     36        * platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp:
     37        (WebCore::FontFamilySpecificationCoreText::fontRanges const):
     38        * platform/graphics/mac/FontCustomPlatformData.cpp:
     39        (WebCore::FontCustomPlatformData::fontPlatformData):
     40
    1412019-05-22  Antti Koivisto  <antti@apple.com>
    242

  • trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.cpp

    r245647 r245672  
    485485        if (!tables)
    486486            return;
     487        bool foundStat = false;
     488        bool foundTrak = false;
    487489        auto size = CFArrayGetCount(tables.get());
    488490        for (CFIndex i = 0; i < size; ++i) {
    … …  
    498500                break;
    499501            case 'STAT':
     502                foundStat = true;
    500503                variationType = VariationType::OpenType18;
    501504                break;
    … …  
    508511                openTypeShaping = true;
    509512                break;
     513            case 'trak':
     514                foundTrak = true;
     515                break;
    510516            }
    511517        }
    512     }
    513 
    514     enum class VariationType {
    515         NotVariable,
    516         TrueTypeGX,
    517         OpenType18
    518     };
     518        if (foundStat && foundTrak)
     519            trackingType = TrackingType::Automatic;
     520        else if (foundTrak)
     521            trackingType = TrackingType::Manual;
     522    }
     523
     524    enum class VariationType : uint8_t { NotVariable, TrueTypeGX, OpenType18, };
    519525    VariationType variationType { VariationType::NotVariable };
     526    enum class TrackingType : uint8_t { None, Automatic, Manual, };
     527    TrackingType trackingType { TrackingType::None };
    520528    bool openTypeShaping { false };
    521529    bool aatShaping { false };
    522530};
    523531
    524 RetainPtr<CTFontRef> preparePlatformFont(CTFontRef originalFont, const FontDescription& fontDescription, const FontFeatureSettings* fontFaceFeatures, const FontVariantSettings* fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities, float size, bool applyWeightWidthSlopeVariations)
    525 {
    526     bool alwaysAddVariations = false;
    527 
    528     // FIXME: Remove when <rdar://problem/29859207> is fixed
     532RetainPtr<CTFontRef> preparePlatformFont(CTFontRef originalFont, const FontDescription& fontDescription, const FontFeatureSettings* fontFaceFeatures, const FontVariantSettings* fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities, bool applyWeightWidthSlopeVariations)
     533{
     534    if (!originalFont)
     535        return originalFont;
     536
     537    FontType fontType { originalFont };
     538
     539    auto fontOpticalSizing = fontDescription.opticalSizing();
     540
    529541#if ENABLE(VARIATION_FONTS)
    530542    auto defaultValues = defaultVariationValues(originalFont);
    531     alwaysAddVariations = !defaultValues.isEmpty();
    532543
    533544    auto fontSelectionRequest = fontDescription.fontSelectionRequest();
    534     auto fontOpticalSizing = fontDescription.opticalSizing();
    535545    auto fontStyleAxis = fontDescription.fontStyleAxis();
     546
     547    bool forceOpticalSizingOn = fontOpticalSizing == FontOpticalSizing::Enabled && fontType.variationType == FontType::VariationType::TrueTypeGX && defaultValues.contains({{'o', 'p', 's', 'z'}});
     548    bool forceVariations = defaultValues.contains({{'w', 'g', 'h', 't'}}) || defaultValues.contains({{'w', 'd', 't', 'h'}}) || (fontStyleAxis == FontStyleAxis::ital && defaultValues.contains({{'i', 't', 'a', 'l'}})) || (fontStyleAxis == FontStyleAxis::slnt && defaultValues.contains({{'s', 'l', 'n', 't'}}));
     549    const auto& variations = fontDescription.variationSettings();
    536550#else
    537551    UNUSED_PARAM(fontFaceCapabilities);
    538     UNUSED_PARAM(size);
    539552    UNUSED_PARAM(applyWeightWidthSlopeVariations);
     553    bool forceOpticalSizingOn = false;
    540554#endif
    541555
    542556    const auto& features = fontDescription.featureSettings();
    543557    const auto& variantSettings = fontDescription.variantSettings();
    544     const auto& variations = fontDescription.variationSettings();
    545558    auto textRenderingMode = fontDescription.textRenderingMode();
    546559
    547     if (!originalFont || (!features.size() && (!alwaysAddVariations && variations.isEmpty()) && (textRenderingMode == TextRenderingMode::AutoTextRendering) && variantSettings.isAllNormal()
    548         && (!fontFaceFeatures || !fontFaceFeatures->size()) && (!fontFaceVariantSettings || fontFaceVariantSettings->isAllNormal())))
     560    // We might want to check fontType.trackingType == FontType::TrackingType::Manual here, but in order to maintain compatibility with the rest of the system, we don't.
     561    bool noFontFeatureSettings = features.isEmpty();
     562#if ENABLE(VARIATION_FONTS)
     563    bool noFontVariationSettings = !forceVariations && variations.isEmpty();
     564#else
     565    bool noFontVariationSettings = true;
     566#endif
     567    bool textRenderingModeIsAuto = textRenderingMode == TextRenderingMode::AutoTextRendering;
     568    bool variantSettingsIsNormal = variantSettings.isAllNormal();
     569    bool dontNeedToApplyOpticalSizing = fontOpticalSizing == FontOpticalSizing::Enabled && !forceOpticalSizingOn;
     570    bool fontFaceDoesntSpecifyFeatures = !fontFaceFeatures || fontFaceFeatures->isEmpty();
     571    bool fontFaceDoesntSpecifyVariations = !fontFaceVariantSettings || fontFaceVariantSettings->isAllNormal();
     572    if (noFontFeatureSettings && noFontVariationSettings && textRenderingModeIsAuto && variantSettingsIsNormal && dontNeedToApplyOpticalSizing && fontFaceDoesntSpecifyFeatures && fontFaceDoesntSpecifyVariations)
    549573        return originalFont;
    550574
    … …  
    580604    for (auto& newFeature : features)
    581605        featuresToBeApplied.set(newFeature.tag(), newFeature.value());
    582 
    583     FontType fontType(originalFont);
    584606
    585607#if ENABLE(VARIATION_FONTS)
    … …  
    620642    }
    621643
    622     if (fontOpticalSizing == FontOpticalSizing::Enabled) {
    623         const float pxToPtRatio = 3.0f / 4;
    624         applyVariation({{'o', 'p', 's', 'z'}}, size * pxToPtRatio);
    625     }
    626 
    627644    for (auto& newVariation : variations)
    628645        applyVariation(newVariation.tag(), newVariation.value());
    629 
    630646#endif // ENABLE(VARIATION_FONTS)
    631647
    … …  
    656672#endif
    657673
    658     if (textRenderingMode == TextRenderingMode::OptimizeLegibility) {
    659         CGFloat size = CTFontGetSize(originalFont);
     674    if (forceOpticalSizingOn || textRenderingMode == TextRenderingMode::OptimizeLegibility) {
     675#if HAVE(CORETEXT_AUTO_OPTICAL_SIZING)
     676        CFDictionaryAddValue(attributes.get(), kCTFontOpticalSizeAttribute, CFSTR("auto"));
     677#else
     678        auto size = CTFontGetSize(originalFont);
    660679        auto sizeNumber = adoptCF(CFNumberCreate(kCFAllocatorDefault, kCFNumberCGFloatType, &size));
    661680        CFDictionaryAddValue(attributes.get(), kCTFontOpticalSizeAttribute, sizeNumber.get());
    662     }
     681#endif
     682    } else if (fontOpticalSizing == FontOpticalSizing::Disabled) {
     683#if HAVE(CORETEXT_AUTO_OPTICAL_SIZING)
     684        CFDictionaryAddValue(attributes.get(), kCTFontOpticalSizeAttribute, CFSTR("none"));
     685#endif
     686    }
     687
    663688    auto descriptor = adoptCF(CTFontDescriptorCreateWithAttributes(attributes.get()));
    664689    auto result = adoptCF(CTFontCreateCopyWithAttributes(originalFont, CTFontGetSize(originalFont), nullptr, descriptor.get()));
    … …  
    12241249    if (!fontLookup.result)
    12251250        fontLookup = platformFontLookupWithFamily(family, request, size, fontDescription.shouldAllowUserInstalledFonts());
    1226     return preparePlatformFont(fontLookup.result.get(), fontDescription, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities, size, !fontLookup.createdFromPostScriptName);
     1251    return preparePlatformFont(fontLookup.result.get(), fontDescription, fontFaceFeatures, fontFaceVariantSettings, fontFaceCapabilities, !fontLookup.createdFromPostScriptName);
    12271252}
    12281253
    … …  
    13751400
    13761401    auto result = lookupFallbackFont(platformData.font(), description.weight(), description.locale(), characters, length);
    1377     result = preparePlatformFont(result.get(), description, nullptr, nullptr, { }, description.computedSize());
     1402    result = preparePlatformFont(result.get(), description, nullptr, nullptr, { });
    13781403
    13791404    if (!result)

  • trunk/Source/WebCore/platform/graphics/cocoa/FontCacheCoreText.h

    r245647 r245672  
    5151};
    5252
    53 RetainPtr<CTFontRef> preparePlatformFont(CTFontRef, const FontDescription&, const FontFeatureSettings* fontFaceFeatures, const FontVariantSettings* fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities, float size, bool applyWeightWidthSlopeVariations = true);
     53RetainPtr<CTFontRef> preparePlatformFont(CTFontRef, const FontDescription&, const FontFeatureSettings* fontFaceFeatures, const FontVariantSettings* fontFaceVariantSettings, FontSelectionSpecifiedCapabilities fontFaceCapabilities, bool applyWeightWidthSlopeVariations = true);
    5454SynthesisPair computeNecessarySynthesis(CTFontRef, const FontDescription&, bool isPlatformFont = false);
    5555RetainPtr<CTFontRef> platformFontWithFamilySpecialCase(const AtomicString& family, FontSelectionRequest, float size, AllowUserInstalledFonts);

  • trunk/Source/WebCore/platform/graphics/cocoa/FontFamilySpecificationCoreText.cpp

    r245647 r245672  
    109109#endif
    110110
    111         font = preparePlatformFont(font.get(), fontDescription, nullptr, nullptr, { }, fontDescription.computedSize());
     111        font = preparePlatformFont(font.get(), fontDescription, nullptr, nullptr, { });
    112112
    113113        bool syntheticBold, syntheticOblique;

  • trunk/Source/WebCore/platform/graphics/mac/FontCustomPlatformData.cpp

    r245647 r245672  
    4747    FontWidthVariant widthVariant = fontDescription.widthVariant();
    4848    RetainPtr<CTFontRef> font = adoptCF(CTFontCreateWithFontDescriptor(modifiedFontDescriptor.get(), size, nullptr));
    49     font = preparePlatformFont(font.get(), fontDescription, &fontFaceFeatures, &fontFaceVariantSettings, fontFaceCapabilities, fontDescription.computedSize());
     49    font = preparePlatformFont(font.get(), fontDescription, &fontFaceFeatures, &fontFaceVariantSettings, fontFaceCapabilities);
    5050    ASSERT(font);
    5151    return FontPlatformData(font.get(), size, bold, italic, orientation, widthVariant, fontDescription.textRenderingMode());
Note: See TracChangeset for help on using the changeset viewer.