Delay activation/draw on GPU tile work completion

cc/tiles/tile_manager.cc

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #include "cc/tiles/tile_manager.h"
 
 #include <stddef.h>
 #include <stdint.h>
 
 #include <algorithm>
@@ skipping 348 matching lines @@
       more_tiles_need_prepare_check_notifier_(
           task_runner_,
           base::Bind(&TileManager::CheckIfMoreTilesNeedToBePrepared,
                      base::Unretained(this))),
       signals_check_notifier_(task_runner_,
                               base::Bind(&TileManager::CheckAndIssueSignals,
                                          base::Unretained(this))),
       has_scheduled_tile_tasks_(false),
       prepare_tiles_count_(0u),
       next_tile_id_(0u),
-      task_set_finished_weak_ptr_factory_(this) {}
+      task_set_finished_weak_ptr_factory_(this),
+      ready_to_draw_callback_weak_ptr_factory_(this) {}
 
 TileManager::~TileManager() {
   FinishTasksAndCleanUp();
 }
 
 void TileManager::FinishTasksAndCleanUp() {
   if (!tile_task_manager_)
     return;
 
   global_state_ = GlobalStateThatImpactsTilePriority();
 
   // This cancels tasks if possible, finishes pending tasks, and release any
   // uninitialized resources.
   tile_task_manager_->Shutdown();
 
   raster_buffer_provider_->Shutdown();
 
   tile_task_manager_->CheckForCompletedTasks();
 
   FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
   tile_task_manager_ = nullptr;
   resource_pool_ = nullptr;
   more_tiles_need_prepare_check_notifier_.Cancel();
   signals_check_notifier_.Cancel();
   task_set_finished_weak_ptr_factory_.InvalidateWeakPtrs();
+  ready_to_draw_callback_weak_ptr_factory_.InvalidateWeakPtrs();
 
   image_controller_.SetImageDecodeCache(nullptr);
   locked_image_tasks_.clear();
 }
 
 void TileManager::SetResources(ResourcePool* resource_pool,
                                ImageDecodeCache* image_decode_cache,
                                TaskGraphRunner* task_graph_runner,
                                RasterBufferProvider* raster_buffer_provider,
                                size_t scheduled_raster_task_limit,
@@ skipping 98 matching lines @@
   // We need to call CheckForCompletedTasks() once in-between each call
   // to ScheduleTasks() to prevent canceled tasks from being scheduled.
   if (!did_check_for_completed_tasks_since_last_schedule_tasks_) {
     tile_task_manager_->CheckForCompletedTasks();
     did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
   }
 
   FreeResourcesForReleasedTiles();
   CleanUpReleasedTiles();
 
+  // The required for activate/draw status of tiles may have changed any time
+  // PrepareTiles is called. If we have any |pending_ready_to_draw_tiles_|,
+  // set a flag so we know to re-compute these requirements.
+  if (!pending_ready_for_draw_tiles_.empty())

[vmpstr, 2016/12/14 18:03:31]

If it's empty, then we won't do much work anyway. How about we just set it
unconditionally?

[ericrk, 2016/12/14 19:53:48]

true enough...

+    pending_tile_requirements_dirty_ = true;
+
   PrioritizedWorkToSchedule prioritized_work = AssignGpuMemoryToTiles();
 
   // Inform the client that will likely require a draw if the highest priority
   // tile that will be rasterized is required for draw.
   client_->SetIsLikelyToRequireADraw(
       !prioritized_work.tiles_to_raster.empty() &&
       prioritized_work.tiles_to_raster.front().tile()->required_for_draw());
 
   // Schedule tile tasks.
   ScheduleTasks(prioritized_work);
 
   TRACE_EVENT_INSTANT1("cc", "DidPrepareTiles", TRACE_EVENT_SCOPE_THREAD,
                        "state", BasicStateAsValue());
   return true;
 }
 
 void TileManager::Flush() {
   TRACE_EVENT0("cc", "TileManager::Flush");
 
   if (!tile_task_manager_) {
     TRACE_EVENT_INSTANT0("cc", "Flush aborted", TRACE_EVENT_SCOPE_THREAD);
     return;
   }
 
   tile_task_manager_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
+  CheckPendingReadyToDrawTiles(true /* issue_signals */);
 
   TRACE_EVENT_INSTANT1("cc", "DidFlush", TRACE_EVENT_SCOPE_THREAD, "stats",
                        RasterTaskCompletionStatsAsValue(flush_stats_));
   flush_stats_ = RasterTaskCompletionStats();
 }
 
 std::unique_ptr<base::trace_event::ConvertableToTraceFormat>
 TileManager::BasicStateAsValue() const {
   std::unique_ptr<base::trace_event::TracedValue> value(
       new base::trace_event::TracedValue());
@@ skipping 123 matching lines @@
       // or drop the tile for scheduling and raster.
       // TODO(sohanjg): Check if we could use a shared analysis
       // canvas which is reset between tiles.
       tile->set_solid_color_analysis_performed(true);
       SkColor color = SK_ColorTRANSPARENT;
       bool is_solid_color =
           prioritized_tile.raster_source()->PerformSolidColorAnalysis(
               tile->content_rect(), tile->raster_scales(), &color);
       if (is_solid_color) {
         tile->draw_info().set_solid_color(color);
-        tile->draw_info().set_was_ever_ready_to_draw();
         if (!tile_is_needed_now)
           tile->draw_info().set_was_a_prepaint_tile();
         client_->NotifyTileStateChanged(tile);
         continue;
       }
     }
 
     // Prepaint tiles that are far away are only processed for images.
     if (!tile->required_for_activation() && !tile->required_for_draw() &&
         prioritized_tile.is_process_for_images_only()) {
@@ skipping 78 matching lines @@
                    "all_tiles_that_need_to_be_rasterized_are_scheduled",
                    all_tiles_that_need_to_be_rasterized_are_scheduled_,
                    "had_enough_memory_to_schedule_tiles_needed_now",
                    had_enough_memory_to_schedule_tiles_needed_now);
   return work_to_schedule;
 }
 
 void TileManager::FreeResourcesForTile(Tile* tile) {
   TileDrawInfo& draw_info = tile->draw_info();
   if (draw_info.resource_) {
     resource_pool_->ReleaseResource(draw_info.resource_);

[vmpstr, 2016/12/14 18:03:31]

Can you add a TakeResource to the draw info that automatically sets it to null?
I'd like to move towards having draw_info.resource_ not be accessible directly
by the tile manager, since it's becoming easy to get the usage wrong. 

[ericrk, 2016/12/14 19:53:48]

sure, added.

-    draw_info.resource_ = nullptr;
+    draw_info.set_resource(nullptr);
   }
 }
 
 void TileManager::FreeResourcesForTileAndNotifyClientIfTileWasReadyToDraw(
     Tile* tile) {
   bool was_ready_to_draw = tile->draw_info().IsReadyToDraw();
   FreeResourcesForTile(tile);
   if (was_ready_to_draw)
     client_->NotifyTileStateChanged(tile);
 }
