Absolute in stacking-context scroller jiggles when scrolled
authorsimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 18:58:09 +0000 (18:58 +0000)
committersimon.fraser@apple.com <simon.fraser@apple.com@268f45cc-cd09-0410-ab3c-d52691b4dbfc>
Thu, 21 Mar 2019 18:58:09 +0000 (18:58 +0000)
https://bugs.webkit.org/show_bug.cgi?id=196010

Reviewed by Zalan Bujtas.

Source/WebCore:

Updating compositing layers after a scroll (in a compositing update on the main thread)
failed to traverse to an absolute layer inside a stacking-context overflow:scroll,
because the overflow's layer didn't have the "hasCompositingAncestor" bit set on it.

This happened because childState.subtreeIsCompositing wasn't being set when indirect
reasons trigger compositing. So clean up RenderLayerCompositor::computeCompositingRequirements()
to set childState.subtreeIsCompositing for "late" compositing decisions, and move the
"Subsequent layers in the parent stacking context also need to composite" chunk
down to after the last compositing decision has been made.

Test: compositing/overflow/absolute-in-overflow.html

* page/scrolling/ScrollingTreeScrollingNode.cpp:
(WebCore::ScrollingTreeScrollingNode::scrollTo):
* page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
(WebCore::ScrollingTreePositionedNode::applyLayerPositions):
* rendering/RenderLayerCompositor.cpp:
(WebCore::RenderLayerCompositor::computeCompositingRequirements):

LayoutTests:

* compositing/overflow/absolute-in-overflow-expected.html: Added.
* compositing/overflow/absolute-in-overflow.html: Added.

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

LayoutTests/ChangeLog
LayoutTests/compositing/overflow/absolute-in-overflow-expected.html [new file with mode: 0644]
LayoutTests/compositing/overflow/absolute-in-overflow.html [new file with mode: 0644]
Source/WebCore/ChangeLog
Source/WebCore/page/scrolling/ScrollingTreeScrollingNode.cpp
Source/WebCore/page/scrolling/cocoa/ScrollingTreePositionedNode.mm
Source/WebCore/rendering/RenderLayerCompositor.cpp

index 57fef80..bd9de71 100644 (file)
@@ -1,3 +1,13 @@
+2019-03-21  Simon Fraser  <simon.fraser@apple.com>
+
+        Absolute in stacking-context scroller jiggles when scrolled
+        https://bugs.webkit.org/show_bug.cgi?id=196010
+
+        Reviewed by Zalan Bujtas.
+
+        * compositing/overflow/absolute-in-overflow-expected.html: Added.
+        * compositing/overflow/absolute-in-overflow.html: Added.
+
 2019-03-21  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] Start tracking implicit transitions at touchStart
diff --git a/LayoutTests/compositing/overflow/absolute-in-overflow-expected.html b/LayoutTests/compositing/overflow/absolute-in-overflow-expected.html
new file mode 100644 (file)
index 0000000..477fd02
--- /dev/null
@@ -0,0 +1,61 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            margin: 0;
+        }
+        #scroller {
+            margin: 10px;
+            background-color: silver;
+            width: 400px;
+            height: 300px;
+            overflow: scroll;
+            opacity: 0.8;
+        }
+        
+        .scrolling-content {
+            height: 1000px;
+        }
+        
+        .absolute {
+            top: 20px;
+            left: 20px;
+            width: 200px;
+            height: 200px;
+            background-color: blue;
+        }
+        
+        .changed {
+            position: absolute;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        if (window.internals)
+            window.internals.settings.setAsyncOverflowScrollingEnabled(true);
+
+        function doTest()
+        {
+            setTimeout(() => {
+                scroller.scrollTop = 200;
+                target.classList.add('changed');
+
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0);
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div id="scroller">
+        <div class="scrolling-content">
+            <div id="target" class="absolute"></div>
+        </div>
+    </div>
+</body>
+</html>
diff --git a/LayoutTests/compositing/overflow/absolute-in-overflow.html b/LayoutTests/compositing/overflow/absolute-in-overflow.html
new file mode 100644 (file)
index 0000000..99e6204
--- /dev/null
@@ -0,0 +1,57 @@
+<!DOCTYPE html>
+<html>
+<head>
+    <style>
+        body {
+            margin: 0;
+        }
+        #scroller {
+            margin: 10px;
+            background-color: silver;
+            width: 400px;
+            height: 300px;
+            overflow: scroll;
+            opacity: 0.8;
+        }
+        
+        .scrolling-content {
+            height: 1000px;
+        }
+        
+        .absolute {
+            position: absolute;
+            top: 20px;
+            left: 20px;
+            width: 200px;
+            height: 200px;
+            background-color: blue;
+        }
+    </style>
+    <script>
+        if (window.testRunner)
+            testRunner.waitUntilDone();
+
+        if (window.internals)
+            window.internals.settings.setAsyncOverflowScrollingEnabled(true);
+
+        function doTest()
+        {
+            setTimeout(() => {
+                scroller.scrollTop = 200;
+
+                if (window.testRunner)
+                    testRunner.notifyDone();
+            }, 0);
+        }
+
+        window.addEventListener('load', doTest, false);
+    </script>
+</head>
+<body>
+    <div id="scroller">
+        <div class="scrolling-content">
+            <div class="absolute"></div>
+        </div>
+    </div>
+</body>
+</html>
index e2baead..3f106f5 100644 (file)
@@ -1,3 +1,29 @@
+2019-03-21  Simon Fraser  <simon.fraser@apple.com>
+
+        Absolute in stacking-context scroller jiggles when scrolled
+        https://bugs.webkit.org/show_bug.cgi?id=196010
+
+        Reviewed by Zalan Bujtas.
+
+        Updating compositing layers after a scroll (in a compositing update on the main thread)
+        failed to traverse to an absolute layer inside a stacking-context overflow:scroll,
+        because the overflow's layer didn't have the "hasCompositingAncestor" bit set on it.
+
+        This happened because childState.subtreeIsCompositing wasn't being set when indirect
+        reasons trigger compositing. So clean up RenderLayerCompositor::computeCompositingRequirements()
+        to set childState.subtreeIsCompositing for "late" compositing decisions, and move the
+        "Subsequent layers in the parent stacking context also need to composite" chunk
+        down to after the last compositing decision has been made.
+
+        Test: compositing/overflow/absolute-in-overflow.html
+
+        * page/scrolling/ScrollingTreeScrollingNode.cpp:
+        (WebCore::ScrollingTreeScrollingNode::scrollTo):
+        * page/scrolling/cocoa/ScrollingTreePositionedNode.mm:
+        (WebCore::ScrollingTreePositionedNode::applyLayerPositions):
+        * rendering/RenderLayerCompositor.cpp:
+        (WebCore::RenderLayerCompositor::computeCompositingRequirements):
+
 2019-03-21  Zalan Bujtas  <zalan@apple.com>
 
         [ContentChangeObserver] Start tracking implicit transitions at touchStart
index d2d97c4..bf4e83a 100644 (file)
@@ -161,6 +161,9 @@ void ScrollingTreeScrollingNode::scrollTo(const FloatPoint& position, ScrollPosi
         return;
 
     m_currentScrollPosition = adjustedScrollPosition(position, clamp);
+    
+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreeScrollingNode " << scrollingNodeID() << " scrollTo " << position << " (delta from last committed position " << (m_lastCommittedScrollPosition - m_currentScrollPosition) << ")");
+
     updateViewportForCurrentScrollPosition();
     currentScrollPositionChanged();
 }
index 3317753..c6b5863 100644 (file)
@@ -87,8 +87,6 @@ void ScrollingTreePositionedNode::applyLayerPositions(const FloatRect&, FloatSiz
             }
         }
     }
