cc: Support texture rect targets for masks

cc/trees/layer_tree_host_impl.cc

Index: cc/trees/layer_tree_host_impl.cc
diff --git a/cc/trees/layer_tree_host_impl.cc b/cc/trees/layer_tree_host_impl.cc
index a3a03286fdad5b4e1cab889d6ab8220b340bf4c9..8bc11aac93f3fd6755dcec3d40b97df5e8a8e012 100644
--- a/cc/trees/layer_tree_host_impl.cc
+++ b/cc/trees/layer_tree_host_impl.cc
@@ -78,6 +78,20 @@
 namespace cc {
 namespace {
 
+bool SupportsTextureTarget(unsigned target,
+                           LayerTreeSettings::TextureTargetPrefs prefs) {

[reveman, 2014/10/22 12:40:39]

I find this utility function a bit confusing. Based on the name, I'd expect it
to be passed a target and the ContextProvider::Capabilities and return true if
target is supported.

+  switch (prefs) {
+    case LayerTreeSettings::USE_DEFAULT_TEXTURE_TARGET:
+      return true;
+    case LayerTreeSettings::FORCE_TEXTURE_2D_TARGET:
+      return target == GL_TEXTURE_2D;
+    case LayerTreeSettings::FORCE_TEXTURE_RECT_TARGET:
+      return target == GL_TEXTURE_RECTANGLE_ARB;
+  }
+  NOTREACHED();
+  return false;
+}
+
 // Small helper class that saves the current viewport location as the user sees
 // it and resets to the same location.
 class ViewportAnchor {
@@ -2013,6 +2027,8 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
 
   ContextProvider* context_provider = output_surface_->context_provider();
   if (!context_provider) {
+    DCHECK(
+        SupportsTextureTarget(GL_TEXTURE_2D, settings_.texture_target_prefs));
     resource_pool_ =
         ResourcePool::Create(resource_provider_.get(),
                              GL_TEXTURE_2D,
@@ -2023,6 +2039,8 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
                                        RasterWorkerPool::GetTaskGraphRunner(),
                                        resource_provider_.get());
   } else if (use_gpu_rasterization_) {
+    DCHECK(
+        SupportsTextureTarget(GL_TEXTURE_2D, settings_.texture_target_prefs));
     resource_pool_ =
         ResourcePool::Create(resource_provider_.get(),
                              GL_TEXTURE_2D,
@@ -2031,16 +2049,24 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
     raster_worker_pool_ = GpuRasterWorkerPool::Create(
         task_runner, context_provider, resource_provider_.get());
   } else if (UseZeroCopyRasterizer()) {
-    resource_pool_ = ResourcePool::Create(
-        resource_provider_.get(),
-        GetMapImageTextureTarget(context_provider->ContextCapabilities()),
-        resource_provider_->best_texture_format());
+    unsigned target =
+        GetMapImageTextureTarget(context_provider->ContextCapabilities());
+    if (settings_.texture_target_prefs ==
+        LayerTreeSettings::FORCE_TEXTURE_2D_TARGET)

[reveman, 2014/10/22 12:40:38]

I don't think this should be a special case for zero-copy. It applies to
one-copy too and possibly to Gpu raster in the future. It would be nice if we
could simply pass settings_.raster_texture_target directly to each of these
functions and maybe just have a check above all this if-else stuff to make sure
target is supported.

[enne (OOO), 2014/10/22 19:20:44]

Maybe I'm mistaken, but I don't think it applies to one copy.  The staging
resource is GetMapImageTextureTarget, but the final resource is GL_TEXTURE_2D?
So, the GLRenderer code that runs will use GL_TEXTURE_2D, right?

[reveman, 2014/10/22 20:36:47]

Correct, GLRenderer is only affected by the target used for drawing. It's the
addition of a new way of configuring this without removing an old that confuses
me. We need to select the raster texture target based on context capabilities,
that's clear. And based on this patch I assume that we need to be able to force
raster texture target for testing purposes too, right? Instead of having to fuse
both of these selection mechanism here is there some way we can have one replace
or override the other so that we only have to deal with one setting or
capability here?

[enne (OOO), 2014/10/22 21:14:30]

I'm not sure that these should be the same flag, as they exercise different
paths in GLRenderer depending on which raster worker pool is being used.  We
probably should test them, but it seems like there needs to be a different
setting.

If you want to test the raster worker pool via pixel tests, then we probably
need to have (pool type) x (raster/staging target type) x (draw target type)
configurations.

[reveman, 2014/10/22 23:13:35]

I think ((pool type) * (raster target type)) + (draw target type) is a
sufficient set of test configurations. If a draw target works with one pool
type, there's no reason it shouldn't work with another. Also, you can't really
separate raster and draw target types. Draw target type is a result of pool type
+ raster target type.

Ideally we wouldn't need use our existing rasterizer implementations to verify
that all draw target types work correctly. But we probably need to add a lot of
code to avoid that. Using the 0-copy rasterizer uniquely (as it supports output
to all target types) to test that all draw target types work correctly seem fine
to me.

+      target = GL_TEXTURE_2D;
+    DCHECK(SupportsTextureTarget(target, settings_.texture_target_prefs));
+    resource_pool_ =
+        ResourcePool::Create(resource_provider_.get(),
+                             target,
+                             resource_provider_->best_texture_format());
 
     raster_worker_pool_ =
         ZeroCopyRasterWorkerPool::Create(proxy_->ImplThreadTaskRunner(),
                                          RasterWorkerPool::GetTaskGraphRunner(),
                                          resource_provider_.get());
   } else if (UseOneCopyRasterizer()) {
+    DCHECK(
+        SupportsTextureTarget(GL_TEXTURE_2D, settings_.texture_target_prefs));
     // We need to create a staging resource pool when using copy rasterizer.
     staging_resource_pool_ = ResourcePool::Create(
         resource_provider_.get(),
@@ -2058,6 +2084,8 @@ void LayerTreeHostImpl::CreateAndSetTileManager() {
                                         resource_provider_.get(),
                                         staging_resource_pool_.get());
   } else {
+    DCHECK(
+        SupportsTextureTarget(GL_TEXTURE_2D, settings_.texture_target_prefs));
     resource_pool_ = ResourcePool::Create(
         resource_provider_.get(),
         GL_TEXTURE_2D,