[Async overflow scrolling] Absolute positioned element inside async overflow scroll...
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 May 2019 15:01:14 +0000 (15:01 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Sat, 25 May 2019 15:01:14 +0000 (15:01 +0000)
https://bugs.webkit.org/show_bug.cgi?id=198237

Reviewed by Antti Koivisto.

Source/WebCore:

The logic in requiresCompositingForIndirectReason() that decides if we need to do
compositing for a layer that needs to move independently of its enclosing scroller
was wrong.

Instead of asking question about the enclosing compositing layer, it needs to ask
whether it has different positioning behavior from the layer that it would
otherwise paint into, which is its paint-order parent.

"paintsIntoProvidedBacking" already does a containing-block check against the
scroller, so we know we don't have to do the check in that case.

Test: scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html

* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeCompositingRequirements):
(WebCore::enclosingCompositedScrollingLayer):
(WebCore::isScrolledByOverflowScrollLayer):
(WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
(WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):
* rendering/RenderLayerCompositor.h:

LayoutTests:

* platform/ios-wk2/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt: Added.
* scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html: Added.

git-svn-id: https://svn.webkit.org/repository/webkit/trunk@245772 268f45cc-cd09-0410-ab3c-d52691b4dbfc

LayoutTests/ChangeLog
LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt [new file with mode: 0644]
LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/rendering/RenderLayerCompositor.cpp
Source/WebCore/rendering/RenderLayerCompositor.h

index 2510909..4f225c7 100644 (file)
@@ -1,5 +1,16 @@
 2019-05-25  Simon Fraser  <simon.fraser@apple.com>
 
+        [Async overflow scrolling] Absolute positioned element inside async overflow scroll didn't get composited sometimes
+        https://bugs.webkit.org/show_bug.cgi?id=198237
+
+        Reviewed by Antti Koivisto.
+
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html: Added.
+
+2019-05-25  Simon Fraser  <simon.fraser@apple.com>
+
         [macOS] Fix programmatic scroll in RTL overflow with async scrolling enabled
         https://bugs.webkit.org/show_bug.cgi?id=198226
 
diff --git a/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt b/LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt
new file mode 100644 (file)
index 0000000..86cd129
--- /dev/null
@@ -0,0 +1,44 @@
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 1)
+    (vertical scroll elasticity 1)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 2
+    (Overflow scrolling node
+      (scrollable area size 220 170)
+      (contents size 220 1020)
+      (parent relative scrollable rect at (19,11) size 220x170)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 1)
+        (vertical scroll elasticity 1)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (10,10))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+      (children 1
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (71,22))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+      )
+    )
+  )
+)
+
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt b/LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller-expected.txt
new file mode 100644 (file)
index 0000000..227bdb1
--- /dev/null
@@ -0,0 +1,44 @@
+
+(Frame scrolling node
+  (scrollable area size 800 600)
+  (contents size 800 600)
+  (parent relative scrollable rect at (0,0) size 800x600)
+  (scrollable area parameters 
+    (horizontal scroll elasticity 2)
+    (vertical scroll elasticity 2)
+    (horizontal scrollbar mode 0)
+    (vertical scrollbar mode 0))
+  (layout viewport at (0,0) size 800x600)
+  (min layout viewport origin (0,0))
+  (max layout viewport origin (0,0))
+  (behavior for fixed 0)
+  (children 2
+    (Overflow scrolling node
+      (scrollable area size 205 155)
+      (contents size 205 1020)
+      (parent relative scrollable rect at (19,11) size 205x155)
+      (scrollable area parameters 
+        (horizontal scroll elasticity 0)
+        (vertical scroll elasticity 0)
+        (horizontal scrollbar mode 0)
+        (vertical scrollbar mode 0)
+        (has enabled vertical scrollbar 1))
+    )
+    (Positioned node
+      (layout constraints 
+        (layer-position-at-last-layout (10,10))
+        (positioning-behavior moves))
+      (related overflow nodes 1)
+      (children 1
+        (Positioned node
+          (layout constraints 
+            (layer-position-at-last-layout (71,22))
+            (positioning-behavior stationary))
+          (related overflow nodes 1)
+        )
+      )
+    )
+  )
+)
+
+
diff --git a/LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html b/LayoutTests/scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html
new file mode 100644 (file)
index 0000000..2d97423
--- /dev/null
@@ -0,0 +1,59 @@
+<!DOCTYPE html> <!-- webkit-test-runner [ internal:AsyncOverflowScrollingEnabled=true ] -->
+<html>
+<head>
+       <title>Tests that we make a compositing layer for an abspost element inside something that paints into the scroller</title>
+    <style>
+           .scroller {
+               overflow: scroll;
+               margin: 10px;
+               background-color: silver;
+               border: 1px solid black;
+               padding: 10px;
+               width: 200px;
+               height: 150px;
+           }
+
+           .stacking {
+                       border: 2px solid gray;
+                       padding: 20px;
+               opacity: 0.75;
+           }
+
+           .box {
+               width: 100px;
+               height: 100px;
+                       background-color: blue;
+           }
+       
+           .absolute {
+               position: absolute;
+               left: 100px;
+           }
+
+           .scrolling-content {
+               height: 1000px;
+           }
+    </style>
+    <script>
+           if (window.testRunner)
+               testRunner.dumpAsText();
+
+           window.addEventListener('load', () => {
+               if (window.internals)
+                   document.getElementById('scrollingTree').innerText = window.internals.scrollingStateTreeAsText() + "\n";
+           }, false);
+    </script>
+</head>
+<body>
+    <div class="container">
+        <div class="scroller">
+            <div class="scrolling-content">
+                <div class="stacking">
+                    <div class="absolute stationary box"></div>
+                </div>
+            </div>
+        </div>
+    </div>
+<pre id="scrollingTree"></pre>
+</body>
+</html>
index 6515e70..e384341 100644 (file)
@@ -1,5 +1,33 @@
 2019-05-25  Simon Fraser  <simon.fraser@apple.com>
 
