RESOLVED FIXED 179946
ScrollingStateNode::reconcileLayerPositionForViewportRect is only called on direct children of the root 
https://bugs.webkit.org/show_bug.cgi?id=179946
Summary ScrollingStateNode::reconcileLayerPositionForViewportRect is only called on d...
Frédéric Wang (:fredw)
Reported 2017-11-22 07:18:27 PST
Created attachment 327452 [details] Testcase AsyncScrollingCoordinator::reconcileViewportConstrainedLayerPositions has the following comment: // FIXME: We'll have to traverse deeper into the tree at some point. for (auto& child : *children) child->reconcileLayerPositionForViewportRect(viewportRect, action); I'm not sure whether this is affecting bug 154399 but the attached testcase shows that reconcileLayerPositionForViewportRect is not called on the fixed node when one scrolls the main frame.
Attachments
Testcase (1.52 KB, text/html)
2017-11-22 07:18 PST, Frédéric Wang (:fredw) 		no flags
Details
WIP Patch (5.36 KB, patch)
2017-11-22 07:27 PST, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (12.46 KB, patch)
2018-01-15 07:03 PST, Frédéric Wang (:fredw) 		no flags
DetailsFormatted DiffDiff
Patch (17.20 KB, patch)
2018-01-16 01:44 PST, Frédéric Wang (:fredw) 		simon.fraser: review+
DetailsFormatted DiffDiff
Show Obsolete (2) View All Add attachment proposed patch, testcase, etc.
Frédéric Wang (:fredw)
Comment 1 2017-11-22 07:27:20 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.
Frédéric Wang (:fredw)
Comment 2 2018-01-15 07:03:12 PST
Created attachment 331334 [details] Patch
Darin Adler
Comment 3 2018-01-15 14:55:18 PST
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?
Frédéric Wang (:fredw)
Comment 4 2018-01-16 01:44:11 PST
Created attachment 331376 [details] Patch
Frédéric Wang (:fredw)
Comment 5 2018-01-16 01:46:13 PST
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.
Frédéric Wang (:fredw)
Comment 6 2018-01-16 01:47:20 PST
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) )
Frédéric Wang (:fredw)
Comment 7 2018-01-24 02:48:40 PST
@Simon: Can you please review that patch?
Frédéric Wang (:fredw)
Comment 8 2018-01-24 23:59:12 PST
Committed r227596: <https://trac.webkit.org/changeset/227596>
Radar WebKit Bug Importer
Comment 9 2018-01-25 00:00:37 PST
<rdar://problem/36857233>
Note You need to log in before you can comment on or make changes to this bug.