Changeset 290730 in webkit


Ignore:
Timestamp:
Mar 2, 2022 7:27:56 AM (5 months ago)
Author:
graouts@webkit.org
Message:

[web-animations] changes to font-size should recompute keyframes
​https://bugs.webkit.org/show_bug.cgi?id=237357

Reviewed by Antti Koivisto.

LayoutTests/imported/w3c:

  • web-platform-tests/css/css-animations/responsive/column-width-001-expected.txt:
  • web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt:
  • web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt:
  • web-platform-tests/web-animations/responsive/fontSize-expected.txt:
  • web-platform-tests/web-animations/responsive/perspective-expected.txt:
  • web-platform-tests/web-animations/responsive/textIndent-expected.txt:
  • web-platform-tests/web-animations/responsive/to-style-change-expected.txt:

Source/WebCore:

When we compute keyframes, it's possible that some values are specified using "em" units
and thus dependent on the current value for font-size. If the font-size changes over time,
we must recompute keyframes to ensure that any such value is updated.

To ensure that we correctly determine when the font-size changed, we pass a pointer to the
style seen when we last udpated animations so that we only consider a change in font-size
provided we've ever resolved animations.

  • animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::propertyAffectingKeyframeResolutionDidChange):
(WebCore::KeyframeEffect::propertyAffectingLogicalPropertiesDidChange): Deleted.

  • animation/KeyframeEffect.h:
  • animation/KeyframeEffectStack.cpp:

(WebCore::KeyframeEffectStack::applyKeyframeEffects):

  • animation/KeyframeEffectStack.h:
  • style/StyleTreeResolver.cpp:

(WebCore::Style::TreeResolver::createAnimatedElementUpdate):

  • style/Styleable.h:

(WebCore::Styleable::applyKeyframeEffects const):

