- Revision
- 239222
- Author
- simon.fra...@apple.com
- Date
- 2018-12-14 11:05:43 -0800 (Fri, 14 Dec 2018)
Log Message
REGRESSION (r233268): contents of an animated element inside overflow:hidden disappear
https://bugs.webkit.org/show_bug.cgi?id=188655
rdar://problem/43382687
Reviewed by Antoine Quint.
Source/WebCore:
The logic that computes animation extent, used by backing store attachment code, failed
to account for the behavior where a keyframe animation with a missing 0% keyframe uses
the transform from the unanimated style. This resulted in the computed extent being wrong,
which caused us to remove the layer's backing store in some scenarios.
Fix both animation code paths to use the renderer style if the first keyframe doesn't
contain a transform.
Tests: compositing/backing/backing-store-attachment-empty-keyframe.html
legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
* animation/KeyframeEffect.cpp:
(WebCore::KeyframeEffect::computeExtentOfTransformAnimation const):
* page/animation/KeyframeAnimation.cpp:
(WebCore::KeyframeAnimation::computeExtentOfTransformAnimation const):
LayoutTests:
* compositing/backing/backing-store-attachment-empty-keyframe-expected.txt: Added.
* compositing/backing/backing-store-attachment-empty-keyframe.html: Added.
* legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt: Added.
* legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html: Added.
Modified Paths
Added Paths
Diff
Modified: trunk/LayoutTests/ChangeLog (239221 => 239222)
--- trunk/LayoutTests/ChangeLog 2018-12-14 18:26:47 UTC (rev 239221)
+++ trunk/LayoutTests/ChangeLog 2018-12-14 19:05:43 UTC (rev 239222)
@@ -1,3 +1,16 @@
+2018-12-14 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r233268): contents of an animated element inside overflow:hidden disappear
+ https://bugs.webkit.org/show_bug.cgi?id=188655
+ rdar://problem/43382687
+
+ Reviewed by Antoine Quint.
+
+ * compositing/backing/backing-store-attachment-empty-keyframe-expected.txt: Added.
+ * compositing/backing/backing-store-attachment-empty-keyframe.html: Added.
+ * legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt: Added.
+ * legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html: Added.
+
2018-12-14 Zalan Bujtas <za...@apple.com>
Unreviewed test gardening.
Added: trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (0 => 239222)
--- trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (rev 0)
+++ trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt 2018-12-14 19:05:43 UTC (rev 239222)
@@ -0,0 +1,30 @@
+(GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 800.00 600.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (bounds 800.00 600.00)
+ (contentsOpaque 1)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (offsetFromRenderer width=-501 height=0)
+ (position 9.00 101.00)
+ (bounds 501.00 150.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (position 501.00 0.00)
+ (bounds 520.00 170.00)
+ (contentsOpaque 1)
+ (drawsContent 1)
+ (backingStoreAttached 1)
+ )
+ )
+ )
+ )
+ )
+ )
+)
+Some text here to force backing store.
Added: trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html (0 => 239222)
--- trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html (rev 0)
+++ trunk/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html 2018-12-14 19:05:43 UTC (rev 239222)
@@ -0,0 +1,73 @@
+<!DOCTYPE html>
+<html>
+<head>
+<style>
+ .container {
+ position: absolute;
+ top: 100px;
+ overflow: hidden;
+ width: 500px;
+ height: 150px;
+ border: 1px solid black;
+ }
+
+ .mover {
+ position: absolute;
+ width: 500px;
+ height: 100%;
+ left: 100%;
+ transform: translateX(-100%);
+ background-color: silver;
+ padding: 10px;
+ }
+
+ .mover.animating {
+ animation: slide 1s linear forwards;
+ }
+
+ @keyframes slide {
+ 100% { transform: translateX(0) }
+ }
+</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()
+ {
+ requestAnimationFrame(function() {
+ // Trigger layer flush
+ document.querySelector('.container').style.width = '501px';
+ dumpLayerTree();
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ }
+
+ window.addEventListener('load', () => {
+ let box = document.getElementById('box');
+ box.addEventListener('animationstart', dumpLayersSoon, false);
+ box.classList.add('animating');
+ }, false);
+</script>
+</head>
+<body>
+<pre id="out"></pre>
+<div class="container">
+ <div id="box" class="mover">
+ Some text here to force backing store.
+ </div>
+</div>
+</body>
+</html>
Added: trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (0 => 239222)
--- trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (rev 0)
+++ trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt 2018-12-14 19:05:43 UTC (rev 239222)
@@ -0,0 +1,31 @@
+(GraphicsLayer
+ (anchor 0.00 0.00)
+ (bounds 800.00 600.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (bounds 800.00 600.00)
+ (contentsOpaque 1)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (offsetFromRenderer width=-501 height=0)
+ (position 9.00 101.00)
+ (bounds 501.00 150.00)
+ (backingStoreAttached 1)
+ (children 1
+ (GraphicsLayer
+ (position 501.00 0.00)
+ (bounds 520.00 170.00)
+ (contentsOpaque 1)
+ (drawsContent 1)
+ (backingStoreAttached 1)
+ (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] [-520.00 0.00 0.00 1.00])
+ )
+ )
+ )
+ )
+ )
+ )
+)
+Some text here to force backing store.
Added: trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html (0 => 239222)
--- trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html (rev 0)
+++ trunk/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html 2018-12-14 19:05:43 UTC (rev 239222)
@@ -0,0 +1,73 @@
+<!DOCTYPE html><!-- webkit-test-runner [ experimental:WebAnimationsCSSIntegrationEnabled=false ] -->
+<html>
+<head>
+<style>
+ .container {
+ position: absolute;
+ top: 100px;
+ overflow: hidden;
+ width: 500px;
+ height: 150px;
+ border: 1px solid black;
+ }
+
+ .mover {
+ position: absolute;
+ width: 500px;
+ height: 100%;
+ left: 100%;
+ transform: translateX(-100%);
+ background-color: silver;
+ padding: 10px;
+ }
+
+ .mover.animating {
+ animation: slide 1s linear forwards;
+ }
+
+ @keyframes slide {
+ 100% { transform: translateX(0) }
+ }
+</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()
+ {
+ requestAnimationFrame(function() {
+ // Trigger layer flush
+ document.querySelector('.container').style.width = '501px';
+ dumpLayerTree();
+ if (window.testRunner)
+ testRunner.notifyDone();
+ });
+ }
+
+ window.addEventListener('load', () => {
+ let box = document.getElementById('box');
+ box.addEventListener('animationstart', dumpLayersSoon, false);
+ box.classList.add('animating');
+ }, false);
+</script>
+</head>
+<body>
+<pre id="out"></pre>
+<div class="container">
+ <div id="box" class="mover">
+ Some text here to force backing store.
+ </div>
+</div>
+</body>
+</html>
Modified: trunk/Source/WebCore/ChangeLog (239221 => 239222)
--- trunk/Source/WebCore/ChangeLog 2018-12-14 18:26:47 UTC (rev 239221)
+++ trunk/Source/WebCore/ChangeLog 2018-12-14 19:05:43 UTC (rev 239222)
@@ -1,3 +1,27 @@
+2018-12-14 Simon Fraser <simon.fra...@apple.com>
+
+ REGRESSION (r233268): contents of an animated element inside overflow:hidden disappear
+ https://bugs.webkit.org/show_bug.cgi?id=188655
+ rdar://problem/43382687
+
+ Reviewed by Antoine Quint.
+
+ The logic that computes animation extent, used by backing store attachment code, failed
+ to account for the behavior where a keyframe animation with a missing 0% keyframe uses
+ the transform from the unanimated style. This resulted in the computed extent being wrong,
+ which caused us to remove the layer's backing store in some scenarios.
+
+ Fix both animation code paths to use the renderer style if the first keyframe doesn't
+ contain a transform.
+
+ Tests: compositing/backing/backing-store-attachment-empty-keyframe.html
+ legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html
+
+ * animation/KeyframeEffect.cpp:
+ (WebCore::KeyframeEffect::computeExtentOfTransformAnimation const):
+ * page/animation/KeyframeAnimation.cpp:
+ (WebCore::KeyframeAnimation::computeExtentOfTransformAnimation const):
+
2018-12-14 Chris Dumez <cdu...@apple.com>
[PSON] Stop exposing PolicyAction::Suspend to WebCore
Modified: trunk/Source/WebCore/animation/KeyframeEffect.cpp (239221 => 239222)
--- trunk/Source/WebCore/animation/KeyframeEffect.cpp 2018-12-14 18:26:47 UTC (rev 239221)
+++ trunk/Source/WebCore/animation/KeyframeEffect.cpp 2018-12-14 19:05:43 UTC (rev 239222)
@@ -1408,23 +1408,30 @@
if (!is<RenderBox>(renderer()))
return true; // Non-boxes don't get transformed;
- RenderBox& box = downcast<RenderBox>(*renderer());
- FloatRect rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
+ auto& box = downcast<RenderBox>(*renderer());
+ auto rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
- FloatRect cumulativeBounds = bounds;
+ auto cumulativeBounds = bounds;
for (const auto& keyframe : m_blendingKeyframes.keyframes()) {
+ const auto* keyframeStyle = keyframe.style();
+
// FIXME: maybe for declarative animations we always say it's true for the first and last keyframe.
- if (!keyframe.containsProperty(CSSPropertyTransform))
- continue;
+ if (!keyframe.containsProperty(CSSPropertyTransform)) {
+ // If the first keyframe is missing transform style, use the current style.
+ if (!keyframe.key())
+ keyframeStyle = &box.style();
+ else
+ continue;
+ }
- LayoutRect keyframeBounds = bounds;
+ auto keyframeBounds = bounds;
bool canCompute;
if (transformFunctionListsMatch())
- canCompute = computeTransformedExtentViaTransformList(rendererBox, *keyframe.style(), keyframeBounds);
+ canCompute = computeTransformedExtentViaTransformList(rendererBox, *keyframeStyle, keyframeBounds);
else
- canCompute = computeTransformedExtentViaMatrix(rendererBox, *keyframe.style(), keyframeBounds);
+ canCompute = computeTransformedExtentViaMatrix(rendererBox, *keyframeStyle, keyframeBounds);
if (!canCompute)
return false;
@@ -1432,7 +1439,7 @@
cumulativeBounds.unite(keyframeBounds);
}
- bounds = LayoutRect(cumulativeBounds);
+ bounds = cumulativeBounds;
return true;
}
Modified: trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp (239221 => 239222)
--- trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2018-12-14 18:26:47 UTC (rev 239221)
+++ trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp 2018-12-14 19:05:43 UTC (rev 239222)
@@ -245,22 +245,29 @@
if (!is<RenderBox>(renderer()))
return true; // Non-boxes don't get transformed;
- RenderBox& box = downcast<RenderBox>(*renderer());
- FloatRect rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
+ auto& box = downcast<RenderBox>(*renderer());
+ auto rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
- FloatRect cumulativeBounds = bounds;
+ auto cumulativeBounds = bounds;
for (auto& keyframe : m_keyframes.keyframes()) {
- if (!keyframe.containsProperty(CSSPropertyTransform))
- continue;
+ const RenderStyle* keyframeStyle = keyframe.style();
- LayoutRect keyframeBounds = bounds;
+ if (!keyframe.containsProperty(CSSPropertyTransform)) {
+ // If the first keyframe is missing transform style, use the current style.
+ if (!keyframe.key())
+ keyframeStyle = &box.style();
+ else
+ continue;
+ }
+
+ auto keyframeBounds = bounds;
bool canCompute;
if (transformFunctionListsMatch())
- canCompute = computeTransformedExtentViaTransformList(rendererBox, *keyframe.style(), keyframeBounds);
+ canCompute = computeTransformedExtentViaTransformList(rendererBox, *keyframeStyle, keyframeBounds);
else
- canCompute = computeTransformedExtentViaMatrix(rendererBox, *keyframe.style(), keyframeBounds);
+ canCompute = computeTransformedExtentViaMatrix(rendererBox, *keyframeStyle, keyframeBounds);
if (!canCompute)
return false;
@@ -268,7 +275,7 @@
cumulativeBounds.unite(keyframeBounds);
}
- bounds = LayoutRect(cumulativeBounds);
+ bounds = cumulativeBounds;
return true;
}
