cc: Avoid issuing COMMANDS_COMPLETED queries unless necessary.

Minor change to the Fence API that allows GLRenderer to not
issue sync queries unless some resource that depend on them
was used to produce a frame.

Also includes some minor renaming cleanup related to usage of
Fences in ResourceProvider code.

BUG=

Review URL: https://codereview.chromium.org/398543004

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284318 0039d316-1c4b-4281-b951-d872f2087c98

cc/output/gl_renderer.cc

@@ -63,9 +63,10 @@ namespace {
 class FallbackFence : public ResourceProvider::Fence {
  public:
   explicit FallbackFence(gpu::gles2::GLES2Interface* gl)
-      : gl_(gl), has_passed_(false) {}
+      : gl_(gl), has_passed_(true) {}
 
   // Overridden from ResourceProvider::Fence:
+  virtual void Set() OVERRIDE { has_passed_ = false; }
   virtual bool HasPassed() OVERRIDE {
     if (!has_passed_) {
       has_passed_ = true;
@@ -235,32 +236,60 @@ struct GLRenderer::PendingAsyncReadPixels {
 class GLRenderer::SyncQuery {
  public:
   explicit SyncQuery(gpu::gles2::GLES2Interface* gl)
-      : gl_(gl), query_id_(0u), weak_ptr_factory_(this) {
+      : gl_(gl), query_id_(0u), is_pending_(false), weak_ptr_factory_(this) {
     gl_->GenQueriesEXT(1, &query_id_);
   }
   virtual ~SyncQuery() { gl_->DeleteQueriesEXT(1, &query_id_); }
 
   scoped_refptr<ResourceProvider::Fence> Begin() {
-    DCHECK(!weak_ptr_factory_.HasWeakPtrs() || !IsPending());
+    DCHECK(!IsPending());
     // Invalidate weak pointer held by old fence.
     weak_ptr_factory_.InvalidateWeakPtrs();
-    gl_->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM, query_id_);
+    // Note: In case the set of drawing commands issued before End() do not
+    // depend on the query, defer BeginQueryEXT call until Set() is called and
+    // query is required.
     return make_scoped_refptr<ResourceProvider::Fence>(
         new Fence(weak_ptr_factory_.GetWeakPtr()));
   }
 
-  void End() { gl_->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM); }
+  void Set() {
+    if (is_pending_)
+      return;
+
+    // Note: BeginQueryEXT on GL_COMMANDS_COMPLETED_CHROMIUM is effectively a
+    // noop relative to GL, so it doesn't matter where it happens but we still
+    // make sure to issue this command when Set() is called (prior to issuing
+    // any drawing commands that depend on query), in case some future extension
+    // can take advantage of this.
+    gl_->BeginQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM, query_id_);
+    is_pending_ = true;
+  }
+
+  void End() {
+    if (!is_pending_)
+      return;
+
+    gl_->EndQueryEXT(GL_COMMANDS_COMPLETED_CHROMIUM);
+  }
 
   bool IsPending() {
-    unsigned available = 1;
+    if (!is_pending_)
+      return false;
+
+    unsigned result_available = 1;
     gl_->GetQueryObjectuivEXT(
-        query_id_, GL_QUERY_RESULT_AVAILABLE_EXT, &available);
-    return !available;
+        query_id_, GL_QUERY_RESULT_AVAILABLE_EXT, &result_available);
+    is_pending_ = !result_available;
+    return is_pending_;
   }
 
   void Wait() {
+    if (!is_pending_)
+      return;
+
     unsigned result = 0;
     gl_->GetQueryObjectuivEXT(query_id_, GL_QUERY_RESULT_EXT, &result);
+    is_pending_ = false;
   }
 
  private:
@@ -270,6 +299,10 @@ class GLRenderer::SyncQuery {
         : query_(query) {}
 
     // Overridden from ResourceProvider::Fence:
+    virtual void Set() OVERRIDE {
+      DCHECK(query_);
+      query_->Set();
+    }
     virtual bool HasPassed() OVERRIDE {
       return !query_ || !query_->IsPending();
     }
@@ -284,6 +317,7 @@ class GLRenderer::SyncQuery {
 
   gpu::gles2::GLES2Interface* gl_;
   unsigned query_id_;
+  bool is_pending_;
   base::WeakPtrFactory<SyncQuery> weak_ptr_factory_;
 
   DISALLOW_COPY_AND_ASSIGN(SyncQuery);

cc/resources/image_raster_worker_pool.cc

@@ -155,7 +155,7 @@ void ImageRasterWorkerPool::ReleaseCanvasForRaster(RasterTask* task) {
   // Map/UnmapImageRasterBuffer 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
   // resource that is currently in-use by the GPU.
-  resource_provider_->EnableReadLockFences(task->resource()->id(), true);
+  resource_provider_->EnableReadLockFences(task->resource()->id());
 }
 
 void ImageRasterWorkerPool::OnRasterFinished() {

cc/resources/pixel_buffer_raster_worker_pool.cc

@@ -412,7 +412,7 @@ void PixelBufferRasterWorkerPool::CheckForCompletedUploads() {
     // Async set pixels commands are not necessarily processed in-sequence with
     // drawing commands. Read lock fences are required to ensure that async
     // commands don't access the resource while used for drawing.
-    resource_provider_->EnableReadLockFences(task->resource()->id(), true);
+    resource_provider_->EnableReadLockFences(task->resource()->id());
 
     DCHECK(std::find(completed_raster_tasks_.begin(),
                      completed_raster_tasks_.end(),

cc/resources/resource_provider.cc

@@ -206,6 +206,7 @@ class QueryFence : public ResourceProvider::Fence {
       : gl_(gl), query_id_(query_id) {}
 
   // Overridden from ResourceProvider::Fence:
+  virtual void Set() OVERRIDE {}
   virtual bool HasPassed() OVERRIDE {
     unsigned available = 1;
     gl_->GetQueryObjectuivEXT(
@@ -241,7 +242,7 @@ ResourceProvider::Resource::Resource()
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
       allocated(false),
-      enable_read_lock_fences(false),
+      read_lock_fences_enabled(false),
       has_shared_bitmap_id(false),
       allow_overlay(false),
       read_lock_fence(NULL),
@@ -257,7 +258,8 @@ ResourceProvider::Resource::Resource()
       hint(TextureUsageAny),
       type(InvalidType),
       format(RGBA_8888),
-      shared_bitmap(NULL) {}
+      shared_bitmap(NULL) {
+}
 
 ResourceProvider::Resource::~Resource() {}
 
@@ -286,7 +288,7 @@ ResourceProvider::Resource::Resource(GLuint texture_id,
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
       allocated(false),
-      enable_read_lock_fences(false),
+      read_lock_fences_enabled(false),
       has_shared_bitmap_id(false),
       allow_overlay(false),
       read_lock_fence(NULL),
@@ -329,7 +331,7 @@ ResourceProvider::Resource::Resource(uint8_t* pixels,
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
       allocated(false),
-      enable_read_lock_fences(false),
+      read_lock_fences_enabled(false),
       has_shared_bitmap_id(!!bitmap),
       allow_overlay(false),
       read_lock_fence(NULL),
@@ -373,7 +375,7 @@ ResourceProvider::Resource::Resource(const SharedBitmapId& bitmap_id,
       pending_set_pixels(false),
       set_pixels_completion_forced(false),
       allocated(false),
-      enable_read_lock_fences(false),
+      read_lock_fences_enabled(false),
       has_shared_bitmap_id(true),
       allow_overlay(false),
       read_lock_fence(NULL),
@@ -1062,8 +1064,11 @@ const ResourceProvider::Resource* ResourceProvider::LockForRead(ResourceId id) {
   }
 
   resource->lock_for_read_count++;
-  if (resource->enable_read_lock_fences)
+  if (resource->read_lock_fences_enabled) {
+    if (current_read_lock_fence_)
+      current_read_lock_fence_->Set();
     resource->read_lock_fence = current_read_lock_fence_;
+  }
 
   return resource;
 }
@@ -1522,8 +1527,11 @@ void ResourceProvider::ReceiveReturnsFromParent(
 
     // Need to wait for the current read lock fence to pass before we can
     // recycle this resource.
-    if (resource->enable_read_lock_fences)
+    if (resource->read_lock_fences_enabled) {
+      if (current_read_lock_fence_)
+        current_read_lock_fence_->Set();
       resource->read_lock_fence = current_read_lock_fence_;
+    }
 
     if (returned.sync_point) {
       DCHECK(!resource->has_shared_bitmap_id);
@@ -2102,10 +2110,9 @@ void ResourceProvider::BindImageForSampling(Resource* resource) {
   resource->dirty_image = false;
 }
 
-void ResourceProvider::EnableReadLockFences(ResourceProvider::ResourceId id,
-                                            bool enable) {
+void ResourceProvider::EnableReadLockFences(ResourceProvider::ResourceId id) {
   Resource* resource = GetResource(id);
-  resource->enable_read_lock_fences = enable;
+  resource->read_lock_fences_enabled = true;
 }
 
 void ResourceProvider::AcquireImage(Resource* resource) {

cc/resources/resource_provider.h

@@ -301,6 +301,8 @@ class CC_EXPORT ResourceProvider {
   class Fence : public base::RefCounted<Fence> {
    public:
     Fence() {}
+
+    virtual void Set() = 0;
     virtual bool HasPassed() = 0;
 
    protected:
@@ -356,7 +358,7 @@ class CC_EXPORT ResourceProvider {
   void SetReadLockFence(Fence* fence) { current_read_lock_fence_ = fence; }
 
   // Enable read lock fences for a specific resource.
-  void EnableReadLockFences(ResourceProvider::ResourceId id, bool enable);
+  void EnableReadLockFences(ResourceProvider::ResourceId id);
 
   // Indicates if we can currently lock this resource for write.
   bool CanLockForWrite(ResourceId id);
@@ -418,7 +420,7 @@ class CC_EXPORT ResourceProvider {
     bool pending_set_pixels : 1;
     bool set_pixels_completion_forced : 1;
     bool allocated : 1;
-    bool enable_read_lock_fences : 1;
+    bool read_lock_fences_enabled : 1;
     bool has_shared_bitmap_id : 1;
     bool allow_overlay : 1;
     scoped_refptr<Fence> read_lock_fence;