- Revision
- 256050
- Author
- repst...@apple.com
- Date
- 2020-02-07 11:35:23 -0800 (Fri, 07 Feb 2020)
Log Message
Cherry-pick r255422. rdar://problem/59264302
[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.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@255422 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-610.1.1.4-branch/LayoutTests/ChangeLog (256049 => 256050)
--- branches/safari-610.1.1.4-branch/LayoutTests/ChangeLog 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/LayoutTests/ChangeLog 2020-02-07 19:35:23 UTC (rev 256050)
@@ -1,5 +1,70 @@
2020-02-07 Russell Epstein <repst...@apple.com>
+ Cherry-pick r255422. rdar://problem/59264302
+
+ [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.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@255422 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-02-07 Russell Epstein <repst...@apple.com>
+
Cherry-pick r255396. rdar://problem/59264302
Web Inspector: add instrumentation for showing existing Web Animations
Modified: branches/safari-610.1.1.4-branch/LayoutTests/platform/win/TestExpectations (256049 => 256050)
--- branches/safari-610.1.1.4-branch/LayoutTests/platform/win/TestExpectations 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/LayoutTests/platform/win/TestExpectations 2020-02-07 19:35:23 UTC (rev 256050)
@@ -4519,3 +4519,6 @@
webkit.org/b/205855 http/wpt/css/css-highlight-api/highlight-text.html [ ImageOnlyFailure ]
webkit.org/b/205856 storage/indexeddb/IDBTransaction-page-cache.html [ Pass Timeout ]
+
+webkit.org/b/206995 webanimations/seeking-by-changing-delay-accelerated.html [ Failure ]
+
Added: branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html (0 => 256050)
--- branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html (rev 0)
+++ branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated-expected.html 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html (0 => 256050)
--- branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html (rev 0)
+++ branches/safari-610.1.1.4-branch/LayoutTests/webanimations/seeking-by-changing-delay-accelerated.html 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/Source/WebCore/ChangeLog (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/ChangeLog 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/ChangeLog 2020-02-07 19:35:23 UTC (rev 256050)
@@ -1,5 +1,90 @@
2020-02-07 Russell Epstein <repst...@apple.com>
+ Cherry-pick r255422. rdar://problem/59264302
+
+ [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.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@255422 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 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-02-07 Russell Epstein <repst...@apple.com>
+
Cherry-pick r255396. rdar://problem/59264302
Web Inspector: add instrumentation for showing existing Web Animations
Modified: branches/safari-610.1.1.4-branch/Source/WebCore/animation/AnimationEffect.cpp (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/animation/AnimationEffect.cpp 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/animation/AnimationEffect.cpp 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/Source/WebCore/animation/CSSAnimation.cpp (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/animation/CSSAnimation.cpp 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/animation/CSSAnimation.cpp 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/Source/WebCore/animation/KeyframeEffect.cpp (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/animation/KeyframeEffect.cpp 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.cpp (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.cpp 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.cpp 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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: branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.h (256049 => 256050)
--- branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.h 2020-02-07 19:35:12 UTC (rev 256049)
+++ branches/safari-610.1.1.4-branch/Source/WebCore/animation/WebAnimation.h 2020-02-07 19:35:23 UTC (rev 256050)
@@ -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);
