Title: [294391] trunk
Revision
294391
Author
grao...@webkit.org
Date
2022-05-18 05:54:37 -0700 (Wed, 18 May 2022)

Log Message

[web-animations] dynamically toggle acceleration of composition animations depending on ability of animations in the effect stack to be accelerated
https://bugs.webkit.org/show_bug.cgi?id=236708
<rdar://89029577>

Reviewed by Dean Jackson.

We currently don't have support for composite operations at the GraphicsLayerCA level. There are many situations
under which an effect depends on another effect lower in the effect stack, and if that effect fails to run accelerated,
such as due to a non-replace composite operation, we should not run any other effect for that property using
acceleration.

While we could add logic to correctly determine the cases where we actually can run effects using acceleration,
it is a relatively involved task and a simpler, less error-prone approach is to correctly run a whole effect
stack containing some non-replace composite operations without any acceleration.

We already did something similar for cases where we'd fail to run an effect targeting a transform property because
GraphicsLayerCA::addAnimation() would return false. Such situations include using a steps() timing function, a
playback rate other than 1 or providing fewer than 2 animation values.

We now use a single mechanism to deal not only with the case of animations that fail to run using GraphicsLayerCA,
but also with animations that we know GraphicsLayerCA cannot handle, such as non-replace composite operations.

To do this, we change the RunningAccelerated enum to have both "Prevented" and "Failed" values instead of the
single No value to determine the reason why an effect does not accelerated. The "Prevented" value indicates that
the effect stack or the effect itself prevented the effect from running accelerated, while the "Failed" value
indicates that while we attempted to run the animation accelerated through GraphicsLayerCA, we failed to.

We now account for the "Failed" value in preventsAcceleration() to determine that an effect should prevent other
effects from running accelerated. This method is used by KeyframeEffectStack to check that all of its effects
are able to run accelerated and to toggle acceleration based on this value.

As an effect's composite operation or keyframes is set, or when pending accelerated actions are applied, we now
instantiate a CanBeAcceleratedMutationScope which will automatically detect if the preventsAcceleration() value
changes before and after such mutations, and informs the effect stack if that is the case to make sure the effect
stack knows whether it is newly able to run or not run using acceleration.

Finally, to be able to correctly interact with GraphicsLayerCA, we must make sure to reuse the same key when creating
keyframes passed to its animation APIs, so we add a m_keyframesName member.

* Source/WebCore/animation/DocumentTimeline.cpp:
(WebCore::DocumentTimeline::applyPendingAcceleratedAnimations):
* Source/WebCore/animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::KeyframeEffect):
(WebCore::KeyframeEffect::copyPropertiesFromSource):
(WebCore::KeyframeEffect::updateBlendingKeyframes):
(WebCore::KeyframeEffect::setBlendingKeyframes):
(WebCore::KeyframeEffect::computeCSSTransitionBlendingKeyframes):
(WebCore::KeyframeEffect::preventsAcceleration const):
(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::addPendingAcceleratedAction):
(WebCore::KeyframeEffect::animationDidChangeTimingProperties):
(WebCore::KeyframeEffect::applyPendingAcceleratedActions):
(WebCore::KeyframeEffect::setComposite):
(WebCore::KeyframeEffect::effectStackNoLongerPreventsAcceleration):
(WebCore::KeyframeEffect::effectStackNoLongerAllowsAcceleration):
(WebCore::KeyframeEffect::abilityToBeAcceleratedDidChange):
(WebCore::KeyframeEffect::CanBeAcceleratedMutationScope::CanBeAcceleratedMutationScope):
(WebCore::KeyframeEffect::CanBeAcceleratedMutationScope::~CanBeAcceleratedMutationScope):
(WebCore::KeyframeEffect::stopAcceleratingTransformRelatedProperties): Deleted.
* Source/WebCore/animation/KeyframeEffect.h:
* Source/WebCore/animation/KeyframeEffectStack.cpp:
(WebCore::KeyframeEffectStack::addEffect):
(WebCore::KeyframeEffectStack::removeEffect):
(WebCore::KeyframeEffectStack::effectAbilityToBeAcceleratedDidChange):
(WebCore::KeyframeEffectStack::allowsAcceleration const):
(WebCore::KeyframeEffectStack::startAcceleratedAnimationsIfPossible):
(WebCore::KeyframeEffectStack::stopAcceleratedAnimations):
(WebCore::KeyframeEffectStack::stopAcceleratingTransformRelatedProperties): Deleted.
(WebCore::KeyframeEffectStack::containsEffectThatPreventsAccelerationOfEffect): Deleted.
* Source/WebCore/animation/KeyframeEffectStack.h:
* Source/WebCore/animation/WebAnimationTypes.h:
* LayoutTests/webanimations/accelerated-animations-and-composite-expected.txt:
* LayoutTests/webanimations/accelerated-animations-and-composite.html:

