Bug 72078 – [Chromium] Compositing with full sized fixed div causes heavy repaints

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

[James Simonsen]

Created attachment 114609 [details]
Reduced test case

This is a reduced case from crosbug.com/21332, which is that techcrunch.com scrolls slowly on ChromeOS. There may be many problems, but at least one of them is that we're repainting every time the user scrolls. One of the troublemakers is a 100%x100% fixed div (#finalizer) that's causing a heavy repaint every time we scroll.

I've attached a simple test case that demonstrates this. If you instrument the code, you'll see that we have to repaint every time you scroll, even though the bright green div isn't visible.

[James Robinson]

The paint rects are significantly smaller if the div is partially onscreen - for example add top:95%. Very strange.

[Xianzhu Wang]

Created attachment 120232 [details]
Test case with negative z-index

In my experiments, I must add a z-index CSS property with a negative value (like 'z-index: -100) to the #finalizer, does the issue occur.

Another experiment is to add 'top: 0px' to #finalizer, then the finalizer will be just under the other contents of the body. It's reasonable to repaint in this case when scrolling. So the excessive repaint might be because WebKit thinks #finalizer is overlapping with the whole body.

[Xianzhu Wang]

I believe the patch to https://bugs.webkit.org/show_bug.cgi?id=75018 should resolve this issue. My investigation showed that before that patch, all layers are reinitialized and repainted on very scroll because the scrollbar layers are recreated and added into the layers hierarchy.

[Xianzhu Wang]

Sorry for misleading. Just verified that https://bugs.webkit.org/show_bug.cgi?id=75018 make this issue better, but doesn't totally resolve this issue.

[Xianzhu Wang]

Created attachment 121032 [details]
patch

Only verified on chromium-android, not on chromeos. Wondering if this CL could resolve the whole problem.

[WebKit Review Bot]

Attachment 121032 [details] did not pass style-queue:

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'Source/WebCore/ChangeLog', u'Source/WebCor..." exit_code: 1

Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHostImpl.h:110:  The parameter name "viewportSize" adds no information, so it should be removed.  [readability/parameter_name] [5]
Source/WebCore/platform/graphics/chromium/cc/CCLayerTreeHost.h:176:  The parameter name "viewportSize" adds no information, so it should be removed.  [readability/parameter_name] [5]
Total errors found: 2 in 11 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[Xianzhu Wang]

Created attachment 121036 [details]
patch v2 (fixed style issues)

[James Robinson]

Comment on attachment 121036 [details]
patch v2 (fixed style issues)

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

I'm guessing the issue here is texture use that's higher than the reclaim limit but lower than the absolute limit, right?  Looks like this without this patch we're reducing to the reclaim limit on every frame, regardless of what's visible.

This change seems reasonable to me, although it's effectively making the reclaim limit much weaker. I think you definitely need tests for a subtle behavior change like this.

> Source/WebKit/chromium/ChangeLog:12
> +        No new tests as it doesn't change any functionality.

you definitely are changing functionality, so a test would be nice

[Vangelis Kokkevis]

Comment on attachment 121036 [details]
patch v2 (fixed style issues)

I think this is a reasonable change and more in the spirit of how the reclaim limit should be functioning. Its role is not to actively go and delete textures over the limit but only do so if we need to allocate a new texture.  

There is still a remaining issue that we don't actually protect textures that we already have and need for the current frame from being deleted. So if we are over the reclaim limit and we get a new layer, there is a good chance we'll go and delete a texture from another layer that's still needed if that other layer comes later in the hierarchy traversal.

Maybe what we should be doing instead is to first reserve all the existing textures that we need for the frame before we try to create new ones.

[Xianzhu Wang]

(In reply to comment #8)
> (From update of attachment 121036 [details])
> View in context: https://bugs.webkit.org/attachment.cgi?id=121036&action=review
> 
> I'm guessing the issue here is texture use that's higher than the reclaim limit but lower than the absolute limit, right?

Yes.

> This change seems reasonable to me, although it's effectively making the reclaim limit much weaker. I think you definitely need tests for a subtle behavior change like this.
> 
> > Source/WebKit/chromium/ChangeLog:12
> > +        No new tests as it doesn't change any functionality.
> 
> you definitely are changing functionality, so a test would be nice

I'll add unit test for the patch.

As this bug has several reasons, and the patch just resolves one of them, I'll create separate bugs for each reason.

[Xianzhu Wang]

Add bug 75018 as the depended bug as it used to be the main reason of this bug.

[Xianzhu Wang]

Comment on attachment 121036 [details]
patch v2 (fixed style issues)

The updated patch will be in bug 75632.

[Xianzhu Wang]

All depending bugs fixed. Closing.