Location:
trunk
Files:
15 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/imported/w3c/ChangeLog

    r290667 r290730  
     12022-03-02  Antoine Quint  <graouts@webkit.org>
     2
     3        [web-animations] changes to font-size should recompute keyframes
     4        https://bugs.webkit.org/show_bug.cgi?id=237357
     5
     6        Reviewed by Antti Koivisto.
     7
     8        * web-platform-tests/css/css-animations/responsive/column-width-001-expected.txt:
     9        * web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt:
     10        * web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt:
     11        * web-platform-tests/web-animations/responsive/fontSize-expected.txt:
     12        * web-platform-tests/web-animations/responsive/perspective-expected.txt:
     13        * web-platform-tests/web-animations/responsive/textIndent-expected.txt:
     14        * web-platform-tests/web-animations/responsive/to-style-change-expected.txt:
     15
    1162022-03-01  Martin Robinson  <mrobinson@webkit.org>
    217

  • trunk/LayoutTests/imported/w3c/web-platform-tests/css/css-animations/responsive/column-width-001-expected.txt

    r262946 r290730  
    11
    2 FAIL column-width responds to font-size changes assert_equals: expected "80px" but got "40px"
    3 FAIL column-width clamps to 0px assert_equals: expected "0px" but got "30px"
     2PASS column-width responds to font-size changes
     3FAIL column-width clamps to 0px assert_equals: expected "0px" but got "20px"
    44FAIL column-width responds to inherited changes assert_equals: expected "auto" but got "30px"
    55

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-expected.txt

    r267649 r290730  
    11
    2 FAIL Effect values reflect changes to font-size on element assert_equals: Effect value after updating font-size expected "300px" but got "150px"
     2PASS Effect values reflect changes to font-size on element
    33FAIL Effect values reflect changes to font-size on parent element assert_equals: Effect value after updating font-size on parent element expected "300px" but got "150px"
    44PASS Effect values reflect changes to font-size when computed style is not immediately flushed
    5 FAIL Effect values reflect changes to font-size from reparenting assert_equals: Effect value after attaching to font-size:20px parent expected "300px" but got "150px"
     5PASS Effect values reflect changes to font-size from reparenting
    66PASS Effect values reflect changes to target element
    77

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/animation-model/keyframe-effects/effect-value-context-filling-expected.txt

    r287549 r290730  
    11
    2 FAIL Filling effect values reflect changes to font-size on element assert_equals: Effect value after updating font-size expected "400px" but got "200px"
     2PASS Filling effect values reflect changes to font-size on element
    33FAIL Filling effect values reflect changes to font-size on parent element assert_equals: Effect value after updating font-size on parent element expected "400px" but got "200px"
    44FAIL Filling effect values reflect changes to variables on element assert_equals: Effect value after updating variable expected "200px" but got "100px"

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/fontSize-expected.txt

    r287924 r290730  
    11
    2 FAIL Relative font size larger responsive to style changes assert_not_equals: got disallowed value "15.6px"
    3 FAIL Relative font size smaller responsive to style changes assert_not_equals: got disallowed value "14.999999px"
    4 FAIL Font size medium responsive to style changes assert_not_equals: got disallowed value "16px"
    5 FAIL Font size initial responsive to style changes assert_not_equals: got disallowed value "13px"
     2PASS Relative font size larger responsive to style changes
     3PASS Relative font size smaller responsive to style changes
     4PASS Font size medium responsive to style changes
     5PASS Font size initial responsive to style changes
    66

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/perspective-expected.txt

    r287924 r290730  
    11
    22PASS perspective responsive to style changes
    3 FAIL perspective clamped to 0px assert_equals: expected "0px" but got "60px"
     3FAIL perspective clamped to 0px assert_equals: expected "0px" but got "100px"
    44FAIL perspective responsive to inherited changes assert_equals: expected "80px" but got "none"
    55

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/textIndent-expected.txt

    r287924 r290730  
    11
    2 FAIL textIndent responsive to style changes assert_not_equals: got disallowed value "100px hanging"
     2PASS textIndent responsive to style changes
    33FAIL textIndent responsive to inherited textIndent changes assert_equals: expected "200px hanging each-line" but got "150px hanging each-line"
    44

  • trunk/LayoutTests/imported/w3c/web-platform-tests/web-animations/responsive/to-style-change-expected.txt

    r287924 r290730  
    11
    2 FAIL Lengths responsive to style changes assert_equals: expected "calc(40% + 150px)" but got "calc(40% + 100px)"
     2PASS Lengths responsive to style changes
    33PASS Percentages responsive to width style changes
    44PASS Numbers responsive to style changes

  • trunk/Source/WebCore/ChangeLog

    r290729 r290730  
     12022-03-02  Antoine Quint  <graouts@webkit.org>
     2
     3        [web-animations] changes to font-size should recompute keyframes
     4        https://bugs.webkit.org/show_bug.cgi?id=237357
     5
     6        Reviewed by Antti Koivisto.
     7
     8        When we compute keyframes, it's possible that some values are specified using "em" units
     9        and thus dependent on the current value for font-size. If the font-size changes over time,
     10        we must recompute keyframes to ensure that any such value is updated.
     11
     12        To ensure that we correctly determine when the font-size changed, we pass a pointer to the
     13        style seen when we last udpated animations so that we only consider a change in font-size
     14        provided we've ever resolved animations.
     15
     16        * animation/KeyframeEffect.cpp:
     17        (WebCore::KeyframeEffect::propertyAffectingKeyframeResolutionDidChange):
     18        (WebCore::KeyframeEffect::propertyAffectingLogicalPropertiesDidChange): Deleted.
     19        * animation/KeyframeEffect.h:
     20        * animation/KeyframeEffectStack.cpp:
     21        (WebCore::KeyframeEffectStack::applyKeyframeEffects):
     22        * animation/KeyframeEffectStack.h:
     23        * style/StyleTreeResolver.cpp:
     24        (WebCore::Style::TreeResolver::createAnimatedElementUpdate):
     25        * style/Styleable.h:
     26        (WebCore::Styleable::applyKeyframeEffects const):
     27
    1282022-03-02  Oriol Brufau  <obrufau@igalia.com>
    229

  • trunk/Source/WebCore/animation/KeyframeEffect.cpp

    r290667 r290730  
    16831683}
    16841684
    1685 void KeyframeEffect::propertyAffectingLogicalPropertiesDidChange(RenderStyle& unanimatedStyle, const Style::ResolutionContext& resolutionContext)
     1685void KeyframeEffect::propertyAffectingKeyframeResolutionDidChange(RenderStyle& unanimatedStyle, const Style::ResolutionContext& resolutionContext)
    16861686{
    16871687    switch (m_blendingKeyframesSource) {

  • trunk/Source/WebCore/animation/KeyframeEffect.h

    r290667 r290730  
    127127    void animationTimingDidChange();
    128128    void transformRelatedPropertyDidChange();
    129     void propertyAffectingLogicalPropertiesDidChange(RenderStyle&, const Style::ResolutionContext&);
     129    void propertyAffectingKeyframeResolutionDidChange(RenderStyle&, const Style::ResolutionContext&);
    130130    OptionSet<AcceleratedActionApplicationResult> applyPendingAcceleratedActions();
    131131

  • trunk/Source/WebCore/animation/KeyframeEffectStack.cpp

    r289598 r290730  
    123123}
    124124
    125 OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle& previousLastStyleChangeEventStyle, const Style::ResolutionContext& resolutionContext)
     125OptionSet<AnimationImpact> KeyframeEffectStack::applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle* previousLastStyleChangeEventStyle, const Style::ResolutionContext& resolutionContext)
    126126{
    127127    OptionSet<AnimationImpact> impact;
    128128
     129    auto& previousStyle = previousLastStyleChangeEventStyle ? *previousLastStyleChangeEventStyle : RenderStyle::defaultStyle();
     130
    129131    auto transformRelatedPropertyChanged = [&]() -> bool {
    130         return !arePointingToEqualData(targetStyle.translate(), previousLastStyleChangeEventStyle.translate())
    131             || !arePointingToEqualData(targetStyle.scale(), previousLastStyleChangeEventStyle.scale())
    132             || !arePointingToEqualData(targetStyle.rotate(), previousLastStyleChangeEventStyle.rotate())
    133             || targetStyle.transform() != previousLastStyleChangeEventStyle.transform();
     132        return !arePointingToEqualData(targetStyle.translate(), previousStyle.translate())
     133            || !arePointingToEqualData(targetStyle.scale(), previousStyle.scale())
     134            || !arePointingToEqualData(targetStyle.rotate(), previousStyle.rotate())
     135            || targetStyle.transform() != previousStyle.transform();
    134136    }();
    135137
    136     auto propertyAffectingLogicalPropertiesChanged = previousLastStyleChangeEventStyle.direction() != targetStyle.direction()
    137         || previousLastStyleChangeEventStyle.writingMode() != targetStyle.writingMode();
     138    auto fontSizeChanged = previousLastStyleChangeEventStyle && previousLastStyleChangeEventStyle->computedFontSize() != targetStyle.computedFontSize();
     139    auto propertyAffectingLogicalPropertiesChanged = previousLastStyleChangeEventStyle && (previousLastStyleChangeEventStyle->direction() != targetStyle.direction() || previousLastStyleChangeEventStyle->writingMode() != targetStyle.writingMode());
    138140
    139141    auto unanimatedStyle = RenderStyle::clone(targetStyle);
    … …  
    142144        ASSERT(effect->animation());
    143145
    144         if (propertyAffectingLogicalPropertiesChanged)
    145             effect->propertyAffectingLogicalPropertiesDidChange(unanimatedStyle, resolutionContext);
     146        if (propertyAffectingLogicalPropertiesChanged || fontSizeChanged)
     147            effect->propertyAffectingKeyframeResolutionDidChange(unanimatedStyle, resolutionContext);
    146148
    147149        effect->animation()->resolve(targetStyle, resolutionContext);

  • trunk/Source/WebCore/animation/KeyframeEffectStack.h

    r289598 r290730  
    5555    bool isCurrentlyAffectingProperty(CSSPropertyID) const;
    5656    bool requiresPseudoElement() const;
    57     OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle& previousLastStyleChangeEventStyle, const Style::ResolutionContext&);
     57    OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle* previousLastStyleChangeEventStyle, const Style::ResolutionContext&);
    5858    bool hasEffectWithImplicitKeyframes() const;
    5959

  • trunk/Source/WebCore/style/StyleTreeResolver.cpp

    r290564 r290730  
    367367    // as animations created via the JS API.
    368368    if (styleable.hasKeyframeEffects()) {
    369         auto previousLastStyleChangeEventStyle = styleable.lastStyleChangeEventStyle() ? RenderStyle::clonePtr(*styleable.lastStyleChangeEventStyle()) : RenderStyle::createPtr();
     369        auto previousLastStyleChangeEventStyle = styleable.lastStyleChangeEventStyle() ? RenderStyle::clonePtr(*styleable.lastStyleChangeEventStyle()) : nullptr;
    370370        // Record the style prior to applying animations for this style change event.
    371371        styleable.setLastStyleChangeEventStyle(RenderStyle::clonePtr(*newStyle));
    372372        // Apply all keyframe effects to the new style.
    373373        auto animatedStyle = RenderStyle::clonePtr(*newStyle);
    374         animationImpact = styleable.applyKeyframeEffects(*animatedStyle, *previousLastStyleChangeEventStyle, resolutionContext);
     374        animationImpact = styleable.applyKeyframeEffects(*animatedStyle, previousLastStyleChangeEventStyle.get(), resolutionContext);
    375375        newStyle = WTFMove(animatedStyle);
    376376

  • trunk/Source/WebCore/style/Styleable.h

    r289455 r290730  
    9797    }
    9898
    99     OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle& previousLastStyleChangeEventStyle, const Style::ResolutionContext& resolutionContext) const
     99    OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle* previousLastStyleChangeEventStyle, const Style::ResolutionContext& resolutionContext) const
    100100    {
    101101        return element.ensureKeyframeEffectStack(pseudoId).applyKeyframeEffects(targetStyle, previousLastStyleChangeEventStyle, resolutionContext);
Note: See TracChangeset for help on using the changeset viewer.