Implement the logic to kick off image decoding jobs for TileManager

cc/tile_manager.cc

Index: cc/tile_manager.cc
diff --git a/cc/tile_manager.cc b/cc/tile_manager.cc
index 80c2c74069c2d06db44d0c03b6c4c04c924b04ae..05f5961dcbffd9884eb7ae3b695369d50ae502c4 100644
--- a/cc/tile_manager.cc
+++ b/cc/tile_manager.cc
@@ -17,6 +17,7 @@
 #include "cc/resource_pool.h"
 #include "cc/switches.h"
 #include "cc/tile.h"
+#include "skia/ext/lazy_pixel_ref.h"
 #include "third_party/skia/include/core/SkDevice.h"
 
 namespace {
@@ -66,6 +67,17 @@ class RasterThread : public base::Thread {
         base::Bind(&RasterThread::RunReply, base::Unretained(this), reply));
   }
 
+  void PostImageDecodingTaskAndReply(const tracked_objects::Location& from_here,
+                                     SkPixelRef* pixel_ref,
+                                     const base::Closure& reply) {
+    ++num_pending_tasks_;
+    message_loop_proxy()->PostTaskAndReply(
+        from_here,
+        base::Bind(&skia::LazyPixelRef::Decode, base::Unretained(
+            static_cast<skia::LazyPixelRef*>(pixel_ref))),
+        base::Bind(&RasterThread::RunReply, base::Unretained(this), reply));
+  }
+
  private:
   static void RunRasterTask(PicturePileImpl* picture_pile,
                             uint8_t* mapped_buffer,
@@ -238,6 +250,10 @@ void TileManager::ManageTiles() {
     }
 
     mts.bin = EVENTUALLY_BIN;
+
+    // Update all the SkPixelRefs this tile intersects.
+    mts.pending_pixel_refs.clear();
+    mts.has_image_decoding_info = false;
   }
 
   // Memory limit policy works by mapping some bin states to the NEVER bin.
@@ -275,6 +291,17 @@ void TileManager::ManageTiles() {
   // Assign gpu memory and determine what tiles need to be rasterized.
   AssignGpuMemoryToTiles();
 
+  // Initialize image information for tiles in the NOW_BIN. For other tiles,
+  // we will get the information later when decoding is about to start.

[reveman, 2012/12/07 21:14:54]

Why not lazily get information for all types?

[qinmin, 2012/12/07 23:41:55]

So if an image covers several tiles(A,B,C,D) in NOW_BIN, and tile A kicks off
for image decoding first, while B, C, D are not scheduled due to task limit.
When A's decoding job finishes, B, C and D will have no idea that the same
picture they are trying to decode just finishes.
This is just to make a tradeoff between the cost of calling
GetImageInformationForTile() and the cost of the above problem.

On 2012/12/07 21:14:54, David Reveman wrote:

+  for (TileVector::reverse_iterator it =
+      tiles_that_need_to_be_rasterized_.rbegin();
+      it != tiles_that_need_to_be_rasterized_.rend(); ++it) {
+    if ((*it)->managed_state().bin == NOW_BIN)
+      GetImageInformationForTile(*it);
+    else
+      break;
+  }
+
   // Finally, kick the rasterizer.
   DispatchMoreRasterTasks();
 }
@@ -332,6 +359,15 @@ void TileManager::AssignGpuMemoryToTiles() {
       tiles_that_need_to_be_rasterized_.end());
 }
 
