- Revision
- 268802
- Author
- alanc...@apple.com
- Date
- 2020-10-21 09:54:53 -0700 (Wed, 21 Oct 2020)
Log Message
Cherry-pick r268771. rdar://problem/70532973
REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
https://bugs.webkit.org/show_bug.cgi?id=217987
<rdar://problem/70418548>
Reviewed by Simon Fraser.
Source/WebCore:
When several animations targetting the same property and the same layer are overlapping, we used to
always override the previous animations. With r268483 we started maintaining all active animations
and let them run, potentially with additivity if the animation could be broken into several animations
each targeting a given transform operation.
On top of that, with r268615 and the support for accelerated animation of individual CSS transform
properties (translate, scale and rotate), all transform-related animations were made additive.
This meant that we would always run active animations targeting "transform" in a way where they would be
additive rather than being replaced.
Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
animation set will always have a 0 index. So now, when we compile a list of transform animations in
GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
ensuring only the top-most transform animation is applied.
We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
remove all animations with the provided name.
Test: webanimations/accelerated-overlapping-transform-animations.html
* platform/graphics/ca/GraphicsLayerCA.cpp:
(WebCore::GraphicsLayerCA::pauseAnimation):
(WebCore::GraphicsLayerCA::removeAnimation):
(WebCore::GraphicsLayerCA::updateAnimations):
LayoutTests:
Add a new test that checks that only the last of two overlapping "transform" animations is applied.
* platform/mac-wk1/TestExpectations:
* webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
* webanimations/accelerated-overlapping-transform-animations.html: Added.
git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
Modified Paths
Added Paths
Diff
Modified: branches/safari-611.1.4-branch/LayoutTests/ChangeLog (268801 => 268802)
--- branches/safari-611.1.4-branch/LayoutTests/ChangeLog 2020-10-21 16:40:44 UTC (rev 268801)
+++ branches/safari-611.1.4-branch/LayoutTests/ChangeLog 2020-10-21 16:54:53 UTC (rev 268802)
@@ -1,3 +1,66 @@
+2020-10-21 Alan Coon <alanc...@apple.com>
+
+ Cherry-pick r268771. rdar://problem/70532973
+
+ REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
+ https://bugs.webkit.org/show_bug.cgi?id=217987
+ <rdar://problem/70418548>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ When several animations targetting the same property and the same layer are overlapping, we used to
+ always override the previous animations. With r268483 we started maintaining all active animations
+ and let them run, potentially with additivity if the animation could be broken into several animations
+ each targeting a given transform operation.
+
+ On top of that, with r268615 and the support for accelerated animation of individual CSS transform
+ properties (translate, scale and rotate), all transform-related animations were made additive.
+
+ This meant that we would always run active animations targeting "transform" in a way where they would be
+ additive rather than being replaced.
+
+ Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
+ animation set will always have a 0 index. So now, when we compile a list of transform animations in
+ GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
+ ensuring only the top-most transform animation is applied.
+
+ We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
+ several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
+ remove all animations with the provided name.
+
+ Test: webanimations/accelerated-overlapping-transform-animations.html
+
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::pauseAnimation):
+ (WebCore::GraphicsLayerCA::removeAnimation):
+ (WebCore::GraphicsLayerCA::updateAnimations):
+
+ LayoutTests:
+
+ Add a new test that checks that only the last of two overlapping "transform" animations is applied.
+
+ * platform/mac-wk1/TestExpectations:
+ * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
+ * webanimations/accelerated-overlapping-transform-animations.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-10-20 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
+ https://bugs.webkit.org/show_bug.cgi?id=217987
+ <rdar://problem/70418548>
+
+ Reviewed by Simon Fraser.
+
+ Add a new test that checks that only the last of two overlapping "transform" animations is applied.
+
+ * platform/mac-wk1/TestExpectations:
+ * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
+ * webanimations/accelerated-overlapping-transform-animations.html: Added.
+
2020-10-20 Alan Coon <alanc...@apple.com>
Cherry-pick r268746. rdar://problem/70497736
Modified: branches/safari-611.1.4-branch/LayoutTests/platform/mac-wk1/TestExpectations (268801 => 268802)
--- branches/safari-611.1.4-branch/LayoutTests/platform/mac-wk1/TestExpectations 2020-10-21 16:40:44 UTC (rev 268801)
+++ branches/safari-611.1.4-branch/LayoutTests/platform/mac-wk1/TestExpectations 2020-10-21 16:54:53 UTC (rev 268802)
@@ -1193,3 +1193,5 @@
webkit.org/b/215325 [ Mojave ] media/remote-control-command-seek.html [ Pass Timeout ]
webkit.org/b/217761 [ Mojave+ Debug ] webgl/2.0.0/conformance/extensions/webgl-compressed-texture-s3tc-srgb.html [ Pass Crash ]
+
+webkit.org/b/217997 webanimations/accelerated-overlapping-transform-animations.html [ Pass Failure ]
Added: branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html (0 => 268802)
--- branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html (rev 0)
+++ branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html 2020-10-21 16:54:53 UTC (rev 268802)
@@ -0,0 +1,14 @@
+<style>
+
+ #target {
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ transform: translateX(200px);
+ }
+
+</style>
+<div id="target"></div>
Added: branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html (0 => 268802)
--- branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html (rev 0)
+++ branches/safari-611.1.4-branch/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html 2020-10-21 16:54:53 UTC (rev 268802)
@@ -0,0 +1,44 @@
+<style>
+
+ #target {
+ position: absolute;
+ top: 0;
+ left: 0;
+ width: 100px;
+ height: 100px;
+ background-color: black;
+ }
+
+</style>
+<div id="target"></div>
+<script src=""
+<script>
+
+(async () => {
+ if (window.testRunner)
+ testRunner.waitUntilDone();
+
+ const target = document.getElementById("target");
+
+ // Make the animations last a whole minute so they don't end while the test is running.
+ const duration = 60 * 1000;
+
+ // Start a first stationary "transform" animation.
+ const firstAnimation = target.animate({ transform: ["translateX(100px)", "translateX(100px)"] }, duration);
+
+ // Wait until this first animation has been applied.
+ await firstAnimation.ready;
+ await UIHelper.ensureStablePresentationUpdate();
+
+ // Now start a second stationary "transform" animation, which should replace the previous animation.
+ const secondAnimation = target.animate({ transform: ["translateX(200px)", "translateX(200px)"] }, duration);
+
+ // Wait until this second animation has been applied.
+ await secondAnimation.ready;
+ await UIHelper.ensureStablePresentationUpdate();
+
+ if (window.testRunner)
+ testRunner.notifyDone();
+})();
+
+</script>
Modified: branches/safari-611.1.4-branch/Source/WebCore/ChangeLog (268801 => 268802)
--- branches/safari-611.1.4-branch/Source/WebCore/ChangeLog 2020-10-21 16:40:44 UTC (rev 268801)
+++ branches/safari-611.1.4-branch/Source/WebCore/ChangeLog 2020-10-21 16:54:53 UTC (rev 268802)
@@ -1,3 +1,87 @@
+2020-10-21 Alan Coon <alanc...@apple.com>
+
+ Cherry-pick r268771. rdar://problem/70532973
+
+ REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
+ https://bugs.webkit.org/show_bug.cgi?id=217987
+ <rdar://problem/70418548>
+
+ Reviewed by Simon Fraser.
+
+ Source/WebCore:
+
+ When several animations targetting the same property and the same layer are overlapping, we used to
+ always override the previous animations. With r268483 we started maintaining all active animations
+ and let them run, potentially with additivity if the animation could be broken into several animations
+ each targeting a given transform operation.
+
+ On top of that, with r268615 and the support for accelerated animation of individual CSS transform
+ properties (translate, scale and rotate), all transform-related animations were made additive.
+
+ This meant that we would always run active animations targeting "transform" in a way where they would be
+ additive rather than being replaced.
+
+ Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
+ animation set will always have a 0 index. So now, when we compile a list of transform animations in
+ GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
+ ensuring only the top-most transform animation is applied.
+
+ We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
+ several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
+ remove all animations with the provided name.
+
+ Test: webanimations/accelerated-overlapping-transform-animations.html
+
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::pauseAnimation):
+ (WebCore::GraphicsLayerCA::removeAnimation):
+ (WebCore::GraphicsLayerCA::updateAnimations):
+
+ LayoutTests:
+
+ Add a new test that checks that only the last of two overlapping "transform" animations is applied.
+
+ * platform/mac-wk1/TestExpectations:
+ * webanimations/accelerated-overlapping-transform-animations-expected.html: Added.
+ * webanimations/accelerated-overlapping-transform-animations.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@268771 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2020-10-20 Antoine Quint <grao...@webkit.org>
+
+ REGRESSION (r268483): Map jumps around while zooming on windy.com, strava.com
+ https://bugs.webkit.org/show_bug.cgi?id=217987
+ <rdar://problem/70418548>
+
+ Reviewed by Simon Fraser.
+
+ When several animations targetting the same property and the same layer are overlapping, we used to
+ always override the previous animations. With r268483 we started maintaining all active animations
+ and let them run, potentially with additivity if the animation could be broken into several animations
+ each targeting a given transform operation.
+
+ On top of that, with r268615 and the support for accelerated animation of individual CSS transform
+ properties (translate, scale and rotate), all transform-related animations were made additive.
+
+ This meant that we would always run active animations targeting "transform" in a way where they would be
+ additive rather than being replaced.
+
+ Any animation targeting "transform" will yield one or several accelerated animations, and the first of this
+ animation set will always have a 0 index. So now, when we compile a list of transform animations in
+ GraphicsLayerCA::updateAnimations(), we reset that list any time we encounted an animation with a 0 index,
+ ensuring only the top-most transform animation is applied.
+
+ We also fix an issue where we didn't account for the possibility that a single KeyframeEffect could yield
+ several transform animations with the same name in pauseAnimation() and removeAnimation(). We now pause or
+ remove all animations with the provided name.
+
+ Test: webanimations/accelerated-overlapping-transform-animations.html
+
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::pauseAnimation):
+ (WebCore::GraphicsLayerCA::removeAnimation):
+ (WebCore::GraphicsLayerCA::updateAnimations):
+
2020-10-20 Alan Coon <alanc...@apple.com>
Revert r268386. rdar://problem/70497386
Modified: branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (268801 => 268802)
--- branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-21 16:40:44 UTC (rev 268801)
+++ branches/safari-611.1.4-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-21 16:54:53 UTC (rev 268802)
@@ -1079,18 +1079,16 @@
{
LOG_WITH_STREAM(Animations, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " pauseAnimation " << animationName << " (is running " << animationIsRunning(animationName) << ")");
- auto index = m_animations.findMatching([&](LayerPropertyAnimation animation) {
- return animation.m_name == animationName && !animation.m_pendingRemoval;
- });
+ for (auto& animation : m_animations) {
+ // There may be several animations with the same name in the case of transform animations
+ // animating multiple components as individual animations.
+ if (animation.m_name == animationName && !animation.m_pendingRemoval) {
+ animation.m_playState = PlayState::PausePending;
+ animation.m_timeOffset = Seconds { timeOffset };
- if (index == notFound)
- return;
-
- auto& animation = m_animations[index];
- animation.m_playState = PlayState::PausePending;
- animation.m_timeOffset = Seconds { timeOffset };
-
- noteLayerPropertyChanged(AnimationChanged);
+ noteLayerPropertyChanged(AnimationChanged);
+ }
+ }
}
void GraphicsLayerCA::removeAnimation(const String& animationName)
@@ -1097,16 +1095,14 @@
{
LOG_WITH_STREAM(Animations, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " removeAnimation " << animationName << " (is running " << animationIsRunning(animationName) << ")");
- auto index = m_animations.findMatching([&](LayerPropertyAnimation animation) {
- return animation.m_name == animationName && !animation.m_pendingRemoval;
- });
-
- if (index == notFound)
- return;
-
- m_animations[index].m_pendingRemoval = true;
-
- noteLayerPropertyChanged(AnimationChanged | CoverageRectChanged);
+ for (auto& animation : m_animations) {
+ // There may be several animations with the same name in the case of transform animations
+ // animating multiple components as individual animations.
+ if (animation.m_name == animationName && !animation.m_pendingRemoval) {
+ animation.m_pendingRemoval = true;
+ noteLayerPropertyChanged(AnimationChanged | CoverageRectChanged);
+ }
+ }
}
void GraphicsLayerCA::platformCALayerAnimationStarted(const String& animationKey, MonotonicTime startTime)
@@ -2960,6 +2956,14 @@
rotateAnimation = &animation;
break;
case AnimatedPropertyTransform:
+ // In the case of animations targeting the "transform" CSS property, there may be several
+ // animations created for a single KeyframeEffect, one for each transform component. In that
+ // case the animation index starts at 0 and increases for each component. If we encounter an
+ // index of 0 this means this animation establishes a new group of animation belonging to a
+ // single KeyframeEffect. As such, since the top-most KeyframeEffect replaces the previous
+ // ones, we can remove all the previously-added "transform" animations.
+ if (!animation.m_index)
+ transformAnimations.clear();
transformAnimations.append(&animation);
break;
case AnimatedPropertyOpacity:
