Changeset 289211 in webkit


Ignore:
Timestamp:
Feb 7, 2022 5:57:38 AM (5 months ago)
Author:
graouts@webkit.org
Message:

[Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
​https://bugs.webkit.org/show_bug.cgi?id=236080
<rdar://problem/88432373>

Reviewed by Dean Jackson.

Source/WebCore:

Accelerated animations can only be started if the keyframe effect's renderer is composited.
Keyframe effects enqueue a list of accelerated actions (play, pause, seek, stop, etc.) when
they can be accelerated, and these actions are performed in sync as the document timeline
is finishing up its "update animations and send events" procedure as we update the page
rendering.

When an animation is started _without_ a delay, the machinery to consider making a renderer
composited happens _before_ we update animations and send events, and since at this stage the
new animation's effect is already in its target's effect stack because it is immediately
"relevant" (per the Web Animations terminology) and targets a property that can be accelerated,
its renderer is indeed composited.

Thus by the time we consider starting this animation's effect with acceleration, it is in a
condition to do so since its renderer is already composited.

However, when an animation is started _with_ a delay, it is not immediately "relevant" and thus
won't appear in its target's effect stack. That will happen once we run the "update animations
and send events procedure" the next time we update the page rendering. But before that, the
effect's renderer will be considered to be made composited and _will not_ be because at this
point there is no effect animating a property that can be accelerated.

As we eventually run the "update animations and send events" procedure, we'll enqueue an
accelerated action to play the animation now that it became relevant, and when that procedure
completes and we try to apply that accelerated action, we'll fail to start an accelerated
animation since the renderer is not composited.

To address this, we only enqueue accelerated actions when the renderer is composited, thus
matching the condition to actually be able to apply this action.

This means we no longer need the animationDidPlay() on effects since we wouldn't be able to
honor enqueuing a "play" accelerated action since the renderer won't be composited yet at this
time in the vast majority of cases.

Test: webanimations/transform-animation-with-delay-yields-accelerated-animation.html

  • animation/AnimationEffect.h:

(WebCore::AnimationEffect::animationDidTick):
(WebCore::AnimationEffect::animationDidPlay): Deleted.

  • animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidPlay): Deleted.

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

(WebCore::WebAnimation::play):

  • animation/AnimationEffect.h:

(WebCore::AnimationEffect::animationDidTick):
(WebCore::AnimationEffect::animationDidPlay): Deleted.

  • animation/KeyframeEffect.cpp:

(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidPlay): Deleted.

  • animation/KeyframeEffect.h:

(WebCore::KeyframeEffect::isAboutToRunAccelerated const):

  • animation/WebAnimation.cpp:

(WebCore::WebAnimation::play):

LayoutTests:

Add a test that runs a "transform" animation with a small delay and checks that it yielded
an accelerated animation. We also rewrite some existing tests to use internals.acceleratedAnimationsForElement()
since relying on a layer tree dump was not reliable, which also resolves some platform-specific
issues.

  • platform/glib/TestExpectations:
  • platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
  • platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
  • platform/win/TestExpectations:
  • webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Make sure overflow doesn't cause an image failure.
  • webanimations/partly-accelerated-transition-by-removing-property-expected.txt:
  • webanimations/partly-accelerated-transition-by-removing-property.html: Rewrite test to check on the accelerated

animation count and ensure the element is already composited to make the test work on WK1 and GTK.

  • webanimations/resources/request-frames-until-true.js: Added.

(const.requestFramesUntilTrue.async resolveCondition):

  • webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt: Added.
  • webanimations/transform-animation-with-delay-yields-accelerated-animation.html: Added.
Location:
trunk
Files:
3 added
2 deleted
11 edited

Legend:

