[cc] Enable specifying a overhang/gutter texture.

[cc] Enable specifying a overhang/gutter texture.

This is in contrast to drawing overhang region with the LTH's background
color.

Specify the texture to use through WebLayerTreeView.

Use the UI resource framework to manage the upload of the linen texture.

BUG=133097
Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=221276

[ccameron, 2013-08-30 01:50:13]

This hooks up the cc/ side of painting gutters with the linen texture.

Before this is enabled, I'll need to
- add the Blink side to actually use this interface
- apply the patch circulated (much) earlier which enables GL_REPEAT
- apply the patch which frees GL resources UI resources in backgrounded tabs

After all of that is done, though, we'll be able to abandon painting and
uploading
the overhang areas every frame.

[enne (OOO), 2013-08-30 15:48:25]

This looks great.  Could you also add some sort of testing so this doesn't
regress in the future?  This isn't that complicated, but I'm always a little
worried about platform-specific code paths that don't have as many eyes on them.

One possibility would be a layer_tree_host_impl_unittest to check that the quads
you get out are right.  Alternatively, a layer tree host pixeltest so that you
can test GL_REPEAT's behavior too.

[ccameron, 2013-08-30 22:30:33]

Thanks!!

I've added a unit test that check the tex coords, etc.

I don't have the GL_REPEAT part yet because that change isn't checked in -- I'll
add that to the tests when it comes in. There are some webkit layout pixel tests
for overhang  that test the end-to-end system.

[ccameron, 2013-09-03 21:46:25]

Pinging this again in case it fell off the bottom of the the long weekend inbox.

[enne (OOO), 2013-09-03 23:18:26]

https://codereview.chromium.org/23805002/diff/12001/cc/resources/scoped_ui_re...
File cc/resources/scoped_ui_resource.h (right):