+        [Async overflow scrolling] Absolute positioned element inside async overflow scroll didn't get composited sometimes
+        https://bugs.webkit.org/show_bug.cgi?id=198237
+
+        Reviewed by Antti Koivisto.
+
+        The logic in requiresCompositingForIndirectReason() that decides if we need to do
+        compositing for a layer that needs to move independently of its enclosing scroller
+        was wrong.
+
+        Instead of asking question about the enclosing compositing layer, it needs to ask
+        whether it has different positioning behavior from the layer that it would
+        otherwise paint into, which is its paint-order parent.
+
+        "paintsIntoProvidedBacking" already does a containing-block check against the
+        scroller, so we know we don't have to do the check in that case.
+
+        Test: scrollingcoordinator/scrolling-tree/absolute-inside-stacking-in-scroller.html
+
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+        (WebCore::enclosingCompositedScrollingLayer):
+        (WebCore::isScrolledByOverflowScrollLayer):
+        (WebCore::isNonScrolledLayerInsideScrolledCompositedAncestor):
+        (WebCore::RenderLayerCompositor::requiresCompositingForIndirectReason const):
+        * rendering/RenderLayerCompositor.h:
+
+2019-05-25  Simon Fraser  <simon.fraser@apple.com>
+
         [macOS] Fix programmatic scroll in RTL overflow with async scrolling enabled
         https://bugs.webkit.org/show_bug.cgi?id=198226
 
index cae45f8..f6d258a 100644 (file)
@@ -1009,7 +1009,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     // Now check for reasons to become composited that depend on the state of descendant layers.
     IndirectCompositingReason indirectCompositingReason;
     if (!willBeComposited && canBeComposited(layer)
-        && requiresCompositingForIndirectReason(layer, compositingState.compositingAncestor, currentState.subtreeIsCompositing, anyDescendantHas3DTransform, layerPaintsIntoProvidedBacking, indirectCompositingReason)) {
+        && requiresCompositingForIndirectReason(layer, currentState.subtreeIsCompositing, anyDescendantHas3DTransform, layerPaintsIntoProvidedBacking, indirectCompositingReason)) {
         layer.setIndirectCompositingReason(indirectCompositingReason);
         layerWillCompositePostDescendants();
     }
@@ -2916,8 +2916,45 @@ bool RenderLayerCompositor::requiresCompositingForOverflowScrolling(const Render
     return layer.hasCompositedScrollableOverflow();
 }
 
+static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
+{
+    const auto* ancestorLayer = layer.parent();
+    while (ancestorLayer) {
+        if (ancestorLayer == &intermediateLayer)
+            sawIntermediateLayer = true;
+
+        if (ancestorLayer->hasCompositedScrollableOverflow())
+            return const_cast<RenderLayer*>(ancestorLayer);
+
+        ancestorLayer = ancestorLayer->parent();
+    }
+
+    return nullptr;
+}
+
+static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
+{
+    bool scrolledByOverflowScroll = false;
+    traverseAncestorLayers(layer, [&](const RenderLayer& ancestorLayer, bool inContainingBlockChain, bool) {
+        if (&ancestorLayer == &overflowScrollLayer) {
+            scrolledByOverflowScroll = inContainingBlockChain;
+            return AncestorTraversal::Stop;
+        }
+        return AncestorTraversal::Continue;
+    });
+    return scrolledByOverflowScroll;
+}
+
+static bool isNonScrolledLayerInsideScrolledCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor, const RenderLayer& scrollingAncestor)
+{
+    bool ancestorMovedByScroller = &compositedAncestor == &scrollingAncestor || isScrolledByOverflowScrollLayer(compositedAncestor, scrollingAncestor);
+    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, scrollingAncestor);
+
+    return ancestorMovedByScroller && !layerMovedByScroller;
+}
+
 // FIXME: why doesn't this handle the clipping cases?
-bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLayer& layer, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason& reason) const
+bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLayer& layer, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason& reason) const
 {
     // When a layer has composited descendants, some effects, like 2d transforms, filters, masks etc must be implemented
     // via compositing so that they also apply to those composited descendants.
@@ -2941,8 +2978,12 @@ bool RenderLayerCompositor::requiresCompositingForIndirectReason(const RenderLay
         }
     }
 
-    if (!paintsIntoProvidedBacking && renderer.isAbsolutelyPositioned() && compositingAncestor && layer.hasCompositedScrollingAncestor()) {
-        if (layerContainingBlockCrossesCoordinatedScrollingBoundary(layer, *compositingAncestor)) {
+    // If this layer scrolls independently from the layer that it would paint into, it needs to get composited.
+    if (!paintsIntoProvidedBacking && layer.hasCompositedScrollingAncestor()) {
+        auto* paintDestination = layer.paintOrderParent();
+        bool paintDestinationIsScrolling = false;
+        auto* scrollingAncestor = enclosingCompositedScrollingLayer(layer, *paintDestination, paintDestinationIsScrolling);
+        if (isNonScrolledLayerInsideScrolledCompositedAncestor(layer, *paintDestination, *scrollingAncestor)) {
             reason = IndirectCompositingReason::OverflowScrollPositioning;
             return true;
         }
@@ -3045,43 +3086,6 @@ bool RenderLayerCompositor::useCoordinatedScrollingForLayer(const RenderLayer& l
     return false;
 }
 
-static RenderLayer* enclosingCompositedScrollingLayer(const RenderLayer& layer, const RenderLayer& intermediateLayer, bool& sawIntermediateLayer)
-{
-    const auto* currLayer = layer.parent();
-    while (currLayer) {
-        if (currLayer == &intermediateLayer)
-            sawIntermediateLayer = true;
-
-        if (currLayer->hasCompositedScrollableOverflow())
-            return const_cast<RenderLayer*>(currLayer);
-
-        currLayer = currLayer->parent();
-    }
-
-    return nullptr;
-}
-
-static bool isScrolledByOverflowScrollLayer(const RenderLayer& layer, const RenderLayer& overflowScrollLayer)
-{
-    bool scrolledByOverflowScroll = false;
-    traverseAncestorLayers(layer, [&](const RenderLayer& ancestorLayer, bool inContainingBlockChain, bool) {
-        if (&ancestorLayer == &overflowScrollLayer) {
-            scrolledByOverflowScroll = inContainingBlockChain;
-            return AncestorTraversal::Stop;
-        }
-        return AncestorTraversal::Continue;
-    });
-    return scrolledByOverflowScroll;
-}
-
-static bool isNonScrolledLayerInsideScrolledCompositedAncestor(const RenderLayer& layer, const RenderLayer& compositedAncestor, const RenderLayer& scrollingAncestor)
-{
-    bool ancestorMovedByScroller = &compositedAncestor == &scrollingAncestor || isScrolledByOverflowScrollLayer(compositedAncestor, scrollingAncestor);
-    bool layerMovedByScroller = isScrolledByOverflowScrollLayer(layer, scrollingAncestor);
-
-    return ancestorMovedByScroller && !layerMovedByScroller;
-}
-
 bool RenderLayerCompositor::layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
 {
     bool compositedAncestorIsInsideScroller = false;
index 12c1353..568ab4d 100644 (file)
@@ -479,7 +479,7 @@ private:
     bool requiresCompositingForPosition(RenderLayerModelObject&, const RenderLayer&, RequiresCompositingData&) const;
     bool requiresCompositingForOverflowScrolling(const RenderLayer&, RequiresCompositingData&) const;
     bool requiresCompositingForEditableImage(RenderLayerModelObject&) const;
-    bool requiresCompositingForIndirectReason(const RenderLayer&, const RenderLayer* compositingAncestor, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason&) const;
+    bool requiresCompositingForIndirectReason(const RenderLayer&, bool hasCompositedDescendants, bool has3DTransformedDescendants, bool paintsIntoProvidedBacking, IndirectCompositingReason&) const;
 
     static bool layerContainingBlockCrossesCoordinatedScrollingBoundary(const RenderLayer&, const RenderLayer& compositedAncestor);