Skip to content

Commit

Permalink
Cherry-pick 262875@main (d566d00). https://bugs.webkit.org/show_bug.c…
Browse files Browse the repository at this point in the history
…gi?id=255338

    REGRESSION (260399@main): animations flicker on https://payto.com.au
    https://bugs.webkit.org/show_bug.cgi?id=255338
    rdar://107532064

    Reviewed by Dean Jackson.

    When creating a new animation of any type (CSS Transition, CSS Animation, Web Animations API)
    that is accelerated, we add it to the list of animations on GraphicsLayerCA regardless of its
    composite order relative to other effects for the given target.

    In the case of https://payto.com.au, a CSS Animation is applied to an element for the "transform"
    property, and that property also yields a CSS Transition that is canceled and recreated on each
    frame (whether this is the right behavior is discussed in w3c/csswg-drafts#8701
    as Firefox, Chrome and Safari all have different behavior).

    That perpetually-recreated CSS Transition is lower in the composite order, but since it's created
    after the CSS Animation, due to how we create accelerated animations it would override the CSS Animation.

    In this patch we change the behavior of DocumentTimeline::applyPendingAcceleratedAnimations() to
    update the entire effect stack with which an effect pending application of an accelerated action
    is associated. This guarantees the effect stack's order to be preserved.

    We also ensure we remove any similarly-named animation before adding new animations in GraphicsLayerCA.

    * LayoutTests/webanimations/accelerated-animation-addition-lower-in-effect-stack-expected.html: Added.
    * LayoutTests/webanimations/accelerated-animation-addition-lower-in-effect-stack.html: Added.
    * Source/WebCore/animation/DocumentTimeline.cpp:
    (WebCore::DocumentTimeline::applyPendingAcceleratedAnimations):
    * Source/WebCore/animation/KeyframeEffect.cpp:
    (WebCore::KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties):
    * Source/WebCore/animation/KeyframeEffect.h:
    * Source/WebCore/animation/KeyframeEffectStack.cpp:
    (WebCore::KeyframeEffectStack::applyPendingAcceleratedActions const):
    * Source/WebCore/animation/KeyframeEffectStack.h:
    * Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp:
    (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
    (WebCore::GraphicsLayerCA::createFilterAnimationsFromKeyframes):

    Canonical link: https://commits.webkit.org/262875@main
  • Loading branch information
graouts authored and aperezdc committed Apr 24, 2023
1 parent 558c551 commit 4e304ac
Show file tree
Hide file tree
Showing 8 changed files with 111 additions and 1 deletion.
@@ -0,0 +1,14 @@
<style>

div {
position: absolute;
top: 0;
left: 0;
width: 100px;
height: 100px;
background-color: black;
translate: 100px;
}

</style>
<div></div>
@@ -0,0 +1,50 @@
<style>

div {
position: absolute;
top: 0;
left: 0;
width: 100px;
height: 100px;
background-color: black;
}

div.animated {
animation: translate 1000s linear forwards;
}

@keyframes translate {
from { translate: 0px }
to { translate: 0px }
}

</style>
<div></div>
<script>

window.testRunner?.waitUntilDone();

(async function () {
const target = document.querySelector("div");

// Start an animation which will remain the highest in composite order.
const animation = target.animate({ "translate": ["100px", "100px"] }, { duration: 1000 * 1000, fill: "forwards" })

// Wait until the animation is committed.
await animation.ready;
await new Promise(requestAnimationFrame);
await new Promise(requestAnimationFrame);
await new Promise(requestAnimationFrame);

// Start a CSS Animation which will be completely replaced by the script-originated animation.
target.classList.add("animated");

// Wait until the animation is committed.
await new Promise(requestAnimationFrame);
await new Promise(requestAnimationFrame);
await new Promise(requestAnimationFrame);

window.testRunner?.notifyDone();
})();

</script>
20 changes: 19 additions & 1 deletion Source/WebCore/animation/DocumentTimeline.cpp
Expand Up @@ -412,14 +412,32 @@ void DocumentTimeline::applyPendingAcceleratedAnimations()
auto acceleratedAnimationsPendingRunningStateChange = m_acceleratedAnimationsPendingRunningStateChange;
m_acceleratedAnimationsPendingRunningStateChange.clear();

HashSet<KeyframeEffectStack*> keyframeEffectStacksToUpdate;

bool hasForcedLayout = false;
for (auto& animation : acceleratedAnimationsPendingRunningStateChange) {
if (auto* keyframeEffect = dynamicDowncast<KeyframeEffect>(animation->effect())) {
if (!hasForcedLayout)
hasForcedLayout |= keyframeEffect->forceLayoutIfNeeded();
keyframeEffect->applyPendingAcceleratedActions();

// If the animation is no longer relevant, apply the pending accelerations in isolation.
if (!animation->isRelevant()) {
keyframeEffect->applyPendingAcceleratedActions();
continue;
}

// Otherwise, this accelerated animation exists in the context of an effect stack and we must
// update all effects in that stack to ensure their sort order is respected.
if (auto target = keyframeEffect->targetStyleable()) {
auto* keyframeEffectStack = target ? target->keyframeEffectStack() : nullptr;
if (keyframeEffectStack)
keyframeEffectStacksToUpdate.add(keyframeEffectStack);
}
}
}

for (auto* keyframeEffectStack : keyframeEffectStacksToUpdate)
keyframeEffectStack->applyPendingAcceleratedActions();
}

