cc: Replace recorded region with direct map lookup

cc/resources/picture_pile.cc

Index: cc/resources/picture_pile.cc
diff --git a/cc/resources/picture_pile.cc b/cc/resources/picture_pile.cc
index 43b13c577941531cebac25736e7ba2003c5b1e93..ec1180e8ca98b411226625f6ba4a950cd89bf56d 100644
--- a/cc/resources/picture_pile.cc
+++ b/cc/resources/picture_pile.cc
@@ -164,14 +164,18 @@ bool PicturePile::Update(
       -kPixelDistanceToRecord,
       -kPixelDistanceToRecord,
       -kPixelDistanceToRecord);
+  recorded_viewport_ = interest_rect;
+  recorded_viewport_.Intersect(gfx::Rect(size()));
 
   bool invalidated = false;
   for (Region::Iterator i(invalidation); i.has_rect(); i.next()) {
     gfx::Rect invalidation = i.rect();
     // Split this inflated invalidation across tile boundaries and apply it
     // to all tiles that it touches.
-    for (TilingData::Iterator iter(&tiling_, invalidation);
-         iter; ++iter) {
+    bool include_borders = true;
+    for (TilingData::Iterator iter(&tiling_, invalidation, include_borders);
+         iter;
+         ++iter) {
       const PictureMapKey& key = iter.index();
 
       PictureMap::iterator picture_it = picture_map_.find(key);
@@ -186,9 +190,9 @@ bool PicturePile::Update(
   // Make a list of all invalid tiles; we will attempt to
   // cluster these into multiple invalidation regions.
   std::vector<gfx::Rect> invalid_tiles;
-
-  for (TilingData::Iterator it(&tiling_, interest_rect);
-       it; ++it) {
+  bool include_borders = true;
+  for (TilingData::Iterator it(&tiling_, interest_rect, include_borders); it;
+       ++it) {
     const PictureMapKey& key = it.index();
     PictureInfo& info = picture_map_[key];
 
@@ -199,17 +203,20 @@ bool PicturePile::Update(
     if (info.NeedsRecording(frame_number, distance_to_visible)) {
       gfx::Rect tile = tiling_.TileBounds(key.first, key.second);
       invalid_tiles.push_back(tile);
+    } else if (recorded_viewport_.Intersects(rect)) {

[vmpstr, 2014/03/18 17:24:54]

Should this have a "&& !info.picture"? If it doesn't need recording, because it
has a picture, then it's correctly in the recorded viewport, no?

[enne (OOO), 2014/03/18 17:52:16]

Oh! Yes!

+      // Recorded viewport is just an optimization for a fully recorded
+      // interest rect.  In this case, a tile in that rect has declined
+      // to be recorded (probably due to frequent invalidations).
+      // TODO(enne): Shrink the recorded_viewport_ rather than clearing.
+      recorded_viewport_ = gfx::Rect();

[enne (OOO), 2014/03/18 00:55:20]

The core issue is that if something gets invalidated offscreen, then this cached
recorded viewport rect that is filled with valid tiles is no longer true.  So,
this needs to be cleared here.

     }
   }
 
   std::vector<gfx::Rect> record_rects;
   ClusterTiles(invalid_tiles, &record_rects);
 
-  if (record_rects.empty()) {
-    if (invalidated)
-      UpdateRecordedRegion();
+  if (record_rects.empty())
     return invalidated;
-  }
 
   for (std::vector<gfx::Rect>::iterator it = record_rects.begin();
        it != record_rects.end();
@@ -245,18 +252,24 @@ bool PicturePile::Update(
       stats_instrumentation->AddRecord(best_duration, recorded_pixel_count);
     }
 
-    for (TilingData::Iterator it(&tiling_, record_rect);
-        it; ++it) {
+    bool found_tile_for_recorded_picture = false;
+
+    bool include_borders = true;
+    for (TilingData::Iterator it(&tiling_, record_rect, include_borders); it;
+         ++it) {
       const PictureMapKey& key = it.index();
       gfx::Rect tile = PaddedRect(key);
       if (record_rect.Contains(tile)) {
         PictureInfo& info = picture_map_[key];
         info.SetPicture(picture);
+        found_tile_for_recorded_picture = true;
       }
     }
+    DCHECK(found_tile_for_recorded_picture);
   }
 
-  UpdateRecordedRegion();
+  has_any_recordings_ = true;
+  DCHECK(CanRasterSlowTileCheck(recorded_viewport_));

[enne (OOO), 2014/03/18 00:55:20]

Just an additional sanity check.

   return true;
 }