Unmodified
Added
Removed

  • trunk/LayoutTests/ChangeLog

    r289203 r289211  
     12022-02-07  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
     4        https://bugs.webkit.org/show_bug.cgi?id=236080
     5        <rdar://problem/88432373>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Add a test that runs a "transform" animation with a small delay and checks that it yielded
     10        an accelerated animation. We also rewrite some existing tests to use internals.acceleratedAnimationsForElement()
     11        since relying on a layer tree dump was not reliable, which also resolves some platform-specific
     12        issues.
     13
     14        * platform/glib/TestExpectations:
     15        * platform/gtk/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
     16        * platform/mac-wk1/webanimations/partly-accelerated-transition-by-removing-property-expected.txt: Removed.
     17        * platform/win/TestExpectations:
     18        * webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html: Make sure overflow doesn't cause an image failure.
     19        * webanimations/partly-accelerated-transition-by-removing-property-expected.txt:
     20        * webanimations/partly-accelerated-transition-by-removing-property.html: Rewrite test to check on the accelerated
     21        animation count and ensure the element is already composited to make the test work on WK1 and GTK.
     22        * webanimations/resources/request-frames-until-true.js: Added.
     23        (const.requestFramesUntilTrue.async resolveCondition):
     24        * webanimations/transform-animation-with-delay-yields-accelerated-animation-expected.txt: Added.
     25        * webanimations/transform-animation-with-delay-yields-accelerated-animation.html: Added.
     26
    1272022-02-07  Rob Buis  <rbuis@igalia.com>
    228

  • trunk/LayoutTests/platform/glib/TestExpectations

    r289193 r289211  
    19411941# The DocumentTimeline.maximumFrameRate property which this test is about is not available on GTK/WPE.
    19421942webanimations/frame-rate/document-timeline-maximum-frame-rate.html [ Skip ]
     1943
     1944# This test requires an internal API which only works on Cocoa ports.
     1945webanimations/transform-animation-with-delay-yields-accelerated-animation.html [ Skip ]
     1946webanimations/partly-accelerated-transition-by-removing-property.html [ Skip ]
    19431947
    19441948# OT-SVG is not implemented on GTK/WPE

  • trunk/LayoutTests/platform/win/TestExpectations

    r289193 r289211  
    43494349[ Win10 ] storage/indexeddb/modern/idbindex-getallkeys-1.html [ Failure ]
    43504350[ Win10 ] transforms/2d/hindi-rotated.html [ Failure ]
    4351 [ Win10 ] webanimations/partly-accelerated-transition-by-removing-property.html [ Failure ]
    43524351
    43534352[ Win10 ] fast/repaint/block-no-inflow-children.html [ ImageOnlyFailure ]
    … …  
    49584957webanimations/frame-rate/document-timeline-maximum-frame-rate.html [ Skip ]
    49594958
     4959# This test requires an internal API which only works on Cocoa ports.
     4960webanimations/transform-animation-with-delay-yields-accelerated-animation.html [ Skip ]
     4961webanimations/partly-accelerated-transition-by-removing-property.html [ Skip ]
     4962
    49604963pointerevents/mouse/pointer-button-and-buttons.html [ Failure ]
    49614964pointerevents/mouse/pointer-capture.html [ Failure ]

  • trunk/LayoutTests/webanimations/accelerated-css-transition-with-easing-y-axis-above-1.html

    r266280 r289211  
    44    body {
    55        background-color: red;
     6        overflow: hidden;
    67    }
    78   

  • trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property-expected.txt

    r255383 r289211  
    1 (GraphicsLayer
    2   (anchor 0.00 0.00)
    3   (bounds 800.00 600.00)
    4   (children 1
    5     (GraphicsLayer
    6       (bounds 800.00 600.00)
    7       (contentsOpaque 1)
    8       (children 1
    9         (GraphicsLayer
    10           (position 100.00 0.00)
    11           (bounds 100.00 100.00)
    12           (opacity 0.50)
    13           (contentsOpaque 1)
    14         )
    15       )
    16     )
    17   )
    18 )
    191
     2PASS Transtioning two properties, one of which can be accelerated while the other cannot, yields only one accelerated animation.
     3

  • trunk/LayoutTests/webanimations/partly-accelerated-transition-by-removing-property.html

    r267188 r289211  
    11<!DOCTYPE html>
    22<body>
    3 <pre id="results"></pre>
    4 <div id="target" style="position: absolute; top: 0; left: 0; width: 100px; height: 100px; background-color: black;"></div>
     3<script src="../resources/testharness.js"></script>
     4<script src="../resources/testharnessreport.js"></script>
     5<script src="resources/request-frames-until-true.js"></script>
     6<style>
     7
     8    #target {
     9        position: absolute;
     10        left: 0;
     11        top: 0;
     12        width: 100px;
     13        height: 100px;
     14        background-color: black;
     15        will-change: opacity;
     16    }
     17
     18</style>
     19<div id="target"></div>
    520<script>
    621
    7 if (window.testRunner) {
    8     testRunner.waitUntilDone();
    9     testRunner.dumpAsText();
    10 }
     22promise_test(async t => {
     23    const target = document.getElementById("target");
    1124
    12 const target = document.getElementById("target");
    13 target.style.opacity = "0.5";
    14 target.style.marginLeft = "100px";
    15 setTimeout(() => {
     25    const acceleratedAnimationsStarted = async numberOfAnimations => {
     26        return requestFramesUntilTrue(
     27            () => internals.acceleratedAnimationsForElement(target).length == numberOfAnimations,
     28            () => !"acceleratedAnimationsForElement" in window.internals);
     29    };
     30
     31    // Set initial styles.
     32    target.style.opacity = "0.5";
     33    target.style.marginLeft = "100px";
     34
     35    await new Promise(setTimeout);
     36
     37    // Now update styles to trigger a transition for properties, only one of which should yield an accelerated animation.
    1638    target.style.removeProperty("opacity");
    1739    target.style.removeProperty("margin-left");
    1840    target.style.transitionProperty = "margin-left opacity";
    1941    target.style.transitionDuration = "1s";
    20     requestAnimationFrame(() => {
    21         if (window.internals)
    22             document.getElementById("results").innerText = internals.layerTreeAsText(document);
    23         if (window.testRunner)
    24             testRunner.notifyDone();
    25     });
    26 });
     42
     43    // Wait a few frames for the opacity animation to be commited.
     44    await acceleratedAnimationsStarted(1);
     45}, "Transtioning two properties, one of which can be accelerated while the other cannot, yields only one accelerated animation.");
    2746
    2847</script>

  • trunk/Source/WebCore/ChangeLog

    r289210 r289211  
     12022-02-07  Antoine Quint  <graouts@webkit.org>
     2
     3        [Web Animations] Starting a transform animation with a 1ms delay doesn't run it accelerated
     4        https://bugs.webkit.org/show_bug.cgi?id=236080
     5        <rdar://problem/88432373>
     6
     7        Reviewed by Dean Jackson.
     8
     9        Accelerated animations can only be started if the keyframe effect's renderer is composited.
     10        Keyframe effects enqueue a list of accelerated actions (play, pause, seek, stop, etc.) when
     11        they can be accelerated, and these actions are performed in sync as the document timeline
     12        is finishing up its "update animations and send events" procedure as we update the page
     13        rendering.
     14
     15        When an animation is started _without_ a delay, the machinery to consider making a renderer
     16        composited happens _before_ we update animations and send events, and since at this stage the
     17        new animation's effect is already in its target's effect stack because it is immediately
     18        "relevant" (per the Web Animations terminology) and targets a property that can be accelerated,
     19        its renderer is indeed composited.
     20
     21        Thus by the time we consider starting this animation's effect with acceleration, it is in a
     22        condition to do so since its renderer is already composited.
     23
     24        However, when an animation is started _with_ a delay, it is not immediately "relevant" and thus
     25        won't appear in its target's effect stack. That will happen once we run the "update animations
     26        and send events procedure" the next time we update the page rendering. But before that, the
     27        effect's renderer will be considered to be made composited and _will not_ be because at this
     28        point there is no effect animating a property that can be accelerated.
     29
     30        As we eventually run the "update animations and send events" procedure, we'll enqueue an
     31        accelerated action to play the animation now that it became relevant, and when that procedure
     32        completes and we try to apply that accelerated action, we'll fail to start an accelerated
     33        animation since the renderer is not composited.
     34
     35        To address this, we only enqueue accelerated actions when the renderer is composited, thus
     36        matching the condition to actually be able to apply this action.
     37
     38        This means we no longer need the animationDidPlay() on effects since we wouldn't be able to
     39        honor enqueuing a "play" accelerated action since the renderer won't be composited yet at this
     40        time in the vast majority of cases.
     41
     42        Test: webanimations/transform-animation-with-delay-yields-accelerated-animation.html
     43
     44        * animation/AnimationEffect.h:
     45        (WebCore::AnimationEffect::animationDidTick):
     46        (WebCore::AnimationEffect::animationDidPlay): Deleted.
     47        * animation/KeyframeEffect.cpp:
     48        (WebCore::KeyframeEffect::updateAcceleratedActions):
     49        (WebCore::KeyframeEffect::animationDidPlay): Deleted.
     50        * animation/KeyframeEffect.h:
     51        * animation/WebAnimation.cpp:
     52        (WebCore::WebAnimation::play):
     53
     54        * animation/AnimationEffect.h:
     55        (WebCore::AnimationEffect::animationDidTick):
     56        (WebCore::AnimationEffect::animationDidPlay): Deleted.
     57        * animation/KeyframeEffect.cpp:
     58        (WebCore::KeyframeEffect::updateAcceleratedActions):
     59        (WebCore::KeyframeEffect::animationDidPlay): Deleted.
     60        * animation/KeyframeEffect.h:
     61        (WebCore::KeyframeEffect::isAboutToRunAccelerated const):
     62        * animation/WebAnimation.cpp:
     63        (WebCore::WebAnimation::play):
     64
    1652022-02-07  Nikolas Zimmermann  <nzimmermann@igalia.com>
    266

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

    r286555 r289211  
    6464
    6565    virtual void animationDidTick() { };
    66     virtual void animationDidPlay() { };
    6766    virtual void animationDidChangeTimingProperties() { };
    6867    virtual void animationWasCanceled() { };

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

    r289048 r289211  
    16171617void KeyframeEffect::updateAcceleratedActions()
    16181618{
     1619    auto* renderer = this->renderer();
     1620    if (!renderer || !renderer->isComposited())
     1621        return;
     1622
    16191623    if (!canBeAccelerated()) {
    16201624        // In the case where this animation is actively targeting a transform-related property and yet
    … …  
    16791683    invalidate();
    16801684    updateAcceleratedActions();
    1681 }
    1682 
    1683 void KeyframeEffect::animationDidPlay()
    1684 {
    1685     if (m_acceleratedPropertiesState != AcceleratedProperties::None)
    1686         addPendingAcceleratedAction(AcceleratedAction::Play);
    16871685}
    16881686

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

    r289158 r289211  
    136136    bool isRunningAccelerated() const { return m_runningAccelerated == RunningAccelerated::Yes; }
    137137
    138     bool isAboutToRunAccelerated() const { return canBeAccelerated() && m_lastRecordedAcceleratedAction != AcceleratedAction::Stop; }
     138    // FIXME: These ignore the fact that some timing functions can prevent acceleration.
     139    bool isAboutToRunAccelerated() const { return m_acceleratedPropertiesState != AcceleratedProperties::None && m_lastRecordedAcceleratedAction != AcceleratedAction::Stop; }
    139140
    140141    bool filterFunctionListsMatch() const override { return m_filterFunctionListsMatch; }
    … …  
    211212    bool isKeyframeEffect() const final { return true; }
    212213    void animationDidTick() final;
    213     void animationDidPlay() final;
    214214    void animationDidChangeTimingProperties() final;
    215215    void animationWasCanceled() final;

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

    r288069 r289211  
    10481048    invalidateEffect();
    10491049
    1050     if (m_effect)
    1051         m_effect->animationDidPlay();
    1052 
    10531050    return { };
    10541051}
Note: See TracChangeset for help on using the changeset viewer.