https://www.fandom.com/explore
<rdar://problem/51589759>
Created attachment 371955 [details] patch
Comment on attachment 371955 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=371955&action=review > Source/WebCore/ChangeLog:3 > + REGRESSION(r245818): Async scrolling: Fixed position banner jumps around page Change the title to make it clear this is about fixed inside sticky. > Source/WebCore/page/scrolling/cocoa/ScrollingTreeFixedNode.mm:106 > + 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<ScrollingTreeStickyNode>(*ancestor); > + overflowScrollDelta += stickyNode.scrollDeltaSinceLastCommit(); > + lastStickyNode = &stickyNode; > + continue; > + } It seems weird for ScrollingTreeFixedNode to have to test all its ancestor node types. This was the problem that the old delta was trying to solve, I think; these ancestors should pass state down the tree for the applyLayerPositions() tree walk. > Source/WebCore/rendering/RenderLayerCompositor.cpp:3057 > + if (ancestor->hasCompositedScrollableOverflow()) > + return true; But this might not be in the ancestor paint order chain. A simple fixed -> non-stacking overflow -> fixed should not need to composite the inner fixed. I think we only need to composite the inner fixed if some paint-order ancestor has a scrolling tree node (but we can't do that check at this stage).
> It seems weird for ScrollingTreeFixedNode to have to test all its ancestor > node types. This was the problem that the old delta was trying to solve, I > think; these ancestors should pass state down the tree for the > applyLayerPositions() tree walk. The old approach had multiple problems. It didn't work unless update started from the root (which it doesn't in most cases). It couldn't handle sticky without additional parameters. This is much easier to reason about and when we have decent test coverage we can factor this differently. > > Source/WebCore/rendering/RenderLayerCompositor.cpp:3057 > > + if (ancestor->hasCompositedScrollableOverflow()) > > + return true; > > But this might not be in the ancestor paint order chain. A simple fixed -> > non-stacking overflow -> fixed should not need to composite the inner fixed. > I think we only need to composite the inner fixed if some paint-order > ancestor has a scrolling tree node (but we can't do that check at this > stage). Unnecessary nodes should be harmless. You'll probably need to write the optimal version of this.
(In reply to Antti Koivisto from comment #4) > Unnecessary nodes should be harmless. You'll probably need to write the > optimal version of this. They have memory cost, potentially significant (triggering jetsams).
Created attachment 371973 [details] patch
Comment on attachment 371973 [details] patch Clearing flags on attachment: 371973 Committed r246367: <https://trac.webkit.org/changeset/246367>
All reviewed patches have been landed. Closing bug.