Diff
Modified: branches/safari-606-branch/LayoutTests/ChangeLog (234093 => 234094)
--- branches/safari-606-branch/LayoutTests/ChangeLog 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/LayoutTests/ChangeLog 2018-07-23 07:09:50 UTC (rev 234094)
@@ -1,3 +1,64 @@
+2018-07-23 Babak Shafiei <[email protected]>
+
+ Cherry-pick r234073. rdar://problem/42451644
+
+ Remove completed animations from GraphicsLayer, thus avoiding excessive backing store allocation
+ https://bugs.webkit.org/show_bug.cgi?id=187844
+ rdar://problem/40387294
+
+ Reviewed by Dean Jackson.
+ Source/WebCore:
+
+ A keyframe animation which animates 3D transforms, and is fill-forwards, currently
+ leaves the GraphicsLayer in a state where it has a "running" animation. However, the
+ logic that computes animation extent in RenderLayerBacking::updateGeometry() only does
+ so for running or paused animations. GraphicsLayer then thinks that it has an active
+ transform animation with unknown extent, and refuses to detach its backing store.
+
+ This triggers excessive layer creation on some sites (e.g. https://www.kqed.org).
+
+ Fix by always removing animations from the GraphicsLayer when they finish, whether
+ or not they fill forwards. This is done by having KeyframeAnimation::onAnimationEnd()
+ always call endAnimation().
+
+ This change only fixes the non-Web Animation code path. webkit.org/b/187845 exists
+ to fix the other code path.
+
+ Also improve some logging that would have revealed this problem sooner.
+
+ Test: compositing/backing/backing-store-attachment-fill-forwards-animation.html
+
+ * page/animation/AnimationBase.h:
+ (WebCore::AnimationBase::endAnimation):
+ * page/animation/ImplicitAnimation.cpp:
+ (WebCore::ImplicitAnimation::endAnimation):
+ * page/animation/ImplicitAnimation.h:
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::endAnimation):
+ (WebCore::KeyframeAnimation::onAnimationEnd):
+ * page/animation/KeyframeAnimation.h:
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::addAnimation):
+ (WebCore::GraphicsLayerCA::updateCoverage):
+
+ LayoutTests:
+
+ * compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt: Added.
+ * compositing/backing/backing-store-attachment-fill-forwards-animation.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234073 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-07-19 Simon Fraser <[email protected]>
+
+ Remove completed animations from GraphicsLayer, thus avoiding excessive backing store allocation
+ https://bugs.webkit.org/show_bug.cgi?id=187844
+ rdar://problem/40387294
+
+ Reviewed by Dean Jackson.
+
+ * compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt: Added.
+ * compositing/backing/backing-store-attachment-fill-forwards-animation.html: Added.
+
2018-07-20 Babak Shafiei <[email protected]>
Revert r233926. rdar://problem/42446531
Added: branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt (0 => 234094)
--- branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt (rev 0)
+++ branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt 2018-07-23 07:09:50 UTC (rev 234094)
@@ -0,0 +1,25 @@
+An element with a finished fill-forwards animation should not trigger backing store outside the viewport.
+
+(GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 785.00 1110.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (bounds 785.00 1110.00)
+ (contentsOpaque 1)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (position 18.00 1010.00)
+ (bounds 100.00 100.00)
+ (contentsOpaque 1)
+ (drawsContent 1)
+ (backingStoreAttached 0)
+ (transform [1.00 0.00 0.00 0.00] [0.00 1.00 0.00 0.00] [0.00 0.00 1.00 0.00] [100.00 0.00 0.00 1.00])
+ )
+ )
+ )
+ )
+)
+Some text here to force backing store.
Added: branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation.html (0 => 234094)
--- branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation.html (rev 0)
+++ branches/safari-606-branch/LayoutTests/compositing/backing/backing-store-attachment-fill-forwards-animation.html 2018-07-23 07:09:50 UTC (rev 234094)
@@ -0,0 +1,67 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ @keyframes move {
+ from { transform: translate3d(0, 0, 0); }
+ to { transform: translate3d(100px, 0, 0); }
+ }
+
+ #box {
+ position: absolute;
+ top: 1000px;
+ width: 100px;
+ height: 100px;
+ background-color: silver;
+ margin: 10px;
+ }
+
+ #box.animating {
+ animation: move linear 0.1s forwards;
+ }
+
+</style>
+<script>
+if (window.testRunner) {
+ testRunner.dumpAsText();
+ testRunner.waitUntilDone();
+}
+
+function dumpLayerTree()
+{
+ if (!window.internals)
+ return;
+
+ var out = document.getElementById('out');
+ out.innerText = internals.layerTreeAsText(document, internals.LAYER_TREE_INCLUDES_BACKING_STORE_ATTACHED);
+}
+
+function dumpLayersSoon()
+{
+ setTimeout(function() {
+ dumpLayerTree();
+ if (window.testRunner)
+ testRunner.notifyDone();
+ }, 0);
+}
+
+function runTest()
+{
+ let box = document.getElementById('box');
+ box.addEventListener('animationend', dumpLayersSoon, false);
+ box.classList.add('animating');
+}
+
+window.addEventListener('load', runTest, false);
+
+</script>
+</head>
+<body>
+<p>An element with a finished fill-forwards animation should not trigger backing store outside the viewport.</p>
+<pre id="out"></pre>
+<div id="box">
+ Some text here to force backing store.
+</div>
+
+</body>
+</html>
Added: branches/safari-606-branch/LayoutTests/platform/ios/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt (0 => 234094)
--- branches/safari-606-branch/LayoutTests/platform/ios/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt (rev 0)
+++ branches/safari-606-branch/LayoutTests/platform/ios/compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt 2018-07-23 07:09:50 UTC (rev 234094)
@@ -0,0 +1,25 @@
+An element with a finished fill-forwards animation should not trigger backing store outside the viewport.
+
+(GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 800.00 1110.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (bounds 800.00 1110.00)
+ (contentsOpaque 1)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (position 18.00 1010.00)
+ (bounds 100.00 100.00)
+ (contentsOpaque 1)
+ (drawsContent 1)
+ (backingStoreAttached 0)
+ (transform [1.00 0.00 0.00 0.00] [0.00 1.00 0.00 0.00] [0.00 0.00 1.00 0.00] [100.00 0.00 0.00 1.00])
+ )
+ )
+ )
+ )
+)
+Some text here to force backing store.
Modified: branches/safari-606-branch/Source/WebCore/ChangeLog (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/ChangeLog 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/ChangeLog 2018-07-23 07:09:50 UTC (rev 234094)
@@ -1,3 +1,93 @@
+2018-07-23 Babak Shafiei <[email protected]>
+
+ Cherry-pick r234073. rdar://problem/42451644
+
+ Remove completed animations from GraphicsLayer, thus avoiding excessive backing store allocation
+ https://bugs.webkit.org/show_bug.cgi?id=187844
+ rdar://problem/40387294
+
+ Reviewed by Dean Jackson.
+ Source/WebCore:
+
+ A keyframe animation which animates 3D transforms, and is fill-forwards, currently
+ leaves the GraphicsLayer in a state where it has a "running" animation. However, the
+ logic that computes animation extent in RenderLayerBacking::updateGeometry() only does
+ so for running or paused animations. GraphicsLayer then thinks that it has an active
+ transform animation with unknown extent, and refuses to detach its backing store.
+
+ This triggers excessive layer creation on some sites (e.g. https://www.kqed.org).
+
+ Fix by always removing animations from the GraphicsLayer when they finish, whether
+ or not they fill forwards. This is done by having KeyframeAnimation::onAnimationEnd()
+ always call endAnimation().
+
+ This change only fixes the non-Web Animation code path. webkit.org/b/187845 exists
+ to fix the other code path.
+
+ Also improve some logging that would have revealed this problem sooner.
+
+ Test: compositing/backing/backing-store-attachment-fill-forwards-animation.html
+
+ * page/animation/AnimationBase.h:
+ (WebCore::AnimationBase::endAnimation):
+ * page/animation/ImplicitAnimation.cpp:
+ (WebCore::ImplicitAnimation::endAnimation):
+ * page/animation/ImplicitAnimation.h:
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::endAnimation):
+ (WebCore::KeyframeAnimation::onAnimationEnd):
+ * page/animation/KeyframeAnimation.h:
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::addAnimation):
+ (WebCore::GraphicsLayerCA::updateCoverage):
+
+ LayoutTests:
+
+ * compositing/backing/backing-store-attachment-fill-forwards-animation-expected.txt: Added.
+ * compositing/backing/backing-store-attachment-fill-forwards-animation.html: Added.
+
+ git-svn-id: https://svn.webkit.org/repository/webkit/trunk@234073 268f45cc-cd09-0410-ab3c-d52691b4dbfc
+
+ 2018-07-19 Simon Fraser <[email protected]>
+
+ Remove completed animations from GraphicsLayer, thus avoiding excessive backing store allocation
+ https://bugs.webkit.org/show_bug.cgi?id=187844
+ rdar://problem/40387294
+
+ Reviewed by Dean Jackson.
+
+ A keyframe animation which animates 3D transforms, and is fill-forwards, currently
+ leaves the GraphicsLayer in a state where it has a "running" animation. However, the
+ logic that computes animation extent in RenderLayerBacking::updateGeometry() only does
+ so for running or paused animations. GraphicsLayer then thinks that it has an active
+ transform animation with unknown extent, and refuses to detach its backing store.
+
+ This triggers excessive layer creation on some sites (e.g. https://www.kqed.org).
+
+ Fix by always removing animations from the GraphicsLayer when they finish, whether
+ or not they fill forwards. This is done by having KeyframeAnimation::onAnimationEnd()
+ always call endAnimation().
+
+ This change only fixes the non-Web Animation code path. webkit.org/b/187845 exists
+ to fix the other code path.
+
+ Also improve some logging that would have revealed this problem sooner.
+
+ Test: compositing/backing/backing-store-attachment-fill-forwards-animation.html
+
+ * page/animation/AnimationBase.h:
+ (WebCore::AnimationBase::endAnimation):
+ * page/animation/ImplicitAnimation.cpp:
+ (WebCore::ImplicitAnimation::endAnimation):
+ * page/animation/ImplicitAnimation.h:
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::endAnimation):
+ (WebCore::KeyframeAnimation::onAnimationEnd):
+ * page/animation/KeyframeAnimation.h:
+ * platform/graphics/ca/GraphicsLayerCA.cpp:
+ (WebCore::GraphicsLayerCA::addAnimation):
+ (WebCore::GraphicsLayerCA::updateCoverage):
+
2018-07-20 Babak Shafiei <[email protected]>
Revert r233926. rdar://problem/42446531
Modified: branches/safari-606-branch/Source/WebCore/page/animation/AnimationBase.h (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/page/animation/AnimationBase.h 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/page/animation/AnimationBase.h 2018-07-23 07:09:50 UTC (rev 234094)
@@ -223,7 +223,7 @@
virtual bool startAnimation(double /*timeOffset*/) { return false; }
// timeOffset is the time at which the animation is being paused.
virtual void pauseAnimation(double /*timeOffset*/) { }
- virtual void endAnimation() { }
+ virtual void endAnimation(bool /*fillingForwards*/ = false) { }
virtual const RenderStyle& unanimatedStyle() const = 0;
Modified: branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.cpp 2018-07-23 07:09:50 UTC (rev 234094)
@@ -147,7 +147,7 @@
setNeedsStyleRecalc(element());
}
-void ImplicitAnimation::endAnimation()
+void ImplicitAnimation::endAnimation(bool)
{
if (auto* renderer = compositedRenderer())
renderer->transitionFinished(m_animatingProperty);
Modified: branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.h (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.h 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/page/animation/ImplicitAnimation.h 2018-07-23 07:09:50 UTC (rev 234094)
@@ -51,7 +51,7 @@
void onAnimationEnd(double elapsedTime) override;
bool startAnimation(double timeOffset) override;
void pauseAnimation(double timeOffset) override;
- void endAnimation() override;
+ void endAnimation(bool fillingForwards = false) override;
bool animate(CompositeAnimation&, const RenderStyle& targetStyle, std::unique_ptr<RenderStyle>& animatedStyle, bool& didBlendStyle);
void getAnimatedStyle(std::unique_ptr<RenderStyle>& animatedStyle) override;
Modified: branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.cpp 2018-07-23 07:09:50 UTC (rev 234094)
@@ -297,7 +297,7 @@
setNeedsStyleRecalc(element());
}
-void KeyframeAnimation::endAnimation()
+void KeyframeAnimation::endAnimation(bool fillingForwards)
{
if (!element())
return;
@@ -306,7 +306,7 @@
renderer->animationFinished(m_keyframes.animationName());
// Restore the original (unanimated) style
- if (!paused())
+ if (!fillingForwards && !paused())
setNeedsStyleRecalc(element());
}
@@ -328,10 +328,7 @@
void KeyframeAnimation::onAnimationEnd(double elapsedTime)
{
sendAnimationEvent(eventNames().animationendEvent, elapsedTime);
- // End the animation if we don't fill forwards. Forward filling
- // animations are ended properly in the class destructor.
- if (!m_animation->fillsForwards())
- endAnimation();
+ endAnimation(m_animation->fillsForwards());
}
bool KeyframeAnimation::sendAnimationEvent(const AtomicString& eventType, double elapsedTime)
Modified: branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.h (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.h 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/page/animation/KeyframeAnimation.h 2018-07-23 07:09:50 UTC (rev 234094)
@@ -69,7 +69,7 @@
void onAnimationEnd(double elapsedTime) override;
bool startAnimation(double timeOffset) override;
void pauseAnimation(double timeOffset) override;
- void endAnimation() override;
+ void endAnimation(bool fillingForwards = false) override;
void overrideAnimations() override;
void resumeOverriddenAnimations() override;
Modified: branches/safari-606-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp (234093 => 234094)
--- branches/safari-606-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2018-07-23 07:07:07 UTC (rev 234093)
+++ branches/safari-606-branch/Source/WebCore/platform/graphics/ca/GraphicsLayerCA.cpp 2018-07-23 07:09:50 UTC (rev 234094)
@@ -1022,7 +1022,7 @@
bool GraphicsLayerCA::addAnimation(const KeyframeValueList& valueList, const FloatSize& boxSize, const Animation* anim, const String& animationName, double timeOffset)
{
- LOG(Animations, "GraphicsLayerCA %p addAnimation %s (can be accelerated %d)", this, animationName.utf8().data(), animationCanBeAccelerated(valueList, anim));
+ LOG(Animations, "GraphicsLayerCA %p %llu addAnimation %s (can be accelerated %d)", this, primaryLayerID(), animationName.utf8().data(), animationCanBeAccelerated(valueList, anim));
ASSERT(!animationName.isEmpty());
@@ -2351,7 +2351,20 @@
|| commitState.ancestorWithTransformAnimationIntersectsCoverageRect // FIXME: Compute backing exactly for descendants of animating layers.
|| (isRunningTransformAnimation() && !animationExtent()); // Create backing if we don't know the animation extent.
- LOG_WITH_STREAM(Compositing, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " setBackingStoreAttached: " << requiresBacking);
+#if !LOG_DISABLED
+ if (requiresBacking) {
+ auto reasonForBacking = [&]() -> const char* {
+ if (m_intersectsCoverageRect)
+ return "intersectsCoverageRect";
+
+ if (commitState.ancestorWithTransformAnimationIntersectsCoverageRect)
+ return "ancestor with transform";
+
+ return "has transform animation with unknown extent";
+ };
+ LOG_WITH_STREAM(Compositing, stream << "GraphicsLayerCA " << this << " id " << primaryLayerID() << " setBackingStoreAttached: " << requiresBacking << " (" << reasonForBacking() << ")");
+ }
+#endif
m_layer->setBackingStoreAttached(requiresBacking);
if (m_layerClones) {