@@ skipping 234 matching lines @@
 
   if (was_canceled) {
     ++flush_stats_.canceled_count;
     resource_pool_->ReleaseResource(resource);
     return;
   }
 
   resource_pool_->OnContentReplaced(resource->id(), tile->id());
   ++flush_stats_.completed_count;
 
-  draw_info.set_use_resource();
-  draw_info.resource_ = resource;
+  draw_info.set_resource(resource);
   draw_info.contents_swizzled_ = DetermineResourceRequiresSwizzle(tile);
-
-  DCHECK(draw_info.IsReadyToDraw());
-  draw_info.set_was_ever_ready_to_draw();
-
-  client_->NotifyTileStateChanged(tile);
+  if (global_state_.tree_priority == SMOOTHNESS_TAKES_PRIORITY &&
+      !raster_buffer_provider_->IsResourceReadyToDraw(resource->id())) {
+    // In SMOOTHNESS_TAKES_PRIORITY mode, we wait for GPU work to complete
+    // for a tile before setting it as ready to draw.
+    pending_ready_for_draw_tiles_.push_back(tile);
+  } else {
+    draw_info.set_resource_ready_for_draw();
+    client_->NotifyTileStateChanged(tile);
+  }
 }
 
 ScopedTilePtr TileManager::CreateTile(const Tile::CreateInfo& info,
                                       int layer_id,
                                       int source_frame_number,
                                       int flags) {
   // We need to have a tile task worker pool to do anything meaningful with
   // tiles.
   DCHECK(tile_task_manager_);
   ScopedTilePtr tile(
@@ skipping 28 matching lines @@
     Tile* tile = all_queue->Top().tile();
     DCHECK(!tile->required_for_activation() ||
            tile->draw_info().IsReadyToDraw());
   }
 #endif
   return true;
 }
 
 bool TileManager::IsReadyToActivate() const {
   TRACE_EVENT0("cc", "TileManager::IsReadyToActivate");
-  return AreRequiredTilesReadyToDraw(
-      RasterTilePriorityQueue::Type::REQUIRED_FOR_ACTIVATION);
+  return pending_required_for_activation_callback_id_ == 0 &&
+         AreRequiredTilesReadyToDraw(
+             RasterTilePriorityQueue::Type::REQUIRED_FOR_ACTIVATION);
 }
 
 bool TileManager::IsReadyToDraw() const {
   TRACE_EVENT0("cc", "TileManager::IsReadyToDraw");
-  return AreRequiredTilesReadyToDraw(
-      RasterTilePriorityQueue::Type::REQUIRED_FOR_DRAW);
+  return pending_required_for_draw_callback_id_ == 0 &&
+         AreRequiredTilesReadyToDraw(
+             RasterTilePriorityQueue::Type::REQUIRED_FOR_DRAW);
 }
 
 void TileManager::CheckAndIssueSignals() {
   TRACE_EVENT0("cc", "TileManager::CheckAndIssueSignals");
   tile_task_manager_->CheckForCompletedTasks();
   did_check_for_completed_tasks_since_last_schedule_tasks_ = true;
 
+  CheckPendingReadyToDrawTiles(false /* issue_signals */);
+
   // Ready to activate.
   if (signals_.ready_to_activate && !signals_.did_notify_ready_to_activate) {
     signals_.ready_to_activate = false;
     if (IsReadyToActivate()) {
       TRACE_EVENT0(TRACE_DISABLED_BY_DEFAULT("cc.debug"),
                    "TileManager::CheckAndIssueSignals - ready to activate");
       signals_.did_notify_ready_to_activate = true;
       client_->NotifyReadyToActivate();
     }
   }
@@ skipping 133 matching lines @@
                     signals_.all_tile_tasks_completed);
   state->EndDictionary();
   return std::move(state);
 }
 
 bool TileManager::UsePartialRaster() const {
   return use_partial_raster_ &&
          raster_buffer_provider_->CanPartialRasterIntoProvidedResource();
 }
 
