cc: Add flag to specify to ResourceProvider to always use GMBs

cc/resources/resource_provider.cc

Index: cc/resources/resource_provider.cc
diff --git a/cc/resources/resource_provider.cc b/cc/resources/resource_provider.cc
index 2f5b0a9d7917ea2fd00472cea2992a0c268b67a2..1a6839be343524318af0d4cbcaee15b63f3cb3de 100644
--- a/cc/resources/resource_provider.cc
+++ b/cc/resources/resource_provider.cc
@@ -324,11 +324,13 @@ scoped_ptr<ResourceProvider> ResourceProvider::Create(
     BlockingTaskRunner* blocking_main_thread_task_runner,
     int highp_threshold_min,
     size_t id_allocation_chunk_size,
+    bool use_gpu_memory_buffers_by_default,
     const std::vector<unsigned>& use_image_texture_targets) {
   scoped_ptr<ResourceProvider> resource_provider(new ResourceProvider(
       output_surface, shared_bitmap_manager, gpu_memory_buffer_manager,
       blocking_main_thread_task_runner, highp_threshold_min,
-      id_allocation_chunk_size, use_image_texture_targets));
+      id_allocation_chunk_size, use_gpu_memory_buffers_by_default,
+      use_image_texture_targets));
   resource_provider->Initialize();
   return resource_provider;
 }
