cc: Remove and Create the correct tiles when resizing live tiles rect

cc/resources/picture_layer_tiling.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/picture_layer_tiling.h"
 
 #include <algorithm>
 #include <cmath>
 #include <limits>
 
 #include "base/debug/trace_event.h"
 #include "base/debug/trace_event_argument.h"
+#include "base/logging.h"
 #include "cc/base/math_util.h"
 #include "cc/resources/tile.h"
 #include "cc/resources/tile_priority.h"
 #include "cc/trees/occlusion_tracker.h"
 #include "ui/gfx/point_conversions.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/safe_integer_conversions.h"
 #include "ui/gfx/size_conversions.h"
 
 namespace cc {
@@ skipping 116 matching lines @@
   scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
   if (tile.get()) {
     DCHECK(!tile->is_shared());
     tiles_[key] = tile;
   }
   return tile.get();
 }
 
 void PictureLayerTiling::CreateMissingTilesInLiveTilesRect() {
   const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
-  bool include_borders = true;
+  bool include_borders = false;
   for (TilingData::Iterator iter(
            &tiling_data_, live_tiles_rect_, include_borders);
        iter;
        ++iter) {
     TileMapKey key = iter.index();
     TileMap::iterator find = tiles_.find(key);
     if (find != tiles_.end())
       continue;
     CreateTile(key.first, key.second, twin_tiling);
   }
+
+  VerifyLiveTilesRect();
 }
 
 void PictureLayerTiling::UpdateTilesToCurrentPile(
     const Region& layer_invalidation,
     const gfx::Size& new_layer_bounds) {
   DCHECK(!new_layer_bounds.IsEmpty());
 
   gfx::Size old_layer_bounds = layer_bounds_;
   layer_bounds_ = new_layer_bounds;
 
   gfx::Size content_bounds =
       gfx::ToCeiledSize(gfx::ScaleSize(layer_bounds_, contents_scale_));
   gfx::Size tile_size = tiling_data_.max_texture_size();
 
   if (layer_bounds_ != old_layer_bounds) {
+    // The SetLiveTilesRect() method would drop tiles outside the new bounds,
+    // but may do so incorrectly if resizing the tiling causes the number of
+    // tiles in the tiling_data_ to change.
+    gfx::Rect content_rect(content_bounds);
+    int before_left = tiling_data_.TileXIndexFromSrcCoord(live_tiles_rect_.x());
+    int before_top = tiling_data_.TileYIndexFromSrcCoord(live_tiles_rect_.y());
+    int before_right =
+        tiling_data_.TileXIndexFromSrcCoord(live_tiles_rect_.right() - 1);
+    int before_bottom =
+        tiling_data_.TileYIndexFromSrcCoord(live_tiles_rect_.bottom() - 1);
+
+    live_tiles_rect_.Intersect(content_rect);

[vmpstr, 2014/08/27 19:13:21]

nit: Put a comment here saying that this is the place where you actually shrink
live tiles rect/tiling data. Otherwise it's kind of easy to skim over it without
noticing

[danakj, 2014/08/27 19:27:01]

Done.

+    tiling_data_.SetTilingSize(content_bounds);
+
+    int after_right = -1;
+    int after_bottom = -1;
+    if (!live_tiles_rect_.IsEmpty()) {
+      after_right =
+          tiling_data_.TileXIndexFromSrcCoord(live_tiles_rect_.right() - 1);
+      after_bottom =
+          tiling_data_.TileYIndexFromSrcCoord(live_tiles_rect_.bottom() - 1);
+    }
+
+    // There is no recycled twin since this is run on the pending tiling.

[vmpstr, 2014/08/27 19:13:21]

Please DCHECK the tree

[danakj, 2014/08/27 19:27:01]

Oh, GetTree, I missed that in the client. Done.

+    PictureLayerTiling* recycled_twin = NULL;
+    DCHECK_EQ(recycled_twin, client_->GetRecycledTwinTiling(this));
+
     // Drop tiles outside the new layer bounds if the layer shrank.
-    SetLiveTilesRect(
-        gfx::IntersectRects(live_tiles_rect_, gfx::Rect(content_bounds)));
-    tiling_data_.SetTilingSize(content_bounds);
+    for (int i = after_right + 1; i <= before_right; ++i) {
+      for (int j = before_top; j <= before_bottom; ++j)
+        RemoveTileAt(i, j, recycled_twin);
+    }
+    for (int i = before_left; i <= before_right; ++i) {

[vmpstr, 2014/08/27 19:13:21]

nit: this can be after_right, I think? Otherwise you're clearing the corner
twice? Maybe?

[danakj, 2014/08/27 19:27:01]

Oh, yah. I think so.

+      for (int j = after_bottom + 1; j <= before_bottom; ++j)
+        RemoveTileAt(i, j, recycled_twin);
+    }
+
+    // If the layer grew, new tiles may exist inside the same live tiles rect.
+    const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+    for (int i = before_right + 1; i <= after_right; ++i) {

[vmpstr, 2014/08/27 19:13:21]

Can you please add a comment in the style of

// ------===
// |    |xx|
// |    |xx|
// ------===

or something where X denotes the spot that this loop touches, etc? I'm not sure
if ascii art is the way to go here, but I think it should clarify what is going
on. I'm just looking at the indecies and trying to visualize the area being
iterated.

[danakj, 2014/08/27 19:27:01]

I changed these from a double for-loop to an if statement + dcheck + single for
loop. Does that help? Do you still want a verbose comment?

+      for (int j = before_top; j <= after_bottom; ++j)
+        CreateTile(i, j, twin_tiling);
+    }
+    for (int i = before_left; i <= before_right; ++i) {
+      for (int j = before_bottom + 1; j <= after_bottom; ++j)
+        CreateTile(i, j, twin_tiling);
+    }
+
     tile_size = client_->CalculateTileSize(content_bounds);
   }
 
   if (tile_size != tiling_data_.max_texture_size()) {
     tiling_data_.SetMaxTextureSize(tile_size);
     // When the tile size changes, the TilingData positions no longer work
     // as valid keys to the TileMap, so just drop all tiles.
     Reset();
   } else {
     Invalidate(layer_invalidation);
   }
 
   PicturePileImpl* pile = client_->GetPile();
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
     it->second->set_picture_pile(pile);
+  VerifyLiveTilesRect();
 }
 
 void PictureLayerTiling::RemoveTilesInRegion(const Region& layer_region) {
   bool recreate_invalidated_tiles = false;
   DoInvalidate(layer_region, recreate_invalidated_tiles);
 }
 
 void PictureLayerTiling::Invalidate(const Region& layer_region) {
   bool recreate_invalidated_tiles = true;
   DoInvalidate(layer_region, recreate_invalidated_tiles);
 }
 
 void PictureLayerTiling::DoInvalidate(const Region& layer_region,
                                       bool recreate_invalidated_tiles) {
   std::vector<TileMapKey> new_tile_keys;
   gfx::Rect expanded_live_tiles_rect =
-      tiling_data_.ExpandRectIgnoringBordersToTileBoundsWithBorders(
-          live_tiles_rect_);
+      tiling_data_.ExpandRectIgnoringBordersToTileBounds(live_tiles_rect_);
   for (Region::Iterator iter(layer_region); iter.has_rect(); iter.next()) {
     gfx::Rect layer_rect = iter.rect();
     gfx::Rect content_rect =
         gfx::ScaleToEnclosingRect(layer_rect, contents_scale_);
+    // Consider tiles inside the live tiles rect even if only their border
+    // pixels intersect the invalidation. But don't consider tiles outside
+    // the live tiles rect with the same conditions, as they won't exist.
+    int border_pixels = tiling_data_.border_texels();
+    content_rect.Inset(-border_pixels, -border_pixels);
     // Avoid needless work by not bothering to invalidate where there aren't
     // tiles.
     content_rect.Intersect(expanded_live_tiles_rect);
     if (content_rect.IsEmpty())
       continue;
-    bool include_borders = true;
+    // Since the content_rect includes border pixels already, don't include
+    // borders when iterating to avoid double counting them.
+    bool include_borders = false;
     for (TilingData::Iterator iter(
              &tiling_data_, content_rect, include_borders);
          iter;
          ++iter) {
-      TileMapKey key(iter.index());
-      TileMap::iterator find = tiles_.find(key);
-      if (find == tiles_.end())
-        continue;
-
-      ReleaseTile(find->second.get(), client_->GetTree());
-
-      tiles_.erase(find);
-      new_tile_keys.push_back(key);
+      // There is no recycled twin since this is run on the pending tiling.

[vmpstr, 2014/08/27 19:13:21]

nit: I think we have a client_->GetTree in this version, can you sprinkle a few
dchecks where you expect this to be run on the pending tree only

[danakj, 2014/08/27 19:27:01]

Done.

+      PictureLayerTiling* recycled_twin = NULL;
+      DCHECK_EQ(recycled_twin, client_->GetRecycledTwinTiling(this));
+      if (RemoveTileAt(iter.index_x(), iter.index_y(), recycled_twin))
+        new_tile_keys.push_back(iter.index());
     }
   }
 
   if (recreate_invalidated_tiles && !new_tile_keys.empty()) {
     for (size_t i = 0; i < new_tile_keys.size(); ++i) {
       // Don't try to share a tile with the twin layer, it's been invalidated so
       // we have to make our own tile here.
       const PictureLayerTiling* twin_tiling = NULL;
       CreateTile(new_tile_keys[i].first, new_tile_keys[i].second, twin_tiling);
     }
@@ skipping 143 matching lines @@
     return texture_rect;
   texture_rect.Offset(-tex_origin.OffsetFromOrigin());
 
   return texture_rect;
 }
 
 gfx::Size PictureLayerTiling::CoverageIterator::texture_size() const {
   return tiling_->tiling_data_.max_texture_size();
 }
 
+bool PictureLayerTiling::RemoveTileAt(int i,
+                                      int j,
+                                      PictureLayerTiling* recycled_twin) {
+  TileMap::iterator found = tiles_.find(TileMapKey(i, j));
+  if (found == tiles_.end())
+    return false;
+  ReleaseTile(found->second.get(), client_->GetTree());
+  tiles_.erase(found);
+  if (recycled_twin) {
+    // Recycled twin does not also have a recycled twin, so pass NULL.
+    recycled_twin->RemoveTileAt(i, j, NULL);
+  }
+  return true;
+}
+
 void PictureLayerTiling::Reset() {
   live_tiles_rect_ = gfx::Rect();
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
+  PictureLayerTiling* recycled_twin = client_->GetRecycledTwinTiling(this);
+  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
     ReleaseTile(it->second.get(), client_->GetTree());
+    if (recycled_twin)
+      recycled_twin->RemoveTileAt(it->first.first, it->first.second, NULL);
+  }
   tiles_.clear();
 }
 
 gfx::Rect PictureLayerTiling::ComputeSkewport(
     double current_frame_time_in_seconds,
     const gfx::Rect& visible_rect_in_content_space) const {
   gfx::Rect skewport = visible_rect_in_content_space;
   if (last_impl_frame_time_in_seconds_ == 0.0)
     return skewport;
 
@@ skipping 86 matching lines @@
 
   last_impl_frame_time_in_seconds_ = current_frame_time_in_seconds;
   last_visible_rect_in_content_space_ = visible_rect_in_content_space;
 
   eviction_tiles_cache_valid_ = false;
 
   TilePriority now_priority(resolution_, TilePriority::NOW, 0);
   float content_to_screen_scale = ideal_contents_scale / contents_scale_;
 
   // Assign now priority to all visible tiles.
-  bool include_borders = true;
+  bool include_borders = false;
   has_visible_rect_tiles_ = false;
   for (TilingData::Iterator iter(
            &tiling_data_, visible_rect_in_content_space, include_borders);
        iter;
        ++iter) {
     TileMap::iterator find = tiles_.find(iter.index());
     if (find == tiles_.end())
       continue;
     has_visible_rect_tiles_ = true;
     Tile* tile = find->second.get();
@@ skipping 81 matching lines @@
     tile->SetPriority(tree, priority);
   }
 
   // Update iteration rects.
   current_visible_rect_ = visible_rect_in_content_space;
   current_skewport_rect_ = skewport;
   current_soon_border_rect_ = soon_border_rect;
   current_eventually_rect_ = eventually_rect;
 }
 
