cc: Optimize decode scheduling for checker-images.

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 672 matching lines @@
 
     // We won't be able to schedule this tile, so break out early.
     if (work_to_schedule.tiles_to_raster.size() >=
         scheduled_raster_task_limit_) {
       all_tiles_that_need_to_be_rasterized_are_scheduled_ = false;
       break;
     }
 
     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
     // more memory to create a raster task.
     MemoryUsage memory_required_by_tile_to_be_scheduled;
     if (!tile->raster_task_.get()) {
       memory_required_by_tile_to_be_scheduled = MemoryUsage::FromConfig(
           tile->desired_texture_size(), DetermineResourceFormat(tile));
     }
 
@@ skipping 101 matching lines @@
 
   scoped_refptr<TileTask> required_for_activation_done_task =
       CreateTaskSetFinishedTask(
           &TileManager::DidFinishRunningTileTasksRequiredForActivation);
   scoped_refptr<TileTask> required_for_draw_done_task =
       CreateTaskSetFinishedTask(
           &TileManager::DidFinishRunningTileTasksRequiredForDraw);
   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();
 
     DCHECK(!task->HasCompleted());
 
     if (tile->required_for_activation()) {
       required_for_activate_count++;
       graph_.edges.push_back(
           TaskGraph::Edge(task, required_for_activation_done_task.get()));
@@ skipping 81 matching lines @@
                     kAllDoneTaskPriority, all_count);
 
   // Synchronize worker with compositor.
   raster_buffer_provider_->OrderingBarrier();
 
   // Schedule running of |raster_queue_|. This replaces any previously
   // scheduled tasks and effectively cancels all tasks not present
   // 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",
                                ScheduledTasksStateAsValue());
 }
 
 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.
   uint64_t resource_content_id = 0;
   Resource* resource = nullptr;
   gfx::Rect invalidated_rect = tile->invalidated_content_rect();
   if (UsePartialRaster() && tile->invalidated_id()) {
     resource = resource_pool_->TryAcquireResourceForPartialRaster(
         tile->id(), tile->invalidated_content_rect(), tile->invalidated_id(),
         &invalidated_rect);
@@ skipping 12 matching lines @@
   RasterSource::PlaybackSettings playback_settings;
   playback_settings.skip_images =
       prioritized_tile.priority().resolution == LOW_RESOLUTION;
 
   // Create and queue all image decode tasks that this tile depends on.
   TileTask::Vector decode_tasks;
   std::vector<DrawImage>& images = scheduled_draw_images_[tile->id()];
   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
   // skip during raster.
   playback_settings.use_image_hijack_canvas =
       !images.empty() || !images_to_skip.empty();
   playback_settings.images_to_skip = std::move(images_to_skip);
 
   // Get the tasks for the required images.
   ImageDecodeCache::TracingInfo tracing_info(
       prepare_tiles_count_, prioritized_tile.priority().priority_bin);
   image_controller_.GetTasksForImagesAndRef(&images, &decode_tasks,
                                             tracing_info);
 
   std::unique_ptr<RasterBuffer> raster_buffer =
       raster_buffer_provider_->AcquireBufferForRaster(
           resource, resource_content_id, tile->invalidated_id());
   return make_scoped_refptr(new RasterTaskImpl(
       this, tile, resource, prioritized_tile.raster_source(), playback_settings,
       prioritized_tile.priority().resolution, invalidated_rect,
       prepare_tiles_count_, std::move(raster_buffer), &decode_tasks,
       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();
 }
 
 void TileManager::OnRasterTaskCompleted(
     std::unique_ptr<RasterBuffer> raster_buffer,
     Tile::Id tile_id,
     Resource* resource,
     bool was_canceled) {
   raster_buffer_provider_->ReleaseBufferForRaster(std::move(raster_buffer));
@@ skipping 425 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