cc: Support texture rect targets for masks

cc/test/layer_tree_pixel_resource_test.cc

+// Copyright 2014 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+#include "cc/test/layer_tree_pixel_resource_test.h"
+
+#include "cc/layers/layer.h"
+#include "cc/resources/bitmap_raster_worker_pool.h"
+#include "cc/resources/gpu_raster_worker_pool.h"
+#include "cc/resources/one_copy_raster_worker_pool.h"
+#include "cc/resources/pixel_buffer_raster_worker_pool.h"
+#include "cc/resources/raster_worker_pool.h"
+#include "cc/resources/resource_pool.h"
+#include "cc/resources/zero_copy_raster_worker_pool.h"
+#include "cc/test/fake_output_surface.h"
+#include "gpu/GLES2/gl2extchromium.h"
+
+namespace cc {
+
+namespace {
+
+bool IsTestCaseSupported(PixelResourceTestCase test_case) {
+  switch (test_case) {
+    case SOFTWARE:
+    case GL_GPU_RASTER_2D_DRAW:
+    case GL_ZERO_COPY_2D_DRAW:
+    case GL_ZERO_COPY_RECT_DRAW:
+    case GL_ASYNC_UPLOAD_2D_DRAW:
+      return true;
+    case GL_ONE_COPY_2D_STAGING_2D_DRAW:
+    case GL_ONE_COPY_RECT_STAGING_2D_DRAW:
+    case GL_ONE_COPY_EXTERNAL_STAGING_2D_DRAW:
+      // TODO(reveman): one copy not supported in unit tests yet.

[enne (OOO), 2014/10/23 19:24:15]

These tests all yell about sync_query, so I've disabled them here.

+      return false;
+    case GL_ZERO_COPY_EXTERNAL_DRAW:
+#if !defined(OS_ANDROID)

[reveman, 2014/10/23 20:39:27]

what happens without this ifdef? what do we need to do to test this everywhere
and not just on android?

[enne (OOO), 2014/10/24 22:21:28]

Lots of GL errors.

I don't know what's needed to support this elsewhere.  Actually, I don't even
know that it works on Android, as all pixel tests are turned off on Android.  I
should just make this false with a TODO.

[reveman, 2014/10/27 17:19:04]

Sounds like we're missing a check to determine if this extension is available.
If we can test this or not doesn't depend on the platform but available
extensions.

[enne (OOO), 2014/10/27 17:56:22]

That seemed likely to me too.  It seems like something that could be followed up
on, along with turning this framework on for other patches.

+      return false;
+#else
+      return true;
+#endif
+    case NUM_PIXEL_RESOURCE_TEST_CASES:
+      NOTREACHED();
+      break;
+  }
+
+  NOTREACHED();
+  return false;
+}

[reveman, 2014/10/23 20:39:27]

I think we should try to get this function removed and all our configurations
tested on all platforms asap.

[enne (OOO), 2014/10/24 22:21:28]

No question! I just wanted to start somewhere.  This patch is about adding
testing support for texture rectangle arb and masks, not testing everything
everywhere.  The other obvious follow-up would be to make more pixel tests use
this framework.

[reveman, 2014/10/27 17:19:04]

Sounds good. I just wanted to make sure we're on the same page.

+
+struct {
+  LayerTreePixelTest::PixelTestType test_type;
+  unsigned staging_texture_target;
+  unsigned draw_texture_target;
+  LayerTreeHostPixelResourceTest::ResourcePoolOption resource_pool_option;
+} ResourcePoolTestConfig[NUM_PIXEL_RESOURCE_TEST_CASES] = {

[reveman, 2014/10/23 20:39:27]

Please consider replacing this with a set of functions with switch cases
instead. GetPixelTestType(PixelResourceTestCase test_case),
GetRasterTextureTarget(PixelResourceTestCase test_case)... or just one function
with one switch statement that returns a ResourcePoolTestConfig if you prefer
that.

As that would allow you to remove NUM_PIXEL_RESOURCE_TEST_CASES and make adding
new configurations easier as the compiler would force us to think about all
cases and all that good stuff..

[enne (OOO), 2014/10/24 22:21:28]

I like all the values set together, because it made it easier to understand the
entire test case.  So, single functions sound strictly worse to me.

To be honest, I don't see much difference at all between a struct with data or a
switch statement that sets values.  Adding a new configuration doesn't make any
difference; the compiler would complain if you added a new enum and didn't
update this array initialization or didn't update a switch statement.

So, sure.  I'll make it a function if that's to your taste.

+      // SOFTWARE
+      {
+       LayerTreePixelTest::PIXEL_TEST_SOFTWARE,
+       GL_INVALID_VALUE,
+       GL_INVALID_VALUE,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_BITMAP_RASTER_WORKER,
+      },
+      // GL_GPU_RASTER_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_INVALID_VALUE,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_GPU_RASTER,
+      },
+      // GL_ONE_COPY_2D_STAGING_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_TEXTURE_RECTANGLE_ARB,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ONE_COPY,
+      },
+      // GL_ONE_COPY_RECT_STAGING_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_TEXTURE_2D,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ONE_COPY,
+      },
+      // GL_ONE_COPY_EXTERNAL_STAGING_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_TEXTURE_EXTERNAL_OES,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ONE_COPY,
+      },
+      // GL_ZERO_COPY_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_INVALID_VALUE,
+       GL_TEXTURE_RECTANGLE_ARB,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ZERO_COPY,
+      },
+      // GL_ZERO_COPY_RECT_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_INVALID_VALUE,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ZERO_COPY,
+      },
+      // GL_ZERO_COPY_EXTERNAL_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_INVALID_VALUE,
+       GL_TEXTURE_EXTERNAL_OES,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_ZERO_COPY,
+      },
+      // GL_ASYNC_UPLOAD_2D_DRAW
+      {
+       LayerTreePixelTest::PIXEL_TEST_GL,
+       GL_INVALID_VALUE,
+       GL_TEXTURE_2D,
+       LayerTreeHostPixelResourceTest::RESOURCE_POOL_PIXEL_BUFFER_RASTER,
+      },
+};
+
+}  // namespace
+
+LayerTreeHostPixelResourceTest::LayerTreeHostPixelResourceTest(
+    PixelResourceTestCase test_case)
+    : staging_texture_target_(
+          ResourcePoolTestConfig[test_case].staging_texture_target),
+      draw_texture_target_(
+          ResourcePoolTestConfig[test_case].draw_texture_target),
+      resource_pool_option_(
+          ResourcePoolTestConfig[test_case].resource_pool_option),
+      test_case_(test_case) {
+  test_type_ = ResourcePoolTestConfig[test_case].test_type;
+}
+
+void LayerTreeHostPixelResourceTest::CreateResourceAndRasterWorkerPool(

[enne (OOO), 2014/10/23 19:24:15]

This function ends up looking really long, but I don't think it's too bad on the
whole.  I think this is better than the settings approach, as it lets us
forcibly set everything in tests as they should be.

+    LayerTreeHostImpl* host_impl,
+    scoped_ptr<RasterWorkerPool>* raster_worker_pool,
+    scoped_ptr<ResourcePool>* resource_pool,
+    scoped_ptr<ResourcePool>* staging_resource_pool) {
+  base::SingleThreadTaskRunner* task_runner =
+      proxy()->HasImplThread() ? proxy()->ImplThreadTaskRunner()
+                               : proxy()->MainThreadTaskRunner();
+  DCHECK(task_runner);
+
+  ContextProvider* context_provider =
+      host_impl->output_surface()->context_provider();
+  ResourceProvider* resource_provider = host_impl->resource_provider();
+  if (resource_pool_option_ == RESOURCE_POOL_BITMAP_RASTER_WORKER) {

[reveman, 2014/10/23 20:39:27]

Can you use a switch statement here instead of this long else-if section?

[enne (OOO), 2014/10/24 22:21:28]

Sure.

+    EXPECT_FALSE(context_provider);
+    EXPECT_EQ(PIXEL_TEST_SOFTWARE, test_type_);
+    *resource_pool =
+        ResourcePool::Create(resource_provider,
+                             draw_texture_target_,
+                             resource_provider->best_texture_format());
+
+    *raster_worker_pool = BitmapRasterWorkerPool::Create(
+        task_runner, RasterWorkerPool::GetTaskGraphRunner(), resource_provider);
+  } else if (resource_pool_option_ == RESOURCE_POOL_GPU_RASTER) {
+    EXPECT_TRUE(context_provider);
+    EXPECT_EQ(PIXEL_TEST_GL, test_type_);
+    *resource_pool =
+        ResourcePool::Create(resource_provider,
+                             draw_texture_target_,
+                             resource_provider->best_texture_format());
+
+    bool use_distance_field_text = false;
+    *raster_worker_pool = GpuRasterWorkerPool::Create(task_runner,
+                                                      context_provider,
+                                                      resource_provider,
+                                                      use_distance_field_text);
+  } else if (resource_pool_option_ == RESOURCE_POOL_ZERO_COPY) {
+    EXPECT_TRUE(context_provider);
+    EXPECT_EQ(PIXEL_TEST_GL, test_type_);
+    EXPECT_TRUE(host_impl->CanUseZeroCopyRasterizer());
+    *resource_pool =
+        ResourcePool::Create(resource_provider,
+                             draw_texture_target_,
+                             resource_provider->best_texture_format());
+
+    *raster_worker_pool = ZeroCopyRasterWorkerPool::Create(
+        task_runner, RasterWorkerPool::GetTaskGraphRunner(), resource_provider);
+  } else if (resource_pool_option_ == RESOURCE_POOL_ONE_COPY) {
+    EXPECT_TRUE(context_provider);
+    EXPECT_EQ(PIXEL_TEST_GL, test_type_);
+    EXPECT_TRUE(host_impl->CanUseOneCopyRasterizer());
+    // We need to create a staging resource pool when using copy rasterizer.
+    *staging_resource_pool =
+        ResourcePool::Create(resource_provider,
+                             staging_texture_target_,
+                             resource_provider->best_texture_format());
+    *resource_pool =
+        ResourcePool::Create(resource_provider,
+                             draw_texture_target_,
+                             resource_provider->best_texture_format());
+
+    *raster_worker_pool =
+        OneCopyRasterWorkerPool::Create(task_runner,
+                                        RasterWorkerPool::GetTaskGraphRunner(),
+                                        context_provider,
+                                        resource_provider,
+                                        staging_resource_pool->get());
+  } else if (resource_pool_option_ == RESOURCE_POOL_PIXEL_BUFFER_RASTER) {
+    EXPECT_TRUE(context_provider);
+    EXPECT_EQ(PIXEL_TEST_GL, test_type_);
+    *resource_pool = ResourcePool::Create(
+        resource_provider,
+        draw_texture_target_,
+        resource_provider->memory_efficient_texture_format());
+
+    size_t max_transfer_buffer_usage_bytes = 1024u * 1024u * 60u;
+    *raster_worker_pool = PixelBufferRasterWorkerPool::Create(
+        task_runner,
+        RasterWorkerPool::GetTaskGraphRunner(),
+        context_provider,
+        resource_provider,
+        max_transfer_buffer_usage_bytes);
+  } else {
+    NOTREACHED();
+  }
+}
+
+void LayerTreeHostPixelResourceTest::RunPixelResourceTest(
+    scoped_refptr<Layer> content_root,
+    base::FilePath file_name) {
+  if (!IsTestCaseSupported(test_case_))
+    return;
+  RunPixelTest(test_type_, content_root, file_name);
+}
+
+ParameterizedPixelResourceTest::ParameterizedPixelResourceTest()
+    : LayerTreeHostPixelResourceTest(GetParam()) {
+}
+
+}  // namespace cc