https://codereview.chromium.org/23805002/diff/12001/cc/resources/scoped_ui_re...
cc/resources/scoped_ui_resource.h:25: gfx::Size GetSize();
const?

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host....
cc/trees/layer_tree_host.cc:146: overhang_ui_resource_.reset();
Why does this need to be explicit?

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host....
cc/trees/layer_tree_host.h:385: scoped_ptr<ScopedUIResource>
overhang_ui_resource_;
Can you leave a comment about what this is, e.g. "If set, this texture is used
to fill in parts of the screen not covered by layers."  I know what overhang is,
but I don't know that this is generally obvious.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:590: layer_rect.x() /
static_cast<float>(resource_size.width()),
Does this also need to have a device scale conversion? layer_rect here is in
device space, so the linen texture will be 1:1 with device pixels.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:593: (layer_rect.x() + layer_rect.width()) /
style nit: layer_rect.right() and layer_rect.bottom()

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2849:
did_activate_pending_tree_(false) { }
style nit: {}.  Consider running 'git cl format' on your patch.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2993: if (quad_list[i]->material ==
DrawQuad::TEXTURE_CONTENT) {
style nit: early-out with a continue here, rather than indenting the whole
block.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2994: TextureDrawQuad* quad =
reinterpret_cast<TextureDrawQuad*>(
TextureDrawQuad::MaterialCast(quad_list[i])

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:3003: (quad->rect.x() +
quad->rect.width()) /
quad->rect.right()

[ccameron, 2013-09-04 02:36:47]

Thanks!!

https://codereview.chromium.org/23805002/diff/12001/cc/resources/scoped_ui_re...
File cc/resources/scoped_ui_resource.h (right):

https://codereview.chromium.org/23805002/diff/12001/cc/resources/scoped_ui_re...
cc/resources/scoped_ui_resource.h:25: gfx::Size GetSize();
On 2013/09/03 23:18:26, enne wrote:
> const?

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host.cc
File cc/trees/layer_tree_host.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host....
cc/trees/layer_tree_host.cc:146: overhang_ui_resource_.reset();
On 2013/09/03 23:18:26, enne wrote:
> Why does this need to be explicit?

The documentation of ScopedUIResource says that a UI resource may not outlive
its client -- the LTH is the client for the resource.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host.h
File cc/trees/layer_tree_host.h (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host....
cc/trees/layer_tree_host.h:385: scoped_ptr<ScopedUIResource>
overhang_ui_resource_;
On 2013/09/03 23:18:26, enne wrote:
> Can you leave a comment about what this is, e.g. "If set, this texture is used
> to fill in parts of the screen not covered by layers."  I know what overhang
is,
> but I don't know that this is generally obvious.

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:590: layer_rect.x() /
static_cast<float>(resource_size.width()),
On 2013/09/03 23:18:26, enne wrote:
> Does this also need to have a device scale conversion? layer_rect here is in
> device space, so the linen texture will be 1:1 with device pixels.

YES!! I just discovered this as your email was coming in.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl.cc:593: (layer_rect.x() + layer_rect.width()) /
On 2013/09/03 23:18:26, enne wrote:
> style nit: layer_rect.right() and layer_rect.bottom()

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
File cc/trees/layer_tree_host_impl_unittest.cc (right):

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2849:
did_activate_pending_tree_(false) { }
On 2013/09/03 23:18:26, enne wrote:
> style nit: {}.  Consider running 'git cl format' on your patch.

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2993: if (quad_list[i]->material ==
DrawQuad::TEXTURE_CONTENT) {
On 2013/09/03 23:18:26, enne wrote:
> style nit: early-out with a continue here, rather than indenting the whole
> block.

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:2994: TextureDrawQuad* quad =
reinterpret_cast<TextureDrawQuad*>(
On 2013/09/03 23:18:26, enne wrote:
> TextureDrawQuad::MaterialCast(quad_list[i])

Done.

https://codereview.chromium.org/23805002/diff/12001/cc/trees/layer_tree_host_...
cc/trees/layer_tree_host_impl_unittest.cc:3003: (quad->rect.x() +
quad->rect.width()) /
On 2013/09/03 23:18:26, enne wrote:
> quad->rect.right()

Done.

[jamesr, 2013-09-04 03:08:32]

content/renderer/ lgtm

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
File content/renderer/gpu/render_widget_compositor.h (right):

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
content/renderer/gpu/render_widget_compositor.h:73: virtual void
setOverhangBitmap(SkBitmap);
in chromium style, all parameters get names even when they're obvious

[enne (OOO), 2013-09-04 17:29:36]

lgtm

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
File content/renderer/gpu/render_widget_compositor.h (right):

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
content/renderer/gpu/render_widget_compositor.h:73: virtual void
setOverhangBitmap(SkBitmap);
On 2013/09/04 03:08:32, jamesr wrote:
> in chromium style, all parameters get names even when they're obvious

Can this also be const SkBitmap& like the LTH::SetOverhangBitmap function?

[ccameron, 2013-09-04 18:41:24]

Thanks!!

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
File content/renderer/gpu/render_widget_compositor.h (right):

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
content/renderer/gpu/render_widget_compositor.h:73: virtual void
setOverhangBitmap(SkBitmap);
On 2013/09/04 03:08:32, jamesr wrote:
> in chromium style, all parameters get names even when they're obvious

Done.

https://codereview.chromium.org/23805002/diff/24001/content/renderer/gpu/rend...
content/renderer/gpu/render_widget_compositor.h:73: virtual void
setOverhangBitmap(SkBitmap);
On 2013/09/04 17:29:37, enne wrote:
> On 2013/09/04 03:08:32, jamesr wrote:
> > in chromium style, all parameters get names even when they're obvious
> 
> Can this also be const SkBitmap& like the LTH::SetOverhangBitmap function?

I remember presubmit or something being upset at this in an earlier version of
the patch, but I don't see that now, so maybe it was a hallucination.

[commit-bot: I haz the power, 2013-09-04 18:42:06]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/ccameron@chromium.org/23805002/36001

[commit-bot: I haz the power, 2013-09-04 21:35:15]

Change committed as 221276