RESOLVED FIXED 208654
When using the scrolling thread, push ScrollingNodeIDs onto PlatformCALayers
https://bugs.webkit.org/show_bug.cgi?id=208654
Summary When using the scrolling thread, push ScrollingNodeIDs onto PlatformCALayers
Simon Fraser (smfr)
Reported 2020-03-05 10:39:29 PST
When using the scrolling thread, push ScrollingNodeIDs onto PlatformCALayers
Attachments
Patch (19.65 KB, patch)
2020-03-05 10:43 PST, Simon Fraser (smfr)
koivisto: review+
Simon Fraser (smfr)
Comment 1 2020-03-05 10:43:19 PST
Radar WebKit Bug Importer
Comment 2 2020-03-05 10:43:57 PST
Antti Koivisto
Comment 3 2020-03-05 11:09:24 PST
Comment on attachment 392597 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=392597&action=review Still r+ to keep things moving. > Source/WebCore/ChangeLog:12 > + We only need one ScrollingNodeID per platform layer, since a given platform layer only > + ever has one scrolling role. I don't think this is true. Specifically a single layer can meaningfully have both ViewportConstrained and Positioning roles and the scrolling tree code deals with this.
Antti Koivisto
Comment 4 2020-03-05 11:13:49 PST
We create a ScrollingTreePositionedNode with a ScrollingTreeFixedNode as its child in this case, both pointing to the same layer. See this code in ScrollingTreeFixedNode::applyLayerPositions() dealing with it: 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; }
Simon Fraser (smfr)
Comment 5 2020-03-05 14:38:04 PST
(In reply to Antti Koivisto from comment #4) > We create a ScrollingTreePositionedNode with a ScrollingTreeFixedNode as its > child in this case, both pointing to the same layer. > > See this code in ScrollingTreeFixedNode::applyLayerPositions() dealing with > it: > > 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; > } True, but for the purposes of overflow scroll hit-testing I don't care about fixed and positioned nodes.
Simon Fraser (smfr)
Comment 6 2020-03-05 14:59:13 PST
Note You need to log in before you can comment on or make changes to this bug.