Canonical link: https://commits.webkit.org/250686@main

Modified Paths

Diff

Modified: trunk/LayoutTests/webanimations/accelerated-animations-and-composite-expected.txt (294390 => 294391)


--- trunk/LayoutTests/webanimations/accelerated-animations-and-composite-expected.txt	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/LayoutTests/webanimations/accelerated-animations-and-composite-expected.txt	2022-05-18 12:54:37 UTC (rev 294391)
@@ -5,4 +5,11 @@
 PASS Setting 'composite' to 'add' on an animation lower down the stack should allow replace animations further up the stack to be accelerated
 PASS Setting 'composite' to 'add' on an animation lower down the stack should prevent replace animations with an implicity keyframe further up the stack to be accelerated
 PASS Setting 'composite' to 'add' on an animation lower down the stack targeting a property that isn't accelerated should allow replace animations with implicit keyframes further up the stack to be accelerated
+PASS Dynamically setting 'composite' on an effect should toggle acceleration
+PASS Dynamically setting 'composite' on a keyframe should toggle acceleration
+PASS Dynamically setting 'composite' to 'add' on an animation further up the stack should toggle acceleration on lower animations in the stack
+PASS Dynamically setting 'composite' to 'add' on an animation lower down the stack should toggle acceleration but always allow replace animations further up the stack to be accelerated
+PASS Dynamically setting 'composite' to 'add' on an animation lower down the stack should toggle acceleration on replace animations with an implicity keyframe further up the stack
+PASS Dynamically setting 'composite' to 'add' on an animation lower down the stack targeting a property that isn't accelerated shouldn't prevent acceleration of animations with implicit keyframes further up the stack to be accelerated
+PASS Adding a composing effect on top of an existing replace effect should prevent both effects from running accelerated
 

Modified: trunk/LayoutTests/webanimations/accelerated-animations-and-composite.html (294390 => 294391)


--- trunk/LayoutTests/webanimations/accelerated-animations-and-composite.html	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/LayoutTests/webanimations/accelerated-animations-and-composite.html	2022-05-18 12:54:37 UTC (rev 294391)
@@ -83,7 +83,7 @@
         { duration }
     ));
     await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
-    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
 }, "Setting 'composite' to 'add' on an animation lower down the stack should allow replace animations further up the stack to be accelerated");
 
 promise_test(async test => {
@@ -123,5 +123,149 @@
     assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
 }, "Setting 'composite' to 'add' on an animation lower down the stack targeting a property that isn't accelerated should allow replace animations with implicit keyframes further up the stack to be accelerated");
 
