cc: Use asynchronous set pixels API for impl-side painting.

cc/tile_manager.cc

Index: cc/tile_manager.cc
diff --git a/cc/tile_manager.cc b/cc/tile_manager.cc
index 80c2c74069c2d06db44d0c03b6c4c04c924b04ae..fdeb9eb7eee72f30cd755e4907231b2c424c1f46 100644
--- a/cc/tile_manager.cc
+++ b/cc/tile_manager.cc
@@ -100,6 +100,7 @@ class RasterThread : public base::Thread {
 ManagedTileState::ManagedTileState()
     : can_use_gpu_memory(false),
       can_be_freed(true),
+      resource_id(0),
       resource_is_being_initialized(false),
       contents_swizzled(false) {
 }
@@ -116,7 +117,8 @@ TileManager::TileManager(
     : client_(client),
       resource_pool_(ResourcePool::Create(resource_provider,
                                           Renderer::ImplPool)),
-      manage_tiles_pending_(false) {
+      manage_tiles_pending_(false),
+      check_for_completed_set_pixels_pending_(false) {
   // Initialize all threads.
   const std::string thread_name_prefix = kRasterThreadNamePrefix;
   while (raster_threads_.size() < num_raster_threads) {
@@ -175,6 +177,13 @@ void TileManager::ScheduleManageTiles() {
   manage_tiles_pending_ = true;
 }
 
+void TileManager::ScheduleCheckForCompletedSetPixels() {
+  if (check_for_completed_set_pixels_pending_)
+    return;
+  client_->ScheduleCheckForCompletedSetPixels();
+  check_for_completed_set_pixels_pending_ = true;
+}
+
 class BinComparator {
 public:
   bool operator() (const Tile* a, const Tile* b) const {
@@ -279,6 +288,29 @@ void TileManager::ManageTiles() {
   DispatchMoreRasterTasks();
 }
 

[epennerAtGoogle, 2012/12/06 00:33:17]

Could this possibly just be called at the beginning of managing tiles, and just
keep scheduling management until all tiles are done uploading? If feels like the
only two things we need are redraw and manage.

Also, could we just traverse all tiles rather than keeping a different list? I'm
sure we will end up having a couple traversals of all tiles where this can fit.

[reveman, 2012/12/06 15:28:20]

For minimal latency I think we want to call this just before we evaluate if the
active tree should be updated.

FYI, the more time there is between issuing the beginSetPixels and checking for
its completion the less likely we are to cause an unnecessary command buffer
flush.

Having the queue allows us to check queries in the order they are issued which
also helps avoid unnecessary flushes.

+void TileManager::CheckForCompletedSetPixels() {

[nduca, 2012/12/06 03:21:30]

I had asked for this before, and I'll ask again.

Uploads. These are uploads. Call them uploads. SetPixels is detail.
CheckForCompletedUploads. ScheduleCheckForCompletedUploads.

[reveman, 2012/12/06 15:28:20]

We've moved away from using the "upload" term in relation to resources [1].
Uploads are specific to texture resources and this code is not restricted to a
specific type of resource. I'd really prefer to not start using that term in
resource type independent code again. We don't need to use SetPixels here
though. How about CheckForCompletedTileInitialization()?

[1] http://src.chromium.org/viewvc/chrome?view=rev&revision=165632

+  check_for_completed_set_pixels_pending_ = false;
+
+  while (!tiles_with_pending_set_pixels_.empty()) {
+    Tile* tile = tiles_with_pending_set_pixels_.front();
+    DCHECK(tile->managed_state().resource);
+
+    // Set pixel tasks complete in the order they are posted.

[nduca, 2012/12/06 16:37:09]

does async upload extension guarantee completion of uploads in the order that
they were issued?

[reveman, 2012/12/06 20:40:35]

Yes and it's advantages not to relax this requirement unless we actually make
use of it on the service side.

+    if (!resource_pool_->resource_provider()->didSetPixelsComplete(
+          tile->managed_state().resource->id())) {
+      ScheduleCheckForCompletedSetPixels();
+      break;
+    }
+
+    // It's now safe to release the pixel buffer.
+    resource_pool_->resource_provider()->releasePixelBuffer(
+        tile->managed_state().resource->id());
+
+    DidFinishTileInitialization(tile);
+    tiles_with_pending_set_pixels_.pop();
+  }
+}
+
 void TileManager::renderingStats(RenderingStats* stats) {
   stats->totalRasterizeTimeInSeconds =
     rendering_stats_.totalRasterizeTimeInSeconds;
@@ -420,16 +452,20 @@ void TileManager::OnRasterTaskCompleted(
 
   // Finish resource initialization if |can_use_gpu_memory| is true.
   if (managed_tile_state.can_use_gpu_memory) {
-    resource_pool_->resource_provider()->setPixelsFromBuffer(resource->id());
-    resource_pool_->resource_provider()->releasePixelBuffer(resource->id());
-
-    // The component order may be bgra if we uploaded bgra pixels to rgba
+    // The component order may be bgra if we're uploading bgra pixels to rgba
     // texture. Mark contents as swizzled if image component order is
     // different than texture format.
     managed_tile_state.contents_swizzled =
         !PlatformColor::sameComponentOrder(tile->format_);
 
-    DidFinishTileInitialization(tile, resource.Pass());
+    // Tile resources can't be freed until upload has completed.
+    managed_tile_state.can_be_freed = false;
+
+    resource_pool_->resource_provider()->beginSetPixels(resource->id());
+    managed_tile_state.resource = resource.Pass();

[nduca, 2012/12/06 16:37:09]

this is getting really insane to me that you have resource and resource_id, and
sometimes one is set and not the other.

PLease pleas please please please make this cleaner.

Again, from my PoV, this would have been so much cleaner all along:
1. put the resource_id into the managed tile state up front
2. have a managed_state.status = {ready, rasterizing, uploading}
3. mutate the status as needed
4. remove resource_id() accessor; ready_resource_id() then can be 0 if non-ready

[reveman, 2012/12/06 20:40:35]

I got rid of ManagedTileState::resource_id by just replacing resource_id() with
a non-inlined version that is conditional on resource_is_being_initialized.

Passing the ownership of ResourcePool::Resource to the tile right away requires
additional changes. When a raster task completes, we call
AssignGpuMemoryToTiles() to determine if we should abort the initialization or
not. The resource can't be freed at this point because we still need to release
the pixel buffer. Here are some options:

1. Keep the ownership of the resource with the raster task until beginSetPixels
is called. (current patch)

2. Keep the ownership of the resource on tile. Take ownership of resource back
before calling AssignGpuMemoryToTiles() in OnRasterTaskCompleted() and then give
it back to the tile when beginSetPixels() is called.

3. Split AssignGpuMemoryToTiles() up into two functions. One that sets
can_use_gpu_memory and one that frees resources.

4. Make ResourcePool::Resource aware of pixel buffers.

Let me know which one you prefer or if you have a better solution.

+    tiles_with_pending_set_pixels_.push(tile);
+
+    ScheduleCheckForCompletedSetPixels();
   } else {
     resource_pool_->resource_provider()->releasePixelBuffer(resource->id());
     resource_pool_->ReleaseResource(resource.Pass());
@@ -439,14 +475,12 @@ void TileManager::OnRasterTaskCompleted(
   DispatchMoreRasterTasks();
 }
 
-void TileManager::DidFinishTileInitialization(
-    Tile* tile, scoped_ptr<ResourcePool::Resource> resource) {
+void TileManager::DidFinishTileInitialization(Tile* tile) {
   ManagedTileState& managed_tile_state = tile->managed_state();
-  DCHECK(!managed_tile_state.resource);
-  managed_tile_state.resource = resource.Pass();
+  DCHECK(managed_tile_state.resource);
+  managed_tile_state.resource_id = managed_tile_state.resource->id();
   managed_tile_state.resource_is_being_initialized = false;
-  // TODO(qinmin): Make this conditional on managed_tile_state.bin == NOW_BIN.
-  client_->ScheduleRedraw();
+  managed_tile_state.can_be_freed = true;
 }
 
 }