-    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << scrollOffsetSinceLastCommit);
-
     auto layerOffset = -scrollOffsetSinceLastCommit;
     if (m_constraints.scrollPositioningBehavior() == ScrollPositioningBehavior::Stationary) {
         // Stationary nodes move in the opposite direction.
@@ -96,6 +94,9 @@ void ScrollingTreePositionedNode::applyLayerPositions(const FloatRect&, FloatSiz
     }
 
     FloatPoint layerPosition = m_constraints.layerPositionAtLastLayout() - layerOffset;
+
+    LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << scrollOffsetSinceLastCommit << " moving layer to " << layerPosition);
+
     [m_layer _web_setLayerTopLeftPosition:layerPosition - m_constraints.alignmentOffset()];
 
     // FIXME: Should our scroller deltas propagate to descendants?
index e1abb72..8bada2b 100644 (file)
@@ -958,6 +958,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         && requiresCompositingForIndirectReason(layer.renderer(), compositingState.compositingAncestor, childState.subtreeIsCompositing, anyDescendantHas3DTransform, indirectCompositingReason)) {
         layer.setIndirectCompositingReason(indirectCompositingReason);
         childState.compositingAncestor = &layer;
+        childState.subtreeIsCompositing = true;
         overlapMap.pushCompositingContainer();
         addToOverlapMapRecursive(overlapMap, layer);
         willBeComposited = true;
@@ -968,10 +969,6 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         layer.reflectionLayer()->setIndirectCompositingReason(willBeComposited ? RenderLayer::IndirectCompositingReason::Stacking : RenderLayer::IndirectCompositingReason::None);
     }
 
-    // Subsequent layers in the parent stacking context also need to composite.
-    compositingState.subtreeIsCompositing |= childState.subtreeIsCompositing;
-    compositingState.fullPaintOrderTraversalRequired |= childState.fullPaintOrderTraversalRequired;
-
     // Set the flag to say that this layer has compositing children.
     layer.setHasCompositingDescendant(childState.subtreeIsCompositing);
 
@@ -987,6 +984,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     if (isCompositedClippingLayer) {
         if (!willBeComposited) {
             childState.compositingAncestor = &layer;
+            childState.subtreeIsCompositing = true;
             overlapMap.pushCompositingContainer();
             addToOverlapMapRecursive(overlapMap, layer);
             willBeComposited = true;
@@ -994,8 +992,7 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
     }
 
 #if ENABLE(CSS_COMPOSITING)
-    if ((willBeComposited && layer.hasBlendMode())
-        || (layer.hasNotIsolatedCompositedBlendingDescendants() && !layer.isolatesCompositedBlending()))
+    if ((willBeComposited && layer.hasBlendMode()) || (layer.hasNotIsolatedCompositedBlendingDescendants() && !layer.isolatesCompositedBlending()))
         compositingState.hasNotIsolatedCompositedBlendingDescendants = true;
 #endif
 
@@ -1013,7 +1010,11 @@ void RenderLayerCompositor::computeCompositingRequirements(RenderLayer* ancestor
         willBeComposited = false;
 #endif
     }
-    
+
+    // Subsequent layers in the parent stacking context also need to composite.
+    compositingState.subtreeIsCompositing |= childState.subtreeIsCompositing;
+    compositingState.fullPaintOrderTraversalRequired |= childState.fullPaintOrderTraversalRequired;
+
     ASSERT(willBeComposited == needsToBeComposited(layer, queryData));
 
     // Create or destroy backing here. However, we can't update geometry because layers above us may become composited