cc: Correct expansion of invalidation for tiles outside of interest rect

Previously we subtracted the interest_rect_over_tiles from the
invalid_rect when looking to see what invalidation would not be recorded
but this computation is wrong.

Instead, we need to expand the invalidation to the bounds of the tiles
it touches (including tiles it touches on borders). Then we can subtract
the interest rect expanded to the bounds of tiles it touches (including
tiles it touches on borders). This makes each expansion match the
iteration of the given rect where we include borders.

If a tile is touched only on the border by invalidation, but the tile is
not touched even on a border by the interest rect, we want to expand the
invalidation to cover that tile.

R=enne, vmpstr
BUG=421729

Review URL: https://codereview.chromium.org/657913002

Cr-Commit-Position: refs/heads/master@{#299750}

Author: danakj

cc/resources/picture_pile.cc

@@ -149,7 +149,9 @@ float ClusterTiles(const std::vector<gfx::Rect>& invalid_tiles,
 
 namespace cc {
 
-PicturePile::PicturePile() : is_suitable_for_gpu_rasterization_(true) {
+PicturePile::PicturePile()
+    : is_suitable_for_gpu_rasterization_(true),
+      pixel_record_distance_(kPixelDistanceToRecord) {
 }
 
 PicturePile::~PicturePile() {
@@ -180,11 +182,7 @@ bool PicturePile::UpdateAndExpandInvalidation(
   }
 
   gfx::Rect interest_rect = visible_layer_rect;
-  interest_rect.Inset(
-      -kPixelDistanceToRecord,
-      -kPixelDistanceToRecord,
-      -kPixelDistanceToRecord,
-      -kPixelDistanceToRecord);
+  interest_rect.Inset(-pixel_record_distance_, -pixel_record_distance_);
   recorded_viewport_ = interest_rect;
   recorded_viewport_.Intersect(gfx::Rect(tiling_size()));
 
@@ -385,23 +383,31 @@ bool PicturePile::UpdateAndExpandInvalidation(
     for (auto& it : picture_map_)
       updated = it.second.Invalidate(frame_number) || updated;
   } else {
-    // 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.
+    // Expand invalidation that is on tiles that aren't in the interest rect and
+    // will not be re-recorded below. 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.
     Region invalidation_expanded_to_full_tiles;
 
     for (Region::Iterator i(*invalidation); i.has_rect(); i.next()) {
       gfx::Rect invalid_rect = i.rect();
 
-      gfx::Rect invalid_rect_outside_interest_rect_tiles = invalid_rect;
+      // This rect covers the bounds (excluding borders) of all tiles whose
+      // bounds (including borders) touch the |interest_rect|. This matches
+      // the iteration of the |invalid_rect| below which includes borders when
+      // calling Invalidate() on pictures.
+      gfx::Rect invalid_rect_outside_interest_rect_tiles =
+          tiling_.ExpandRectToTileBounds(invalid_rect);
+      // We subtract the |interest_rect_over_tiles| which represents the bounds
+      // of tiles that will be re-recorded below. This matches the iteration of
+      // |interest_rect| below which includes borders.
       // 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_expanded_to_full_tiles.Union(
+          invalid_rect_outside_interest_rect_tiles);
 
       // Split this inflated invalidation across tile boundaries and apply it
       // to all tiles that it touches.

cc/resources/picture_pile.h

@@ -51,6 +51,8 @@ class CC_EXPORT PicturePile : public PicturePileBase {
     is_suitable_for_gpu_rasterization_ = false;
   }
 
+  void SetPixelRecordDistanceForTesting(int d) { pixel_record_distance_ = d; }
+
  protected:
   virtual ~PicturePile();
 
@@ -60,6 +62,7 @@ class CC_EXPORT PicturePile : public PicturePileBase {
   void DetermineIfSolidColor();
 
   bool is_suitable_for_gpu_rasterization_;
+  int pixel_record_distance_;
 
   DISALLOW_COPY_AND_ASSIGN(PicturePile);
 };

cc/resources/picture_pile_unittest.cc

@@ -99,6 +99,82 @@ class PicturePileTest : public PicturePileTestBase, public testing::Test {
   virtual void SetUp() override { InitializeData(); }
 };
 
+TEST_F(PicturePileTest, InvalidationOnTileBorderOutsideInterestRect) {
+  // Don't expand the interest rect past what we invalidate.
+  pile_->SetPixelRecordDistanceForTesting(0);
+
+  gfx::Size tile_size(100, 100);
+  pile_->tiling().SetMaxTextureSize(tile_size);
+
+  gfx::Size pile_size(400, 400);
+  SetTilingSize(pile_size);
+
+  // We have multiple tiles.
+  EXPECT_GT(pile_->tiling().num_tiles_x(), 2);
+  EXPECT_GT(pile_->tiling().num_tiles_y(), 2);
+
+  // Record everything.
+  Region invalidation(tiling_rect());
+  UpdateAndExpandInvalidation(&invalidation, tiling_size(), tiling_rect());
+
+  // +----------+-----------------+-----------+
+  // |          |     VVVV     1,0|           |
+  // |          |     VVVV        |           |
+  // |          |     VVVV        |           |
+  // |       ...|.................|...        |
+  // |       ...|.................|...        |
+  // +----------+-----------------+-----------+
+  // |       ...|                 |...        |
+  // |       ...|                 |...        |
+  // |       ...|                 |...        |
+  // |       ...|                 |...        |
+  // |       ...|              1,1|...        |
+  // +----------+-----------------+-----------+
+  // |       ...|.................|...        |
+  // |       ...|.................|...        |
+  // +----------+-----------------+-----------+
+  //
+  // .. = border pixels for tile 1,1
+  // VV = interest rect (what we will record)
+  //
+  // The first invalidation is inside VV, so it does not touch border pixels of
+  // tile 1,1.
+  //
+  // The second invalidation goes below VV into the .. border pixels of 1,1.
+
+  // This is the VV interest rect which will be entirely inside 1,0 and not
+  // touch the border of 1,1.
+  gfx::Rect interest_rect(
+      pile_->tiling().TilePositionX(1) + pile_->tiling().border_texels(),
+      0,
+      10,
+      pile_->tiling().TileSizeY(0) - pile_->tiling().border_texels());
+
+  // Invalidate tile 1,0 only. This is a rect that avoids the borders of any
+  // other tiles.
+  gfx::Rect invalidate_tile = interest_rect;
+  // This should cause the tile 1,0 to be invalidated and re-recorded. The
+  // invalidation did not need to be expanded.
+  invalidation = invalidate_tile;
+  UpdateAndExpandInvalidation(&invalidation, tiling_size(), interest_rect);
+  EXPECT_EQ(invalidate_tile, invalidation);
+
+  // Invalidate tile 1,0 and 1,1 by invalidating something that only touches the
+  // border of 1,1 (and is inside the tile bounds of 1,0). This is a 10px wide
+  // strip from the top of the tiling onto the border pixels of tile 1,1 that
+  // avoids border pixels of any other tiles.
+  gfx::Rect invalidate_border = interest_rect;
+  invalidate_border.Inset(0, 0, 0, -1);
+  // This should cause the tile 1,0 and 1,1 to be invalidated. The 1,1 tile will
+  // not be re-recorded since it does not touch the interest rect, so the
+  // invalidation should be expanded to cover all of 1,1.
+  invalidation = invalidate_border;
+  UpdateAndExpandInvalidation(&invalidation, tiling_size(), interest_rect);
+  Region expected_invalidation = invalidate_border;
+  expected_invalidation.Union(pile_->tiling().TileBounds(1, 1));
+  EXPECT_EQ(expected_invalidation.ToString(), invalidation.ToString());
+}
+
 TEST_F(PicturePileTest, SmallInvalidateInflated) {
   // Invalidate something inside a tile.
   Region invalidate_rect(gfx::Rect(50, 50, 1, 1));