CC: Have GPU-raster tiles exactly match the viewport.

CC: Have GPU-raster tiles exactly match the viewport.

The previous logic doesn't account for divide truncation and
border texels, resulting in an additional visible tile with
only a couple of rows visible. This is hidden by the URL bar
sometimes, but it still affects full-screen, web-apps and
desktop browsers.

BUG=365877
Committed: https://crrev.com/30d771737be5a083038f141eec2e6331f69bc47f
Cr-Commit-Position: refs/heads/master@{#299201}

[epennerAtGoogle, 2014-10-09 01:41:17]

epenner@google.com changed reviewers:
+ enne@chromium.org, ernstm@chromium.org, vmpstr@chromium.org

[epenner, 2014-10-09 03:47:58]

Ptal.

This needs to land first but looks like it will land shortly:
https://codereview.chromium.org/605773004/

[ernstm, 2014-10-09 22:05:48]

On 2014/10/09 03:47:58, epenner wrote:
> Ptal.
> 
> This needs to land first but looks like it will land shortly:
> https://codereview.chromium.org/605773004/

LGTM

[epenner, 2014-10-09 23:53:35]

Rebased this on the last one.

Ptal for Enne or Vlad for landing this one.

[enne (OOO), 2014-10-10 00:06:05]

https://codereview.chromium.org/626113004/diff/110001/cc/layers/picture_layer...
File cc/layers/picture_layer_impl_unittest.cc (right):

https://codereview.chromium.org/626113004/diff/110001/cc/layers/picture_layer...
cc/layers/picture_layer_impl_unittest.cc:4423: // Tile-height doubles to 50%
when width shrinks to <= 50%.
For what it's worth, I think this "viewport / 4" makes a lot of sense for long
scrollable layers.  But it doesn't make really all that much sense for most
other layers that (currently) get created.

I guess I'm trying to say this relative 50% is kind of weird and maybe should be
absolute.  Would it make sense to have a "max untiled size" for ganesh as well,
so that small layers don't get divided up by viewport size too?

[epennerAtGoogle, 2014-10-10 00:34:55]

> I guess I'm trying to say this relative 50% is kind of weird and maybe should
be
> absolute.  Would it make sense to have a "max untiled size" for ganesh as
well,
> so that small layers don't get divided up by viewport size too?

I wanted to preserve some level of 'increase the height once the width becomes
reasonably small'. So the questions I guess are:
1.) When is a layer's width considered small enough (is <=50% viewport okay for
that?)
2.) What value should we increase the tile-height to in that case?

Some concern I have with a fixed size is that it might not scale uniformly on
different devices, requiring tuning of that constant like our software-tile
size... But otherwise I'm fine with it and just don't know a good size, and when
to start using it (see 1.)).

[epennerAtGoogle, 2014-10-10 01:19:06]

Thinking a bit about the trade-off:

Without something really fancy, I think the trade-off currently is allocation
thrashing vs the cpu-cost of doing too many passes in order to recycle tiles
better. I guess my hope would be that the RoundUp(64) is helping us enough to
smooth out the allocation thrashing, and therefore the best choice for now is to
have relatively large tiles to minimize the cpu-cost of doing multiple passes...
Once they are relatively large, 1/4 of screen pixels seems reasonable to me,
which is what ends up happening in most cases.

Manfred, do you have an opinion on this?

In terms of fancy future solutions, I figure we could probably use exclusively
1/4 viewport tiles some day, combined with a scratch-pad, some copies, and
rotation/sub-allocation for small stuff.

[ernstm, 2014-10-10 17:38:06]

On 2014/10/10 01:19:06, epennerAtGoogle wrote:
> Thinking a bit about the trade-off:
> 
> Without something really fancy, I think the trade-off currently is allocation
> thrashing vs the cpu-cost of doing too many passes in order to recycle tiles
> better. I guess my hope would be that the RoundUp(64) is helping us enough to
> smooth out the allocation thrashing, and therefore the best choice for now is
to
> have relatively large tiles to minimize the cpu-cost of doing multiple
passes...
> Once they are relatively large, 1/4 of screen pixels seems reasonable to me,
> which is what ends up happening in most cases.
> 
> Manfred, do you have an opinion on this?
> 
> In terms of fancy future solutions, I figure we could probably use exclusively
> 1/4 viewport tiles some day, combined with a scratch-pad, some copies, and
> rotation/sub-allocation for small stuff.

I think the relative threshold is nice, because it is device independent. Maybe
we should extend it to use a divisor of 1 if content_bounds.width() <=
viewport_width / 4. In general we want as few tiles as possible with Ganesh.

[enne (OOO), 2014-10-10 17:54:00]

On 2014/10/10 at 01:19:06, epenner wrote:

> In terms of fancy future solutions, I figure we could probably use exclusively
1/4 viewport tiles some day, combined with a scratch-pad, some copies, and
rotation/sub-allocation for small stuff.

In terms of future solutions, I think anything based on relative size of the
viewport is going to be awkward for desktop when it can change the size of the
viewport.  It's fine to punt on this, but we're probably going to need to
introduce absolute values here somewhere at some point.

[epennerAtGoogle, 2014-10-10 18:46:33]

> I think the relative threshold is nice, because it is device independent.
Maybe
> we should extend it to use a divisor of 1 if content_bounds.width() <=
> viewport_width / 4. In general we want as few tiles as possible with Ganesh.

If you think that's best, sure. That's in the latest patch.

Also, I made the minimum height 128 to preserve the original intended logic,
however if the goal is to have minimal tiles, I've made it 256 again. The bug
was effectively making it 512, hopefully 256 will work okay to start.

> In terms of future solutions, I think anything based on relative size of the
> viewport is going to be awkward for desktop when it can change the size of the
> viewport.  It's fine to punt on this, but we're probably going to need to
> introduce absolute values here somewhere at some point.

Ahh, yeah that's a good point. Hmm, would it be okay to just round up to 64
after calculating the ideal size, on any platform where the viewport changes
fluidly? That would make roughly the same as an animating layer size. For that I
could just add a settings.fluid_viewport setting, set it to false on Android and
that's it.

For a static size, I'd rather punt unless there is a concrete approach
suggested. I just don't have a clean approach in mind yet for this.

[enne (OOO), 2014-10-10 18:55:50]

I don't have a clean approach in mind either for static sizes.  :)

lgtm % ernstm

[ernstm, 2014-10-10 20:39:49]

On 2014/10/10 18:55:50, enne wrote:
> I don't have a clean approach in mind either for static sizes.  :)
> 
> lgtm % ernstm

LGTM. This patch is a clear improvement over what we had before. Finding an
optimal configuration would require benchmarking on a lot of pages with real
invalidation patterns, which we can't do today. We still need to revisit the
tile size code when we go to desktop platforms, as Enne pointed out.

[epennerAtGoogle, 2014-10-10 20:59:59]

Landing. I realized I didn't upload before for divisor of 1 and min-height being
256. Feel free to un-cq if you have any concerns.

[epennerAtGoogle, 2014-10-10 21:00:48]

The CQ bit was checked by epenner@google.com

[commit-bot: I haz the power, 2014-10-10 21:01:20]

CQ is trying da patch. Follow status at
 https://chromium-cq-status.appspot.com/patch-status/626113004/290001

[commit-bot: I haz the power, 2014-10-10 22:15:11]

Committed patchset #7 (id:290001)

[commit-bot: I haz the power, 2014-10-10 22:16:23]

Patchset 7 (id:??) landed as
https://crrev.com/30d771737be5a083038f141eec2e6331f69bc47f
Cr-Commit-Position: refs/heads/master@{#299201}