cc: Expand invalidation to full tile when recording is missing for the tile.

cc/layers/picture_layer_impl.cc

Index: cc/layers/picture_layer_impl.cc
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
index 250dafa34eac9f2ed6d7846e18f102e5797c3352..e3b18e0e06aed1a0792e5b019746c7cfe7c37f45 100644
--- a/cc/layers/picture_layer_impl.cc
+++ b/cc/layers/picture_layer_impl.cc
@@ -653,6 +653,7 @@ gfx::Size PictureLayerImpl::CalculateTileSize(
 }
 
 void PictureLayerImpl::SyncFromActiveLayer(const PictureLayerImpl* other) {
+  TRACE_EVENT0("cc", "SyncFromActiveLayer");
   DCHECK(!other->needs_post_commit_initialization_);
   DCHECK(other->tilings_);
 
@@ -669,22 +670,25 @@ void PictureLayerImpl::SyncFromActiveLayer(const PictureLayerImpl* other) {
   raster_contents_scale_ = other->raster_contents_scale_;
   low_res_raster_contents_scale_ = other->low_res_raster_contents_scale_;
 
-  // Add synthetic invalidations for any recordings that were dropped.  As
-  // tiles are updated to point to this new pile, this will force the dropping
-  // of tiles that can no longer be rastered.  This is not ideal, but is a
-  // trade-off for memory (use the same pile as much as possible, by switching
-  // during DidBecomeActive) and for time (don't bother checking every tile
-  // during activation to see if the new pile can still raster it).
-  for (int x = 0; x < pile_->num_tiles_x(); ++x) {
-    for (int y = 0; y < pile_->num_tiles_y(); ++y) {
-      bool previously_had = other->pile_->HasRecordingAt(x, y);
-      bool now_has = pile_->HasRecordingAt(x, y);
-      if (now_has || !previously_had)
-        continue;
-      gfx::Rect layer_rect = pile_->tile_bounds(x, y);
-      invalidation_.Union(layer_rect);
-    }
-  }
+  // Add synthetic invalidations to drop tiles from the active twin's tiling
+  // that we may not have recordings for. As tiles are updated to point to this
+  // new pile, this will force the dropping of tiles that can no longer be
+  // rastered.
+  // This is not ideal, as our pile may be able to raster the tiles, but
+  // checking this is expensive.
+  // 1. To find all the recorded pile tiles and build a region is more costly
+  // (up to 5-6x more costly in some experiments).
+  // 2. To check every raster tile and verify that the pile can record it before
+  // we activate and drop the active pile is much more expensive.
+  // 3. But we want to use the same pile for all of the tilings to avoid having
+  // to keep around old piles of recordings.
+  // This causes tiles outside the recorded viewport to be always dropped, but
+  // we expect these to be far from the viewport, since they were outside of the
+  // recording interest rect, so re-rastering those tiles is the tradeoff for
+  // speed here.
+  Region outside_recorded_viewport = Region(pile_->tiling_rect());
+  outside_recorded_viewport.Subtract(pile_->recorded_viewport());
+  invalidation_.Union(outside_recorded_viewport);

[enne (OOO), 2014/05/27 21:09:22]

There's a performance bug here.  As the comment on the member variable suggests,
recorded viewport is empty if it's not valid.

If something declines to be recorded (animated gif offscreen but within the
recorded viewport), then recorded viewport will be set to empty.  If recorded
viewport is empty then you union the entire pile into invalidation.  So
offscreen animated gifs => reraster the world.

Should dropping tiles / adding invalidations outside the recorded viewport be
the job of picture layer and picture pile, who might be able to access the
desired recorded viewport vs. the actual pessimistic one which may be empty for
edge cases?

 
   // Union in the other newly exposed regions as invalid.
   Region difference_region = Region(gfx::Rect(bounds()));