cc: Add initial GPU-to-GPU copy rasterizer implementation.

cc: Add initial GPU-to-GPU copy rasterizer implementation.

This adds a derived RasterWorkerPool class that use a smaller
resource pool for staging and initialize tile resources by
copying the contents of a staging resource to a tile resource
using CHROMIUM_copy_texture.

Zero-copy is still used by default when map-image rasterizer
is enabled using --enable-map-image. GPU-to-GPU copy
rasterizer is used when --disable-zero-copy switch is
provided in combination with --enable-map-image. This
zero-copy setting is also added to about:flags.

BUG=269808
TBR=sky

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

[vmpstr, 2014-04-23 17:25:46]

It's a little bit misleading that the flags seem to indicate that zero copy is
enabled by default, which isn't true. Map image thing needs to be enabled, and
then zero copy is on by default. So map image and disable zero copy makes it
this new image copy thing, as I understand it.

Do you think it's possible to make it more clear somehow? Comments maybe, or
maybe some other combination of flags or something?

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
File cc/resources/image_copy_raster_worker_pool.cc (right):

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
cc/resources/image_copy_raster_worker_pool.cc:197:
resource_pool_->ReleaseResource(resource.Pass());
Will this do the right thing if copy is in progress?

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
File cc/resources/image_copy_raster_worker_pool.h (right):

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
cc/resources/image_copy_raster_worker_pool.h:30: ResourcePool* resource_pool);
Should this be named staging_resource_pool or something?

https://codereview.chromium.org/236313006/diff/60001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/60001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:73: const int
kStagingPoolMemoryLimitWhenVisible = 32 * 1024 * 1024;
Can you put a TODO here to maybe move this into global_state or something like
that? Similar to our other memory limits. My concern is that this will be hard
to tune per platform.

[reveman, 2014-04-24 19:20:42]

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
File cc/resources/image_copy_raster_worker_pool.cc (right):

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
cc/resources/image_copy_raster_worker_pool.cc:197:
resource_pool_->ReleaseResource(resource.Pass());
On 2014/04/23 17:25:46, vmpstr wrote:
> Will this do the right thing if copy is in progress?

Yes, the resource will be busy until the sync query fence created by RP has
passed.

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
File cc/resources/image_copy_raster_worker_pool.h (right):

https://codereview.chromium.org/236313006/diff/60001/cc/resources/image_copy_...
cc/resources/image_copy_raster_worker_pool.h:30: ResourcePool* resource_pool);
On 2014/04/23 17:25:46, vmpstr wrote:
> Should this be named staging_resource_pool or something?

As far as this code cares, it's just some resource pool that can be used to get
resources from. LTHI could decide to use the same as for tile resources, doesn't
matter to this code. So assuming that this is some specific type of resource
pool here doesn't seem right.

https://codereview.chromium.org/236313006/diff/60001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/60001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:73: const int
kStagingPoolMemoryLimitWhenVisible = 32 * 1024 * 1024;
On 2014/04/23 17:25:46, vmpstr wrote:
> Can you put a TODO here to maybe move this into global_state or something like
> that? Similar to our other memory limits. My concern is that this will be hard
> to tune per platform.

Latest patch uses GetMaxTransferBufferUsageBytes instead, which is our current
way of tuning the amount of memory used for pending uploads. That limit might
not be perfect in this case but I think it's what we should be using long term.

[vmpstr, 2014-04-25 19:05:40]

lgtm + nits

https://codereview.chromium.org/236313006/diff/80001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/80001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1215: // We currently use hard-coded limits for
staging pool limits.
nit: Is this comment still correct? We're using GetMaxTransferBufferUsageBytes

https://codereview.chromium.org/236313006/diff/80001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/236313006/diff/80001/chrome/app/generated_res...
chrome/app/generated_resources.grd:6449: +        Disable zero-copy rasterizer
Is it worth it to say "Requires map-image rasterizers" or something?

[reveman, 2014-04-25 20:18:25]

https://codereview.chromium.org/236313006/diff/80001/cc/trees/layer_tree_host...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/80001/cc/trees/layer_tree_host...
cc/trees/layer_tree_host_impl.cc:1215: // We currently use hard-coded limits for
staging pool limits.
On 2014/04/25 19:05:41, vmpstr wrote:
> nit: Is this comment still correct? We're using GetMaxTransferBufferUsageBytes

Updated comment.

https://codereview.chromium.org/236313006/diff/80001/chrome/app/generated_res...
File chrome/app/generated_resources.grd (right):

https://codereview.chromium.org/236313006/diff/80001/chrome/app/generated_res...
chrome/app/generated_resources.grd:6449: +        Disable zero-copy rasterizer
On 2014/04/25 19:05:41, vmpstr wrote:
> Is it worth it to say "Requires map-image rasterizers" or something?

Done.

[reveman, 2014-04-30 19:01:09]

I was able to clean this up a bit now that TEXTURE_RECTANGLE_ARB is supported by
CHROMIUM_copy_texture.

--enable/disable-map-image becomes --enable/disable-zero-copy and that still
enables/disables our traditional zero-copy rasterizer currently used by default
for android webview and software compositing. This new rasterizer is enabled
with --enable-one-copy.

Please take a look.

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1951:
GetRendererCapabilities().using_shared_memory_resources) &&
This should get us the behavior we want with the Software Renderer, which is to
always use zero-copy. Let me know if it's better to a new explicit Renderer
capability for this instead of using |using_shared_memory_resources|.

[piman, 2014-04-30 22:18:38]

lgtm

[vmpstr, 2014-04-30 22:42:41]

still lgtm + a couple more nits

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1838: bool use_zero_copy,
I would DCHECK(!use_zero_copy || !use_one_copy) for sanity

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1950: (settings_.use_zero_copy ||
nit: consider determining this ahead of time:

bool use_zero_copy = ...;
bool use_one_copy = ...;
CreateAndSetTileManager(..., use_zero_copy, use_one_copy, ...);

[boliu, 2014-04-30 22:51:25]

android_webview and DeferredInitialize/ReleaseGL path lgtm

[reveman, 2014-04-30 23:23:56]

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
File cc/trees/layer_tree_host_impl.cc (right):

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1838: bool use_zero_copy,
On 2014/04/30 22:42:41, vmpstr wrote:
> I would DCHECK(!use_zero_copy || !use_one_copy) for sanity

That's currently allowed and I don't think we need to prevent the user from
requesting both. use_zero_copy takes precedence when both are specified.

FYI, I would have made this an enum and multi choice about:flag instead if my
intentions were not to make one_copy the default behavior asap and simply remove
that flag.

https://codereview.chromium.org/236313006/diff/130001/cc/trees/layer_tree_hos...
cc/trees/layer_tree_host_impl.cc:1950: (settings_.use_zero_copy ||
On 2014/04/30 22:42:41, vmpstr wrote:
> nit: consider determining this ahead of time:
> 
> bool use_zero_copy = ...;
> bool use_one_copy = ...;
> CreateAndSetTileManager(..., use_zero_copy, use_one_copy, ...);

Good idea. Done.

[reveman, 2014-04-30 23:26:32]

The CQ bit was checked by reveman@chromium.org

[commit-bot: I haz the power, 2014-04-30 23:28:56]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/reveman@chromium.org/236313006/150001

[commit-bot: I haz the power, 2014-05-01 06:05:22]

Change committed as 267455