1260799 - Infinite painting on verily.com

Status: REOPENED

(Core :: Web Painting, defect)

Description

[Kartikaya Gupta (email:[e-mail])]

Turn on nglayout.debug.paint_flashing and go to verily.com. The background of the page seems to be repainting continuously even when the user isn't doing anything. Without paintflashing there is no obvious reason why that should be happening.

100% repro on OS X aurora, nightly, and release. Assuming it impacts beta as well :)

Comment 1

[Timothy Nikkel (:tnikkel)]

The background of the page is constantly changing colors, just slowly.

Comment 2

[Markus Stange [:mstange]]

But all they're doing is animating a transform on a layer that has a gradient. This shouldn't repaint as much as it does.

Comment 3

[Timothy Nikkel (:tnikkel)]

Hmm, I don't see any paint flashing, I tested on OS X with release, beta, and nightly (e10s on and off).

Comment 4

[Kartikaya Gupta (email:[e-mail])]

Could be that I'm not getting OMTA because the layer is too big relative to the screen on my non-retina mac. I'll check later.

Comment 5

[Markus Stange [:mstange]]

You're probably right. But the layer should still be active, and we should only paint the parts that come into view due to the animation, not the whole layer. Maybe we're not using tiles for the layer and the layer size changes slightly.

Comment 6

[Kartikaya Gupta (email:[e-mail])]

Attachment: verily_video.mov

I had to apply the patches from bug 1254408 to discover this (since bug 1196114 dropped the stdout warnings), but yes, for me it is bailing out of OMTA - "frame size (1375, 4560) is larger than the viewport (1547, 413)"

(In reply to Markus Stange [:mstange] from comment #5)
> You're probably right. But the layer should still be active, and we should
> only paint the parts that come into view due to the animation, not the whole
> layer.

I think that's what's happening - the parts that are flashing are the parts where I can see the background either directly or through transparent stuff on top of it. See attached video for example. Or did you mean something else?

Comment 7

[Markus Stange [:mstange]]

Attachment: verily with a tiled layer - only invalidating newly-added parts

Here's what it should look like.

I got this by returning LayerManager::SCROLLABLE from ContainerState::GetLayerCreationHint unconditionally. This makes the layer use a multi tiled content client, which is able to resize without throwing everything away.

Comment 8

[Markus Stange [:mstange]]

The layer is position:fixed, so it doesn't inherit scrollability from the root scroll frame.

Matt, sounds like we should be using tiles for all layers in animated (non-prerendered?) transforms. Unless the layer is smaller than a single tile, of course. Do you agree? And we need to rename the layer creation hint from SCROLLABLE to something else, maybe something along the lines of NEED_CHEAP_RESIZING?

Comment 9

[Markus Stange [:mstange]]

Oh, it's not just size changes that invalidate a single tiled layer. Moving the visible region while keeping the size fixed also invalidates, because we're not using buffer rotation. I think that's what's happening on this page.

Comment 10

[Matt Woodrow (:mattwoodrow)]

(In reply to Markus Stange [:mstange] from comment #8)
> The layer is position:fixed, so it doesn't inherit scrollability from the
> root scroll frame.
> 
> Matt, sounds like we should be using tiles for all layers in animated
> (non-prerendered?) transforms. Unless the layer is smaller than a single
> tile, of course. Do you agree? And we need to rename the layer creation hint
> from SCROLLABLE to something else, maybe something along the lines of
> NEED_CHEAP_RESIZING?

Why does being transformed imply that we would resize frequently?

Can we instead fix the single tile code to copy the contents to the new buffer when resizing?

We could also track buffer re-allocations and promote a layer to tiling if we get too many.

Comment 11

[Markus Stange [:mstange]]

(In reply to Matt Woodrow (:mattwoodrow) from comment #10)
> (In reply to Markus Stange [:mstange] from comment #8)
> > The layer is position:fixed, so it doesn't inherit scrollability from the
> > root scroll frame.
> > 
> > Matt, sounds like we should be using tiles for all layers in animated
> > (non-prerendered?) transforms. Unless the layer is smaller than a single
> > tile, of course. Do you agree? And we need to rename the layer creation hint
> > from SCROLLABLE to something else, maybe something along the lines of
> > NEED_CHEAP_RESIZING?
> 
> Why does being transformed imply that we would resize frequently?

Ok, let's say we only do this for transforms that are animated by a CSS animation, and that are not pre-rendered.
The transform being animated means that the layer is probably moving. The transform not being pre-rendered usually means that the transformed element was bigger than the viewport, and thus clipped by the viewport. This combination is a pretty good indicator for a layer needing to resize, I think.

> Can we instead fix the single tile code to copy the contents to the new
> buffer when resizing?

Let's just do that anyway.

> We could also track buffer re-allocations and promote a layer to tiling if
> we get too many.

This feels like too much magic to me.