Bug 196508

Summary: Async overflow scroll on iOS paints slowly if it has a negative z-index child
Product: WebKit Reporter: Simon Fraser (smfr) <simon.fraser>
Component: ScrollingAssignee: Simon Fraser (smfr) <simon.fraser>
Status: ASSIGNED    
Severity: Normal CC: commit-queue, dino, simon.fraser, tsavell, 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=198458
Attachments:
Description Flags
Testcase
none
Patch
dino: review+
Patch none

Simon Fraser (smfr)
Reported 2019-04-02 13:42:23 PDT
Created attachment 366531 [details] Testcase We seem to get the clipping wrong when painting if we have a negative z-index child.
Attachments
Testcase (1.17 KB, text/html)
2019-04-02 13:42 PDT, Simon Fraser (smfr)
no flags
Patch (15.22 KB, patch)
2019-06-01 12:18 PDT, Simon Fraser (smfr)
dino: review+
Patch (20.89 KB, patch)
2019-06-01 16:25 PDT, Simon Fraser (smfr)
no flags
Radar WebKit Bug Importer
Comment 1 2019-04-02 13:49:20 PDT
Simon Fraser (smfr)
Comment 2 2019-04-02 18:53:11 PDT
We seem to be using the main-thread-computed clip rects when painting, and should not.
Simon Fraser (smfr)
Comment 3 2019-05-30 17:12:43 PDT
We're clipping the foreground layer to the visible area which is wrong.
Simon Fraser (smfr)
Comment 4 2019-05-30 22:20:39 PDT
And we're triggering repaints on every scroll when setting offsetFromRendering, and resizing the layer.
Simon Fraser (smfr)
Comment 5 2019-06-01 11:47:21 PDT
Fixing flashiness in bug 198458.
Simon Fraser (smfr)
Comment 6 2019-06-01 12:18:32 PDT
Dean Jackson
Comment 7 2019-06-01 14:15:10 PDT
Comment on attachment 371119 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=371119&action=review > Source/WebCore/ChangeLog:13 > + Fix by updating the fore- and background-layers last (nothing elese has dependencies typo: else > Source/WebCore/rendering/RenderLayerBacking.cpp:1274 > + if (m_scrolledContentsLayer) { > + foregroundSize = m_scrolledContentsLayer->size(); > + foregroundOffset = m_scrolledContentsLayer->offsetFromRenderer() - toLayoutSize(m_scrolledContentsLayer->scrollOffset()); > + needsDisplayOnOffsetChange = GraphicsLayer::DontSetNeedsDisplay; > + } else { > + foregroundSize = primaryGraphicsLayerRect.size(); > + foregroundOffset = m_graphicsLayer->offsetFromRenderer(); > + } > + > + if (hasClippingLayer()) { > + // If we have a clipping layer (which clips descendants), then the foreground layer is a child of it, > + // so that it gets correctly sorted with children. In that case, position relative to the clipping layer. > + foregroundSize = FloatSize(clippingBox.size()); > + foregroundOffset = toFloatSize(clippingBox.location()); > + } This can be if (hasClippingLayer()) { } else { if (m_scrolledContentsLayer) { } else { } } Although I guess you still need the needsDisplayOnOffsetChange... :(
Simon Fraser (smfr)
Comment 8 2019-06-01 16:25:19 PDT
WebKit Commit Bot
Comment 9 2019-06-01 19:56:02 PDT
Comment on attachment 371129 [details] Patch Clearing flags on attachment: 371129 Committed r246018: <https://trac.webkit.org/changeset/246018>
Truitt Savell
Comment 10 2019-06-03 13:47:08 PDT
The new test compositing/repaint/scroller-with-foreground-layer-repaints.html added in https://trac.webkit.org/changeset/246018/webkit is flakey: History: http://webkit-test-results.webkit.org/dashboards/flakiness_dashboard.html#showAllRuns=true&tests=compositing%2Frepaint%2Fscroller-with-foreground-layer-repaints.html Diff: --- /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/compositing/repaint/scroller-with-foreground-layer-repaints-expected.txt +++ /Volumes/Data/slave/mojave-release-tests-wk1/build/layout-test-results/compositing/repaint/scroller-with-foreground-layer-repaints-actual.txt @@ -33,9 +33,6 @@ (offsetFromRenderer width=1 height=1) (bounds 305.00 305.00) (drawsContent 1) - (repaint rects - (rect 0.00 0.00 305.00 305.00) - ) ) ) )
Simon Fraser (smfr)
Comment 11 2019-06-03 14:36:23 PDT
This is the standard WK1 flakiness. Please mark the test as flakey or skipped.
Truitt Savell
Comment 12 2019-06-11 14:45:13 PDT
Note You need to log in before you can comment on or make changes to this bug.