@@ -386,10 +388,10 @@ ResourceId ResourceProvider::CreateResource(const gfx::Size& size,
   DCHECK(!size.IsEmpty());
   switch (default_resource_type_) {
     case RESOURCE_TYPE_GL_TEXTURE:
-      return CreateGLTexture(size,
-                             GL_TEXTURE_2D,
-                             hint,
-                             format);
+      return CreateGLTexture(size, use_gpu_memory_buffers_by_default_
+                                       ? GetImageTextureTarget(format)
+                                       : GL_TEXTURE_2D,
+                             hint, format);
     case RESOURCE_TYPE_BITMAP:
       DCHECK_EQ(RGBA_8888, format);
       return CreateBitmap(size);
@@ -860,13 +862,10 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
     ScopedWriteLockGpuMemoryBuffer(ResourceProvider* resource_provider,
                                    ResourceId resource_id)
     : resource_provider_(resource_provider),
-      resource_(resource_provider->LockForWrite(resource_id)),
-      gpu_memory_buffer_manager_(resource_provider->gpu_memory_buffer_manager_),
-      gpu_memory_buffer_(nullptr),
-      size_(resource_->size),
-      format_(resource_->format) {
+      resource_(resource_provider->LockForWrite(resource_id)) {
   DCHECK_EQ(RESOURCE_TYPE_GL_TEXTURE, resource_->type);
-  std::swap(gpu_memory_buffer_, resource_->gpu_memory_buffer);
+  gpu_memory_buffer_.reset(resource_->gpu_memory_buffer);
+  resource_->gpu_memory_buffer = nullptr;
 }
 
 ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
@@ -876,27 +875,7 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
   if (!gpu_memory_buffer_)
     return;
 
-  resource_provider_->LazyCreate(resource_);
-
-  if (!resource_->image_id) {
-    GLES2Interface* gl = resource_provider_->ContextGL();
-    DCHECK(gl);
-
-#if defined(OS_CHROMEOS)
-    // TODO(reveman): GL_COMMANDS_ISSUED_CHROMIUM is used for synchronization
-    // on ChromeOS to avoid some performance issues. This only works with
-    // shared memory backed buffers. crbug.com/436314
-    DCHECK_EQ(gpu_memory_buffer_->GetHandle().type, gfx::SHARED_MEMORY_BUFFER);
-#endif
-
-    resource_->image_id = gl->CreateImageCHROMIUM(
-        gpu_memory_buffer_->AsClientBuffer(), size_.width(), size_.height(),
-        GLInternalFormat(resource_->format));
-  }
-
-  std::swap(resource_->gpu_memory_buffer, gpu_memory_buffer_);
-  resource_->allocated = true;
-  resource_->dirty_image = true;
+  resource_provider_->SetGpuMemoryBuffer(resource_, gpu_memory_buffer_.Pass());
 
   // GpuMemoryBuffer provides direct access to the memory used by the GPU.
   // Read lock fences are required to ensure that we're not trying to map a
@@ -906,14 +885,13 @@ ResourceProvider::ScopedWriteLockGpuMemoryBuffer::
 
 gfx::GpuMemoryBuffer*
 ResourceProvider::ScopedWriteLockGpuMemoryBuffer::GetGpuMemoryBuffer() {
-  if (gpu_memory_buffer_)
-    return gpu_memory_buffer_;
-  scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer =
-      gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(
-          size_, BufferFormat(format_),
-          gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
-  gpu_memory_buffer_ = gpu_memory_buffer.release();
-  return gpu_memory_buffer_;
+  if (!gpu_memory_buffer_) {
+    gpu_memory_buffer_ =
+        resource_provider_->gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(
+            resource_->size, BufferFormat(resource_->format),
+            gfx::BufferUsage::GPU_READ_CPU_READ_WRITE);
+  }
+  return gpu_memory_buffer_.get();
 }
 
 ResourceProvider::ScopedWriteLockGr::ScopedWriteLockGr(
@@ -1002,6 +980,7 @@ ResourceProvider::ResourceProvider(
     BlockingTaskRunner* blocking_main_thread_task_runner,
     int highp_threshold_min,
     size_t id_allocation_chunk_size,
+    bool use_gpu_memory_buffers_by_default,
     const std::vector<unsigned>& use_image_texture_targets)
     : output_surface_(output_surface),
       shared_bitmap_manager_(shared_bitmap_manager),
@@ -1012,6 +991,7 @@ ResourceProvider::ResourceProvider(
       next_id_(1),
       next_child_(1),
       default_resource_type_(RESOURCE_TYPE_BITMAP),
+      use_gpu_memory_buffers_by_default_(use_gpu_memory_buffers_by_default),
       use_texture_storage_ext_(false),
       use_texture_format_bgra_(false),
       use_texture_usage_hint_(false),
@@ -1309,8 +1289,7 @@ void ResourceProvider::TransferResource(GLES2Interface* gl,
     LazyCreate(source);
     DCHECK(source->gl_id);
     DCHECK(source->origin == Resource::INTERNAL);
-    if (source->image_id) {
-      DCHECK(source->dirty_image);
+    if (source->image_id && source->dirty_image) {
       gl->BindTexture(resource->mailbox_holder.texture_target, source->gl_id);
       BindImageForSampling(source);
     }
@@ -1515,9 +1494,14 @@ void ResourceProvider::LazyAllocate(Resource* resource) {
   gfx::Size& size = resource->size;
   ResourceFormat format = resource->format;
   gl->BindTexture(resource->target, resource->gl_id);
-  if (use_texture_storage_ext_ &&
-      IsFormatSupportedForStorage(format, use_texture_format_bgra_) &&
-      (resource->hint & TEXTURE_HINT_IMMUTABLE)) {
+  if (use_gpu_memory_buffers_by_default_) {
+    SetGpuMemoryBuffer(
+        resource, gpu_memory_buffer_manager_->AllocateGpuMemoryBuffer(
+                      size, BufferFormat(format), gfx::BufferUsage::SCANOUT));
+    BindImageForSampling(resource);

[reveman, 2015/11/02 17:28:53]

Why is this BindImageForSampling call needed?

[ccameron, 2015/11/02 22:07:34]

This can be moved to the ScopedWriteLockGL this to the lock for GL read/write.

+  } else if (use_texture_storage_ext_ &&
+             IsFormatSupportedForStorage(format, use_texture_format_bgra_) &&
+             (resource->hint & TEXTURE_HINT_IMMUTABLE)) {
     GLenum storage_format = TextureToStorageFormat(format);
     gl->TexStorage2DEXT(resource->target, 1, storage_format, size.width(),
                         size.height());
@@ -1531,6 +1515,36 @@ void ResourceProvider::LazyAllocate(Resource* resource) {
   }
 }
 
+void ResourceProvider::SetGpuMemoryBuffer(

[reveman, 2015/11/02 17:28:53]

hm, this function is marking the image as dirty and lazy creating one if needed.
Can this be renamed to something like MarkImageAsDirty or LazyCreateImage and
not take a GMB as parameter but instead expect it to be set? I feel like that
would make it a bit easier to understand what this does.

[ccameron, 2015/11/02 22:07:34]

Sounds better --done.

+    Resource* resource,
+    scoped_ptr<gfx::GpuMemoryBuffer> gpu_memory_buffer) {
+  DCHECK(!resource->gpu_memory_buffer);

[reveman, 2015/11/02 17:28:53]

can these 3 lines just become DCHECK(resource->gpu_memory_buffer)?

[ccameron, 2015/11/02 22:07:34]

Yeah, this becomes much simpler this way.

+  if (!gpu_memory_buffer)
+    return;
+
+  LazyCreate(resource);

[reveman, 2015/11/02 17:28:53]

I think this should be moved out of this function now that we use it from
LazyAllocate.

[ccameron, 2015/11/02 22:07:34]

Sg. Added a LazyCreate to the ~ScopedWriteLockGpuMemoryBuffer and a
DCHECK(gl_id) to ensure that it is hit in both paths.

+
+  if (!resource->image_id) {
+    GLES2Interface* gl = ContextGL();
+    DCHECK(gl);
+
+#if defined(OS_CHROMEOS)
+    // TODO(reveman): GL_COMMANDS_ISSUED_CHROMIUM is used for synchronization
+    // on ChromeOS to avoid some performance issues. This only works with
+    // shared memory backed buffers. crbug.com/436314
+    DCHECK_EQ(gpu_memory_buffer->GetHandle().type, gfx::SHARED_MEMORY_BUFFER);
+#endif
+    resource->image_id = gl->CreateImageCHROMIUM(
+        gpu_memory_buffer->AsClientBuffer(), resource->size.width(),
+        resource->size.height(), GLInternalFormat(resource->format));
+  }
+
+  resource->gpu_memory_buffer = gpu_memory_buffer.release();
+  resource->allocated = true;

[reveman, 2015/11/02 17:28:53]

hm, this should not be set here as that means setting it twice in the
LAzyAllocate case.

[ccameron, 2015/11/02 22:07:34]

Moved up to the caller and added a DCHECK for it.

+  resource->dirty_image = true;
+  resource->is_overlay_candidate = true;

[reveman, 2015/11/02 17:28:53]

might be better to set this by the code that allocates the storage too

[ccameron, 2015/11/02 22:07:34]

Done.

+}
+
 void ResourceProvider::BindImageForSampling(Resource* resource) {
   GLES2Interface* gl = ContextGL();
   DCHECK(resource->gl_id);