cc: Explicitly invalidate all dropped recordings on the main thread.

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 6341e88b3688889df468543cecf2c652136f20c9..3c03559b2c2fe7324f3ef826b1aa21cd2adba9cf 100644
--- a/cc/resources/picture_pile.cc
+++ b/cc/resources/picture_pile.cc
@@ -152,6 +152,7 @@ bool PicturePile::UpdateAndExpandInvalidation(
     SkColor background_color,
     bool contents_opaque,
     bool contents_fill_bounds_completely,
+    const gfx::Rect& layer_bounds_rect,
     const gfx::Rect& visible_layer_rect,
     int frame_number,
     Picture::RecordingMode recording_mode,
@@ -160,6 +161,15 @@ bool PicturePile::UpdateAndExpandInvalidation(
   contents_opaque_ = contents_opaque;
   contents_fill_bounds_completely_ = contents_fill_bounds_completely;
 
+  bool updated = false;
+
+  Region resize_invalidation;
+  gfx::Rect old_tiling_rect = tiling_rect();
+  if (old_tiling_rect != layer_bounds_rect) {
+    tiling_.SetTilingRect(layer_bounds_rect);
+    updated = true;
+  }
+
   gfx::Rect interest_rect = visible_layer_rect;
   interest_rect.Inset(
       -kPixelDistanceToRecord,
@@ -172,11 +182,82 @@ bool PicturePile::UpdateAndExpandInvalidation(
   gfx::Rect interest_rect_over_tiles =
       tiling_.ExpandRectToTileBounds(interest_rect);
 
-  Region invalidation_expanded_to_full_tiles;
+  if (old_tiling_rect != layer_bounds_rect) {
+    has_any_recordings_ = false;
 
-  bool invalidated = false;
+    if (tiling_rect().origin() != old_tiling_rect.origin()) {
+      // If the origin changes we just do something simple.
+      picture_map_.clear();
+      invalidation->Union(old_tiling_rect);
+      invalidation->Union(tiling_rect());
+    } else {
+      // Drop recordings that are outside the new layer bounds or that changed
+      // size.
+      std::vector<PictureMapKey> to_erase;
+      int min_toss_x = tiling_.num_tiles_x();
+      if (tiling_rect().right() > old_tiling_rect.right()) {
+        min_toss_x =
+            tiling_.FirstBorderTileXIndexFromSrcCoord(old_tiling_rect.right());
+      }
+      int min_toss_y = tiling_.num_tiles_y();
+      if (tiling_rect().bottom() > old_tiling_rect.bottom()) {
+        min_toss_y =
+            tiling_.FirstBorderTileYIndexFromSrcCoord(old_tiling_rect.bottom());
+      }
+      for (PictureMap::const_iterator it = picture_map_.begin();
+           it != picture_map_.end();
+           ++it) {
+        const PictureMapKey& key = it->first;
+        if (key.first < min_toss_x && key.second < min_toss_y) {
+          has_any_recordings_ |= !!it->second.GetPicture();
+          continue;
+        }
+        to_erase.push_back(key);
+      }
+
+      for (size_t i = 0; i < to_erase.size(); ++i)
+        picture_map_.erase(to_erase[i]);
+
+      // Invalidate dropped recordings that changed size that are outside the
+      // interest rect so that raster tiles that depend on these recordings will
+      // be dropped.
+      gfx::Rect old_tiling_rect_over_tiles =
+          tiling_.ExpandRectToTileBounds(old_tiling_rect);
+      if (min_toss_x < tiling_.num_tiles_x()) {
+        gfx::Rect right_side(tiling_.TilePositionX(min_toss_x),
+                             old_tiling_rect_over_tiles.y(),
+                             tiling_.TileSizeX(min_toss_x),
+                             old_tiling_rect_over_tiles.height());
+        right_side.Subtract(interest_rect_over_tiles);

[enne (OOO), 2014/07/11 00:12:04]

Can you leave a comment describing why this needs to be subtracted, and point me
at a test that verifies this edge case?

[danakj, 2014/07/11 17:06:17]

The comment is up on 221. What do you think I should add to that? Thanks for
pointing out the tests, I will add.

[enne (OOO), 2014/07/11 17:30:05]

Right, the comment says 'what' but not 'why.  I think it's not obvious why you
are subtracting the interest rect here rather than just unioning in the
invalidation from outside.

[danakj, 2014/07/11 18:06:27]

These Subtract calls are covered by:
PicturePileTest.ResizePileInsideInterestRect
PicturePileTest.SmallResizePileInsideInterestRect

[enne (OOO), 2014/07/11 18:21:54]

Sort of.  It looks like all of your tests test shrinking where the subtract
produces no invalidation.  Could you have one that shrinks enough such that some
tiles outside the interest_rect_over_tiles get dropped?

+        resize_invalidation.Union(right_side);
+      }
+      if (min_toss_y < tiling_.num_tiles_y()) {
+        gfx::Rect bottom_side(old_tiling_rect_over_tiles.x(),
+                              tiling_.TilePositionY(min_toss_y),
+                              old_tiling_rect_over_tiles.width(),
+                              tiling_.TileSizeY(min_toss_y));
+        bottom_side.Subtract(interest_rect_over_tiles);
+        resize_invalidation.Union(bottom_side);
+      }
+    }
+  }
+
+  Region invalidation_expanded_to_full_tiles;
   for (Region::Iterator i(*invalidation); i.has_rect(); i.next()) {
     gfx::Rect invalid_rect = i.rect();
+
+    // Expand invalidation that is outside tiles that intersect the interest
+    // rect. These tiles are no longer valid and should be considerered fully
+    // invalid, so we can know to not keep around raster tiles that intersect
+    // with these recording tiles.
+    gfx::Rect invalid_rect_outside_interest_rect_tiles = invalid_rect;
+    // TODO(danakj): We should have a Rect-subtract-Rect-to-2-rects operator
+    // instead of using Rect::Subtract which gives you the bounding box of the
+    // subtraction.
+    invalid_rect_outside_interest_rect_tiles.Subtract(interest_rect_over_tiles);
+    invalidation_expanded_to_full_tiles.Union(tiling_.ExpandRectToTileBounds(
+        invalid_rect_outside_interest_rect_tiles));
+
     // Split this inflated invalidation across tile boundaries and apply it
     // to all tiles that it touches.
     bool include_borders = true;
@@ -190,23 +271,18 @@ bool PicturePile::UpdateAndExpandInvalidation(
         continue;
 
       // Inform the grid cell that it has been invalidated in this frame.
-      invalidated = picture_it->second.Invalidate(frame_number) || invalidated;
+      updated = picture_it->second.Invalidate(frame_number) || updated;
+      // Invalidate drops the picture so the whole tile better be invalidated if
+      // it won't be re-recorded below.
+      DCHECK(
+          tiling_.TileBounds(key.first, key.second).Intersects(interest_rect) ||
+          invalidation_expanded_to_full_tiles.Contains(
+              tiling_.TileBounds(key.first, key.second)));
     }
-
-    // Expand invalidation that is outside tiles that intersect the interest
-    // rect. These tiles are no longer valid and should be considerered fully
-    // invalid, so we can know to not keep around raster tiles that intersect
-    // with these recording tiles.
-    gfx::Rect invalid_rect_outside_interest_rect_tiles = invalid_rect;
-    // TODO(danakj): We should have a Rect-subtract-Rect-to-2-rects operator
-    // instead of using Rect::Subtract which gives you the bounding box of the
-    // subtraction.
-    invalid_rect_outside_interest_rect_tiles.Subtract(interest_rect_over_tiles);
-    invalidation_expanded_to_full_tiles.Union(tiling_.ExpandRectToTileBounds(
-        invalid_rect_outside_interest_rect_tiles));
   }
 
   invalidation->Union(invalidation_expanded_to_full_tiles);
+  invalidation->Union(resize_invalidation);
 
   // Make a list of all invalid tiles; we will attempt to
   // cluster these into multiple invalidation regions.
@@ -244,7 +320,7 @@ bool PicturePile::UpdateAndExpandInvalidation(
   ClusterTiles(invalid_tiles, &record_rects);
 
   if (record_rects.empty())
-    return invalidated;
+    return updated;
 
   for (std::vector<gfx::Rect>::iterator it = record_rects.begin();
        it != record_rects.end();
@@ -310,4 +386,11 @@ bool PicturePile::UpdateAndExpandInvalidation(
   return true;
 }
 
+void PicturePile::SetEmptyBounds() {
+  tiling_.SetTilingRect(gfx::Rect());
+  picture_map_.clear();
+  has_any_recordings_ = false;
+  recorded_viewport_ = gfx::Rect();
+}
+
 }  // namespace cc