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

Description Simon Fraser (smfr) 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.
Comment 1 Radar WebKit Bug Importer 2019-04-02 13:49:20 PDT
<rdar://problem/49532709>
Comment 2 Simon Fraser (smfr) 2019-04-02 18:53:11 PDT
We seem to be using the main-thread-computed clip rects when painting, and should not.
Comment 3 Simon Fraser (smfr) 2019-05-30 17:12:43 PDT
We're clipping the foreground layer to the visible area which is wrong.
Comment 4 Simon Fraser (smfr) 2019-05-30 22:20:39 PDT
And we're triggering repaints on every scroll when setting offsetFromRendering, and resizing the layer.
Comment 5 Simon Fraser (smfr) 2019-06-01 11:47:21 PDT
Fixing flashiness in bug 198458.
Comment 6 Simon Fraser (smfr) 2019-06-01 12:18:32 PDT
Created attachment 371119 [details]
Patch
Comment 7 Dean Jackson 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... :(
Comment 8 Simon Fraser (smfr) 2019-06-01 16:25:19 PDT
Created attachment 371129 [details]
Patch
Comment 9 WebKit Commit Bot 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>
Comment 10 Truitt Savell 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)
-                  )
                 )
               )
             )
Comment 11 Simon Fraser (smfr) 2019-06-03 14:36:23 PDT
This is the standard WK1 flakiness. Please mark the test as flakey or skipped.
Comment 12 Truitt Savell 2019-06-11 14:45:13 PDT
Marked test as flakey in:
https://trac.webkit.org/changeset/246334/webkit