Bug 110430 – Non-painting fixed elements should not cause repaints on scroll

• Status: RESOLVED
• Resolution: FIXED
• Component: Layout and Rendering

[Simon Fraser (smfr)]

In bug 108112 I made empty position:fixed elements not composited. However, that now makes us repaint those (empty) layers on every scroll.

[Simon Fraser (smfr)]

Created attachment 189460 [details]
Testcase

[Radar WebKit Bug Importer]

<rdar://problem/13260967>

[Xianzhu Wang]

In bug 113238, sfmr wrote:
Where is NotCompositedForNoVisibleContent actually consulted? I would expect to see a test for it in FrameView::scrollContentsFastPath(), but do not.

wangxianzhu wrote:
Seems that we need the same test in FrameView::scrollContentsFastPath() (as in ScrollingCoordinator)... Will work on this.

[Xianzhu Wang]

Created attachment 195196 [details]
For EWS to generate layout test baseline

[Xianzhu Wang]

Comment on attachment 195196 [details]
For EWS to generate layout test baseline

Not for formal review yet.

[Xianzhu Wang]

Created attachment 195198 [details]
Patch

[Xianzhu Wang]

Comment on attachment 195198 [details]
Patch

Passed most EWS. The expectations seem ok.

[Simon Fraser (smfr)]

Comment on attachment 195198 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=195198&action=review

> Source/WebCore/page/FrameView.cpp:1624
> +        if (layer->viewportConstrainedNotCompositedReason() == RenderLayer::NotCompositedForNoVisibleContent) {
> +            // Don't invalidate for fixed layers that don't have visible contents.
> +            continue;

I think we should also skip if the reason is NotCompositedForBoundsOutOfView.

[Xianzhu Wang]

Created attachment 195371 [details]
Add out-of-view case

[Simon Fraser (smfr)]

Comment on attachment 195371 [details]
Add out-of-view case

View in context: https://bugs.webkit.org/attachment.cgi?id=195371&action=review

Code changes look good, but the tests should be proper repaint tests.

> LayoutTests/ChangeLog:13
> +        * compositing/repaint/scroll-fixed-layer-no-content.html: Added.
> +        * compositing/repaint/scroll-fixed-layer-no-content-expected.png: Added.
> +        * compositing/repaint/scroll-fixed-layer-no-content-expected.txt: Added.
> +        * compositing/repaint/scroll-fixed-layer-out-of-view.html: Added.
> +        * compositing/repaint/scroll-fixed-layer-out-of-view-expected.png: Added.
> +        * compositing/repaint/scroll-fixed-layer-out-of-view-expected.txt: Added.

These tests should be text tests that dump repaint rects.

[Xianzhu Wang]

Created attachment 195385 [details]
New style repaint tests

[Simon Fraser (smfr)]

Comment on attachment 195385 [details]
New style repaint tests

View in context: https://bugs.webkit.org/attachment.cgi?id=195385&action=review

> LayoutTests/compositing/repaint/scroll-fixed-layer-no-content-expected.txt:1
> +

It would be nice if the output looked like: "There should be no repaints here."

[Build Bot]

Comment on attachment 195385 [details]
New style repaint tests

Attachment 195385 [details] did not pass win-ews (win):
Output: http://webkit-commit-queue.appspot.com/results/17230761

[Xianzhu Wang]

Created attachment 195571 [details]
For landing

[WebKit Review Bot]

Comment on attachment 195571 [details]
For landing

Clearing flags on attachment: 195571

Committed r147120: <http://trac.webkit.org/changeset/147120>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.