cc: Optimize decode scheduling for checker-images.

cc/tiles/tile_manager.cc

Index: cc/tiles/tile_manager.cc
diff --git a/cc/tiles/tile_manager.cc b/cc/tiles/tile_manager.cc
index 76d0a2ac9b8254cfd5fe382522880e43354be1f8..427a2b438b79dd2af50ef8cd7256086f2d085579 100644
--- a/cc/tiles/tile_manager.cc
+++ b/cc/tiles/tile_manager.cc
@@ -691,7 +691,7 @@ TileManager::PrioritizedWorkToSchedule TileManager::AssignGpuMemoryToTiles() {
     tile->scheduled_priority_ = schedule_priority++;
 
     DCHECK(tile->draw_info().mode() == TileDrawInfo::OOM_MODE ||

[vmpstr, 2017/03/17 18:32:13]

Can you comment this DCHECK, since it's getting a bit complicated

[Khushal, 2017/03/27 13:57:33]

Its good that you pointed this out. I hadn't realized that the rest of the
function would have double-counted the memory usage for the checker-imaged
tiles.

-           !tile->draw_info().IsReadyToDraw());
+           !tile->draw_info().IsReadyToDraw() || tile->is_checker_imaged());
 
     // If the tile already has a raster_task, then the memory used by it is
     // already accounted for in memory_usage. Otherwise, we'll have to acquire
@@ -813,17 +813,31 @@ void TileManager::ScheduleTasks(
   scoped_refptr<TileTask> all_done_task =
       CreateTaskSetFinishedTask(&TileManager::DidFinishRunningAllTileTasks);
 
+  CheckerImageTracker::ImageDecodeQueue checker_image_decode_queue;
+
   // Build a new task queue containing all task currently needed. Tasks
   // are added in order of priority, highest priority task first.
   for (auto& prioritized_tile : tiles_that_need_to_be_rasterized) {
     Tile* tile = prioritized_tile.tile();
 
     DCHECK(tile->draw_info().requires_resource());
-    DCHECK(!tile->draw_info().resource());
+    DCHECK(!tile->draw_info().resource() || tile->is_checker_imaged());
+
+    bool process_tile_for_checker_images_only =

[vmpstr, 2017/03/17 18:32:13]

Comment pls

+        tile->draw_info().resource() && tile->is_checker_imaged();
+    if (process_tile_for_checker_images_only) {
+      std::vector<DrawImage> images;
+      ImageIdFlatSet checkered_images;
+      FilterImagesForCheckering(prioritized_tile, &images, &checkered_images,
+                                &checker_image_decode_queue);
+      DCHECK(!checkered_images.empty())
+          << "Checker-imaged state is sticky on a Tile";

[vmpstr, 2017/03/17 18:32:13]

nit: "should be sticky", since when this fails it will say

"Checker-imaged state is sticky on a Tile" which makes it sound like it failed
because it's sticky

+      continue;
+    }
 
     if (!tile->raster_task_) {
-      tile->raster_task_ =
-          CreateRasterTask(prioritized_tile, raster_color_space);
+      tile->raster_task_ = CreateRasterTask(
+          prioritized_tile, raster_color_space, &checker_image_decode_queue);
     }
 
     TileTask* task = tile->raster_task_.get();
@@ -925,6 +939,12 @@ void TileManager::ScheduleTasks(
   // in |raster_queue_|.
   tile_task_manager_->ScheduleTasks(&graph_);
 
+  // Schedule running of the checker-image decode queue. This replaces the
+  // previously scheduled queue and effectively cancels image decodes from the
+  // previously scheduled queue, if not already started.
+  checker_image_tracker_.SetImageDecodeQueue(

[vmpstr, 2017/03/17 18:32:13]

Can you call this ScheduleImageDecodes or something?

[Khushal, 2017/03/27 13:57:33]

Done.

+      std::move(checker_image_decode_queue));
+
   did_check_for_completed_tasks_since_last_schedule_tasks_ = false;
 
   TRACE_EVENT_ASYNC_STEP_INTO1("cc", "ScheduledTasks", this, "running", "state",
@@ -933,7 +953,8 @@ void TileManager::ScheduleTasks(
 
 scoped_refptr<TileTask> TileManager::CreateRasterTask(
     const PrioritizedTile& prioritized_tile,
-    const gfx::ColorSpace& color_space) {
+    const gfx::ColorSpace& color_space,
+    CheckerImageTracker::ImageDecodeQueue* checker_image_decode_queue) {
   Tile* tile = prioritized_tile.tile();
 
   // Get the resource.
@@ -966,10 +987,10 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
   ImageIdFlatSet images_to_skip;
   images.clear();
   if (!playback_settings.skip_images) {
-    prioritized_tile.raster_source()->GetDiscardableImagesInRect(
-        tile->enclosing_layer_rect(), tile->contents_scale(), &images);
-    checker_image_tracker_.FilterImagesForCheckeringForTile(
-        &images, &images_to_skip, prioritized_tile.tile()->tiling()->tree());
+    FilterImagesForCheckering(prioritized_tile, &images, &images_to_skip,
+                              checker_image_decode_queue);
+    if (!images_to_skip.empty())
+      tile->set_is_checker_imaged();
   }
 
   // We can skip the image hijack canvas if we have no images, or no images to
@@ -994,6 +1015,31 @@ scoped_refptr<TileTask> TileManager::CreateRasterTask(
       use_gpu_rasterization_));
 }
 
+void TileManager::FilterImagesForCheckering(
+    const PrioritizedTile& prioritized_tile,
+    std::vector<DrawImage>* images_to_decode,
+    ImageIdFlatSet* checker_images,
+    CheckerImageTracker::ImageDecodeQueue* checker_image_decode_queue) {
+  DCHECK_EQ(images_to_decode->size(), 0u);
+  DCHECK_EQ(checker_images->size(), 0u);
+
+  const Tile* tile = prioritized_tile.tile();
+  std::vector<DrawImage> images_in_tile;
+  prioritized_tile.raster_source()->GetDiscardableImagesInRect(

[vmpstr, 2017/03/17 18:32:13]

I'd prefer you leave this call outside and pass the vector to this function (and
pass Tile* instead of PrioritizedTile). It's easier to test, and easier to
reason about IMO.

[Khushal, 2017/03/27 13:57:33]

Done.

+      tile->enclosing_layer_rect(), tile->contents_scale(), &images_in_tile);
+  for (auto draw_image : images_in_tile) {
+    const sk_sp<const SkImage>& image = draw_image.image();
+    DCHECK(image->isLazyGenerated());
+    if (checker_image_tracker_.ShouldCheckerImage(image,
+                                                  tile->tiling()->tree())) {
+      checker_images->insert(image->uniqueID());
+      checker_image_decode_queue->push_back(std::move(image));
+    } else {
+      images_to_decode->push_back(draw_image);
+    }
+  }
+}
+
 void TileManager::ResetSignalsForTesting() {
   signals_.reset();
 }