cc: Add predictive pre-painting.

cc: Add predictive pre-painting.

When scrolling quickly and with painting just barely able to keep up,
it's possible get into a bad state where we always do visible
painting and never get a chance to paint ahead of the fold. This
shows up as flickering at the bottom of the page.

This patch has two parts. First, it uses a scroll prediction when
choosing what to paint (both for 'visible' and off-screen painting).
Second, it adds a very simple scroll predictor based on the change
in the visibleContentRect. The second part should almost certainly
be improved (use impl-thread scroll/animation directions/velocities)
but this provides an initial estimate that helps in the most important
case (basic scrolling of big layers).

This doesn't actually change the priorities of textures in the scroll
direction (but changing the visible rect does this partially).
This is not an issue currently since the what-to-paint logic doesn't
strictly adhere to priority order.

BUG=121957
Committed: http://src.chromium.org/viewvc/chrome?view=rev&revision=163665

[epenner, 2012-10-18 03:13:00]

Rebased. ptal.

[enne (OOO), 2012-10-18 19:13:08]

It seems a little to me like "idle paint rect" and "predicted visible paint
rect" are trying to do the same thing here, and all the update functions now
have to know about three rects (idle, predicted, visible).

Just to simplify this complexity, is there some way to abstract all of this and
have one function that gives the rect that we want to paint and upload this
frame?

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc
File cc/tiled_layer.cc (right):

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc#newcode673
cc/tiled_layer.cc:673: int x = rect.x() + ((delta.width() < 0)  ? delta.width() 
: 0);
Why do you need this clamping on the negative end, but not on the positive
content bounds end? Don't you intersect with content bounds later anyway?

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc#newcode698
cc/tiled_layer.cc:698: // Bound the prediction to at most 2 tiles, and clamp to
content bounds.
The 2 tiles ahead guess seems a little magical.  Has this worked well in
practice for you?

At the very least, can you pull all these numbers here and in idle paint rect up
into named constants?

[epennerAtGoogle, 2012-10-18 19:58:54]

I'll see if there is anything I can do to combine the idle part and the visible
part so there is less stuff to track.

The tricky thing is that you do want to paint quite far ahead of the scroll if
you have spare cycles (idle), but also not give up and only paint the 'visible'
area if there is new visible content as well.

On a tangent, the biggest issue remaining (even after this stuff) is new visible
dirty content showing up while scrolling. If we could conditionally (when
scrolling) go into another paint/commit cycle when we detect there is still
pre-painting to do in the scroll direction, rather than giving control back to
webkit, then we could devote all the paint time to pre-painting instead of
starting over again and painting visible stuff. This could scroll paint speed at
the expense of possibly completely blocking updates when scrolling quickly.

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc
File cc/tiled_layer.cc (right):

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc#newcode673
cc/tiled_layer.cc:673: int x = rect.x() + ((delta.width() < 0)  ? delta.width() 
: 0);
When expanding a rect in the direction of an arbitrary vector, the width/height
are always increased. But the x/y are only changed if you are expanding in the
negative direction.

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc#newcode698
cc/tiled_layer.cc:698: // Bound the prediction to at most 2 tiles, and clamp to
content bounds.
There needs to be some kind of clamp to prevent arbitrary sized paints, since we
don't move the visible rect but only expand it. Downstream we used the size if
the visible rect to clamp how much extra painting is done but I was more
conservative here.

The amount of extra painting basically chooses the ratio of time that is
committed to pre-painting in cases where there is also visible invalidations.

[epenner, 2012-10-18 20:40:43]

Another quick comment. This only expands the visible rect and never just 'moves'
it using the prediction. When nothing is changing it's effectively the same, but
when the content is changing it insures that an incorrect prediction doesn't
cause additional visible invalid stuff to show up. This is where I think we
should ideally not even allow new invalid stuff when scrolling (block webkit
longer)

[enne (OOO), 2012-10-19 18:40:56]

lgtm

I think I have wrapped my head around all the different rects, and I don't
really see any way to combine them.

visibleRect = current visible rect, used as input for predicted visible rect

predictedVisibleRect = used for prioritization / "normal" painting

idlePaintRect = biggest rect, used if predictedVisibleRect doesn't paint
anything

One other small nit: can you pull the numbers for "how many rows/columns of
tiles do we expand the idle rect by" and "how many rows/columns of tiles do we
expand the predicted scroll rect by" up into constants?

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc
File cc/tiled_layer.cc (right):

https://codereview.chromium.org/11193010/diff/3002/cc/tiled_layer.cc#newcode673
cc/tiled_layer.cc:673: int x = rect.x() + ((delta.width() < 0)  ? delta.width() 
: 0);
Ah, gotcha, thanks.

[epenner, 2012-10-19 18:59:06]

There is one cool idea Dana and I have discussed, which would determine the next
paint rect purely based on priorities (be it visible or pre-paint).  But it's
tricky to avoid accumulating a huge pre-paint rect along due to diagonals.

It could work though. The priorities would need to be carefully crafted, and the
accumulated paint rect constrained to grow as only a row or column.

[danakj, 2012-10-19 19:02:30]

On 2012/10/19 18:59:06, epenner wrote:
> There is one cool idea Dana and I have discussed, which would determine the
next
> paint rect purely based on priorities (be it visible or pre-paint).  But it's
> tricky to avoid accumulating a huge pre-paint rect along due to diagonals.
> 
> It could work though. The priorities would need to be carefully crafted, and
the
> accumulated paint rect constrained to grow as only a row or column.

It also occured to me we could rely less on careful priorities in that scheme,
and when adding a tile based on its priority (modified by velocity), check to
see that it is in the right direction from the visibleContentRect.

We could say something like "only 1 row/column of tiles allowed in non-primary
scroll directions". Then we'd have a clear guard against exloding the paint rect
and more freedom to make simpler priority schemes.

[epennerAtGoogle, 2012-10-19 19:09:18]

> It also occured to me we could rely less on careful priorities in that scheme,
> and when adding a tile based on its priority (modified by velocity), check to
> see that it is in the right direction from the visibleContentRect.
> 
> We could say something like "only 1 row/column of tiles allowed in non-primary
> scroll directions". Then we'd have a clear guard against exloding the paint
rect
> and more freedom to make simpler priority schemes.

This sounds interesting. Okay, unfortunately I'm doing the merge today, but as
soon as I'm done I'll try to wrap this up. If I can quickly do something better
I will, but otherwise I'll leave as is for further improvement.  In any case
I'll put all constants into one place.

[commit-bot: I haz the power, 2012-10-23 18:58:57]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/epenner@chromium.org/11193010/13001

[commit-bot: I haz the power, 2012-10-23 20:26:51]

Change committed as 163665