- Revision
- 231840
- Author
- grao...@webkit.org
- Date
- 2018-05-16 01:27:54 -0700 (Wed, 16 May 2018)
Log Message
REGRESSION (r230574): Interrupted hardware transitions don't behave correctly
https://bugs.webkit.org/show_bug.cgi?id=185299
<rdar://problem/39630230>
Reviewed by Simon Fraser.
Source/WebCore:
In r230574, the fix for webkit.org/b/184518, we changed the processing order in GraphicsLayerCA::updateAnimations() to first
process m_uncomittedAnimations and then m_animationsToProcess, so we are guaranteed animations exist before we attempt to pause
or seek them. This broke interrupting and resuming hardware animations (such as an interrupted CSS Transition or an animation
running in a non-visible tab) since a pause operation recorded _before_ an animation was added would be paused anyway since
the animation was now first added, and then paused. The fix is simply to clear any pending AnimationProcessingAction for a
newly-uncommitted animation.
Test: transitions/interrupted-transition-hardware.html
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::createAnimationFromKeyframes):
(WebCore::GraphicsLayerCA::appendToUncommittedAnimations):
(WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
* platform/graphics/ca/GraphicsLayerCA.h:
(WebCore::GraphicsLayerCA::LayerPropertyAnimation::LayerPropertyAnimation):
LayoutTests:
Add a new test where we interrupt a transition and check that upon returning to the original value,
an animated value is still used and not the initial value. This test fails prior to this patch.
* transitions/interrupted-transition-hardware-expected.html: Added.
* transitions/interrupted-transition-hardware.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (231839 => 231840)
--- trunk/LayoutTests/ChangeLog 2018-05-16 07:05:27 UTC (rev 231839)
+++ trunk/LayoutTests/ChangeLog 2018-05-16 08:27:54 UTC (rev 231840)
@@ -1,3 +1,17 @@
+2018-05-16 Antoine Quint <grao...@apple.com>
+
+ REGRESSION (r230574): Interrupted hardware transitions don't behave correctly
+ https://bugs.webkit.org/show_bug.cgi?id=185299
+ <rdar://problem/39630230>
+
+ Reviewed by Simon Fraser.
+
+ Add a new test where we interrupt a transition and check that upon returning to the original value,
+ an animated value is still used and not the initial value. This test fails prior to this patch.
+
+ * transitions/interrupted-transition-hardware-expected.html: Added.
+ * transitions/interrupted-transition-hardware.html: Added.
+
2018-05-15 Commit Queue <commit-qu...@webkit.org>
Unreviewed, rolling out r231765.
Added: trunk/LayoutTests/transitions/interrupted-transition-hardware-expected.html (0 => 231840)
--- trunk/LayoutTests/transitions/interrupted-transition-hardware-expected.html (rev 0)
+++ trunk/LayoutTests/transitions/interrupted-transition-hardware-expected.html 2018-05-16 08:27:54 UTC (rev 231840)
@@ -0,0 +1,22 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+
+ div {
+ position: absolute;
+ left: 0;
+ top: 0;
+ height: 100px;
+ width: 100px;
+ transform: translateX(25px);
+ background-color: green;
+ }
+
+ </style>
+</head>
+<body>
+ <div id="cover"></div>
+</body>
+</html>
Added: trunk/LayoutTests/transitions/interrupted-transition-hardware.html (0 => 231840)
--- trunk/LayoutTests/transitions/interrupted-transition-hardware.html (rev 0)
+++ trunk/LayoutTests/transitions/interrupted-transition-hardware.html 2018-05-16 08:27:54 UTC (rev 231840)
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+
+<html>
+<head>
+ <style>
+
+ div {
+ position: absolute;
+ left: 0;
+ top: 0;
+ height: 100px;
+ width: 100px;
+ }
+
+ #test {
+ background-color: red;
+ transition: transform calc(24 * 60 * 60s) linear;
+ transition-delay: calc(-12 * 60 * 60s);
+ transform: none;
+ }
+
+ #test.transitions {
+ transform: translateX(100px);
+ }
+
+ #cover {
+ transform: translateX(25px);
+ background-color: green;
+ }
+
+ </style>
+</head>
+<body>
+
+ <div id="test"></div>
+ <div id="cover"></div>
+
+ <script type="text/_javascript_">
+
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ const test = document.getElementById("test");
+
+ requestAnimationFrame(() => {
+ test.classList.add("transitions");
+ requestAnimationFrame(() => {
+ test.classList.remove("transitions");
+ requestAnimationFrame(() => {
+ requestAnimationFrame(() => {
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ });
+ });
+ });
+
+ </script>
+
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (231839 => 231840)
--- trunk/Source/WebCore/ChangeLog 2018-05-16 07:05:27 UTC (rev 231839)
+++ trunk/Source/WebCore/ChangeLog 2018-05-16 08:27:54 UTC (rev 231840)
@@ -1,3 +1,27 @@
+2018-05-16 Antoine Quint <grao...@apple.com>
+
+ REGRESSION (r230574): Interrupted hardware transitions don't behave correctly
+ https://bugs.webkit.org/show_bug.cgi?id=185299
+ <rdar://problem/39630230>
+
+ Reviewed by Simon Fraser.
+
+ In r230574, the fix for webkit.org/b/184518, we changed the processing order in GraphicsLayerCA::updateAnimations() to first
+ process m_uncomittedAnimations and then m_animationsToProcess, so we are guaranteed animations exist before we attempt to pause
+ or seek them. This broke interrupting and resuming hardware animations (such as an interrupted CSS Transition or an animation
+ running in a non-visible tab) since a pause operation recorded _before_ an animation was added would be paused anyway since
+ the animation was now first added, and then paused. The fix is simply to clear any pending AnimationProcessingAction for a
+ newly-uncommitted animation.
+
+ Test: transitions/interrupted-transition-hardware.html
+
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::createAnimationFromKeyframes):
+ (WebCore::GraphicsLayerCA::appendToUncommittedAnimations):
+ (WebCore::GraphicsLayerCA::createTransformAnimationsFromKeyframes):
+ * platform/graphics/ca/GraphicsLayerCA.h:
+ (WebCore::GraphicsLayerCA::LayerPropertyAnimation::LayerPropertyAnimation):
+
2018-05-15 Yusuke Suzuki <utatane....@gmail.com>
[JSC] Check TypeInfo first before calling getCallData when we would like to check whether given object is a function
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (231839 => 231840)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2018-05-16 07:05:27 UTC (rev 231839)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2018-05-16 08:27:54 UTC (rev 231840)
@@ -3015,7 +3015,7 @@
if (!valuesOK)
return false;
- m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
+ appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
return true;
}
@@ -3042,7 +3042,7 @@
if (!validMatrices)
return false;
- m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
+ appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, 0, timeOffset));
return true;
}
@@ -3104,12 +3104,21 @@
ASSERT(valuesOK);
- m_uncomittedAnimations.append(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, internalFilterPropertyIndex, timeOffset));
+ appendToUncommittedAnimations(LayerPropertyAnimation(caAnimation.releaseNonNull(), animationName, valueList.property(), animationIndex, internalFilterPropertyIndex, timeOffset));
}
return true;
}
+void GraphicsLayerCA::appendToUncommittedAnimations(LayerPropertyAnimation&& animation)
+{
+ // Since we're adding a new animation, make sure we clear any pending AnimationProcessingAction for this animation
+ // as these are applied after we've committed new animations.
+ m_animationsToProcess.remove(animation.m_name);
+
+ m_uncomittedAnimations.append(WTFMove(animation));
+}
+
bool GraphicsLayerCA::createFilterAnimationsFromKeyframes(const KeyframeValueList& valueList, const Animation* animation, const String& animationName, Seconds timeOffset)
{
#if ENABLE(FILTERS_LEVEL_2)
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h (231839 => 231840)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2018-05-16 07:05:27 UTC (rev 231839)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.h 2018-05-16 08:27:54 UTC (rev 231840)
@@ -459,8 +459,29 @@
moveOrCopyAnimations(Copy, fromLayer, toLayer);
}
+ // This represents the animation of a single property. There may be multiple transform animations for
+ // a single transition or keyframe animation, so index is used to distinguish these.
+ struct LayerPropertyAnimation {
+ LayerPropertyAnimation(Ref<PlatformCAAnimation>&& caAnimation, const String& animationName, AnimatedPropertyID property, int index, int subIndex, Seconds timeOffset)
+ : m_animation(WTFMove(caAnimation))
+ , m_name(animationName)
+ , m_property(property)
+ , m_index(index)
+ , m_subIndex(subIndex)
+ , m_timeOffset(timeOffset)
+ { }
+
+ RefPtr<PlatformCAAnimation> m_animation;
+ String m_name;
+ AnimatedPropertyID m_property;
+ int m_index;
+ int m_subIndex;
+ Seconds m_timeOffset;
+ };
+
bool appendToUncommittedAnimations(const KeyframeValueList&, const TransformOperations*, const Animation*, const String& animationName, const FloatSize& boxSize, int animationIndex, Seconds timeOffset, bool isMatrixAnimation);
bool appendToUncommittedAnimations(const KeyframeValueList&, const FilterOperation*, const Animation*, const String& animationName, int animationIndex, Seconds timeOffset);
+ void appendToUncommittedAnimations(LayerPropertyAnimation&&);
enum LayerChange : uint64_t {
NoChange = 0,
@@ -572,26 +593,6 @@
RetainPtr<CGImageRef> m_uncorrectedContentsImage;
RetainPtr<CGImageRef> m_pendingContentsImage;
- // This represents the animation of a single property. There may be multiple transform animations for
- // a single transition or keyframe animation, so index is used to distinguish these.
- struct LayerPropertyAnimation {
- LayerPropertyAnimation(Ref<PlatformCAAnimation>&& caAnimation, const String& animationName, AnimatedPropertyID property, int index, int subIndex, Seconds timeOffset)
- : m_animation(WTFMove(caAnimation))
- , m_name(animationName)
- , m_property(property)
- , m_index(index)
- , m_subIndex(subIndex)
- , m_timeOffset(timeOffset)
- { }
-
- RefPtr<PlatformCAAnimation> m_animation;
- String m_name;
- AnimatedPropertyID m_property;
- int m_index;
- int m_subIndex;
- Seconds m_timeOffset;
- };
-
// Uncommitted transitions and animations.
Vector<LayerPropertyAnimation> m_uncomittedAnimations;
