WebKit Bugzilla
Attachment 357283 Details for
Bug 188655
: REGRESSION (r233268): contents of an animated element inside overflow:hidden disappear
Home
|
New
|
Browse
|
Search
|
[?]
|
Reports
|
Requests
|
Help
|
New Account
|
Log In
Remember
[x]
|
Forgot Password
Login:
[x]
[patch]
Patch
bug-188655-20181213185045.patch (text/plain), 14.18 KB, created by
Simon Fraser (smfr)
on 2018-12-13 18:50:46 PST
(
hide
)
Description:
Patch
Filename:
MIME Type:
Creator:
Simon Fraser (smfr)
Created:
2018-12-13 18:50:46 PST
Size:
14.18 KB
patch
obsolete
>Subversion Revision: 239170 >diff --git a/Source/WebCore/ChangeLog b/Source/WebCore/ChangeLog >index c7c333bbc491e236532813334d0f41a531b75075..ee395b9b8c83ae10bba4de646a28179781698cde 100644 >--- a/Source/WebCore/ChangeLog >+++ b/Source/WebCore/ChangeLog >@@ -1,3 +1,27 @@ >+2018-12-13 Simon Fraser <simon.fraser@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 NOBODY (OOPS!). >+ >+ 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-13 Eric Carlson <eric.carlson@apple.com> > > [MediaStream] Calculate width or height when constraints contain only the other >diff --git a/Source/WebCore/animation/KeyframeEffect.cpp b/Source/WebCore/animation/KeyframeEffect.cpp >index 11c878aef6b20127829afb41a772c2c0cb77ec8d..ce1bb329a2e30402b5c4a1c27eef8ec9e2a4228b 100644 >--- a/Source/WebCore/animation/KeyframeEffect.cpp >+++ b/Source/WebCore/animation/KeyframeEffect.cpp >@@ -1408,23 +1408,30 @@ bool KeyframeEffect::computeExtentOfTransformAnimation(LayoutRect& bounds) const > 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 @@ bool KeyframeEffect::computeExtentOfTransformAnimation(LayoutRect& bounds) const > cumulativeBounds.unite(keyframeBounds); > } > >- bounds = LayoutRect(cumulativeBounds); >+ bounds = cumulativeBounds; > return true; > } > >diff --git a/Source/WebCore/page/animation/KeyframeAnimation.cpp b/Source/WebCore/page/animation/KeyframeAnimation.cpp >index fefe0101150177b3c16e5db8c83804cb6063967e..f999ff643c50b7824cf9263a56431ea59d7d8b18 100644 >--- a/Source/WebCore/page/animation/KeyframeAnimation.cpp >+++ b/Source/WebCore/page/animation/KeyframeAnimation.cpp >@@ -245,22 +245,29 @@ bool KeyframeAnimation::computeExtentOfTransformAnimation(LayoutRect& bounds) co > 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(); >+ >+ 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; >@@ -268,7 +275,7 @@ bool KeyframeAnimation::computeExtentOfTransformAnimation(LayoutRect& bounds) co > cumulativeBounds.unite(keyframeBounds); > } > >- bounds = LayoutRect(cumulativeBounds); >+ bounds = cumulativeBounds; > return true; > } > >diff --git a/LayoutTests/ChangeLog b/LayoutTests/ChangeLog >index fc660cd3292734e120aa41ed9f5baa9dbf919b19..0f458ede4ac214c246004f857eb582b957465364 100644 >--- a/LayoutTests/ChangeLog >+++ b/LayoutTests/ChangeLog >@@ -1,3 +1,16 @@ >+2018-12-13 Simon Fraser <simon.fraser@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 NOBODY (OOPS!). >+ >+ * 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-13 Eric Carlson <eric.carlson@apple.com> > > [MediaStream] Calculate width or height when constraints contain only the other >diff --git a/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt b/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..fc41ef0988ed4f1196352d49d500c08f556037bd >--- /dev/null >+++ b/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt >@@ -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) >+ (anchor 0.50 0.50) >+ (bounds 521.00 170.00) >+ (contentsOpaque 1) >+ (drawsContent 1) >+ (backingStoreAttached 1) >+ ) >+ ) >+ ) >+ ) >+ ) >+ ) >+) >+Some text here to force backing store. >diff --git a/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html b/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html >new file mode 100644 >index 0000000000000000000000000000000000000000..07ebba6d43fdf91b420c451e8aaea475b460451d >--- /dev/null >+++ b/LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html >@@ -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: 100%; >+ 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> >diff --git a/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt b/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt >new file mode 100644 >index 0000000000000000000000000000000000000000..9b46f232b9daf2a496c50c3f51ba03de980f0a24 >--- /dev/null >+++ b/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt >@@ -0,0 +1,32 @@ >+(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) >+ (anchor 0.50 0.50) >+ (bounds 521.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. >diff --git a/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html b/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html >new file mode 100644 >index 0000000000000000000000000000000000000000..5d6b063eff16e90a9e6d4d4c6ad36d6568827555 >--- /dev/null >+++ b/LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html >@@ -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: 100%; >+ 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>
You cannot view the attachment while viewing its details because your browser does not support IFRAMEs.
View the attachment on a separate page
.
View Attachment As Diff
View Attachment As Raw
Flags:
graouts
:
review+
ews-watchlist
:
commit-queue-
Actions:
View
|
Formatted Diff
|
Diff
Attachments on
bug 188655
:
347266
|
354513
| 357283 |
357295