Changeset 243211 in webkit

Timestamp:

Mar 20, 2019 9:44:49 AM (5 years ago)

Author:

Simon Fraser

Message:

Some elements lag behind async overflow scrolling on palace-games.com
​https://bugs.webkit.org/show_bug.cgi?id=195934

Reviewed by Zalan Bujtas.
Source/WebCore:

The logic added in r242997 was wrong for an absolutely-positioned layer whose containig block was
inside an overflow:scroll, but whose compositing ancestor was outside. This is a case where
we need to make a Positioning node for the absolute layer, because it needs to move along
with the scrolled content.

There are six permutations of overflow, containing block and compositing ancestor that we care about.
Showing renderer (aka DOM) order, they are (where <- indicates child <- parent):

layer <- cb <- ca <- os -- no positioned node required
layer <- cb <- os <- ca -- compositing parent skips overflow, need a "Moved" Positioned node.
layer <- ov <- cb <- ca -- no positioned node required
layer <- ov <- ca <- cb -- no positioned node required
layer <- ca <- cb <- ov -- no positioned node required
layer <- ca <- ov <- cb -- containing block skips overflow, need a "Stationary" Positioned node.

[cb = containing block, ca = compositing ancestor, ov = overflow scroll]

Test: scrollingcoordinator/scrolling-tree/positioned-nodes-complex.html

• rendering/RenderLayer.cpp:

(WebCore::outputPaintOrderTreeRecursive):

• rendering/RenderLayerCompositor.cpp:

(WebCore::layerParentedAcrossCoordinatedScrollingBoundary):
(WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):

LayoutTests:

Add a test with more combinations of overflow, containing block and compositing ancestor.

• platform/ios-wk2/scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt: Added.
• scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt: Added.
• scrollingcoordinator/scrolling-tree/positioned-nodes-complex.html: Added.

Location:

trunk

Files:

• LayoutTests/ChangeLog (modified) (1 diff)
• LayoutTests/platform/ios-wk2/scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt (added)
• LayoutTests/scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt (added)
• LayoutTests/scrollingcoordinator/scrolling-tree/positioned-nodes-complex.html (added)
• Source/WebCore/ChangeLog (modified) (1 diff)
• Source/WebCore/rendering/RenderLayer.cpp (modified) (1 diff)
• Source/WebCore/rendering/RenderLayerCompositor.cpp (modified) (3 diffs)

trunk/LayoutTests/ChangeLog

@@ +1 @@
+2019-03-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Some elements lag behind async overflow scrolling on palace-games.com
+        https://bugs.webkit.org/show_bug.cgi?id=195934
+
+        Reviewed by Zalan Bujtas.
+        
+        Add a test with more combinations of overflow, containing block and compositing ancestor.
+
+        * platform/ios-wk2/scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/positioned-nodes-complex-expected.txt: Added.
+        * scrollingcoordinator/scrolling-tree/positioned-nodes-complex.html: Added.
+
 2019-03-20  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/ChangeLog

@@ +1 @@
+2019-03-20  Simon Fraser  <simon.fraser@apple.com>
+
+        Some elements lag behind async overflow scrolling on palace-games.com
+        https://bugs.webkit.org/show_bug.cgi?id=195934
+
+        Reviewed by Zalan Bujtas.
+
+        The logic added in r242997 was wrong for an absolutely-positioned layer whose containig block was
+        inside an overflow:scroll, but whose compositing ancestor was outside. This is a case where
+        we need to make a Positioning node for the absolute layer, because it needs to move along
+        with the scrolled content.
+        
+        There are six permutations of overflow, containing block and compositing ancestor that we care about.
+        Showing renderer (aka DOM) order, they are (where <- indicates child <- parent):
+        
+        layer <- cb <- ca <- os -- no positioned node required
+        layer <- cb <- os <- ca -- compositing parent skips overflow, need a "Moved" Positioned node.
+        layer <- ov <- cb <- ca -- no positioned node required
+        layer <- ov <- ca <- cb -- no positioned node required
+        layer <- ca <- cb <- ov -- no positioned node required
+        layer <- ca <- ov <- cb -- containing block skips overflow, need a "Stationary" Positioned node.
+        
+        [cb = containing block, ca = compositing ancestor, ov = overflow scroll]
+
+        Test: scrollingcoordinator/scrolling-tree/positioned-nodes-complex.html
+
+        * rendering/RenderLayer.cpp:
+        (WebCore::outputPaintOrderTreeRecursive):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::layerParentedAcrossCoordinatedScrollingBoundary):
+        (WebCore::RenderLayerCompositor::computeCoordinatedPositioningForLayer const):
+
 2019-03-20  Simon Fraser  <simon.fraser@apple.com>
 

trunk/Source/WebCore/rendering/RenderLayer.cpp

@@ -6747 +6747 @@
         auto positionedNodeID = backing.scrollingNodeIDForRole(WebCore::ScrollCoordinationRole::Positioning);
 
-        if (scrollingNodeID || frameHostingNodeID || viewportConstrainedNodeID) {
+        if (scrollingNodeID || frameHostingNodeID || viewportConstrainedNodeID || positionedNodeID) {
             stream << " {";
             bool first = true;

trunk/Source/WebCore/rendering/RenderLayerCompositor.cpp

@@ -2923 +2923 @@
 }
 
-// Is there scrollable overflow between this layer and its composited ancestor?
-static bool layerParentedAcrossCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor)
+// Is there scrollable overflow between this layer and its composited ancestor, and the containing block is the scroller or inside the scroller.
+static bool layerParentedAcrossCoordinatedScrollingBoundary(const RenderLayer& layer, const RenderLayer& compositedAncestor, bool checkContainingBlock, bool& containingBlockIsInsideOverflow)
 {
     ASSERT(layer.isComposited());
 
     for (const auto* currLayer = layer.parent(); currLayer != &compositedAncestor; currLayer = currLayer->parent()) {
+        if (checkContainingBlock && currLayer->renderer().canContainAbsolutelyPositionedObjects())
+            containingBlockIsInsideOverflow = true;
+
         if (currLayer->hasCompositedScrollableOverflow())
             return true;
@@ -2952 +2955 @@
 
     auto& renderer = layer.renderer();
-    if (renderer.isOutOfFlowPositioned() && renderer.style().position() == PositionType::Absolute) {
+    bool containingBlockCanSkipOverflow = renderer.isOutOfFlowPositioned() && renderer.style().position() == PositionType::Absolute;
+    if (containingBlockCanSkipOverflow) {
         if (layerContainingBlockCrossesCoordinatedScrollingBoundary(layer, *compositedAncestor))
             return ScrollPositioningBehavior::Stationary;
-
-        return ScrollPositioningBehavior::None;
     }
 
@@ -2962 +2964 @@
     //    outside the overflow:scroll. In that case, we have to move the layer via the scrolling tree to make
     //    it move along with the overflow scrolling.
-    if (layerParentedAcrossCoordinatedScrollingBoundary(layer, *compositedAncestor))
+    bool containingBlockIsInsideOverflow = !containingBlockCanSkipOverflow;
+    if (layerParentedAcrossCoordinatedScrollingBoundary(layer, *compositedAncestor, containingBlockCanSkipOverflow, containingBlockIsInsideOverflow) && containingBlockIsInsideOverflow)
         return ScrollPositioningBehavior::Moves;