+// The following tests check the dynamic interruption and/or start of accelerated animations
+// after changes made to other effects in the stack.
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animation = target.animate({ transform: "translateX(100px)" }, duration);
+
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Setting the effect composition to "add" means that the animation should
+    // no longer be accelerated.
+    animation.effect.composite = "add";
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Resetting the effect composition to "replace" means that the animation
+    // should be accelerated again.
+    animation.effect.composite = "replace";
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+}, "Dynamically setting 'composite' on an effect should toggle acceleration");
+
+promise_test(async test => {
+    const keyframes = composite => [{ offset: 1, composite, transform: "translateX(100px)" }];
+
+    const target = createDiv(test);
+    const animation = target.animate(keyframes("replace"), duration);
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Setting the keyframe composition to "add" means that the animation should
+    // no longer be accelerated.
+    animation.effect.setKeyframes(keyframes("add"));
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Resetting the keyframe composition to "replace" means that the animation
+    // should be accelerated again.
+    animation.effect.setKeyframes(keyframes("replace"));
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+}, "Dynamically setting 'composite' on a keyframe should toggle acceleration");
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animations = [];
+
+    animations.push(target.animate({ transform: "translateX(100px)" }, duration));
+    animations.push(target.animate({ transform: "translateY(100px)" }, duration));
+
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2, "Step 1");
+
+    // Make the second animation a composing animation. This will prevent the first animation
+    // from running accelerated as well.
+    animations[1].effect.composite = "add";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0, "Step 2");
+
+    // Reset to the original state.
+    animations[1].effect.composite = "replace";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2, "Step 3");
+}, "Dynamically setting 'composite' to 'add' on an animation further up the stack should toggle acceleration on lower animations in the stack");
+
+promise_test(async test => {
+    const target = createDiv(test);
+
+    const animations = [];
+    animations.push(target.animate({ transform: "translateX(100px)" }, duration));
+    animations.push(target.animate({ transform: ["none", "translateY(100px)"] }, duration));
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2);
+
+    // Make the first animation a composing animation. This will prevent the other animation from running accelerated.
+    animations[0].effect.composite = "add";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Reset to the original state.
+    animations[0].effect.composite = "replace";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2);
+}, "Dynamically setting 'composite' to 'add' on an animation lower down the stack should toggle acceleration but always allow replace animations further up the stack to be accelerated");
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animations = [];
+
+    animations.push(target.animate({ transform: "translateX(100px)" }, duration));
+    animations.push(target.animate({ transform: "translateX(100px)" }, duration));
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2);
+
+    // Make the first animation a composing animation. This will prevent both animations
+    // from running accelerated since the second animation uses the first animation as
+    // input due to using an implicit 0% keyframe.
+    animations[0].effect.composite = "add";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+
+    // Reset to the original state.
+    animations[0].effect.composite = "replace";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 2);
+}, "Dynamically setting 'composite' to 'add' on an animation lower down the stack should toggle acceleration on replace animations with an implicity keyframe further up the stack");
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animations = [];
+
+    animations.push(target.animate({ marginLeft: "100px" }, duration));
+    animations.push(target.animate({ transform: "translateX(100px)" }, duration));
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Make the first animation a composing animation. This should not prevent the other animation from running accelerated.
+    animations[0].effect.composite = "add";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    // Reset to the original state.
+    animations[0].effect.composite = "replace";
+    await Promise.all(animations.map(animation => animationReadyToAnimateAccelerated(animation)));
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+}, "Dynamically setting 'composite' to 'add' on an animation lower down the stack targeting a property that isn't accelerated shouldn't prevent acceleration of animations with implicit keyframes further up the stack to be accelerated");
+
+promise_test(async test => {
+    const target = createDiv(test);
+    const animation = target.animate({ transform: "translateX(100px)" }, duration);
+
+    await animationReadyToAnimateAccelerated(animation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 1);
+
+    const composingAnimation = target.animate(
+        { transform: "translateY(100px)" },
+        { composite: "add", duration }
+    );
+
+    await animationReadyToAnimateAccelerated(composingAnimation);
+    assert_equals(internals.acceleratedAnimationsForElement(target).length, 0);
+}, "Adding a composing effect on top of an existing replace effect should prevent both effects from running accelerated");
+
 </script>
 </body>

Modified: trunk/Source/WebCore/animation/DocumentTimeline.cpp (294390 => 294391)


--- trunk/Source/WebCore/animation/DocumentTimeline.cpp	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/DocumentTimeline.cpp	2022-05-18 12:54:37 UTC (rev 294391)
@@ -390,13 +390,6 @@
     auto acceleratedAnimationsPendingRunningStateChange = m_acceleratedAnimationsPendingRunningStateChange;
     m_acceleratedAnimationsPendingRunningStateChange.clear();
 
-    // Animations may fail to run accelerated for reasons private to GraphicsLayerCA. If that happens, and the animation
-    // in question targets a transform-related property, we must prevent all other transform-related animations for this
-    // element to run accelerated since we can't run some transform-related animations accelerated, and some not. To do
-    // this, we keep a list of all KeyframeEffectStack objects containing an effect that failed to start a transform-related
-    // animation so that we can return any transform-related accelerated animation to run non-accelerated.
-    HashSet<KeyframeEffectStack*> effectStacksContainingEffectThatFailedToRunAcceleratedTransformRelatedAnimation;
-
     bool hasForcedLayout = false;
     for (auto& animation : acceleratedAnimationsPendingRunningStateChange) {
         auto* effect = animation->effect();
@@ -406,16 +399,8 @@
         auto& keyframeEffect = downcast<KeyframeEffect>(*effect);
         if (!hasForcedLayout)
             hasForcedLayout |= keyframeEffect.forceLayoutIfNeeded();
-        auto pendingAccelerationActionResult = keyframeEffect.applyPendingAcceleratedActions();
-        if (pendingAccelerationActionResult.contains(AcceleratedActionApplicationResult::TransformRelatedAnimationCannotBeAccelerated)) {
-            ASSERT(keyframeEffect.targetStyleable());
-            ASSERT(keyframeEffect.targetStyleable()->keyframeEffectStack());
-            effectStacksContainingEffectThatFailedToRunAcceleratedTransformRelatedAnimation.add(keyframeEffect.targetStyleable()->keyframeEffectStack());
-        }
+        keyframeEffect.applyPendingAcceleratedActions();
     }