void DocumentTimeline::enqueueAnimationEvent(AnimationEventBase& event)
Expand Down
15 changes: 15 additions & 0 deletions Source/WebCore/animation/KeyframeEffect.cpp
Expand Up @@ -1868,6 +1868,21 @@ void KeyframeEffect::animationSuspensionStateDidChange(bool animationIsSuspended
addPendingAcceleratedAction(animationIsSuspended ? AcceleratedAction::Pause : AcceleratedAction::Play);
}

void KeyframeEffect::applyPendingAcceleratedActionsOrUpdateTimingProperties()
{
#if ENABLE(THREADED_ANIMATION_RESOLUTION)
if (threadedAnimationResolutionEnabled())
return;
#endif

if (m_pendingAcceleratedActions.isEmpty()) {
m_pendingAcceleratedActions.append(AcceleratedAction::UpdateProperties);
applyPendingAcceleratedActions();
m_pendingAcceleratedActions.clear();
} else
applyPendingAcceleratedActions();
}

void KeyframeEffect::applyPendingAcceleratedActions()
{
CanBeAcceleratedMutationScope mutationScope(this);
Expand Down
1 change: 1 addition & 0 deletions Source/WebCore/animation/KeyframeEffect.h
Expand Up @@ -129,6 +129,7 @@ class KeyframeEffect final : public AnimationEffect, public CSSPropertyBlendingC
enum class RecomputationReason : uint8_t { LogicalPropertyChange, Other };
std::optional<RecomputationReason> recomputeKeyframesIfNecessary(const RenderStyle* previousUnanimatedStyle, const RenderStyle& unanimatedStyle, const Style::ResolutionContext&);
void applyPendingAcceleratedActions();
void applyPendingAcceleratedActionsOrUpdateTimingProperties();

void willChangeRenderer();

Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/animation/KeyframeEffectStack.cpp
Expand Up @@ -268,5 +268,10 @@ void KeyframeEffectStack::didApplyCascade(const RenderStyle& styleBeforeCascade,
effect->acceleratedPropertiesOverriddenByCascadeDidChange();
}

void KeyframeEffectStack::applyPendingAcceleratedActions() const
{
for (auto& effect : m_effects)
effect->applyPendingAcceleratedActionsOrUpdateTimingProperties();
}

} // namespace WebCore
2 changes: 2 additions & 0 deletions Source/WebCore/animation/KeyframeEffectStack.h
Expand Up @@ -72,6 +72,8 @@ class KeyframeEffectStack {

const HashSet<AnimatableProperty>& acceleratedPropertiesOverriddenByCascade() const { return m_acceleratedPropertiesOverriddenByCascade; }

void applyPendingAcceleratedActions() const;

private:
void ensureEffectsAreSorted();
bool hasMatchingEffect(const Function<bool(const KeyframeEffect&)>&) const;
Expand Down
5 changes: 5 additions & 0 deletions Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp
Expand Up @@ -3480,6 +3480,9 @@ bool GraphicsLayerCA::createTransformAnimationsFromKeyframes(const KeyframeValue

const auto& primitives = prefix.primitives();
unsigned numberOfSharedPrimitives = valueList.size() > 1 ? primitives.size() : 0;

removeAnimation(animationName);

for (unsigned animationIndex = 0; animationIndex < numberOfSharedPrimitives; ++animationIndex) {
if (!appendToUncommittedAnimations(valueList, primitives[animationIndex], animation, animationName, boxSize, animationIndex, timeOffset, false /* isMatrixAnimation */, keyframesShouldUseAnimationWideTimingFunction))
return false;
Expand Down Expand Up @@ -3543,6 +3546,8 @@ bool GraphicsLayerCA::createFilterAnimationsFromKeyframes(const KeyframeValueLis
return false;
}

removeAnimation(animationName);

for (int animationIndex = 0; animationIndex < numAnimations; ++animationIndex) {
if (!appendToUncommittedAnimations(valueList, operations.operations().at(animationIndex).get(), animation, animationName, animationIndex, timeOffset, keyframesShouldUseAnimationWideTimingFunction))
return false;
Expand Down

0 comments on commit 4e304ac

Please sign in to comment.