cc: Do GatherPixelRefs from skia at record time

cc/resources/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/resources/tile_manager.h"
 
 #include <algorithm>
 #include <string>
 
 #include "base/bind.h"
@@ skipping 617 matching lines @@
       tile->drawing_info().set_solid_color(
         managed_tile_state.picture_pile_analysis.solid_color);
       DidFinishTileInitialization(tile);
     } else if (managed_tile_state.picture_pile_analysis.is_transparent) {
       tile->drawing_info().set_transparent();
       DidFinishTileInitialization(tile);
     }
   }
 }
 
 void TileManager::GatherPixelRefsForTile(Tile* tile) {

[reveman, 2013/04/22 19:56:06]

How about we get rid of this function completely now that iterating over pixel
refs is cheap?

For the time being you probably need a |decoded_pixel_refs| set in
ManagedTileState to make sure we don't get stuck trying to decode an image
forever but that can go away once I land cancellation and reprioritization.

[vmpstr, 2013/04/22 22:38:16]

Let's leave that as a follow-up. I would prefer this patch doesn't extend too
far past picture/picture_pile_impl

   TRACE_EVENT0("cc", "TileManager::GatherPixelRefsForTile");
   ManagedTileState& managed_tile_state = tile->managed_state();
   if (managed_tile_state.need_to_gather_pixel_refs) {
     base::TimeTicks start_time =
         rendering_stats_instrumentation_->StartRecording();
-    tile->picture_pile()->GatherPixelRefs(
-        tile->content_rect_,
-        tile->contents_scale_,
-        managed_tile_state.pending_pixel_refs);
+    for (PicturePileImpl::LazyPixelRefIterator pixel_ref_iter(

[enne (OOO), 2013/04/22 19:10:56]

Looks fine to me.  I think I generally indent 5 spaces for the other two
statements to match up with the parenthesis in "for (", but I think what you've
written is legal style.

If you're not already using it, I highly recommend clang-format.  It's not
always right, but it normally does a good job even with these sorts of
constructs.

+            tile->content_rect(),
+            tile->contents_scale(),
+            tile->picture_pile());
+        pixel_ref_iter;
+        ++pixel_ref_iter) {
+      managed_tile_state.pending_pixel_refs.push_back(*pixel_ref_iter);
+      ++pixel_ref_iter;
+    }
     managed_tile_state.need_to_gather_pixel_refs = false;
     base::TimeDelta duration =
         rendering_stats_instrumentation_->EndRecording(start_time);
     rendering_stats_instrumentation_->AddImageGathering(duration);

[reveman, 2013/04/22 19:56:06]

These stats should now be collected in Picture::GatherAllPixelRefs() instead,
right?

[vmpstr, 2013/04/22 22:38:16]

I'm not sure. It depends what we want to gather. It's still useful to know how
long it took to look-up the pixel refs (to ensure that it is, in fact, quick).

[reveman, 2013/04/23 14:33:23]

a trace event is enough to check that this is quick. it only makes sense to
collect stats for things that have a very variable cost based on page contents.

   }
 }
 
 void TileManager::DispatchImageDecodeTasksForTile(Tile* tile) {
   GatherPixelRefsForTile(tile);
   std::list<skia::LazyPixelRef*>& pending_pixel_refs =
       tile->managed_state().pending_pixel_refs;
   std::list<skia::LazyPixelRef*>::iterator it = pending_pixel_refs.begin();
   while (it != pending_pixel_refs.end()) {
     if (pending_decode_tasks_.end() != pending_decode_tasks_.find(
@@ skipping 145 matching lines @@
   // of this could be that this tile is no longer allowed to use gpu
   // memory and in that case we need to abort initialization and free all
   // associated resources before calling DispatchMoreTasks().
   if (manage_tiles_call_count_when_dispatched != manage_tiles_call_count_)
     AssignGpuMemoryToTiles();
 
   ManagedTileState& managed_tile_state = tile->managed_state();
 
   // Finish resource initialization if |can_use_gpu_memory| is true.
   if (managed_tile_state.can_use_gpu_memory) {
-
     // Tile resources can't be freed until upload has completed.
     tile->drawing_info().can_be_freed_ = false;
 
     resource_pool_->resource_provider()->BeginSetPixels(resource->id());
     has_performed_uploads_since_last_flush_ = true;
 
     tile->drawing_info().resource_ = resource.Pass();
 
     bytes_pending_upload_ += tile->bytes_consumed_if_allocated();
     tiles_with_pending_upload_.push(tile);
@@ skipping 148 matching lines @@
     skia::LazyPixelRef* pixel_ref,
     RenderingStatsInstrumentation* stats_instrumentation) {
   TRACE_EVENT0("cc", "TileManager::RunImageDecodeTask");
   base::TimeTicks start_time = stats_instrumentation->StartRecording();
   pixel_ref->Decode();
   base::TimeDelta duration = stats_instrumentation->EndRecording(start_time);
   stats_instrumentation->AddDeferredImageDecode(duration);
 }
 
 }  // namespace cc