- Revision
- 256341
- Author
- [email protected]
- Date
- 2020-02-11 12:49:00 -0800 (Tue, 11 Feb 2020)
Log Message
Cherry-pick r256181. rdar://problem/59349184
There's an event loop cycle between an animation finishing, and it being removed from GraphicsLayerCA
https://bugs.webkit.org/show_bug.cgi?id=207361
<rdar://problem/59280370>
Reviewed by Simon Fraser.
Source/WebCore:
Animations should be removed from GraphicsLayersCAs in the same rendering update that changes the playState of the
animation to "finished", to avoid layer flush where a GraphicsLayersCAs has no extent set, but thinks there is
an active transform animation.
To do this, instead of enqueuing accelerated actions when resolving animations during style resolution, in
KeyframeAnimation::apply(), we enqueue them as soon as an animation's current time may have changed: in
WebAnimation::tick() and WebAnimation::play() with supporting functions newly exposed on AnimationEffect.
Now, accelerated animations are enqueued and applied during the "update animations and send events" procedure.
* animation/AnimationEffect.h:
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::apply):
(WebCore::KeyframeEffect::updateAcceleratedActions):
(WebCore::KeyframeEffect::animationDidTick):
(WebCore::KeyframeEffect::animationDidPlay):
* animation/KeyframeEffect.h:
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::play):
(WebCore::WebAnimation::tick):
LayoutTests:
Lower the number of frames to wait after an animation completes by one to check that the accelerated animation has been removed
to show that we enqueue accelerated actions as part of the "update animations and send events" procedure.
* webanimations/accelerated-animation-removal-upon-transition-completion.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256181 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Diff
Modified: branches/safari-609.1.17.0-branch/LayoutTests/ChangeLog (256340 => 256341)
--- branches/safari-609.1.17.0-branch/LayoutTests/ChangeLog 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/LayoutTests/ChangeLog 2020-02-11 20:49:00 UTC (rev 256341)
@@ -1,5 +1,61 @@
2020-02-11 Russell Epstein <[email protected]>
+ Cherry-pick r256181. rdar://problem/59349184
+
+ There's an event loop cycle between an animation finishing, and it being removed from GraphicsLayerCA
+ https://bugs.webkit.org/show_bug.cgi?id=207361
+ <rdar://problem/59280370>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ Animations should be removed from GraphicsLayersCAs in the same rendering update that changes the playState of the
+ animation to "finished", to avoid layer flush where a GraphicsLayersCAs has no extent set, but thinks there is
+ an active transform animation.
+
+ To do this, instead of enqueuing accelerated actions when resolving animations during style resolution, in
+ KeyframeAnimation::apply(), we enqueue them as soon as an animation's current time may have changed: in
+ WebAnimation::tick() and WebAnimation::play() with supporting functions newly exposed on AnimationEffect.
+
+ Now, accelerated animations are enqueued and applied during the "update animations and send events" procedure.
+
+ * animation/AnimationEffect.h:
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::apply):
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidTick):
+ (WebCore::KeyframeEffect::animationDidPlay):
+ * animation/KeyframeEffect.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::play):
+ (WebCore::WebAnimation::tick):
+
+ LayoutTests:
+
+ Lower the number of frames to wait after an animation completes by one to check that the accelerated animation has been removed
+ to show that we enqueue accelerated actions as part of the "update animations and send events" procedure.
+
+ * webanimations/accelerated-animation-removal-upon-transition-completion.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256181 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-02-10 Antoine Quint <[email protected]>
+
+ There's an event loop cycle between an animation finishing, and it being removed from GraphicsLayerCA
+ https://bugs.webkit.org/show_bug.cgi?id=207361
+ <rdar://problem/59280370>
+
+ Reviewed by Simon Fraser.
+
+ Lower the number of frames to wait after an animation completes by one to check that the accelerated animation has been removed
+ to show that we enqueue accelerated actions as part of the "update animations and send events" procedure.
+
+ * webanimations/accelerated-animation-removal-upon-transition-completion.html: Added.
+
+2020-02-11 Russell Epstein <[email protected]>
+
Cherry-pick r256095. rdar://problem/59349202
Extent of a composited animation should not include the untransformed position
Modified: branches/safari-609.1.17.0-branch/LayoutTests/webanimations/accelerated-animation-removal-upon-transition-completion.html (256340 => 256341)
--- branches/safari-609.1.17.0-branch/LayoutTests/webanimations/accelerated-animation-removal-upon-transition-completion.html 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/LayoutTests/webanimations/accelerated-animation-removal-upon-transition-completion.html 2020-02-11 20:49:00 UTC (rev 256341)
@@ -35,7 +35,7 @@
const target = document.getElementById("target");
target.style.transform = "translate3d(100px, 0, 0)";
target.getAnimations()[0].finished.then(() => {
- waitNFrames(3, () => {
+ waitNFrames(2, () => {
assert_equals(internals.acceleratedAnimationsForElement(target).length, 0, "There should be no accelerated animation after the animation completed.");
t.done();
});
Modified: branches/safari-609.1.17.0-branch/Source/WebCore/ChangeLog (256340 => 256341)
--- branches/safari-609.1.17.0-branch/Source/WebCore/ChangeLog 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/Source/WebCore/ChangeLog 2020-02-11 20:49:00 UTC (rev 256341)
@@ -1,5 +1,77 @@
2020-02-11 Russell Epstein <[email protected]>
+ Cherry-pick r256181. rdar://problem/59349184
+
+ There's an event loop cycle between an animation finishing, and it being removed from GraphicsLayerCA
+ https://bugs.webkit.org/show_bug.cgi?id=207361
+ <rdar://problem/59280370>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ Animations should be removed from GraphicsLayersCAs in the same rendering update that changes the playState of the
+ animation to "finished", to avoid layer flush where a GraphicsLayersCAs has no extent set, but thinks there is
+ an active transform animation.
+
+ To do this, instead of enqueuing accelerated actions when resolving animations during style resolution, in
+ KeyframeAnimation::apply(), we enqueue them as soon as an animation's current time may have changed: in
+ WebAnimation::tick() and WebAnimation::play() with supporting functions newly exposed on AnimationEffect.
+
+ Now, accelerated animations are enqueued and applied during the "update animations and send events" procedure.
+
+ * animation/AnimationEffect.h:
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::apply):
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidTick):
+ (WebCore::KeyframeEffect::animationDidPlay):
+ * animation/KeyframeEffect.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::play):
+ (WebCore::WebAnimation::tick):
+
+ LayoutTests:
+
+ Lower the number of frames to wait after an animation completes by one to check that the accelerated animation has been removed
+ to show that we enqueue accelerated actions as part of the "update animations and send events" procedure.
+
+ * webanimations/accelerated-animation-removal-upon-transition-completion.html: Added.
+
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@256181 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-02-10 Antoine Quint <[email protected]>
+
+ There's an event loop cycle between an animation finishing, and it being removed from GraphicsLayerCA
+ https://bugs.webkit.org/show_bug.cgi?id=207361
+ <rdar://problem/59280370>
+
+ Reviewed by Simon Fraser.
+
+ Animations should be removed from GraphicsLayersCAs in the same rendering update that changes the playState of the
+ animation to "finished", to avoid layer flush where a GraphicsLayersCAs has no extent set, but thinks there is
+ an active transform animation.
+
+ To do this, instead of enqueuing accelerated actions when resolving animations during style resolution, in
+ KeyframeAnimation::apply(), we enqueue them as soon as an animation's current time may have changed: in
+ WebAnimation::tick() and WebAnimation::play() with supporting functions newly exposed on AnimationEffect.
+
+ Now, accelerated animations are enqueued and applied during the "update animations and send events" procedure.
+
+ * animation/AnimationEffect.h:
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::apply):
+ (WebCore::KeyframeEffect::updateAcceleratedActions):
+ (WebCore::KeyframeEffect::animationDidTick):
+ (WebCore::KeyframeEffect::animationDidPlay):
+ * animation/KeyframeEffect.h:
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::play):
+ (WebCore::WebAnimation::tick):
+
+2020-02-11 Russell Epstein <[email protected]>
+
Cherry-pick r256095. rdar://problem/59349202
Extent of a composited animation should not include the untransformed position
Modified: branches/safari-609.1.17.0-branch/Source/WebCore/animation/AnimationEffect.h (256340 => 256341)
--- branches/safari-609.1.17.0-branch/Source/WebCore/animation/AnimationEffect.h 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/Source/WebCore/animation/AnimationEffect.h 2020-02-11 20:49:00 UTC (rev 256341)
@@ -60,6 +60,8 @@
virtual void apply(RenderStyle&) = 0;
virtual void invalidate() = 0;
+ virtual void animationDidTick() = 0;
+ virtual void animationDidPlay() = 0;
virtual void animationDidSeek() = 0;
virtual void animationWasCanceled() = 0;
virtual void animationSuspensionStateDidChange(bool) = 0;
Modified: branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.cpp (256340 => 256341)
--- branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-02-11 20:49:00 UTC (rev 256341)
@@ -1084,8 +1084,6 @@
auto computedTiming = getComputedTiming();
m_phaseAtLastApplication = computedTiming.phase;
- updateAcceleratedActions(computedTiming);
-
InspectorInstrumentation::willApplyKeyframeEffect(*m_target, *this, computedTiming);
if (!computedTiming.progress)
@@ -1331,11 +1329,13 @@
return nullptr;
}
-void KeyframeEffect::updateAcceleratedActions(ComputedEffectTiming computedTiming)
+void KeyframeEffect::updateAcceleratedActions()
{
if (m_acceleratedPropertiesState == AcceleratedProperties::None)
return;
+ auto computedTiming = getComputedTiming();
+
// If we're not already running accelerated, the only thing we're interested in is whether we need to start the animation
// which we need to do once we're in the active phase. Otherwise, there's no change in accelerated state to consider.
bool isActive = computedTiming.phase == AnimationEffectPhase::Active;
@@ -1378,6 +1378,18 @@
animation()->acceleratedStateDidChange();
}
+void KeyframeEffect::animationDidTick()
+{
+ invalidate();
+ updateAcceleratedActions();
+}
+
+void KeyframeEffect::animationDidPlay()
+{
+ if (m_acceleratedPropertiesState != AcceleratedProperties::None)
+ addPendingAcceleratedAction(AcceleratedAction::Play);
+}
+
void KeyframeEffect::animationDidSeek()
{
// There is no need to seek if we're not playing an animation already. If seeking
Modified: branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.h (256340 => 256341)
--- branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.h 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/Source/WebCore/animation/KeyframeEffect.h 2020-02-11 20:49:00 UTC (rev 256341)
@@ -115,6 +115,8 @@
void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle);
void apply(RenderStyle&) override;
void invalidate() override;
+ void animationDidTick() final;
+ void animationDidPlay() final;
void animationDidSeek() final;
void animationWasCanceled() final;
void animationSuspensionStateDidChange(bool) final;
@@ -163,7 +165,7 @@
void copyPropertiesFromSource(Ref<KeyframeEffect>&&);
ExceptionOr<void> processKeyframes(JSC::JSGlobalObject&, JSC::Strong<JSC::JSObject>&&);
void addPendingAcceleratedAction(AcceleratedAction);
- void updateAcceleratedActions(ComputedEffectTiming);
+ void updateAcceleratedActions();
void setAnimatedPropertiesInStyle(RenderStyle&, double);
TimingFunction* timingFunctionForKeyframeAtIndex(size_t);
Ref<const Animation> backingAnimationForCompositedRenderer() const;
Modified: branches/safari-609.1.17.0-branch/Source/WebCore/animation/WebAnimation.cpp (256340 => 256341)
--- branches/safari-609.1.17.0-branch/Source/WebCore/animation/WebAnimation.cpp 2020-02-11 20:48:56 UTC (rev 256340)
+++ branches/safari-609.1.17.0-branch/Source/WebCore/animation/WebAnimation.cpp 2020-02-11 20:49:00 UTC (rev 256341)
@@ -974,6 +974,9 @@
invalidateEffect();
+ if (m_effect)
+ m_effect->animationDidPlay();
+
return { };
}
@@ -1189,7 +1192,8 @@
if (hasPendingPlayTask())
runPendingPlayTask();
- invalidateEffect();
+ if (!isEffectInvalidationSuspended() && m_effect)
+ m_effect->animationDidTick();
}
void WebAnimation::resolve(RenderStyle& targetStyle)
