Pass correct texture hint for GPU raster

Pass correct texture hint for GPU raster

Currently, ResourcePool creates all resources with
ResourceProvider::TEXTURE_HINT_IMMUTABLE. This is fine for SW raster,
however we should be using TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER for GPU
raster. Using the correct flag passes additional information to ANGLE
via a GL extension and causes us to skip an unnecessary texture copy.

R=vmpstr
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/da59d418f54604ba2451cd0ef3a9cd42c05ca530
Cr-Commit-Position: refs/heads/master@{#433437}

[ericrk, 2016-11-17 02:22:43]

Description was changed from

==========
Pass correct texture hint for GPU raster

Currently, ResourcePool creates all resources with
ResourceProvider::TEXTURE_HINT_IMMUTABLE. This is fine for SW raster,
however we should be using TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER for GPU
raster. Using the correct flag passes additional information to ANGLE
via a GL extension and causes us to skip an unnecessary texture copy.

R=vmpstr
==========

to

==========
Pass correct texture hint for GPU raster

Currently, ResourcePool creates all resources with
ResourceProvider::TEXTURE_HINT_IMMUTABLE. This is fine for SW raster,
however we should be using TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER for GPU
raster. Using the correct flag passes additional information to ANGLE
via a GL extension and causes us to skip an unnecessary texture copy.

R=vmpstr
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

[ericrk, 2016-11-17 02:24:42]

The CQ bit was checked by ericrk@chromium.org to run a CQ dry run

[commit-bot: I haz the power, 2016-11-17 02:26:01]

Dry run: CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 04:28:45]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-17 04:28:45]

Dry run: Try jobs failed on following builders:
  android_arm64_dbg_recipe on master.tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_clang_dbg_recipe on master.tryserver.chromium.android (JOB_TIMED_OUT,
no build URL)
  android_compile_dbg on master.tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)
  android_cronet on master.tryserver.chromium.android (JOB_TIMED_OUT, no build
URL)
  cast_shell_android on master.tryserver.chromium.android (JOB_TIMED_OUT, no
build URL)

[vmpstr, 2016-11-17 19:17:56]

lgtm

https://codereview.chromium.org/2507963003/diff/1/cc/resources/resource_pool.h
File cc/resources/resource_pool.h (right):

https://codereview.chromium.org/2507963003/diff/1/cc/resources/resource_pool....
cc/resources/resource_pool.h:170: ResourceProvider* resource_provider_;
While here, do you mind initializing these as well? Even if they are overriden
by all constructors

[ericrk, 2016-11-17 21:18:27]

The CQ bit was checked by ericrk@chromium.org

[ericrk, 2016-11-17 21:18:27]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2507963003/#ps20001
(title: "Default initialize more vars")

[commit-bot: I haz the power, 2016-11-17 21:20:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-17 23:08:53]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-17 23:08:54]

Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)

[ericrk, 2016-11-17 23:09:26]

The CQ bit was checked by ericrk@chromium.org

[commit-bot: I haz the power, 2016-11-17 23:11:29]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 00:06:33]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 00:06:34]

Try jobs failed on following builders:
  linux_trusty_blink_rel on master.tryserver.blink (JOB_FAILED,
http://build.chromium.org/p/tryserver.blink/builders/linux_trusty_blink_rel/b...)
  linux_chromium_chromeos_ozone_rel_ng on master.tryserver.chromium.linux
(JOB_FAILED,
http://build.chromium.org/p/tryserver.chromium.linux/builders/linux_chromium_...)

[ericrk, 2016-11-18 00:17:27]

The CQ bit was checked by ericrk@chromium.org

[commit-bot: I haz the power, 2016-11-18 00:19:10]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-18 03:12:29]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-18 03:12:32]

Failed to apply patch for cc/resources/resource_pool.h:
While running git apply --index -p1;
  error: patch failed: cc/resources/resource_pool.h:99
  error: cc/resources/resource_pool.h: patch does not apply

Patch:       cc/resources/resource_pool.h
Index: cc/resources/resource_pool.h
diff --git a/cc/resources/resource_pool.h b/cc/resources/resource_pool.h
index
a1bd9d696cb14933dcdd1d63d5d3e4aef5117dd1..d8aaae781b9b0cdebd8c6eb0db28b66352df52a5
100644
--- a/cc/resources/resource_pool.h
+++ b/cc/resources/resource_pool.h
@@ -34,18 +34,17 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
       base::SingleThreadTaskRunner* task_runner,
       gfx::BufferUsage usage,
       const base::TimeDelta& expiration_delay) {
-    std::unique_ptr<ResourcePool> pool(new ResourcePool(
-        resource_provider, task_runner, true, expiration_delay));
-    pool->usage_ = usage;
-    return pool;
+    return base::WrapUnique(new ResourcePool(resource_provider, task_runner,
+                                             usage, expiration_delay));
   }
 
   static std::unique_ptr<ResourcePool> Create(
       ResourceProvider* resource_provider,
       base::SingleThreadTaskRunner* task_runner,
+      ResourceProvider::TextureHint hint,
       const base::TimeDelta& expiration_delay) {
     return base::WrapUnique(new ResourcePool(resource_provider, task_runner,
-                                             false, expiration_delay));
+                                             hint, expiration_delay));
   }
 
   ~ResourcePool() override;