-
-    for (auto& effectStack : effectStacksContainingEffectThatFailedToRunAcceleratedTransformRelatedAnimation)
-        effectStack->stopAcceleratingTransformRelatedProperties(UseAcceleratedAction::No);
 }
 
 void DocumentTimeline::enqueueAnimationEvent(AnimationEventBase& event)

Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (294390 => 294391)


--- trunk/Source/WebCore/animation/KeyframeEffect.cpp	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp	2022-05-18 12:54:37 UTC (rev 294391)
@@ -549,7 +549,8 @@
 }
 
 KeyframeEffect::KeyframeEffect(Element* target, PseudoId pseudoId)
-    : m_target(target)
+    : m_keyframesName(makeAtomString("keyframe-effect-"_s, UUID::createVersion4Weak()))
+    , m_target(target)
     , m_pseudoId(pseudoId)
 {
 }
@@ -585,7 +586,7 @@
     setIterationDuration(source->iterationDuration());
     updateStaticTimingProperties();
 
-    KeyframeList keyframeList(makeAtomString("keyframe-effect-"_s, UUID::createVersion4Weak()));
+    KeyframeList keyframeList(m_keyframesName);
     keyframeList.copyKeyframes(source->m_blendingKeyframes);
     setBlendingKeyframes(keyframeList);
 }
@@ -811,7 +812,7 @@
     if (!m_blendingKeyframes.isEmpty() || !m_target)
         return;
 
