Summary: | ScrollingStateNode::reconcileLayerPositionForViewportRect is only called on direct children of the root | ||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | WebKit | Reporter: | Frédéric Wang (:fredw) <fred.wang> | ||||||||||
Component: | Frames | Assignee: | Frédéric Wang (:fredw) <fred.wang> | ||||||||||
Status: | RESOLVED FIXED | ||||||||||||
Severity: | Normal | CC: | cmarcelo, darin, dvoytenko, ews-watchlist, jamesr, luiz, simon.fraser, tonikitoo, webkit-bug-importer | ||||||||||
Priority: | P2 | Keywords: | InRadar | ||||||||||
Version: | WebKit Nightly Build | ||||||||||||
Hardware: | Unspecified | ||||||||||||
OS: | Unspecified | ||||||||||||
See Also: | https://bugs.webkit.org/show_bug.cgi?id=180002 | ||||||||||||
Bug Depends on: | |||||||||||||
Bug Blocks: | 154399 | ||||||||||||
Attachments: |
|
Description
Frédéric Wang (:fredw)
2017-11-22 07:18:27 PST
Created attachment 327453 [details]
WIP Patch
This is patch to recursively calls reconcileLayerPositionForViewportRect but I'm not sure how to emulate non-programmatic scroll on iOS in order to write a test.
Created attachment 331334 [details]
Patch
Comment on attachment 331334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331334&action=review > Source/WebCore/page/scrolling/ScrollingStateNode.cpp:103 > +void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action) ScrollingStateStickyNode::reconcileLayerPositionForViewportRect and ScrollingStateFixedNode::reconcileLayerPositionForViewportRect don’t call through to ScrollingStateNode::reconcileLayerPositionForViewportRect. Can those nodes have children? Is it OK to not call reconcileLayerPositionForViewportRect on those children? Created attachment 331376 [details]
Patch
Comment on attachment 331334 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=331334&action=review >> Source/WebCore/page/scrolling/ScrollingStateNode.cpp:103 >> +void ScrollingStateNode::reconcileLayerPositionForViewportRect(const LayoutRect& viewportRect, ScrollingLayerPositionAction action) > > ScrollingStateStickyNode::reconcileLayerPositionForViewportRect and ScrollingStateFixedNode::reconcileLayerPositionForViewportRect don’t call through to ScrollingStateNode::reconcileLayerPositionForViewportRect. > > Can those nodes have children? Is it OK to not call reconcileLayerPositionForViewportRect on those children? Yes, I guess sticky/fixed nodes should also call reconcileLayerPositionForViewportRect on their descendants. I modified the code and extended the tests accordingly. For the record, here is the diff of the graphic layers of the test before and after the patch: diff --git a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt index fc9b542a67d..1b6f6dbeaba 100644 --- a/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt +++ b/LayoutTests/fast/scrolling/ios/reconcile-layer-position-recursive-expected.txt @@ -70,44 +70,48 @@ (contentsScale 2.00) (children 2 (GraphicsLayer - (position 7.00 -993.00) + (position 7.00 28.00) + (approximate position 7.00 28.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 0.00 x 0.00) - (coverage rect -7.00, 993.00 200.00 x 245.00) + (visible rect 0.00, 0.00 110.00 x 110.00) + (coverage rect -7.00, -28.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) (children 1 (GraphicsLayer (position 55.00 55.00) + (approximate position 55.00 55.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 0.00 x 0.00) - (coverage rect -62.00, 938.00 200.00 x 245.00) + (visible rect 0.00, 0.00 110.00 x 110.00) + (coverage rect -62.00, -83.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) ) ) ) (GraphicsLayer - (position 105.00 105.00) + (position 105.00 395.00) + (approximate position 105.00 395.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 95.00 x 110.00) - (coverage rect -105.00, -105.00 200.00 x 245.00) + (visible rect 0.00, 0.00 0.00 x 0.00) + (coverage rect -105.00, -395.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) (children 1 (GraphicsLayer (position 55.00 55.00) + (approximate position 55.00 55.00) (bounds 110.00 110.00) (contentsOpaque 1) (drawsContent 1) - (visible rect 0.00, 0.00 40.00 x 85.00) - (coverage rect -160.00, -160.00 200.00 x 245.00) + (visible rect 0.00, 0.00 0.00 x 0.00) + (coverage rect -160.00, -450.00 200.00 x 245.00) (intersects coverage rect 1) (contentsScale 2.00) ) @Simon: Can you please review that patch? Committed r227596: <https://trac.webkit.org/changeset/227596> |