+void TileManager::CheckPendingReadyToDrawTiles(bool issue_signals) {
+  bool in_smoothness_mode =
+      global_state_.tree_priority == SMOOTHNESS_TAKES_PRIORITY;
+  ResourceProvider::ResourceIdArray required_for_activation_ids;
+  ResourceProvider::ResourceIdArray required_for_draw_ids;
+
+  for (auto it = pending_ready_for_draw_tiles_.begin();
+       it != pending_ready_for_draw_tiles_.end();) {
+    Tile* tile = *it;
+    Resource* resource = tile->draw_info().resource_;

[vmpstr, 2016/12/14 18:03:31]

Can you change this to a getter instead?

[ericrk, 2016/12/14 19:53:48]

Done.

+    if (!resource) {
+      // The tile has been evicted or deleted. Remove it from our list.
+      it = pending_ready_for_draw_tiles_.erase(it);
+      continue;
+    }
+
+    if (!in_smoothness_mode ||
+        raster_buffer_provider_->IsResourceReadyToDraw(resource->id())) {
+      // Tile is ready to draw.
+      tile->draw_info().set_resource_ready_for_draw();
+      client_->NotifyTileStateChanged(tile);
+      it = pending_ready_for_draw_tiles_.erase(it);
+      continue;
+    }
+
+    if (pending_tile_requirements_dirty_)
+      tile->tiling()->UpdateRequiredStatesOnTile(tile);
+    if (tile->required_for_activation())
+      required_for_activation_ids.push_back(resource->id());
+    if (tile->required_for_draw())
+      required_for_draw_ids.push_back(resource->id());
+
+    ++it;
+  }
+
+  if (required_for_activation_ids.empty()) {
+    pending_required_for_activation_callback_id_ = 0;
+  } else {
+    pending_required_for_activation_callback_id_ =
+        raster_buffer_provider_->SetReadyToDrawCallback(
+            required_for_activation_ids,
+            base::Bind(&TileManager::CheckPendingReadyToDrawTiles,
+                       ready_to_draw_callback_weak_ptr_factory_.GetWeakPtr(),
+                       true /* issue_signals */),
+            pending_required_for_activation_callback_id_);
+  }
+
+  if (required_for_draw_ids.empty()) {
+    pending_required_for_draw_callback_id_ = 0;
+  } else {
+    pending_required_for_draw_callback_id_ =
+        raster_buffer_provider_->SetReadyToDrawCallback(
+            required_for_draw_ids,
+            base::Bind(&TileManager::CheckPendingReadyToDrawTiles,
+                       ready_to_draw_callback_weak_ptr_factory_.GetWeakPtr(),
+                       true /* issue_signals */),
+            pending_required_for_draw_callback_id_);
+  }
+
+  // Update our signals now that we know whether we have pending resources.
+  signals_.ready_to_activate =

[vmpstr, 2016/12/14 18:03:31]

I think there might be some counter-intuitive interaction here:

Suppose we get a DidFinishRunningTileTasksRequiredForDraw, with
DidFinishRunningTileTasksRequiredForActivation still pending (but about to run):

- The calling code would set ready_to_draw to true, ready_to_activate would be
false and we're in CheckAndIssueSignals
- This function is called and it determines (say we're not in smoothness mode)
that ready_to_activate is true and sets as such
- The AreTilesReady code runs and issues an activation signal
- Then, DidFinishRunningTileTasksRequiredForActivation runs and unconditionally
sets ready_to_activate to true again and runs the same code again
- This would issue a second activation (bad?)

I wonder if these setters should be ... &= ... and in the sync token callback
set both signals to true... although that might still issue a second activation
if there's a pending raster task signal, no?

[ericrk, 2016/12/14 19:53:48]

Looked through the code. I believe this is prevented by
Signals::did_notify_ready_to_activate_, which blocks further activations until
it is reset. We only reset it in PrepareTiles, so I think this is fine.

+      (pending_required_for_activation_callback_id_ == 0);
+  signals_.ready_to_draw = (pending_required_for_draw_callback_id_ == 0);
+
+  if (issue_signals && (signals_.ready_to_activate || signals_.ready_to_draw))
+    signals_check_notifier_.Schedule();
+
+  // We've just updated all pending tile requirements if necessary.
+  pending_tile_requirements_dirty_ = false;
+}
+
 // Utility function that can be used to create a "Task set finished" task that
 // posts |callback| to |task_runner| when run.
 scoped_refptr<TileTask> TileManager::CreateTaskSetFinishedTask(
     void (TileManager::*callback)()) {
   return make_scoped_refptr(new TaskSetFinishedTaskImpl(
       task_runner_,
       base::Bind(callback, task_set_finished_weak_ptr_factory_.GetWeakPtr())));
 }
 
 TileManager::MemoryUsage::MemoryUsage()
     : memory_bytes_(0), resource_count_(0) {}
 
 TileManager::MemoryUsage::MemoryUsage(size_t memory_bytes,
                                       size_t resource_count)
     : memory_bytes_(static_cast<int64_t>(memory_bytes)),
       resource_count_(static_cast<int>(resource_count)) {
   // MemoryUsage is constructed using size_ts, since it deals with memory and
   // the inputs are typically size_t. However, during the course of usage (in
-  // particular operator-=) can cause internal values to become negative. Thus,
-  // member variables are signed.
+  // particular operator-=) can cause internal values to become negative.
+  // Thus, member variables are signed.
   DCHECK_LE(memory_bytes,
             static_cast<size_t>(std::numeric_limits<int64_t>::max()));
   DCHECK_LE(resource_count,
             static_cast<size_t>(std::numeric_limits<int>::max()));
 }
 
 // static
 TileManager::MemoryUsage TileManager::MemoryUsage::FromConfig(
     const gfx::Size& size,
     ResourceFormat format) {
   // We can use UncheckedSizeInBytes here since this is used with a tile
-  // size which is determined by the compositor (it's at most max texture size).
+  // size which is determined by the compositor (it's at most max texture
+  // size).
   return MemoryUsage(ResourceUtil::UncheckedSizeInBytes<size_t>(size, format),
                      1);
 }
 
 // static
 TileManager::MemoryUsage TileManager::MemoryUsage::FromTile(const Tile* tile) {
   const TileDrawInfo& draw_info = tile->draw_info();
   if (draw_info.resource_) {
     return MemoryUsage::FromConfig(draw_info.resource_->size(),
                                    draw_info.resource_->format());
@@ skipping 39 matching lines @@
   all_tile_tasks_completed = false;
   did_notify_all_tile_tasks_completed = false;
 }
 
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule() = default;
 TileManager::PrioritizedWorkToSchedule::PrioritizedWorkToSchedule(
     PrioritizedWorkToSchedule&& other) = default;
 TileManager::PrioritizedWorkToSchedule::~PrioritizedWorkToSchedule() = default;
 
 }  // namespace cc