cc: Move RasterBuffer implementations from ResourceProvider to RasterWorkerPool implementations.

cc/resources/image_copy_raster_worker_pool.cc

Index: cc/resources/image_copy_raster_worker_pool.cc
diff --git a/cc/resources/image_copy_raster_worker_pool.cc b/cc/resources/image_copy_raster_worker_pool.cc
index dda3b7c41ca46c065921ad1a029bf3e198261511..c36498a00237f005fdfa622e8b5ddeb96a80a20d 100644
--- a/cc/resources/image_copy_raster_worker_pool.cc
+++ b/cc/resources/image_copy_raster_worker_pool.cc
@@ -5,15 +5,108 @@
 #include "cc/resources/image_copy_raster_worker_pool.h"
 
 #include <algorithm>
+#include <limits>
 
 #include "base/debug/trace_event.h"
 #include "base/debug/trace_event_argument.h"
 #include "cc/debug/traced_value.h"
+#include "cc/resources/raster_buffer.h"
 #include "cc/resources/resource_pool.h"
 #include "cc/resources/scoped_resource.h"
 #include "gpu/command_buffer/client/gles2_interface.h"
+#include "third_party/skia/include/utils/SkNullCanvas.h"
 
 namespace cc {
+namespace {
+
+// Number of resources to copy at a time.
+const int kBatchSize = 8;
+
+class RasterBufferImpl : public RasterBuffer {
+ public:
+  RasterBufferImpl(ImageCopyRasterWorkerPool* worker_pool,
+                   const Resource* resource,
+                   const base::Closure& copy_batch_of_resources_callback)
+      : worker_pool_(worker_pool),

[vmpstr, 2014/09/16 22:15:55]

client? Up to you.

[reveman, 2014/09/17 14:40:56]

This is different in latest patch. PTAL.

+        resource_(resource),
+        copy_batch_of_resources_callback_(copy_batch_of_resources_callback),
+        raster_resource_(
+            worker_pool->resource_pool()->AcquireResource(resource->size())),
+        buffer_(NULL),
+        stride_(0) {
+    // This RasterBuffer implementation 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.
+    worker_pool_->resource_provider()->EnableReadLockFences(
+        raster_resource_->id());
+
+    // Acquire and map image for raster resource.
+    worker_pool_->resource_provider()->AcquireImage(raster_resource_->id());
+    buffer_ = worker_pool_->resource_provider()->MapImage(
+        raster_resource_->id(), &stride_);
+  }
+
+  virtual ~RasterBufferImpl() {
+    if (!raster_resource_)
+      return;
+
+    // Unmap image for raster resource if a copy operation was never scheduled.
+    worker_pool_->resource_provider()->UnmapImage(raster_resource_->id());
+
+    // Return resource to pool so it can be used for another RasterBuffer
+    // instance.
+    worker_pool_->resource_pool()->ReleaseResource(raster_resource_.Pass());
+  }
+
+  // Overridden from RasterBuffer:
+  virtual skia::RefPtr<SkCanvas> AcquireSkCanvas() OVERRIDE {
+    if (!buffer_)
+      return skia::AdoptRef(SkCreateNullCanvas());
+
+    RasterWorkerPool::AcquireBitmapForBuffer(
+        &bitmap_, buffer_, resource_->format(), resource_->size(), stride_);
+    return skia::AdoptRef(new SkCanvas(bitmap_));
+  }
+  virtual void ReleaseSkCanvas(const skia::RefPtr<SkCanvas>& canvas) OVERRIDE {
+    if (!buffer_)
+      return;
+
+    RasterWorkerPool::ReleaseBitmapForBuffer(
+        &bitmap_, buffer_, resource_->format());
+
+    size_t pending_copy_operations =
+        worker_pool_->ScheduleCopy(raster_resource_.Pass(), resource_);
+
+    // Post task that will copy a batch of resources whenever the number of
+    // pending copy operations is a multiple of the batch size.
+    if ((pending_copy_operations % kBatchSize) == 0) {
+      worker_pool_->task_runner()->PostTask(FROM_HERE,
+                                            copy_batch_of_resources_callback_);
+    }
+  }
+
+ private:
+  ImageCopyRasterWorkerPool* worker_pool_;
+  const Resource* resource_;
+  const base::Closure copy_batch_of_resources_callback_;
+  scoped_ptr<ScopedResource> raster_resource_;
+  uint8_t* buffer_;
+  int stride_;
+  SkBitmap bitmap_;
+
+  DISALLOW_COPY_AND_ASSIGN(RasterBufferImpl);
+};
+
+}  // namespace
+
+ImageCopyRasterWorkerPool::ResourceCopyOperation::ResourceCopyOperation(
+    scoped_ptr<ScopedResource> src,
+    const Resource* dst)
+    : src(src.Pass()), dst(dst) {
+}
+
+ImageCopyRasterWorkerPool::ResourceCopyOperation::~ResourceCopyOperation() {
+}
 
 // static
 scoped_ptr<RasterWorkerPool> ImageCopyRasterWorkerPool::Create(
@@ -42,15 +135,18 @@ ImageCopyRasterWorkerPool::ImageCopyRasterWorkerPool(
       context_provider_(context_provider),
       resource_provider_(resource_provider),
       resource_pool_(resource_pool),
-      has_performed_copy_since_last_flush_(false),
       raster_tasks_pending_(false),
       raster_tasks_required_for_activation_pending_(false),
-      raster_finished_weak_ptr_factory_(this) {
+      raster_finished_weak_ptr_factory_(this),
+      copy_batch_of_resources_weak_ptr_factory_(this) {
   DCHECK(context_provider_);
+  copy_batch_of_resources_callback_ =
+      base::Bind(&ImageCopyRasterWorkerPool::CopyResources,
+                 copy_batch_of_resources_weak_ptr_factory_.GetWeakPtr(),
+                 kBatchSize);
 }
 
 ImageCopyRasterWorkerPool::~ImageCopyRasterWorkerPool() {
-  DCHECK_EQ(0u, raster_task_states_.size());
 }
 
 Rasterizer* ImageCopyRasterWorkerPool::AsRasterizer() { return this; }
@@ -145,6 +241,18 @@ void ImageCopyRasterWorkerPool::CheckForCompletedTasks() {
 
   task_graph_runner_->CollectCompletedTasks(namespace_token_,
                                             &completed_tasks_);
+
+  // Copy all remaining resources.
+  CopyResources(std::numeric_limits<size_t>::max());
+
+  // Cancel existing CopyResources callbacks.
+  copy_batch_of_resources_weak_ptr_factory_.InvalidateWeakPtrs();
+  copy_batch_of_resources_callback_ =
+      base::Bind(&ImageCopyRasterWorkerPool::CopyResources,
+                 copy_batch_of_resources_weak_ptr_factory_.GetWeakPtr(),
+                 kBatchSize);
+
+  // We can now process completed tasks after having copied all resources.
   for (Task::Vector::const_iterator it = completed_tasks_.begin();
        it != completed_tasks_.end();
        ++it) {
@@ -157,47 +265,26 @@ void ImageCopyRasterWorkerPool::CheckForCompletedTasks() {
     task->RunReplyOnOriginThread();
   }
   completed_tasks_.clear();
-
-  FlushCopies();
 }
 
-RasterBuffer* ImageCopyRasterWorkerPool::AcquireBufferForRaster(
-    RasterTask* task) {
-  DCHECK_EQ(task->resource()->format(), resource_pool_->resource_format());
-  scoped_ptr<ScopedResource> resource(
-      resource_pool_->AcquireResource(task->resource()->size()));
-  RasterBuffer* raster_buffer =
-      resource_provider_->AcquireImageRasterBuffer(resource->id());
-  DCHECK(std::find_if(raster_task_states_.begin(),
-                      raster_task_states_.end(),
-                      RasterTaskState::TaskComparator(task)) ==
-         raster_task_states_.end());
-  raster_task_states_.push_back(RasterTaskState(task, resource.release()));
-  return raster_buffer;
+scoped_ptr<RasterBuffer> ImageCopyRasterWorkerPool::AcquireBufferForRaster(
+    const Resource* resource) {
+  DCHECK_EQ(resource->format(), resource_pool_->resource_format());
+  return make_scoped_ptr<RasterBuffer>(
+      new RasterBufferImpl(this, resource, copy_batch_of_resources_callback_));
 }
 
-void ImageCopyRasterWorkerPool::ReleaseBufferForRaster(RasterTask* task) {
-  RasterTaskState::Vector::iterator it =
-      std::find_if(raster_task_states_.begin(),
-                   raster_task_states_.end(),
-                   RasterTaskState::TaskComparator(task));
-  DCHECK(it != raster_task_states_.end());
-  scoped_ptr<ScopedResource> resource(it->resource);
-  std::swap(*it, raster_task_states_.back());
-  raster_task_states_.pop_back();
-
-  bool content_has_changed =
-      resource_provider_->ReleaseImageRasterBuffer(resource->id());
-
-  // |content_has_changed| can be false as result of task being canceled or
-  // task implementation deciding not to modify bitmap (ie. analysis of raster
-  // commands detected content as a solid color).
-  if (content_has_changed) {
-    resource_provider_->CopyResource(resource->id(), task->resource()->id());
-    has_performed_copy_since_last_flush_ = true;
-  }
+void ImageCopyRasterWorkerPool::ReleaseBufferForRaster(
+    scoped_ptr<RasterBuffer> buffer) {
+  // Nothing to do here. RasterBufferImpl destructor cleans up after itself.
+}
 
-  resource_pool_->ReleaseResource(resource.Pass());
+size_t ImageCopyRasterWorkerPool::ScheduleCopy(scoped_ptr<ScopedResource> src,
+                                               const Resource* dst) {
+  base::AutoLock lock(pending_resource_copy_operations_lock_);
+  pending_resource_copy_operations_.push_back(
+      make_scoped_ptr(new ResourceCopyOperation(src.Pass(), dst)));
+  return pending_resource_copy_operations_.size();
 }
 
 void ImageCopyRasterWorkerPool::OnRasterFinished() {
@@ -220,12 +307,44 @@ void ImageCopyRasterWorkerPool::OnRasterRequiredForActivationFinished() {
   client_->DidFinishRunningTasksRequiredForActivation();
 }
 
-void ImageCopyRasterWorkerPool::FlushCopies() {
-  if (!has_performed_copy_since_last_flush_)
+void ImageCopyRasterWorkerPool::CopyResources(size_t count) {
+  TRACE_EVENT1(
+      "cc", "ImageCopyRasterWorkerPool::CopyResources", "count", count);
+
+  ResourceCopyOperation::Deque resource_copy_operations;
+
+  {
+    base::AutoLock lock(pending_resource_copy_operations_lock_);
+
+    for (size_t i = 0; i < count; ++i) {

[vmpstr, 2014/09/16 22:15:55]

maybe:

while (count-- && !pending_resource_copy_operations_.empty()) {
  ...
}

[reveman, 2014/09/17 14:40:56]

I prefer to pick one variable that is used as the conditional of loops when
possible and instead break out of the loop if additional conditions   need to be
satisfied. I find this a bit easier to read but that's probably just a personal
preference. It does allow for comments to be added in a clean way to explain
each condition that force us to break out of the loop.

+      if (pending_resource_copy_operations_.empty())
+        break;
+
+      resource_copy_operations.push_back(
+          pending_resource_copy_operations_.take_front());
+    }
+  }
+
+  // Early out to avoid unnecessary ShallowFlushCHROMIUM call.
+  if (resource_copy_operations.empty())
     return;
 
+  while (!resource_copy_operations.empty()) {
+    scoped_ptr<ResourceCopyOperation> resource_copy_operation =
+        resource_copy_operations.take_front();
+
+    // First unmap source resource image.
+    resource_provider_->UnmapImage(resource_copy_operation->src->id());
+
+    // Copy contents of source resource to destination resource.
+    resource_provider_->CopyResource(resource_copy_operation->src->id(),
+                                     resource_copy_operation->dst->id());
+
+    // Return source resource to pool.
+    resource_pool_->ReleaseResource(resource_copy_operation->src.Pass());
+  }
+
   context_provider_->ContextGL()->ShallowFlushCHROMIUM();
-  has_performed_copy_since_last_flush_ = false;
 }
 
 scoped_refptr<base::debug::ConvertableToTraceFormat>
@@ -233,7 +352,6 @@ ImageCopyRasterWorkerPool::StateAsValue() const {
   scoped_refptr<base::debug::TracedValue> state =
       new base::debug::TracedValue();
 
-  state->SetInteger("pending_count", raster_task_states_.size());
   state->SetBoolean("tasks_required_for_activation_pending",
                     raster_tasks_required_for_activation_pending_);
   state->BeginDictionary("staging_state");