- Revision
- 255422
- Author
- grao...@webkit.org
- Date
- 2020-01-30 05:47:58 -0800 (Thu, 30 Jan 2020)
Log Message
[Web Animations] Changing the delay of an accelerated animation doesn't seek the animation
https://bugs.webkit.org/show_bug.cgi?id=206990
<rdar://problem/58675608>
Reviewed by Antti Koivisto.
Source/WebCore:
Test: webanimations/seeking-by-changing-delay-accelerated.html
In order to seek an accelerated animation, we need to update the animation on the target element's backing GraphicsLayer. We do this by enqueuing an
AcceleratedAction:Seek command which is done by calling KeyframeEffect::animationDidSeek(), which we would only call from WebAnimation::setCurrentTime().
However, seeking can be performed by modifying the animation's effect's timing.
We now call WebAnimation::effectTimingDidChange() with an optional ComputedEffectTiming for call sites that want to provide timing data prior to
modifying timing properties. This allows WebAnimation::effectTimingDidChange() to compare the previous progress with the new progress to determine if the
animation was seeked, so KeyframeEffect::animationDidSeek() may be called.
There are two places where we now call WebAnimation::effectTimingDidChange() with the previous timing data. First, when updateTiming() is called
through the _javascript_ API (AnimationEffect::updateTiming) and when a CSS Animation's timing has been modified by changing some of the animation CSS
properties (CSSAnimation::syncPropertiesWithBackingAnimation).
* animation/AnimationEffect.cpp:
(WebCore::AnimationEffect::updateTiming): Compute the previous timing data and provide it to WebAnimation::effectTimingDidChange().
* animation/CSSAnimation.cpp:
(WebCore::CSSAnimation::syncPropertiesWithBackingAnimation): Compute the previous timing data and provide it to WebAnimation::effectTimingDidChange().
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::computeAcceleratedPropertiesState): Drive-by fix for faulty logic introduced in a recent patch (r255383).
(WebCore::KeyframeEffect::applyPendingAcceleratedActions): We need to reset the m_isRunningAccelerated flag when an animation was supposed to be stopped but
couldn't be because the target's layer backing was removed prior to the accelerated action being committed.
* animation/WebAnimation.cpp:
(WebCore::WebAnimation::effectTimingDidChange): If previous timing data was provided, check whether its progress differs from the current timing data and
call KeyframeEffect::animationDidSeek().
* animation/WebAnimation.h:
LayoutTests:
Add a new test which would fail prior to this patch where we pause an animation after it has started playing accelerated and
change its delay to check that it correctly seeks the animation.
* webanimations/seeking-by-changing-delay-accelerated-expected.html: Added.
* webanimations/seeking-by-changing-delay-accelerated.html: Added.
* platform/win/TestExpectations: Mark the new test as failing.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (255421 => 255422)
--- trunk/LayoutTests/ChangeLog 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/LayoutTests/ChangeLog 2020-01-30 13:47:58 UTC (rev 255422)
@@ -1,3 +1,18 @@
+2020-01-30 Antoine Quint <grao...@apple.com>
+
+ [Web Animations] Changing the delay of an accelerated animation doesn't seek the animation
+ https://bugs.webkit.org/show_bug.cgi?id=206990
+ <rdar://problem/58675608>
+
+ Reviewed by Antti Koivisto.
+
+ Add a new test which would fail prior to this patch where we pause an animation after it has started playing accelerated and
+ change its delay to check that it correctly seeks the animation.
+
+ * webanimations/seeking-by-changing-delay-accelerated-expected.html: Added.
+ * webanimations/seeking-by-changing-delay-accelerated.html: Added.
+ * platform/win/TestExpectations: Mark the new test as failing.
+
2020-01-30 Carlos Garcia Campos <cgar...@igalia.com>
REGRESSION(r253636): [GTK] Mouse cursor changes using onMouseXYZ are erratic
Modified: trunk/LayoutTests/platform/win/TestExpectations (255421 => 255422)
--- trunk/LayoutTests/platform/win/TestExpectations 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/LayoutTests/platform/win/TestExpectations 2020-01-30 13:47:58 UTC (rev 255422)
@@ -4528,3 +4528,4 @@
webkit.org/b/206165 fast/dom/Range/getBoundingClientRect.html [ Failure ]
+webkit.org/b/206995 webanimations/seeking-by-changing-delay-accelerated.html [ Failure ]
Added: trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html (0 => 255422)
--- trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html (rev 0)
+++ trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html 2020-01-30 13:47:58 UTC (rev 255422)
@@ -0,0 +1,21 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+
+#target {
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ transform: translateX(50px);
+}
+
+</style>
+</head>
+<body>
+<div id="target"></div>
+</body>
+</html>
Added: trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html (0 => 255422)
--- trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html (rev 0)
+++ trunk/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html 2020-01-30 13:47:58 UTC (rev 255422)
@@ -0,0 +1,48 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+
+#target {
+ position: absolute;
+ left: 0;
+ top: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+}
+
+</style>
+</head>
+<body>
+<div id="target"></div>
+<script>
+
+if (window.testRunner)
+ testRunner.waitUntilDone();
+
+const animation = document.getElementById("target").animate({ transform: ["translateX(0)", "translateX(100px)"] }, 1000);
+
+animation.ready.then(() => {
+ // Ensure accelerated animations have been commmitted by waiting for the next frame.
+ requestAnimationFrame(() => {
+ // We pause the animation right away.
+ animation.pause();
+
+ // Then on the next frame, we change its delay to seek it to its mid-point.
+ requestAnimationFrame(() => {
+ const effect = animation.effect;
+ const duration = effect.getTiming().duration;
+ const delay = animation.currentTime - 0.5 * duration;
+ animation.effect.updateTiming({ delay });
+
+ // Then wait another frame to ensure the seek was committed.
+ if (window.testRunner)
+ requestAnimationFrame(() => testRunner.notifyDone());
+ });
+ });
+});
+
+</script>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (255421 => 255422)
--- trunk/Source/WebCore/ChangeLog 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/ChangeLog 2020-01-30 13:47:58 UTC (rev 255422)
@@ -1,3 +1,38 @@
+2020-01-30 Antoine Quint <grao...@apple.com>
+
+ [Web Animations] Changing the delay of an accelerated animation doesn't seek the animation
+ https://bugs.webkit.org/show_bug.cgi?id=206990
+ <rdar://problem/58675608>
+
+ Reviewed by Antti Koivisto.
+
+ Test: webanimations/seeking-by-changing-delay-accelerated.html
+
+ In order to seek an accelerated animation, we need to update the animation on the target element's backing GraphicsLayer. We do this by enqueuing an
+ AcceleratedAction:Seek command which is done by calling KeyframeEffect::animationDidSeek(), which we would only call from WebAnimation::setCurrentTime().
+ However, seeking can be performed by modifying the animation's effect's timing.
+
+ We now call WebAnimation::effectTimingDidChange() with an optional ComputedEffectTiming for call sites that want to provide timing data prior to
+ modifying timing properties. This allows WebAnimation::effectTimingDidChange() to compare the previous progress with the new progress to determine if the
+ animation was seeked, so KeyframeEffect::animationDidSeek() may be called.
+
+ There are two places where we now call WebAnimation::effectTimingDidChange() with the previous timing data. First, when updateTiming() is called
+ through the _javascript_ API (AnimationEffect::updateTiming) and when a CSS Animation's timing has been modified by changing some of the animation CSS
+ properties (CSSAnimation::syncPropertiesWithBackingAnimation).
+
+ * animation/AnimationEffect.cpp:
+ (WebCore::AnimationEffect::updateTiming): Compute the previous timing data and provide it to WebAnimation::effectTimingDidChange().
+ * animation/CSSAnimation.cpp:
+ (WebCore::CSSAnimation::syncPropertiesWithBackingAnimation): Compute the previous timing data and provide it to WebAnimation::effectTimingDidChange().
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::computeAcceleratedPropertiesState): Drive-by fix for faulty logic introduced in a recent patch (r255383).
+ (WebCore::KeyframeEffect::applyPendingAcceleratedActions): We need to reset the m_isRunningAccelerated flag when an animation was supposed to be stopped but
+ couldn't be because the target's layer backing was removed prior to the accelerated action being committed.
+ * animation/WebAnimation.cpp:
+ (WebCore::WebAnimation::effectTimingDidChange): If previous timing data was provided, check whether its progress differs from the current timing data and
+ call KeyframeEffect::animationDidSeek().
+ * animation/WebAnimation.h:
+
2020-01-30 Noam Rosenthal <n...@webkit.org>
REGRESSION (r254406): Gmail.com star/favorite icons are not rendering
Modified: trunk/Source/WebCore/animation/AnimationEffect.cpp (255421 => 255422)
--- trunk/Source/WebCore/animation/AnimationEffect.cpp 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/animation/AnimationEffect.cpp 2020-01-30 13:47:58 UTC (rev 255422)
@@ -345,6 +345,10 @@
if (!timing)
return { };
+ Optional<ComputedEffectTiming> previousTiming;
+ if (m_animation)
+ previousTiming = getComputedTiming();
+
// 1. If the iterationStart member of input is present and less than zero, throw a TypeError and abort this procedure.
if (timing->iterationStart) {
if (timing->iterationStart.value() < 0)
@@ -413,7 +417,7 @@
updateStaticTimingProperties();
if (m_animation)
- m_animation->effectTimingDidChange();
+ m_animation->effectTimingDidChange(previousTiming);
return { };
}
Modified: trunk/Source/WebCore/animation/CSSAnimation.cpp (255421 => 255422)
--- trunk/Source/WebCore/animation/CSSAnimation.cpp 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/animation/CSSAnimation.cpp 2020-01-30 13:47:58 UTC (rev 255422)
@@ -65,6 +65,8 @@
auto& animation = backingAnimation();
auto* animationEffect = effect();
+ auto previousTiming = animationEffect->getComputedTiming();
+
switch (animation.fillMode()) {
case AnimationFillMode::None:
animationEffect->setFill(FillMode::None);
@@ -101,7 +103,7 @@
animationEffect->setDelay(Seconds(animation.delay()));
animationEffect->setIterationDuration(Seconds(animation.duration()));
animationEffect->updateStaticTimingProperties();
- effectTimingDidChange();
+ effectTimingDidChange(previousTiming);
// Synchronize the play state
if (animation.playState() == AnimationPlayState::Playing && playState() == WebAnimation::PlayState::Paused) {
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (255421 => 255422)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2020-01-30 13:47:58 UTC (rev 255422)
@@ -1127,7 +1127,7 @@
break;
}
- if (!hasSomeAcceleratedProperties && !hasSomeUnacceleratedProperties)
+ if (!hasSomeAcceleratedProperties)
m_acceleratedPropertiesState = AcceleratedProperties::None;
else if (hasSomeUnacceleratedProperties)
m_acceleratedPropertiesState = AcceleratedProperties::Some;
@@ -1410,8 +1410,10 @@
if (!renderer || !renderer->isComposited()) {
// The renderer may no longer be composited because the accelerated animation ended before we had a chance to update it,
// in which case if we asked for the animation to stop, we can discard the current set of accelerated actions.
- if (m_lastRecordedAcceleratedAction == AcceleratedAction::Stop)
+ if (m_lastRecordedAcceleratedAction == AcceleratedAction::Stop) {
m_pendingAcceleratedActions.clear();
+ m_isRunningAccelerated = false;
+ }
return;
}
Modified: trunk/Source/WebCore/animation/WebAnimation.cpp (255421 => 255422)
--- trunk/Source/WebCore/animation/WebAnimation.cpp 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/animation/WebAnimation.cpp 2020-01-30 13:47:58 UTC (rev 255422)
@@ -144,11 +144,19 @@
--m_suspendCount;
}
-void WebAnimation::effectTimingDidChange()
+void WebAnimation::effectTimingDidChange(Optional<ComputedEffectTiming> previousTiming)
{
timingDidChange(DidSeek::No, SynchronouslyNotify::Yes);
InspectorInstrumentation::didChangeWebAnimationEffectTiming(*this);
+
+ if (!previousTiming)
+ return;
+
+ auto* effect = this->effect();
+ ASSERT(effect);
+ if (previousTiming->progress != effect->getComputedTiming().progress)
+ effect->animationDidSeek();
}
void WebAnimation::setEffect(RefPtr<AnimationEffect>&& newEffect)
Modified: trunk/Source/WebCore/animation/WebAnimation.h (255421 => 255422)
--- trunk/Source/WebCore/animation/WebAnimation.h 2020-01-30 13:27:46 UTC (rev 255421)
+++ trunk/Source/WebCore/animation/WebAnimation.h 2020-01-30 13:47:58 UTC (rev 255422)
@@ -26,6 +26,7 @@
#pragma once
#include "ActiveDOMObject.h"
+#include "ComputedEffectTiming.h"
#include "EventTarget.h"
#include "ExceptionOr.h"
#include "IDLTypes.h"
@@ -32,6 +33,7 @@
#include "WebAnimationUtilities.h"
#include <wtf/Forward.h>
#include <wtf/Markable.h>
+#include <wtf/Optional.h>
#include <wtf/RefCounted.h>
#include <wtf/Seconds.h>
#include <wtf/UniqueRef.h>
@@ -126,7 +128,7 @@
bool isCompletelyAccelerated() const;
bool isRelevant() const { return m_isRelevant; }
void updateRelevance();
- void effectTimingDidChange();
+ void effectTimingDidChange(Optional<ComputedEffectTiming> = WTF::nullopt);
void suspendEffectInvalidation();
void unsuspendEffectInvalidation();
void setSuspended(bool);