-    KeyframeList keyframeList(makeAtomString("keyframe-effect-"_s, UUID::createVersion4Weak()));
+    KeyframeList keyframeList(m_keyframesName);
     auto& styleResolver = m_target->styleResolver();
 
     for (auto& keyframe : m_parsedKeyframes) {
@@ -900,6 +901,8 @@
 
 void KeyframeEffect::setBlendingKeyframes(KeyframeList& blendingKeyframes)
 {
+    CanBeAcceleratedMutationScope mutationScope(this);
+
     m_blendingKeyframes = WTFMove(blendingKeyframes);
     m_animatedProperties.clear();
 
@@ -1037,7 +1040,7 @@
     if (m_target)
         Style::loadPendingResources(*toStyle, *document(), m_target.get());
 
-    KeyframeList keyframeList(makeAtomString("keyframe-effect-"_s, UUID::createVersion4Weak()));
+    KeyframeList keyframeList(m_keyframesName);
     keyframeList.addProperty(property);
 
     KeyframeValue fromKeyframeValue(0, RenderStyle::clonePtr(*oldStyle));
@@ -1600,7 +1603,10 @@
 
 bool KeyframeEffect::preventsAcceleration() const
 {
-    return m_acceleratedPropertiesState != AcceleratedProperties::None && !canBeAccelerated();
+    if (m_acceleratedPropertiesState == AcceleratedProperties::None)
+        return false;
+
+    return !canBeAccelerated() || m_runningAccelerated == RunningAccelerated::Failed;
 }
 
 void KeyframeEffect::updateAcceleratedActions()
@@ -1609,19 +1615,8 @@
     if (!renderer || !renderer->isComposited())
         return;
 
-    if (!canBeAccelerated()) {
-        // In the case where this animation is actively targeting a transform-related property and yet
-        // cannot be accelerated, we must notify the effect stack such that any running accelerated
-        // transform-related animation targeting this element reverts to running non-accelerated.
-        if (isTargetingTransformRelatedProperty()
-            && animation()->playState() == WebAnimation::PlayState::Running
-            && getComputedTiming().phase == AnimationEffectPhase::Active) {
-            ASSERT(targetStyleable());
-            ASSERT(targetStyleable()->keyframeEffectStack());
-            targetStyleable()->keyframeEffectStack()->stopAcceleratingTransformRelatedProperties(UseAcceleratedAction::Yes);
-        }
+    if (!canBeAccelerated())
         return;
-    }
 
     auto computedTiming = getComputedTiming();
 
@@ -1656,6 +1651,9 @@
 
 void KeyframeEffect::addPendingAcceleratedAction(AcceleratedAction action)
 {
+    if (m_runningAccelerated == RunningAccelerated::Prevented || m_runningAccelerated == RunningAccelerated::Failed)
+        return;
+
     if (action == m_lastRecordedAcceleratedAction)
         return;
 
@@ -1677,9 +1675,14 @@
 {
     computeSomeKeyframesUseStepsTimingFunction();
 
-    if (isRunningAccelerated() || isAboutToRunAccelerated())
-        addPendingAcceleratedAction(canBeAccelerated() ? AcceleratedAction::UpdateTiming : AcceleratedAction::Stop);
-    else if (canBeAccelerated())
+    if (isRunningAccelerated() || isAboutToRunAccelerated()) {
+        if (canBeAccelerated())
+            addPendingAcceleratedAction(AcceleratedAction::UpdateTiming);
+        else {
+            abilityToBeAcceleratedDidChange();
+            addPendingAcceleratedAction(AcceleratedAction::Stop);
+        }
+    } else if (canBeAccelerated())
         m_runningAccelerated = RunningAccelerated::NotStarted;
 }
 
@@ -1721,9 +1724,9 @@
         addPendingAcceleratedAction(animationIsSuspended ? AcceleratedAction::Pause : AcceleratedAction::Play);
 }
 
-OptionSet<AcceleratedActionApplicationResult> KeyframeEffect::applyPendingAcceleratedActions()
+void KeyframeEffect::applyPendingAcceleratedActions()
 {
-    OptionSet<AcceleratedActionApplicationResult> result;
+    CanBeAcceleratedMutationScope mutationScope(this);
 
     // Once an accelerated animation has been committed, we no longer want to force a layout.
     // This should have been performed by a call to forceLayoutIfNeeded() prior to applying
@@ -1731,7 +1734,7 @@
     m_needsForcedLayout = false;
 
     if (m_pendingAcceleratedActions.isEmpty())
-        return result;
+        return;
 
     auto* renderer = this->renderer();
     if (!renderer || !renderer->isComposited()) {
@@ -1741,7 +1744,7 @@
             m_pendingAcceleratedActions.clear();
             m_runningAccelerated = RunningAccelerated::NotStarted;
         }
-        return result;
+        return;
     }
 
     auto pendingAcceleratedActions = m_pendingAcceleratedActions;
@@ -1751,19 +1754,19 @@
     auto timeOffset = animation()->currentTime().value_or(0_s).seconds() - delay().seconds();
 
     auto startAnimation = [&]() -> RunningAccelerated {
-        if (m_runningAccelerated == RunningAccelerated::Yes)
+        if (isRunningAccelerated())
             renderer->animationFinished(m_blendingKeyframes.animationName());
 
-        if (!m_hasImplicitKeyframeForAcceleratedProperty)
-            return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), m_blendingKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
-
         ASSERT(m_target);
         auto* effectStack = m_target->keyframeEffectStack(m_pseudoId);
         ASSERT(effectStack);
 
-        if (effectStack->containsEffectThatPreventsAccelerationOfEffect(*this))
-            return RunningAccelerated::No;
+        if (!effectStack->allowsAcceleration())
+            return RunningAccelerated::Prevented;
 
+        if (!m_hasImplicitKeyframeForAcceleratedProperty)
+            return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), m_blendingKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::Failed;
+
         // We need to resolve all animations up to this point to ensure any forward-filling
         // effect is accounted for when computing the "from" value for the accelerated animation.
         auto underlyingStyle = [&]() {
@@ -1782,7 +1785,7 @@
         KeyframeList explicitKeyframes(m_blendingKeyframes.animationName());
         explicitKeyframes.copyKeyframes(m_blendingKeyframes);
         explicitKeyframes.fillImplicitKeyframes(*this, *underlyingStyle);
-        return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), explicitKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::No;
+        return renderer->startAnimation(timeOffset, backingAnimationForCompositedRenderer(), explicitKeyframes) ? RunningAccelerated::Yes : RunningAccelerated::Failed;
     };
 
     for (const auto& action : pendingAcceleratedActions) {
@@ -1789,12 +1792,10 @@
         switch (action) {
         case AcceleratedAction::Play:
             m_runningAccelerated = startAnimation();
-            LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " Play, started accelerated: " << (m_runningAccelerated == RunningAccelerated::Yes));
-            if (m_runningAccelerated == RunningAccelerated::No) {
+            LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " Play, started accelerated: " << isRunningAccelerated());
+            if (!isRunningAccelerated()) {
                 m_lastRecordedAcceleratedAction = AcceleratedAction::Stop;
-                if (isTargetingTransformRelatedProperty())
-                    result.add(AcceleratedActionApplicationResult::TransformRelatedAnimationCannotBeAccelerated);
-                return result;
+                return;
             }
             break;
         case AcceleratedAction::Pause:
@@ -1802,7 +1803,7 @@
             break;
         case AcceleratedAction::UpdateTiming:
             m_runningAccelerated = startAnimation();
-            LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " UpdateTiming, started accelerated: " << (m_runningAccelerated == RunningAccelerated::Yes));
+            LOG_WITH_STREAM(Animations, stream << "KeyframeEffect " << this << " applyPendingAcceleratedActions " << m_blendingKeyframes.animationName() << " UpdateTiming, started accelerated: " << isRunningAccelerated());
             if (animation()->playState() == WebAnimation::PlayState::Paused)
                 renderer->animationPaused(timeOffset, m_blendingKeyframes.animationName());
             break;
