Changeset 239222 in webkit

Timestamp:

Dec 14, 2018 11:05:43 AM (5 years ago)

Author:

Simon Fraser

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.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (added)
• LayoutTests/compositing/backing/backing-store-attachment-empty-keyframe.html (added)
• LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe-expected.txt (added)
• LayoutTests/legacy-animation-engine/compositing/backing/backing-store-attachment-empty-keyframe.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/animation/KeyframeEffect.cpp (modified) (2 diffs)
• Source/WebCore/page/animation/KeyframeAnimation.cpp (modified) (2 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2018-12-14  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 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  <zalan@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2018-12-14  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 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  <cdumez@apple.com>
 

trunk/Source/WebCore/animation/KeyframeEffect.cpp

@@ -1409 +1409 @@
         return true; // Non-boxes don't get transformed;
 
-    RenderBox& box = downcast<RenderBox>(*renderer());
-    FloatRect rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
-
-    FloatRect cumulativeBounds = bounds;
+    auto& box = downcast<RenderBox>(*renderer());
+    auto rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
+
+    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;
-
-        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)
@@ -1433 +1440 @@
     }
 
-    bounds = LayoutRect(cumulativeBounds);
+    bounds = cumulativeBounds;
     return true;
 }

trunk/Source/WebCore/page/animation/KeyframeAnimation.cpp

@@ -246 +246 @@
         return true; // Non-boxes don't get transformed;
 
-    RenderBox& box = downcast<RenderBox>(*renderer());
-    FloatRect rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
-
-    FloatRect cumulativeBounds = bounds;
+    auto& box = downcast<RenderBox>(*renderer());
+    auto rendererBox = snapRectToDevicePixels(box.borderBoxRect(), box.document().deviceScaleFactor());
+
+    auto cumulativeBounds = bounds;
 
     for (auto& keyframe : m_keyframes.keyframes()) {
-        if (!keyframe.containsProperty(CSSPropertyTransform))
-            continue;
-
-        LayoutRect keyframeBounds = bounds;
+        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;
+        }
+
+        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)
@@ -269 +276 @@
     }
 
-    bounds = LayoutRect(cumulativeBounds);
+    bounds = cumulativeBounds;
     return true;
 }