cc: Partial tile update for one-copy raster.

cc/raster/one_copy_tile_task_worker_pool.cc

Index: cc/raster/one_copy_tile_task_worker_pool.cc
diff --git a/cc/raster/one_copy_tile_task_worker_pool.cc b/cc/raster/one_copy_tile_task_worker_pool.cc
index f4db3993fd723d0f04c1f613ae564cb71f1721d1..1a814a8b411607e0c94b85daa91e246a1b1e15ca 100644
--- a/cc/raster/one_copy_tile_task_worker_pool.cc
+++ b/cc/raster/one_copy_tile_task_worker_pool.cc
@@ -26,19 +26,33 @@ class RasterBufferImpl : public RasterBuffer {
                    ResourceProvider* resource_provider,
                    ResourcePool* resource_pool,
                    ResourceFormat resource_format,
-                   const Resource* resource)
+                   const Resource* upload_resource,
+                   uint64_t new_content_id,
+                   uint64_t previous_content_id)
       : worker_pool_(worker_pool),
         resource_provider_(resource_provider),
         resource_pool_(resource_pool),
-        resource_(resource),
-        raster_resource_(
-            resource_pool->AcquireResource(resource->size(), resource_format)),
-        lock_(new ResourceProvider::ScopedWriteLockGpuMemoryBuffer(
-            resource_provider_,
-            raster_resource_->id())),
-        sequence_(0) {}
+        upload_resource_(upload_resource),
+        new_content_id_(new_content_id),
+        reusing_raster_resource_(true),
+        sequence_(0) {
+    if (worker_pool->have_persistent_gpu_memory_buffers() &&
+        previous_content_id) {
+      raster_resource_ = resource_pool->TryAcquireResourceWithContentId(
+          upload_resource->size(), resource_format, previous_content_id);
+    }
+    if (!raster_resource_) {
+      raster_resource_ = resource_pool->AcquireResource(upload_resource->size(),
+                                                        resource_format);
+      reusing_raster_resource_ = false;
+    }
+
+    lock_.reset(new ResourceProvider::ScopedWriteLockGpuMemoryBuffer(
+        resource_provider_, raster_resource_->id()));
+  }
 
   ~RasterBufferImpl() override {
+    bool did_playback = !lock_;

[reveman, 2015/05/28 05:07:24]

you can use |sequence_| to determine if raster_resource_ was updated

[danakj, 2015/05/28 18:54:46]

Done.

     // Release write lock in case a copy was never scheduled.
     lock_.reset();
 
@@ -47,18 +61,20 @@ class RasterBufferImpl : public RasterBuffer {
     if (sequence_)
       worker_pool_->AdvanceLastIssuedCopyTo(sequence_);
 
-    // Return raster resource to pool so it can be used by another RasterBuffer
+    // Return resources to pool so they can be used by another RasterBuffer
     // instance.
-    if (raster_resource_)

[reveman, 2015/05/28 05:07:24]

I guess the removal of this is just a leftover from my partial copy patch,
correct?

[danakj, 2015/05/28 18:54:46]

Yep

-      resource_pool_->ReleaseResource(raster_resource_.Pass());
+    uint64_t id(did_playback ? new_content_id_ : 0);
+    resource_pool_->ReleaseResource(raster_resource_.Pass(), id);
   }
 
   // Overridden from RasterBuffer:
   void Playback(const RasterSource* raster_source,
-                const gfx::Rect& rect,
+                const gfx::Rect& raster_full_rect,
+                const gfx::Rect& raster_dirty_rect,
                 float scale) override {
     sequence_ = worker_pool_->PlaybackAndScheduleCopyOnWorkerThread(
-        lock_.Pass(), raster_resource_.get(), resource_, raster_source, rect,
+        reusing_raster_resource_, lock_.Pass(), raster_resource_.get(),
+        upload_resource_, raster_source, raster_full_rect, raster_dirty_rect,
         scale);
   }
 
@@ -66,7 +82,9 @@ class RasterBufferImpl : public RasterBuffer {
   OneCopyTileTaskWorkerPool* worker_pool_;
   ResourceProvider* resource_provider_;
   ResourcePool* resource_pool_;
-  const Resource* resource_;
+  const Resource* upload_resource_;
+  uint64_t new_content_id_;
+  bool reusing_raster_resource_;
   scoped_ptr<ScopedResource> raster_resource_;
   scoped_ptr<ResourceProvider::ScopedWriteLockGpuMemoryBuffer> lock_;
   CopySequenceNumber sequence_;
@@ -94,11 +112,11 @@ const int kFailedAttemptsBeforeWaitIfNeeded = 256;
 }  // namespace
 
 OneCopyTileTaskWorkerPool::CopyOperation::CopyOperation(
-    scoped_ptr<ResourceProvider::ScopedWriteLockGpuMemoryBuffer> write_lock,
+    scoped_ptr<ResourceProvider::ScopedWriteLockGpuMemoryBuffer> src_write_lock,
     const Resource* src,
     const Resource* dst,
     const gfx::Rect& rect)
-    : write_lock(write_lock.Pass()), src(src), dst(dst), rect(rect) {
+    : src_write_lock(src_write_lock.Pass()), src(src), dst(dst), rect(rect) {
 }
 
 OneCopyTileTaskWorkerPool::CopyOperation::~CopyOperation() {
@@ -111,10 +129,12 @@ scoped_ptr<TileTaskWorkerPool> OneCopyTileTaskWorkerPool::Create(
     ContextProvider* context_provider,
     ResourceProvider* resource_provider,
     ResourcePool* resource_pool,
-    size_t max_bytes_per_copy_operation) {
+    size_t max_bytes_per_copy_operation,
+    bool have_persistent_gpu_memory_buffers) {
   return make_scoped_ptr<TileTaskWorkerPool>(new OneCopyTileTaskWorkerPool(
       task_runner, task_graph_runner, context_provider, resource_provider,
-      resource_pool, max_bytes_per_copy_operation));
+      resource_pool, max_bytes_per_copy_operation,
+      have_persistent_gpu_memory_buffers));
 }
 
 OneCopyTileTaskWorkerPool::OneCopyTileTaskWorkerPool(
@@ -123,7 +143,8 @@ OneCopyTileTaskWorkerPool::OneCopyTileTaskWorkerPool(
     ContextProvider* context_provider,
     ResourceProvider* resource_provider,
     ResourcePool* resource_pool,
-    size_t max_bytes_per_copy_operation)
+    size_t max_bytes_per_copy_operation,
+    bool have_persistent_gpu_memory_buffers)
     : task_runner_(task_runner),
       task_graph_runner_(task_graph_runner),
       namespace_token_(task_graph_runner->GetNamespaceToken()),
@@ -131,6 +152,7 @@ OneCopyTileTaskWorkerPool::OneCopyTileTaskWorkerPool(
       resource_provider_(resource_provider),
       resource_pool_(resource_pool),
       max_bytes_per_copy_operation_(max_bytes_per_copy_operation),
+      have_persistent_gpu_memory_buffers_(have_persistent_gpu_memory_buffers),
       last_issued_copy_operation_(0),
       last_flushed_copy_operation_(0),
       lock_(),
@@ -269,12 +291,12 @@ ResourceFormat OneCopyTileTaskWorkerPool::GetResourceFormat() {
 }
 
 scoped_ptr<RasterBuffer> OneCopyTileTaskWorkerPool::AcquireBufferForRaster(
-    const Resource* resource) {
-  DCHECK_EQ(resource->format(), resource_provider_->best_texture_format());
-  return make_scoped_ptr<RasterBuffer>(
-      new RasterBufferImpl(this, resource_provider_, resource_pool_,
-                           resource_provider_->best_texture_format(),
-                           resource));
+    const TileTaskData& data) {
+  DCHECK_EQ(data.resource->format(), resource_provider_->best_texture_format());
+  return make_scoped_ptr<RasterBuffer>(new RasterBufferImpl(
+      this, resource_provider_, resource_pool_,
+      resource_provider_->best_texture_format(), data.resource,
+      data.new_content_id, data.previous_content_id));
 }
 
 void OneCopyTileTaskWorkerPool::ReleaseBufferForRaster(
@@ -284,33 +306,46 @@ void OneCopyTileTaskWorkerPool::ReleaseBufferForRaster(
 
 CopySequenceNumber
 OneCopyTileTaskWorkerPool::PlaybackAndScheduleCopyOnWorkerThread(
-    scoped_ptr<ResourceProvider::ScopedWriteLockGpuMemoryBuffer> write_lock,
-    const Resource* src,
-    const Resource* dst,
+    bool reusing_raster_resource,
+    scoped_ptr<ResourceProvider::ScopedWriteLockGpuMemoryBuffer>
+        raster_resource_write_lock,
+    const Resource* raster_resource,
+    const Resource* upload_resource,
     const RasterSource* raster_source,
-    const gfx::Rect& rect,
+    const gfx::Rect& raster_full_rect,
+    const gfx::Rect& raster_dirty_rect,
     float scale) {
-  gfx::GpuMemoryBuffer* gpu_memory_buffer = write_lock->GetGpuMemoryBuffer();
+  gfx::GpuMemoryBuffer* gpu_memory_buffer =
+      raster_resource_write_lock->GetGpuMemoryBuffer();
   if (gpu_memory_buffer) {
     void* data = NULL;
     bool rv = gpu_memory_buffer->Map(&data);
     DCHECK(rv);
     int stride;
     gpu_memory_buffer->GetStride(&stride);
-    TileTaskWorkerPool::PlaybackToMemory(data, src->format(), src->size(),
-                                         stride, raster_source, rect, scale);
+
+    gfx::Rect playback_rect = raster_full_rect;
+    if (reusing_raster_resource) {
+      playback_rect.Intersect(raster_dirty_rect);
+    }
+    DCHECK(!playback_rect.IsEmpty())
+        << "Why are we rastering a tile that's not dirty?";
+    TileTaskWorkerPool::PlaybackToMemory(
+        data, raster_resource->format(), raster_resource->size(), stride,
+        raster_source, raster_full_rect, playback_rect, scale);
     gpu_memory_buffer->Unmap();
   }
 
   base::AutoLock lock(lock_);
 
   CopySequenceNumber sequence = 0;
-  size_t bytes_per_row =
-      (BitsPerPixel(src->format()) * src->size().width()) / 8;
+  size_t bytes_per_row = (BitsPerPixel(raster_resource->format()) *
+                          raster_resource->size().width()) /
+                         8;
   size_t chunk_size_in_rows = std::max(
       kMinRowsPerCopyOperation, max_bytes_per_copy_operation_ / bytes_per_row);
   size_t y = 0;
-  size_t height = src->size().height();
+  size_t height = raster_resource->size().height();
   while (y < height) {
     int failed_attempts = 0;
     while ((pending_copy_operations_.size() + issued_copy_operation_count_) >=
@@ -345,10 +380,9 @@ OneCopyTileTaskWorkerPool::PlaybackAndScheduleCopyOnWorkerThread(
 
     // |write_lock| is passed to the first copy operation as it needs to be
     // released before we can issue a copy.
-    pending_copy_operations_.push_back(make_scoped_ptr(
-        new CopyOperation(write_lock.Pass(), src, dst,
-                          gfx::Rect(0, y, src->size().width(), rows_to_copy))));
-
+    pending_copy_operations_.push_back(make_scoped_ptr(new CopyOperation(
+        raster_resource_write_lock.Pass(), raster_resource, upload_resource,
+        gfx::Rect(0, y, raster_resource->size().width(), rows_to_copy))));

[reveman, 2015/05/28 05:07:24]

hard to see if this code was changed. looks like it's only renaming. would be
nice to avoid that unless it's important somehow?

[danakj, 2015/05/28 18:54:46]

this is just renaming, to make it clear what resource we're rastering vs
uploading to. They are still src/dst inside CopyOperation (it's a src->dst copy)
but over here we're rastering some resource and rastering to the "src" was super
unclear. This helped me get my head around what was going on in this class, so
I'd like to keep it?

[reveman, 2015/05/29 14:50:26]

OK, I see, just some renaming that makes sense independent of partial updates..

     y += rows_to_copy;
 
     // Acquire a sequence number for this copy operation.
@@ -426,7 +460,7 @@ void OneCopyTileTaskWorkerPool::IssueCopyOperations(int64 count) {
     scoped_ptr<CopyOperation> copy_operation = copy_operations.take_front();
 
     // Remove the write lock.
-    copy_operation->write_lock.reset();
+    copy_operation->src_write_lock.reset();
 
     // Copy contents of source resource to destination resource.
     resource_provider_->CopyResource(copy_operation->src->id(),