@@ -1811,7 +1812,7 @@
             renderer->animationFinished(m_blendingKeyframes.animationName());
             if (!document()->renderTreeBeingDestroyed())
                 m_target->invalidateStyleAndLayerComposition();
-            m_runningAccelerated = canBeAccelerated() ? RunningAccelerated::NotStarted : RunningAccelerated::No;
+            m_runningAccelerated = canBeAccelerated() ? RunningAccelerated::NotStarted : RunningAccelerated::Prevented;
             break;
         case AcceleratedAction::TransformChange:
             renderer->transformRelatedPropertyDidChange();
@@ -1819,34 +1820,9 @@
         }
     }
 
-    if (m_runningAccelerated == RunningAccelerated::No && isTargetingTransformRelatedProperty())
-        result.add(AcceleratedActionApplicationResult::TransformRelatedAnimationCannotBeAccelerated);
-
-    return result;
+    return;
 }
 
-void KeyframeEffect::stopAcceleratingTransformRelatedProperties(UseAcceleratedAction useAcceleratedAction)
-{
-    if (!isRunningAcceleratedTransformRelatedAnimation())
-        return;
-
-    if (useAcceleratedAction == UseAcceleratedAction::Yes) {
-        addPendingAcceleratedAction(AcceleratedAction::Stop);
-        return;
-    }
-
-    auto* renderer = this->renderer();
-    if (!renderer || !renderer->isComposited())
-        return;
-
-    ASSERT(document());
-    renderer->animationFinished(m_blendingKeyframes.animationName());
-    if (!document()->renderTreeBeingDestroyed())
-        m_target->invalidateStyleAndLayerComposition();
-
-    m_runningAccelerated = RunningAccelerated::No;
-}
-
 Ref<const Animation> KeyframeEffect::backingAnimationForCompositedRenderer() const
 {
     auto effectAnimation = animation();
@@ -2137,6 +2113,7 @@
     if (m_compositeOperation == compositeOperation)
         return;
 
+    CanBeAcceleratedMutationScope mutationScope(this);
     m_compositeOperation = compositeOperation;
     invalidate();
 }
@@ -2266,4 +2243,43 @@
     }();
 }
 
