Merge 254509@main - [Nicosia] Async Scrolling: some elements are jump…

…y in gitlab https://bugs.webkit.org/show_bug.cgi?id=245175 Reviewed by Simon Fraser and Žan Doberšek. Bring several fixes for fixed elements that landed in cocoa but not in nicosia specific code. * Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp: (WebCore::ScrollingTreeFixedNode::applyLayerPositions): * Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.cpp: (WebCore::ScrollingTreeOverflowScrollProxyNode::scrollDeltaSinceLastCommit const): * Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.h: * Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.cpp: (WebCore::ScrollingTreePositionedNode::scrollDeltaSinceLastCommit const): (WebCore::ScrollingTreePositionedNode::applyLayerPositions): * Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.h: * Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNodeNicosia.h: Canonical link: https://commits.webkit.org/254509@main (cherry picked from commit 414681c)
WebKit · Sep 15, 2022 · 0ee81e0 · 0ee81e0
1 parent 3945f86
commit 0ee81e0
Show file tree

Hide file tree

Showing 6 changed files with 67 additions and 10 deletions.
diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp b/Source/WebCore/page/scrolling/nicosia/ScrollingTreeFixedNode.cpp
@@ -36,7 +36,10 @@
 #include "ScrollingStateFixedNode.h"
 #include "ScrollingTree.h"
 #include "ScrollingTreeFrameScrollingNode.h"
+#include "ScrollingTreeOverflowScrollProxyNode.h"
 #include "ScrollingTreeOverflowScrollingNode.h"