@@ -99,9 +98,16 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   }
 
  protected:
+  // Constructor for creating GPU memory buffer resources.
+  ResourcePool(ResourceProvider* resource_provider,
+               base::SingleThreadTaskRunner* task_runner,
+               gfx::BufferUsage usage,
+               const base::TimeDelta& expiration_delay);
+
+  // Constructor for creating standard resources.
   ResourcePool(ResourceProvider* resource_provider,
                base::SingleThreadTaskRunner* task_runner,
-               bool use_gpu_memory_buffers,
+               ResourceProvider::TextureHint hint,
                const base::TimeDelta& expiration_delay);
 
   bool ResourceUsageTooHigh();
@@ -161,14 +167,15 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   bool HasEvictableResources() const;
   base::TimeTicks GetUsageTimeForLRUResource() const;
 
-  ResourceProvider* resource_provider_;
-  bool use_gpu_memory_buffers_;
-  gfx::BufferUsage usage_;
-  size_t max_memory_usage_bytes_;
-  size_t max_resource_count_;
-  size_t in_use_memory_usage_bytes_;
-  size_t total_memory_usage_bytes_;
-  size_t total_resource_count_;
+  ResourceProvider* resource_provider_ = nullptr;
+  bool use_gpu_memory_buffers_ = false;
+  gfx::BufferUsage usage_ = gfx::BufferUsage::GPU_READ_CPU_READ_WRITE;
+  ResourceProvider::TextureHint hint_ = ResourceProvider::TEXTURE_HINT_DEFAULT;
+  size_t max_memory_usage_bytes_ = 0;
+  size_t max_resource_count_ = 0;
+  size_t in_use_memory_usage_bytes_ = 0;
+  size_t total_memory_usage_bytes_ = 0;
+  size_t total_resource_count_ = 0;
 
   // Holds most recently used resources at the front of the queue.
   using ResourceDeque = std::deque<std::unique_ptr<PoolResource>>;
@@ -179,7 +186,7 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   InUseResourceMap in_use_resources_;
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-  bool evict_expired_resources_pending_;
+  bool evict_expired_resources_pending_ = false;
   const base::TimeDelta resource_expiration_delay_;
 
   base::WeakPtrFactory<ResourcePool> weak_ptr_factory_;

[ericrk, 2016-11-19 01:33:23]

The CQ bit was checked by ericrk@chromium.org

[commit-bot: I haz the power, 2016-11-19 01:33:58]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-19 03:38:21]

The CQ bit was unchecked by commit-bot@chromium.org

[commit-bot: I haz the power, 2016-11-19 03:38:23]

Failed to apply patch for cc/resources/resource_pool.h:
While running git apply --index -p1;
  error: patch failed: cc/resources/resource_pool.h:99
  error: cc/resources/resource_pool.h: patch does not apply

Patch:       cc/resources/resource_pool.h
Index: cc/resources/resource_pool.h
diff --git a/cc/resources/resource_pool.h b/cc/resources/resource_pool.h
index
a1bd9d696cb14933dcdd1d63d5d3e4aef5117dd1..d8aaae781b9b0cdebd8c6eb0db28b66352df52a5
100644
--- a/cc/resources/resource_pool.h
+++ b/cc/resources/resource_pool.h
@@ -34,18 +34,17 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
       base::SingleThreadTaskRunner* task_runner,
       gfx::BufferUsage usage,
       const base::TimeDelta& expiration_delay) {
-    std::unique_ptr<ResourcePool> pool(new ResourcePool(
-        resource_provider, task_runner, true, expiration_delay));
-    pool->usage_ = usage;
-    return pool;
+    return base::WrapUnique(new ResourcePool(resource_provider, task_runner,
+                                             usage, expiration_delay));
   }
 
   static std::unique_ptr<ResourcePool> Create(
       ResourceProvider* resource_provider,
       base::SingleThreadTaskRunner* task_runner,
+      ResourceProvider::TextureHint hint,
       const base::TimeDelta& expiration_delay) {
     return base::WrapUnique(new ResourcePool(resource_provider, task_runner,
-                                             false, expiration_delay));
+                                             hint, expiration_delay));
   }
 
   ~ResourcePool() override;
@@ -99,9 +98,16 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   }
 
  protected:
+  // Constructor for creating GPU memory buffer resources.
+  ResourcePool(ResourceProvider* resource_provider,
+               base::SingleThreadTaskRunner* task_runner,
+               gfx::BufferUsage usage,
+               const base::TimeDelta& expiration_delay);
+
+  // Constructor for creating standard resources.
   ResourcePool(ResourceProvider* resource_provider,
                base::SingleThreadTaskRunner* task_runner,
-               bool use_gpu_memory_buffers,
+               ResourceProvider::TextureHint hint,
                const base::TimeDelta& expiration_delay);
 
   bool ResourceUsageTooHigh();
@@ -161,14 +167,15 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   bool HasEvictableResources() const;
   base::TimeTicks GetUsageTimeForLRUResource() const;
 
-  ResourceProvider* resource_provider_;
-  bool use_gpu_memory_buffers_;
-  gfx::BufferUsage usage_;
-  size_t max_memory_usage_bytes_;
-  size_t max_resource_count_;
-  size_t in_use_memory_usage_bytes_;
-  size_t total_memory_usage_bytes_;
-  size_t total_resource_count_;
+  ResourceProvider* resource_provider_ = nullptr;
+  bool use_gpu_memory_buffers_ = false;
+  gfx::BufferUsage usage_ = gfx::BufferUsage::GPU_READ_CPU_READ_WRITE;
+  ResourceProvider::TextureHint hint_ = ResourceProvider::TEXTURE_HINT_DEFAULT;
+  size_t max_memory_usage_bytes_ = 0;
+  size_t max_resource_count_ = 0;
+  size_t in_use_memory_usage_bytes_ = 0;
+  size_t total_memory_usage_bytes_ = 0;
+  size_t total_resource_count_ = 0;
 
   // Holds most recently used resources at the front of the queue.
   using ResourceDeque = std::deque<std::unique_ptr<PoolResource>>;
@@ -179,7 +186,7 @@ class CC_EXPORT ResourcePool : public
base::trace_event::MemoryDumpProvider,
   InUseResourceMap in_use_resources_;
 
   scoped_refptr<base::SingleThreadTaskRunner> task_runner_;
-  bool evict_expired_resources_pending_;
+  bool evict_expired_resources_pending_ = false;
   const base::TimeDelta resource_expiration_delay_;
 
   base::WeakPtrFactory<ResourcePool> weak_ptr_factory_;

[ericrk, 2016-11-19 23:44:57]

The CQ bit was checked by ericrk@chromium.org

[ericrk, 2016-11-19 23:44:57]

The patchset sent to the CQ was uploaded after l-g-t-m from vmpstr@chromium.org
Link to the patchset: https://codereview.chromium.org/2507963003/#ps40001
(title: "rebase")

[commit-bot: I haz the power, 2016-11-19 23:45:05]

CQ is trying da patch. Follow status at
 
https://chromium-cq-status.appspot.com/v2/patch-status/codereview.chromium.or...

[commit-bot: I haz the power, 2016-11-20 00:44:12]

CQ is committing da patch.

Bot data: {"patchset_id": 40001, "attempt_start_ts": 1479599097342470,
"parent_rev": ["429168fc27a9b6760eb90144ab17f7bcdfbd282a", null], "commit_rev":
["a57ef36831af6480588ef285bb7a6b106722f816", null]}

[commit-bot: I haz the power, 2016-11-20 00:44:34]

Committed patchset #3 (id:40001)

[commit-bot: I haz the power, 2016-11-20 00:45:56]

Description was changed from

==========
Pass correct texture hint for GPU raster

Currently, ResourcePool creates all resources with
ResourceProvider::TEXTURE_HINT_IMMUTABLE. This is fine for SW raster,
however we should be using TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER for GPU
raster. Using the correct flag passes additional information to ANGLE
via a GL extension and causes us to skip an unnecessary texture copy.

R=vmpstr
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel
==========

to

==========
Pass correct texture hint for GPU raster

Currently, ResourcePool creates all resources with
ResourceProvider::TEXTURE_HINT_IMMUTABLE. This is fine for SW raster,
however we should be using TEXTURE_HINT_IMMUTABLE_FRAMEBUFFER for GPU
raster. Using the correct flag passes additional information to ANGLE
via a GL extension and causes us to skip an unnecessary texture copy.

R=vmpstr
CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Committed: https://crrev.com/da59d418f54604ba2451cd0ef3a9cd42c05ca530
Cr-Commit-Position: refs/heads/master@{#433437}
==========

[commit-bot: I haz the power, 2016-11-20 00:45:58]

Patchset 3 (id:??) landed as
https://crrev.com/da59d418f54604ba2451cd0ef3a9cd42c05ca530
Cr-Commit-Position: refs/heads/master@{#433437}