+void TileManager::GetImageInformationForTile(Tile* tile) {
+  ManagedTileState& managed_state = tile->managed_state();
+  if (!managed_state.has_image_decoding_info) {
+    const_cast<PicturePileImpl *>(tile->picture_pile())->GatherPixelRefs(
+        tile->rect_inside_picture_, managed_state.pending_pixel_refs);
+    managed_state.has_image_decoding_info = true;
+  }
+}
+
 void TileManager::FreeResourcesForTile(Tile* tile) {
   ManagedTileState& managed_tile_state = tile->managed_state();
   DCHECK(managed_tile_state.can_be_freed);
@@ -339,28 +375,133 @@ void TileManager::FreeResourcesForTile(Tile* tile) {
     resource_pool_->ReleaseResource(managed_tile_state.resource.Pass());
 }
 
+RasterThread* TileManager::GetFreeRasterThread() {
+  RasterThread* thread = 0;
+  for (RasterThreadVector::iterator it = raster_threads_.begin();
+       it != raster_threads_.end(); ++it) {
+    if ((*it)->num_pending_tasks() == kNumPendingRasterTasksPerThread)
+      continue;
+    // Check if this is the best thread we've found so far.
+    if (!thread || (*it)->num_pending_tasks() < thread->num_pending_tasks())
+      thread = *it;
+  }
+  return thread;
+}
+
 void TileManager::DispatchMoreRasterTasks() {
   while (!tiles_that_need_to_be_rasterized_.empty()) {
-    RasterThread* thread = 0;
-
-    for (RasterThreadVector::iterator it = raster_threads_.begin();
-         it != raster_threads_.end(); ++it) {
-      if ((*it)->num_pending_tasks() == kNumPendingRasterTasksPerThread)
-        continue;
-      // Check if this is the best thread we've found so far.
-      if (!thread || (*it)->num_pending_tasks() < thread->num_pending_tasks())
-        thread = *it;
-    }
-
+    RasterThread* thread = GetFreeRasterThread();
     // Stop dispatching tasks when all threads are busy.
     if (!thread)
         return;
 
-    DispatchOneRasterTask(thread, tiles_that_need_to_be_rasterized_.back());
+    Tile* tile = tiles_that_need_to_be_rasterized_.back();
+    std::vector<SkPixelRef*> pixel_refs;
+    if (HasUndecodedImages(tile, pixel_refs)) {
+      tiles_waiting_for_image_decoding_.push_back(tile);
+      DispatchMoreImageDecodingTasks(pixel_refs);
+    } else {
+      DispatchOneRasterTask(thread, tile);
+    }
     tiles_that_need_to_be_rasterized_.pop_back();
   }
 }
 
+void TileManager::DispatchMoreImageDecodingTasks(
+    std::vector<SkPixelRef*>& pixel_refs) {
+  if (pixel_refs.empty())
+    return;
+
+  RasterThread* thread = 0;
+  for (std::vector<SkPixelRef*>::iterator it = pixel_refs.begin();
+      it != pixel_refs.end(); ++it) {
+    thread = GetFreeRasterThread();
+    if (!thread)
+      return;
+    DispatchOneImageDecodingTask(thread, *it);
+  }
+}
+
+bool TileManager::HasUndecodedImages(Tile* tile,

[reveman, 2012/12/07 21:14:54]

Would it be cleaner to change this function to
DispatchImageDecodingTasksForTile? It could still return true if there was
images that needed decoding but you wouldn't need the temporary |unstarted|
vector.

[qinmin, 2012/12/07 23:41:55]

ok, combined HasUndecodedImages() and DispatchMoreImageDecodingTasks into
HasDispatchedImageDecodingTasks()
On 2012/12/07 21:14:54, David Reveman wrote:

+                                     std::vector<SkPixelRef*>& unstarted) {
+  ManagedTileState& managed_state = tile->managed_state();
+  if (!managed_state.has_image_decoding_info)
+    GetImageInformationForTile(tile);
+  while (true) {
+    bool has_removed_pending_tasks = false;

[reveman, 2012/12/07 21:14:54]

I think this nested loop is both hard to read and inefficient. can you please do
this without nesting the loops. e.g. put pixel refs that should be remove in
temporary list.

[qinmin, 2012/12/07 23:41:55]

Done.

+    for (std::vector<SkPixelRef*>::iterator it =
+        managed_state.pending_pixel_refs.begin();
+        it != managed_state.pending_pixel_refs.end(); ++it) {
+      if (pending_decode_tasks_.end() != pending_decode_tasks_.find(
+          (*it)->getGenerationID()))
+        continue;
+      if (static_cast<skia::LazyPixelRef*>(*it)->PrepareToDecode(
+          skia::LazyPixelRef::PrepareParams())) {
+        managed_state.pending_pixel_refs.erase(it);
+        has_removed_pending_tasks = true;
+        break;
+      } else {
+        unstarted.push_back(*it);
+      }
+    }
+    if (!has_removed_pending_tasks)
+      break;
+  }
+  return !managed_state.pending_pixel_refs.empty();
+}
+
+void TileManager::DispatchOneImageDecodingTask(RasterThread* thread,
+                                               SkPixelRef* pixel_ref) {
+  TRACE_EVENT0("cc", "TileManager::SpawnImageDecodingTask");

[reveman, 2012/12/07 21:14:54]

TileManager::DispatchOneImageDecodingTask

[qinmin, 2012/12/07 23:41:55]

Done.

+  uint32_t pixel_ref_id = pixel_ref->getGenerationID();
+  DCHECK(pending_decode_tasks_.end() ==
+      pending_decode_tasks_.find(pixel_ref_id));
+  pending_decode_tasks_[pixel_ref_id] = pixel_ref;
+
+  // TODO(qinmin): Not sure how we can guarantee SkPixelRef will not get deleted
+  // while it is on the raster thread. In SkBitmap, the SkPixelRef is declared
+  // as mutable.

[reveman, 2012/12/07 21:14:54]

SkPixelRef is owned by the SkPicture, right? Keeping the PicturePileImpl alive
should guarantee that the SkPicture is alive so you should probably pass a pile
ref to OnImageDecodingTaskCompleted.

[qinmin, 2012/12/07 23:41:55]

ok, just holding a scoped_refptr of the tile until the reply function is called.
On 2012/12/07 21:14:54, David Reveman wrote:

+  thread->PostImageDecodingTaskAndReply(
+          FROM_HERE,
+          pixel_ref,
+          base::Bind(&TileManager::OnImageDecodingTaskCompleted,
+                     base::Unretained(this),
+                     pixel_ref_id));
+}
+
+void TileManager::OnImageDecodingTaskCompleted(uint32_t pixel_ref_id) {
+  TRACE_EVENT0("cc", "TileManager::OnImageDecoded");
+  pending_decode_tasks_.erase(pixel_ref_id);
+  while (true) {

[reveman, 2012/12/07 21:14:54]

this is similar to the function above but you have three nested loops here.
please remove the outer most loop.

[qinmin, 2012/12/07 23:41:55]

Done.

+    bool no_tile_finished_decoding = true;
+    // Tiles are in increasing priority in the image decoding queue. But since
+    // we are appending tiles to the back of the rasterize queue, we need to
+    // iterate through tiles reversely.
+    for (TileVector::reverse_iterator tile_it =
+        tiles_waiting_for_image_decoding_.rbegin();
+        tile_it != tiles_waiting_for_image_decoding_.rend(); ++tile_it) {
+      std::vector<SkPixelRef*>& pixel_refs =
+          (*tile_it)->managed_state().pending_pixel_refs;
+      for (std::vector<SkPixelRef*>::iterator pixel_it =
+          pixel_refs.begin(); pixel_it != pixel_refs.end(); ++pixel_it) {
+        if (pixel_ref_id == (*pixel_it)->getGenerationID()) {
+          pixel_refs.erase(pixel_it);
+          tiles_that_need_to_be_rasterized_.push_back(*tile_it);
+          tiles_waiting_for_image_decoding_.erase(--tile_it.base());
+          no_tile_finished_decoding = false;
+          break;
+        }
+      }
+      if (!no_tile_finished_decoding);
+        break;
+    }
+    if (no_tile_finished_decoding)
+      break;
+  }
+
+  DispatchMoreRasterTasks();
+}
+
 void TileManager::DispatchOneRasterTask(
     RasterThread* thread, scoped_refptr<Tile> tile) {
   TRACE_EVENT0("cc", "TileManager::DispatchOneRasterTask");