cc: Use CHROMIUM_copy_image for one-copy tile updates.

cc/raster/staging_buffer_pool.cc

Index: cc/raster/staging_buffer_pool.cc
diff --git a/cc/raster/staging_buffer_pool.cc b/cc/raster/staging_buffer_pool.cc
index 0e2a88a14a7f84fa26e17b42c8e320c702438eca..7453d4f9a1882cf39de7a97221367124a480b3fb 100644
--- a/cc/raster/staging_buffer_pool.cc
+++ b/cc/raster/staging_buffer_pool.cc
@@ -15,76 +15,51 @@
 #include "cc/debug/traced_value.h"
 #include "cc/resources/scoped_resource.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
+#include "ui/gfx/gpu_fence.h"
 #include "ui/gfx/gpu_memory_buffer_tracing.h"
 
 namespace cc {
 namespace {
 
-// Delay between checking for query result to be available.
-const int kCheckForQueryResultAvailableTickRateMs = 1;
-
-// Number of attempts to allow before we perform a check that will wait for
-// query to complete.
-const int kMaxCheckForQueryResultAvailableAttempts = 256;
+// Maximum amount of time to wait for fence before we call glFinish().
+const int kWaitForFenceMaxTimeMs = 256;
 
 // Delay before a staging buffer might be released.
 const int kStagingBufferExpirationDelayMs = 1000;
 
-bool CheckForQueryResult(gpu::gles2::GLES2Interface* gl, unsigned query_id) {
-  unsigned complete = 1;
-  gl->GetQueryObjectuivEXT(query_id, GL_QUERY_RESULT_AVAILABLE_EXT, &complete);
-  return !!complete;
-}
-
-void WaitForQueryResult(gpu::gles2::GLES2Interface* gl, unsigned query_id) {
-  TRACE_EVENT0("cc", "WaitForQueryResult");
-
-  int attempts_left = kMaxCheckForQueryResultAvailableAttempts;
-  while (attempts_left--) {
-    if (CheckForQueryResult(gl, query_id))
-      break;
-
-    // We have to flush the context to be guaranteed that a query result will
-    // be available in a finite amount of time.
-    gl->ShallowFlushCHROMIUM();
+bool WaitForGpuFence(gfx::GpuFence* gpu_fence) {
+  TRACE_EVENT0("cc", "WaitForGpuFence");
 
-    base::PlatformThread::Sleep(base::TimeDelta::FromMilliseconds(
-        kCheckForQueryResultAvailableTickRateMs));
+  base::TimeTicks now = base::TimeTicks::Now();
+  base::TimeTicks end =
+      now + base::TimeDelta::FromMilliseconds(kWaitForFenceMaxTimeMs);
+  while (now < end) {
+    if (gpu_fence->Wait(end - now))

[ericrk, 2017/01/10 03:50:31]

nit: I understand that things like this sometimes return early, but I wonder if
it's nicer for the GpuFence::Wait implementation to handle this logic? Not sure
if the "may return early" is desirable in other use cases.

[reveman, 2017/01/10 19:37:55]

That was my initial approach but I decided it was better to just push this up to
the caller where it can in some cases be handled more efficiently and it's also
consistent with base::ConditionVariable
(https://cs.chromium.org/chromium/src/base/synchronization/condition_variable....)

+      return true;
+    now = base::TimeTicks::Now();
   }
-
-  unsigned result = 0;
-  gl->GetQueryObjectuivEXT(query_id, GL_QUERY_RESULT_EXT, &result);
+  return false;
 }
 
 }  // namespace
 
 StagingBuffer::StagingBuffer(const gfx::Size& size, ResourceFormat format)
-    : size(size),
-      format(format),
-      texture_id(0),
-      image_id(0),
-      query_id(0),
-      content_id(0) {}
+    : size(size), format(format), image_id(0), fence_id(0), content_id(0) {}
 
 StagingBuffer::~StagingBuffer() {
-  DCHECK_EQ(texture_id, 0u);
   DCHECK_EQ(image_id, 0u);
-  DCHECK_EQ(query_id, 0u);
+  DCHECK_EQ(fence_id, 0u);
 }
 
 void StagingBuffer::DestroyGLResources(gpu::gles2::GLES2Interface* gl) {
-  if (query_id) {
-    gl->DeleteQueriesEXT(1, &query_id);
-    query_id = 0;
+  if (fence_id) {
+    gl->DestroyFenceCHROMIUM(fence_id);
+    fence_id = 0;
   }
   if (image_id) {
     gl->DestroyImageCHROMIUM(image_id);
     image_id = 0;
   }
-  if (texture_id) {
-    gl->DeleteTextures(1, &texture_id);
-    texture_id = 0;
-  }
 }
 
 void StagingBuffer::OnMemoryDump(base::trace_event::ProcessMemoryDump* pmd,
@@ -249,22 +224,18 @@ std::unique_ptr<StagingBuffer> StagingBufferPool::AcquireStagingBuffer(
 
   std::unique_ptr<StagingBuffer> staging_buffer;
 
-  ContextProvider::ScopedContextLock scoped_context(worker_context_provider_);
-
-  gpu::gles2::GLES2Interface* gl = scoped_context.ContextGL();
-  DCHECK(gl);
-
   // Check if any busy buffers have become available.
-  if (resource_provider_->use_sync_query()) {
-    while (!busy_buffers_.empty()) {
-      if (!CheckForQueryResult(gl, busy_buffers_.front()->query_id))
-        break;
+  while (!busy_buffers_.empty()) {
+    if (!busy_buffers_.front()->gpu_fence->IsSignaled())
+      break;
 
-      MarkStagingBufferAsFree(busy_buffers_.front().get());
-      free_buffers_.push_back(PopFront(&busy_buffers_));
-    }
+    busy_buffers_.front()->gpu_fence->Reset();
+    MarkStagingBufferAsFree(busy_buffers_.front().get());
+    free_buffers_.push_back(PopFront(&busy_buffers_));
   }
 
+  bool flushed = false;
+
   // Wait for memory usage of non-free buffers to become less than the limit.
   while (
       (staging_buffer_usage_in_bytes_ - free_staging_buffer_usage_in_bytes_) >=
@@ -273,14 +244,33 @@ std::unique_ptr<StagingBuffer> StagingBufferPool::AcquireStagingBuffer(
     if (busy_buffers_.empty())
       break;
 
-    if (resource_provider_->use_sync_query()) {
-      WaitForQueryResult(gl, busy_buffers_.front()->query_id);
+    if (!flushed) {
+      ContextProvider::ScopedContextLock scoped_context(
+          worker_context_provider_);
+      gpu::gles2::GLES2Interface* gl = scoped_context.ContextGL();
+      DCHECK(gl);
+
+      // We have to flush the context to be guaranteed that fence will signel in
+      // a finite amount of time.
+      gl->ShallowFlushCHROMIUM();
+      flushed = true;
+    }
+
+    if (WaitForGpuFence(busy_buffers_.front()->gpu_fence.get())) {
+      busy_buffers_.front()->gpu_fence->Reset();
       MarkStagingBufferAsFree(busy_buffers_.front().get());
       free_buffers_.push_back(PopFront(&busy_buffers_));
     } else {
-      // Fall-back to glFinish if CHROMIUM_sync_query is not available.
+      ContextProvider::ScopedContextLock scoped_context(
+          worker_context_provider_);
+      gpu::gles2::GLES2Interface* gl = scoped_context.ContextGL();
+      DCHECK(gl);
+
+      // glFinish() should result in the fence signaling or the context is lost
+      // and fence will never signal.
       gl->Finish();
       while (!busy_buffers_.empty()) {
+        busy_buffers_.front()->gpu_fence->Reset();
         MarkStagingBufferAsFree(busy_buffers_.front().get());
         free_buffers_.push_back(PopFront(&busy_buffers_));
       }
@@ -329,7 +319,15 @@ std::unique_ptr<StagingBuffer> StagingBufferPool::AcquireStagingBuffer(
     if (free_buffers_.empty())
       break;
 
-    free_buffers_.front()->DestroyGLResources(gl);
+    {
+      ContextProvider::ScopedContextLock scoped_context(

[ericrk, 2017/01/10 03:50:31]

not sure what the overhead of locking/re-locking, but I wonder if it's worth
making scoped_context a unique_ptr or base::Optional scoped outside of the while
loop. It could be initialized the first time it would be used - so it's only
taken once, if necessary.

I guess that brings the MarkStagingBuffersAsBusy and RemoveStagingBuffers calls
into the lock scope, but they seem pretty lightweight?

[reveman, 2017/01/10 19:37:55]

Makes sense. I'll address this as I rebase this patch.

+          worker_context_provider_);
+      gpu::gles2::GLES2Interface* gl = scoped_context.ContextGL();
+      DCHECK(gl);
+
+      free_buffers_.front()->DestroyGLResources(gl);
+    }
+
     MarkStagingBufferAsBusy(free_buffers_.front().get());
     RemoveStagingBuffer(free_buffers_.front().get());
     free_buffers_.pop_front();