+#include "ScrollingTreePositionedNode.h"
+#include "ScrollingTreeStickyNodeNicosia.h"
 #include <wtf/text/TextStream.h>
 
 namespace WebCore {
@@ -74,21 +77,50 @@ void ScrollingTreeFixedNode::applyLayerPositions()
 {
     auto computeLayerPosition = [&] {
         FloatSize overflowScrollDelta;
-        // FIXME: This code is wrong in complex cases where the fixed element is inside a positioned node as
-        //        the scroll container order does not match the scrolling tree ancestor order.
-        for (auto* node = parent(); node; node = node->parent()) {
-            if (is<ScrollingTreeFrameScrollingNode>(*node)) {
+        ScrollingTreeStickyNodeNicosia* lastStickyNode = nullptr;
+        for (auto* ancestor = parent(); ancestor; ancestor = ancestor->parent()) {
+            if (is<ScrollingTreeFrameScrollingNode>(*ancestor)) {
                 // Fixed nodes are positioned relative to the containing frame scrolling node.
                 // We bail out after finding one.
-                auto layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(*node).layoutViewport();
+                auto layoutViewport = downcast<ScrollingTreeFrameScrollingNode>(*ancestor).layoutViewport();
                 return m_constraints.layerPositionForViewportRect(layoutViewport) - overflowScrollDelta;
             }
 
-            if (is<ScrollingTreeOverflowScrollingNode>(*node)) {
+            if (is<ScrollingTreeOverflowScrollingNode>(*ancestor)) {
                 // To keep the layer still during async scrolling we adjust by how much the position has changed since layout.
-                auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*node);
-                auto localDelta = overflowNode.lastCommittedScrollPosition() - overflowNode.currentScrollPosition();
-                overflowScrollDelta += localDelta;
+                auto& overflowNode = downcast<ScrollingTreeOverflowScrollingNode>(*ancestor);
+                overflowScrollDelta -= overflowNode.scrollDeltaSinceLastCommit();
+                continue;
+            }
+
+            if (is<ScrollingTreeOverflowScrollProxyNode>(*ancestor)) {
+                // To keep the layer still during async scrolling we adjust by how much the position has changed since layout.
+                auto& overflowNode = downcast<ScrollingTreeOverflowScrollProxyNode>(*ancestor);
+                overflowScrollDelta -= overflowNode.scrollDeltaSinceLastCommit();
+                continue;
+            }
+
+            if (is<ScrollingTreePositionedNode>(*ancestor)) {
+                auto& positioningAncestor = downcast<ScrollingTreePositionedNode>(*ancestor);
+                // See if sticky node already handled this positioning node.
+                // FIXME: Include positioning node information to sticky/fixed node to avoid these tests.
+                if (lastStickyNode && lastStickyNode->layer() == positioningAncestor.layer())
+                    continue;
+                if (positioningAncestor.layer() != m_layer)
+                    overflowScrollDelta -= positioningAncestor.scrollDeltaSinceLastCommit();
+                continue;
+            }
+
+            if (is<ScrollingTreeStickyNode>(*ancestor)) {
+                auto& stickyNode = downcast<ScrollingTreeStickyNodeNicosia>(*ancestor);
+                overflowScrollDelta += stickyNode.scrollDeltaSinceLastCommit();
+                lastStickyNode = &stickyNode;
+                continue;
+            }
+
+            if (is<ScrollingTreeFixedNode>(*ancestor)) {
+                // The ancestor fixed node has already applied the needed corrections to say put.
+                return m_constraints.layerPositionAtLastLayout() - overflowScrollDelta;
             }
         }
         ASSERT_NOT_REACHED();

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.cpp b/Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.cpp
@@ -73,6 +73,16 @@ void ScrollingTreeOverflowScrollProxyNode::commitStateBeforeChildren(const Scrol
     }
 }
 
+FloatSize ScrollingTreeOverflowScrollProxyNode::scrollDeltaSinceLastCommit() const
+{
+    if (auto* node = scrollingTree().nodeForID(m_overflowScrollingNodeID)) {
+        if (is<ScrollingTreeOverflowScrollingNode>(node))
+            return downcast<ScrollingTreeOverflowScrollingNode>(*node).scrollDeltaSinceLastCommit();
+    }
+
+    return { };
+}
+
 void ScrollingTreeOverflowScrollProxyNode::applyLayerPositions()
 {
     FloatPoint scrollOffset;

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.h b/Source/WebCore/page/scrolling/nicosia/ScrollingTreeOverflowScrollProxyNode.h
@@ -43,6 +43,8 @@ class ScrollingTreeOverflowScrollProxyNode final : public ScrollingTreeNode {
     static Ref<ScrollingTreeOverflowScrollProxyNode> create(ScrollingTree&, ScrollingNodeID);
     virtual ~ScrollingTreeOverflowScrollProxyNode();
 
+    FloatSize scrollDeltaSinceLastCommit() const;
+
     ScrollingNodeID overflowScrollingNodeID() const { return m_overflowScrollingNodeID; }
 
 private:

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.cpp b/Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.cpp
@@ -71,7 +71,7 @@ void ScrollingTreePositionedNode::commitStateBeforeChildren(const ScrollingState
         scrollingTree().activePositionedNodes().add(*this);
 }
 
-void ScrollingTreePositionedNode::applyLayerPositions()
+FloatSize ScrollingTreePositionedNode::scrollDeltaSinceLastCommit() const
 {
     FloatSize delta;
     for (auto nodeID : m_relatedOverflowScrollingNodes) {
@@ -83,6 +83,13 @@ void ScrollingTreePositionedNode::applyLayerPositions()
         }
     }
 
+    // Positioned nodes compensate for scrolling, so negate the scroll delta.
+    return -delta;
+}
+
+void ScrollingTreePositionedNode::applyLayerPositions()
+{
+    FloatSize delta = scrollDeltaSinceLastCommit();
     FloatPoint layerPosition = m_constraints.layerPositionAtLastLayout() + delta;
 
     LOG_WITH_STREAM(Scrolling, stream << "ScrollingTreePositionedNode " << scrollingNodeID() << " applyLayerPositions: overflow delta " << delta << " moving layer to " << layerPosition);

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.h b/Source/WebCore/page/scrolling/nicosia/ScrollingTreePositionedNode.h
@@ -45,8 +45,12 @@ class ScrollingTreePositionedNode final : public ScrollingTreeNode {
     static Ref<ScrollingTreePositionedNode> create(ScrollingTree&, ScrollingNodeID);
     virtual ~ScrollingTreePositionedNode();
 
+    Nicosia::CompositionLayer* layer() const { return m_layer.get(); }
+
     const Vector<ScrollingNodeID>& relatedOverflowScrollingNodes() const { return m_relatedOverflowScrollingNodes; }
 
+    FloatSize scrollDeltaSinceLastCommit() const;
+
 private:
     ScrollingTreePositionedNode(ScrollingTree&, ScrollingNodeID);
 

diff --git a/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNodeNicosia.h b/Source/WebCore/page/scrolling/nicosia/ScrollingTreeStickyNodeNicosia.h
@@ -45,6 +45,8 @@ class ScrollingTreeStickyNodeNicosia final : public ScrollingTreeStickyNode {
     static Ref<ScrollingTreeStickyNodeNicosia> create(ScrollingTree&, ScrollingNodeID);
     virtual ~ScrollingTreeStickyNodeNicosia() = default;
 
+    Nicosia::CompositionLayer* layer() const { return m_layer.get(); }
+
 private:
     ScrollingTreeStickyNodeNicosia(ScrollingTree&, ScrollingNodeID);