Title: [268771] trunk
- Revision
- 268771
- Author
- grao...@webkit.org
- Date
- 2020-10-20 15:58:48 -0700 (Tue, 20 Oct 2020)
Log Message
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.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (268770 => 268771)
--- trunk/LayoutTests/ChangeLog 2020-10-20 22:47:25 UTC (rev 268770)
+++ trunk/LayoutTests/ChangeLog 2020-10-20 22:58:48 UTC (rev 268771)
@@ -1,3 +1,17 @@
+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 Truitt Savell <tsav...@apple.com>
imported/w3c/web-platform-tests/webaudio/the-audio-api/the-audioworklet-interface/audioworkletnode-output-channel-count.https.html is a flaky failure
Modified: trunk/LayoutTests/platform/mac-wk1/TestExpectations (268770 => 268771)
--- trunk/LayoutTests/platform/mac-wk1/TestExpectations 2020-10-20 22:47:25 UTC (rev 268770)
+++ trunk/LayoutTests/platform/mac-wk1/TestExpectations 2020-10-20 22:58:48 UTC (rev 268771)
@@ -1199,3 +1199,4 @@
webkit.org/b/217934 inspector/debugger/breakpoint-resolve-when-script-added.html [ Skip ]
+webkit.org/b/217997 webanimations/accelerated-overlapping-transform-animations.html [ Pass Failure ]
Added: trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html (0 => 268771)
--- trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations-expected.html 2020-10-20 22:58:48 UTC (rev 268771)
@@ -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: trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html (0 => 268771)
--- trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html (rev 0)
+++ trunk/LayoutTests/webanimations/accelerated-overlapping-transform-animations.html 2020-10-20 22:58:48 UTC (rev 268771)
@@ -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: trunk/Source/WebCore/ChangeLog (268770 => 268771)
--- trunk/Source/WebCore/ChangeLog 2020-10-20 22:47:25 UTC (rev 268770)
+++ trunk/Source/WebCore/ChangeLog 2020-10-20 22:58:48 UTC (rev 268771)
@@ -1,3 +1,38 @@
+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 Jer Noble <jer.no...@apple.com>
[media-session] Basic support for MediaSession.setPositionState() and MediaSession.setActionHandler()
Modified: trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (268770 => 268771)
--- trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-20 22:47:25 UTC (rev 268770)
+++ trunk/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2020-10-20 22:58:48 UTC (rev 268771)
@@ -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:
_______________________________________________
webkit-changes mailing list
webkit-changes@lists.webkit.org
https://lists.webkit.org/mailman/listinfo/webkit-changes
