WebKit Bugzilla
New
Browse
Log In
×
Sign in with GitHub
or
Remember my login
Create Account
·
Forgot Password
Forgotten password account recovery
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+
Details
Formatted Diff
Diff
View All
Add attachment
proposed patch, testcase, etc.
Simon Fraser (smfr)
Comment 1
2020-03-05 10:43:19 PST
Created
attachment 392597
[details]
Patch
Radar WebKit Bug Importer
Comment 2
2020-03-05 10:43:57 PST
<
rdar://problem/60090331
>
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
https://trac.webkit.org/changeset/257949/webkit
Note
You need to
log in
before you can comment on or make changes to this bug.
Top of Page
Format For Printing
XML
Clone This Bug