-void PictureLayerTiling::RemoveTileAt(int i, int j) {
-  TileMapKey key(i, j);
-  TileMap::iterator found = tiles_.find(key);
-  if (found == tiles_.end())
-    return;
-  ReleaseTile(found->second.get(), client_->GetTree());
-  tiles_.erase(found);
-}
-
 void PictureLayerTiling::SetLiveTilesRect(
     const gfx::Rect& new_live_tiles_rect) {
   DCHECK(new_live_tiles_rect.IsEmpty() ||
          gfx::Rect(tiling_size()).Contains(new_live_tiles_rect))
       << "tiling_size: " << tiling_size().ToString()
       << " new_live_tiles_rect: " << new_live_tiles_rect.ToString();
   if (live_tiles_rect_ == new_live_tiles_rect)
     return;
 
   // Iterate to delete all tiles outside of our new live_tiles rect.
   PictureLayerTiling* recycled_twin = client_->GetRecycledTwinTiling(this);
   for (TilingData::DifferenceIterator iter(&tiling_data_,
                                            live_tiles_rect_,
                                            new_live_tiles_rect);
        iter;
        ++iter) {
-    TileMapKey key(iter.index());
-    TileMap::iterator found = tiles_.find(key);
-    // If the tile was outside of the recorded region, it won't exist even
-    // though it was in the live rect.
-    if (found != tiles_.end()) {
-      ReleaseTile(found->second.get(), client_->GetTree());
-      tiles_.erase(found);
-      if (recycled_twin)
-        recycled_twin->RemoveTileAt(iter.index_x(), iter.index_y());
-    }
+    RemoveTileAt(iter.index_x(), iter.index_y(), recycled_twin);
   }
 
   const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
 
   // Iterate to allocate new tiles for all regions with newly exposed area.
   for (TilingData::DifferenceIterator iter(&tiling_data_,
                                            new_live_tiles_rect,
                                            live_tiles_rect_);
        iter;
        ++iter) {
     TileMapKey key(iter.index());
     CreateTile(key.first, key.second, twin_tiling);
   }
 
   live_tiles_rect_ = new_live_tiles_rect;
+  VerifyLiveTilesRect();
+}
+
+void PictureLayerTiling::VerifyLiveTilesRect() {
+#if DCHECK_IS_ON
+  for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+    if (!it->second.get())
+      continue;
+    DCHECK(it->first.first < tiling_data_.num_tiles_x())
+        << this << " " << it->first.first << "," << it->first.second
+        << " num_tiles_x " << tiling_data_.num_tiles_x() << " live_tiles_rect "
+        << live_tiles_rect_.ToString();
+    DCHECK(it->first.second < tiling_data_.num_tiles_y())
+        << this << " " << it->first.first << "," << it->first.second
+        << " num_tiles_y " << tiling_data_.num_tiles_y() << " live_tiles_rect "
+        << live_tiles_rect_.ToString();
+    DCHECK(tiling_data_.TileBounds(it->first.first, it->first.second)
+               .Intersects(live_tiles_rect_))
+        << this << " " << it->first.first << "," << it->first.second
+        << " tile bounds "
+        << tiling_data_.TileBounds(it->first.first, it->first.second).ToString()
+        << " live_tiles_rect " << live_tiles_rect_.ToString();
+  }
+#endif
 }
 
 void PictureLayerTiling::DidBecomeRecycled() {
   // DidBecomeActive below will set the active priority for tiles that are
   // still in the tree. Calling this first on an active tiling that is becoming
   // recycled takes care of tiles that are no longer in the active tree (eg.
   // due to a pending invalidation).
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
     it->second->SetPriority(ACTIVE_TREE, TilePriority());
   }
@@ skipping 274 matching lines @@
     PictureLayerTiling* tiling,
     WhichTree tree)
     : tiling_(tiling), phase_(VISIBLE_RECT), tree_(tree), current_tile_(NULL) {
   if (!tiling_->has_visible_rect_tiles_) {
     AdvancePhase();
     return;
   }
 
   visible_iterator_ = TilingData::Iterator(&tiling_->tiling_data_,
                                            tiling_->current_visible_rect_,
-                                           true /* include_borders */);
+                                           false /* include_borders */);
   if (!visible_iterator_) {
     AdvancePhase();
     return;
   }
 
   current_tile_ =
       tiling_->TileAt(visible_iterator_.index_x(), visible_iterator_.index_y());
   if (!current_tile_ || !TileNeedsRaster(current_tile_))
     ++(*this);
 }
@@ skipping 136 matching lines @@
   DCHECK(*this);
   do {
     ++current_eviction_tiles_index_;
   } while (current_eviction_tiles_index_ != eviction_tiles_->size() &&
            !(*eviction_tiles_)[current_eviction_tiles_index_]->HasResources());
 
   return *this;
 }
 
 }  // namespace cc