+void KeyframeEffect::effectStackNoLongerPreventsAcceleration()
+{
+    if (m_runningAccelerated == RunningAccelerated::Failed)
+        return;
+
+    if (m_runningAccelerated == RunningAccelerated::Prevented)
+        m_runningAccelerated = RunningAccelerated::NotStarted;
+
+    updateAcceleratedActions();
+}
+
+void KeyframeEffect::effectStackNoLongerAllowsAcceleration()
+{
+    addPendingAcceleratedAction(AcceleratedAction::Stop);
+}
+
+void KeyframeEffect::abilityToBeAcceleratedDidChange()
+{
+    if (!m_inTargetEffectStack)
+        return;
+
+    ASSERT(m_target);
+    if (auto* effectStack = m_target->keyframeEffectStack(m_pseudoId))
+        effectStack->effectAbilityToBeAcceleratedDidChange(*this);
+}
+
+KeyframeEffect::CanBeAcceleratedMutationScope::CanBeAcceleratedMutationScope(KeyframeEffect* effect)
+    : m_effect(effect)
+{
+    ASSERT(effect);
+    m_couldOriginallyPreventAcceleration = effect->preventsAcceleration();
+}
+
+KeyframeEffect::CanBeAcceleratedMutationScope::~CanBeAcceleratedMutationScope()
+{
+    if (m_effect && m_couldOriginallyPreventAcceleration != m_effect->preventsAcceleration())
+        m_effect->abilityToBeAcceleratedDidChange();
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/animation/KeyframeEffect.h (294390 => 294391)


--- trunk/Source/WebCore/animation/KeyframeEffect.h	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/KeyframeEffect.h	2022-05-18 12:54:37 UTC (rev 294391)
@@ -127,7 +127,7 @@
     void animationTimingDidChange();
     void transformRelatedPropertyDidChange();
     void propertyAffectingKeyframeResolutionDidChange(RenderStyle&, const Style::ResolutionContext&);
-    OptionSet<AcceleratedActionApplicationResult> applyPendingAcceleratedActions();
+    void applyPendingAcceleratedActions();
 
     void willChangeRenderer();
 
@@ -172,11 +172,12 @@
     bool requiresPseudoElement() const;
     bool hasImplicitKeyframes() const;
 
-    void stopAcceleratingTransformRelatedProperties(UseAcceleratedAction);
-
     void keyframesRuleDidChange();
 
+    bool canBeAccelerated() const;
     bool preventsAcceleration() const;
+    void effectStackNoLongerPreventsAcceleration();
+    void effectStackNoLongerAllowsAcceleration();
 
     static String CSSPropertyIDToIDLAttributeName(CSSPropertyID);
 
@@ -186,8 +187,19 @@
     enum class AcceleratedAction : uint8_t { Play, Pause, UpdateTiming, TransformChange, Stop };
     enum class BlendingKeyframesSource : uint8_t { CSSAnimation, CSSTransition, WebAnimation };
     enum class AcceleratedProperties : uint8_t { None, Some, All };
-    enum class RunningAccelerated : uint8_t { NotStarted, Yes, No };
+    enum class RunningAccelerated : uint8_t { NotStarted, Yes, Prevented, Failed };
 
+    class CanBeAcceleratedMutationScope {
+        WTF_MAKE_NONCOPYABLE(CanBeAcceleratedMutationScope);
+    public:
+        CanBeAcceleratedMutationScope(KeyframeEffect*);
+        ~CanBeAcceleratedMutationScope();
+
+    private:
+        KeyframeEffect* m_effect;
+        bool m_couldOriginallyPreventAcceleration;
+    };
+
     Document* document() const;
     void updateEffectStackMembership();
     void copyPropertiesFromSource(Ref<KeyframeEffect>&&);
@@ -194,7 +206,6 @@
     void didChangeTargetStyleable(const std::optional<const Styleable>&);
     ExceptionOr<void> processKeyframes(JSC::JSGlobalObject&, Document&, JSC::Strong<JSC::JSObject>&&);
     void addPendingAcceleratedAction(AcceleratedAction);
-    bool canBeAccelerated() const;
     bool isCompletelyAccelerated() const { return m_acceleratedPropertiesState == AcceleratedProperties::All; }
     void updateAcceleratedActions();
     void setAnimatedPropertiesInStyle(RenderStyle&, double);
@@ -220,6 +231,7 @@
 #endif
     void computeHasImplicitKeyframeForAcceleratedProperty();
     void computeHasKeyframeComposingAcceleratedProperty();
+    void abilityToBeAcceleratedDidChange();
 
     // AnimationEffect
     bool isKeyframeEffect() const final { return true; }
@@ -233,6 +245,7 @@
     bool ticksContinouslyWhileActive() const final;
     std::optional<double> progressUntilNextStep(double) const final;
 
+    AtomString m_keyframesName;
     KeyframeList m_blendingKeyframes { emptyAtom() };
     HashSet<CSSPropertyID> m_animatedProperties;
     HashSet<CSSPropertyID> m_inheritedProperties;
@@ -247,7 +260,7 @@
     CompositeOperation m_compositeOperation { CompositeOperation::Replace };
     AcceleratedProperties m_acceleratedPropertiesState { AcceleratedProperties::None };
     AnimationEffectPhase m_phaseAtLastApplication { AnimationEffectPhase::Idle };
-    RunningAccelerated m_runningAccelerated { RunningAccelerated::No };
+    RunningAccelerated m_runningAccelerated { RunningAccelerated::NotStarted };
     bool m_needsForcedLayout { false };
     bool m_triggersStackingContext { false };
     size_t m_transformFunctionListsMatchPrefix { 0 };

Modified: trunk/Source/WebCore/animation/KeyframeEffectStack.cpp (294390 => 294391)


--- trunk/Source/WebCore/animation/KeyframeEffectStack.cpp	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/KeyframeEffectStack.cpp	2022-05-18 12:54:37 UTC (rev 294391)
@@ -54,12 +54,18 @@
     effect.invalidate();
     m_effects.append(effect);
     m_isSorted = false;
+
+    if (m_effects.size() > 1 && effect.preventsAcceleration())
+        stopAcceleratedAnimations();
+
     return true;
 }
 
 void KeyframeEffectStack::removeEffect(KeyframeEffect& effect)
 {
-    m_effects.removeFirst(&effect);
+    auto removedEffect = m_effects.removeFirst(&effect);
+    if (removedEffect && !m_effects.isEmpty() && !effect.canBeAccelerated())
+        startAcceleratedAnimationsIfPossible();
 }
 
 bool KeyframeEffectStack::requiresPseudoElement() const
