Bug 193539

Created attachment 359389 [details] wip

Comment on attachment 359389 [details] wip View in context: https://bugs.webkit.org/attachment.cgi?id=359389&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:3421 > + m_clipLayer = m_scrollLayer; Hmm, this seems like it could lead to programming mistakes. I don't think any other code considers the fact that these could be the same layer.

Not sure how different this bug is, but FYI I had patches in bug 173833, bug 192303, bug 182868 with various tests that were pending review.

(In reply to Frédéric Wang (:fredw) from comment #3) > Not sure how different this bug is, but FYI I had patches in bug 173833, bug > 192303, bug 182868 with various tests that were pending review. Interesting! I wasn't aware of that patch. Looks very similar. The main differences seem to be - this patch ties to asyncFrameScrollingEnabled internal setting and doesn't enable the feature by default - eliminates the unnecessary clip layer in async scrolling case (UIScrollView clips). - uses the same ScrolliongTreeFrameScrollingNode type for both main frame and subframe, switching operation based on the scroll layer type (so the setting also works) - is missing some of the bug fixes - doesn't add tests I propose we land this version as a starting point that can be experimented with and then work towards integrating your tests and bug fixes. How does that sound?

(In reply to Antti Koivisto from comment #4) > (In reply to Frédéric Wang (:fredw) from comment #3) > > Not sure how different this bug is, but FYI I had patches in bug 173833, bug > > 192303, bug 182868 with various tests that were pending review. > > Interesting! I wasn't aware of that patch. Looks very similar. The main > differences seem to be > > - this patch ties to asyncFrameScrollingEnabled internal setting and doesn't > enable the feature by default > - eliminates the unnecessary clip layer in async scrolling case > (UIScrollView clips). > - uses the same ScrolliongTreeFrameScrollingNode type for both main frame > and subframe, switching operation based on the scroll layer type (so the > setting also works) > - is missing some of the bug fixes > - doesn't add tests > > I propose we land this version as a starting point that can be experimented > with and then work towards integrating your tests and bug fixes. How does > that sound? OK, that sounds good to me. I think the two last points are important to have at some point :-)

<rdar://problem/47379873>

Created attachment 359465 [details] patch

Created attachment 359466 [details] patch

Created attachment 359474 [details] patch

Created attachment 359475 [details] patch

Created attachment 359476 [details] patch

Attachment 359476 [details] did not pass style-queue: ERROR: Source/WebCore/page/scrolling/ios/ScrollingTreeFrameScrollingNodeIOS.h:64: Inline functions should not be in classes annotated with WEBCORE_EXPORT. Remove the macro from the class and apply it to each appropriate method, or move the inline function definition out-of-line. [build/webcore_export] [4] Total errors found: 1 in 14 files If any of these errors are false positives, please file a bug against check-webkit-style.

Created attachment 359478 [details] patch

Comment on attachment 359478 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=359478&action=review > Source/WebCore/rendering/RenderLayerCompositor.cpp:3415 > +#if PLATFORM(IOS) if PLATFORM(IOS_FAMILY) Ideally this wouldn't be a platform #ifdef, but something else. > LayoutTests/platform/ios-wk2/compositing/tiling/tiled-drawing-async-frame-scrolling-expected.txt:46 > - (coverage rect 0.00, 0.00 300.00 x 150.00) > + (coverage rect -10.00, -10.00 800.00 x 600.00) This seems odd.

Created attachment 359513 [details] patch

Created attachment 359514 [details] patch

Comment on attachment 359514 [details] patch Clearing flags on attachment: 359514 Committed r240162: <https://trac.webkit.org/changeset/240162>

All reviewed patches have been landed. Closing bug.

*** Bug 173833 has been marked as a duplicate of this bug. ***