Bug 196508 - Async overflow scroll on iOS paints slowly if it has a negative z-index child
Summary: Async overflow scroll on iOS paints slowly if it has a negative z-index child
Status: ASSIGNED
Alias: None
Product: WebKit
Classification: Unclassified
Component: Scrolling (show other bugs)
Version: WebKit Nightly Build
Hardware: Unspecified Unspecified
: P2 Normal
Assignee: Simon Fraser (smfr)
URL:
Keywords: InRadar
Depends on:
Blocks:
 
Reported: 2019-04-02 13:42 PDT by Simon Fraser (smfr)
Modified: 2019-06-11 14:45 PDT (History)
5 users (show)

See Also:


Attachments
Testcase (1.17 KB, text/html)
2019-04-02 13:42 PDT, Simon Fraser (smfr)
no flags Details
Patch (15.22 KB, patch)
2019-06-01 12:18 PDT, Simon Fraser (smfr)
dino: review+
Details | Formatted Diff | Diff
Patch (20.89 KB, patch)
2019-06-01 16:25 PDT, Simon Fraser (smfr)
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
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