@@ -172,12 +178,6 @@
     return impact;
 }
 
-void KeyframeEffectStack::stopAcceleratingTransformRelatedProperties(UseAcceleratedAction useAcceleratedAction)
-{
-    for (auto& effect : m_effects)
-        effect->stopAcceleratingTransformRelatedProperties(useAcceleratedAction);
-}
-
 void KeyframeEffectStack::clearInvalidCSSAnimationNames()
 {
     m_invalidCSSAnimationNames.clear();
@@ -198,18 +198,44 @@
     m_invalidCSSAnimationNames.add(name);
 }
 
-bool KeyframeEffectStack::containsEffectThatPreventsAccelerationOfEffect(const KeyframeEffect& potentiallyAcceleratedEffect)
+void KeyframeEffectStack::effectAbilityToBeAcceleratedDidChange(const KeyframeEffect& effect)
 {
-    ensureEffectsAreSorted();
+    ASSERT(m_effects.contains(&effect));
+    if (effect.preventsAcceleration())
+        stopAcceleratedAnimations();
+    else
+        startAcceleratedAnimationsIfPossible();
+}
 
+bool KeyframeEffectStack::allowsAcceleration() const
+{
+    // We could try and be a lot smarter here and do this on a per-property basis and
+    // account for fully replacing effects which could co-exist with effects that
+    // don't support acceleration lower in the stack, etc. But, if we are not able to run
+    // all effects that could support acceleration using acceleration, then we might
+    // as well not run any at all since we'll be updating effects for this stack
+    // for each animation frame. So for now, we simply return false if any effect in the
+    // stack is unable to be accelerated.
     for (auto& effect : m_effects) {
-        if (effect.get() == &potentiallyAcceleratedEffect)
-            continue;
         if (effect->preventsAcceleration())
-            return true;
+            return false;
     }
+    return true;
+}
 
-    return false;
+void KeyframeEffectStack::startAcceleratedAnimationsIfPossible()
+{
+    if (!allowsAcceleration())
+        return;
+
+    for (auto& effect : m_effects)
+        effect->effectStackNoLongerPreventsAcceleration();
 }
 
+void KeyframeEffectStack::stopAcceleratedAnimations()
+{
+    for (auto& effect : m_effects)
+        effect->effectStackNoLongerAllowsAcceleration();
+}
+
 } // namespace WebCore

Modified: trunk/Source/WebCore/animation/KeyframeEffectStack.h (294390 => 294391)


--- trunk/Source/WebCore/animation/KeyframeEffectStack.h	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/KeyframeEffectStack.h	2022-05-18 12:54:37 UTC (rev 294391)
@@ -57,10 +57,9 @@
     OptionSet<AnimationImpact> applyKeyframeEffects(RenderStyle& targetStyle, const RenderStyle* previousLastStyleChangeEventStyle, const Style::ResolutionContext&);
     bool hasEffectWithImplicitKeyframes() const;
 
-    bool containsEffectThatPreventsAccelerationOfEffect(const KeyframeEffect&);
+    void effectAbilityToBeAcceleratedDidChange(const KeyframeEffect&);
+    bool allowsAcceleration() const;
 
-    void stopAcceleratingTransformRelatedProperties(UseAcceleratedAction);
-
     void clearInvalidCSSAnimationNames();
     bool hasInvalidCSSAnimationNames() const;
     bool containsInvalidCSSAnimationName(const String&) const;
@@ -69,6 +68,9 @@
 private:
     void ensureEffectsAreSorted();
 
+    void startAcceleratedAnimationsIfPossible();
+    void stopAcceleratedAnimations();
+
     Vector<WeakPtr<KeyframeEffect>> m_effects;
     HashSet<String> m_invalidCSSAnimationNames;
     RefPtr<const AnimationList> m_cssAnimationList;

Modified: trunk/Source/WebCore/animation/WebAnimationTypes.h (294390 => 294391)


--- trunk/Source/WebCore/animation/WebAnimationTypes.h	2022-05-18 11:24:06 UTC (rev 294390)
+++ trunk/Source/WebCore/animation/WebAnimationTypes.h	2022-05-18 12:54:37 UTC (rev 294391)
@@ -55,10 +55,6 @@
     ForcesStackingContext   = 1 << 1
 };
 
-enum class AcceleratedActionApplicationResult {
-    TransformRelatedAnimationCannotBeAccelerated  = 1 << 0
-};
-
 enum class UseAcceleratedAction : uint8_t { Yes, No };
 
 using MarkableDouble = Markable<double, WebAnimationsMarkableDoubleTraits>;
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes

Reply via email to