Makes tile-creation lazy

cc/resources/picture_layer_tiling.cc

Index: cc/resources/picture_layer_tiling.cc
diff --git a/cc/resources/picture_layer_tiling.cc b/cc/resources/picture_layer_tiling.cc
index 5f74b704d19fbf9ebb1eca67e5797633ab7d40b9..1bab2b3e7680d9b0a1c984611faa02932290617f 100644
--- a/cc/resources/picture_layer_tiling.cc
+++ b/cc/resources/picture_layer_tiling.cc
@@ -21,7 +21,7 @@ scoped_ptr<PictureLayerTiling> PictureLayerTiling::Create(
 }
 
 scoped_ptr<PictureLayerTiling> PictureLayerTiling::Clone() const {
-  return make_scoped_ptr(new PictureLayerTiling(*this));
+  return make_scoped_ptr(new PictureLayerTiling(*this, 0));
 }
 
 PictureLayerTiling::PictureLayerTiling(float contents_scale)
@@ -33,6 +33,19 @@ PictureLayerTiling::PictureLayerTiling(float contents_scale)
       last_impl_frame_time_(0) {
 }
 
+PictureLayerTiling::PictureLayerTiling(const PictureLayerTiling& other, int)

[enne (OOO), 2013/03/27 16:16:27]

I'm assuming this "int" here is a way to have an overloaded copy constructor,
but that's an awkward way to overload something.  If you need to do something
like this please just use the default constructor and have Clone() set exactly
the parameters you need with a comment about why you're not copying tiles_ and
last_interest_rect_.

[whunt, 2013/03/27 17:42:02]

I agree, that's reasonable.

+    : contents_scale_(other.contents_scale_),
+      layer_bounds_(other.layer_bounds_),
+      resolution_(other.resolution_),
+      client_(other.client_),
+      tiling_data_(other.tiling_data_),
+      tiles_(),
+      interest_rect_(),
+      last_interest_rect_(other.last_interest_rect_),
+      last_source_frame_number_(other.last_source_frame_number_),
+      last_impl_frame_time_(other.last_impl_frame_time_) {
+}
+
 PictureLayerTiling::~PictureLayerTiling() {
 }
 
@@ -56,11 +69,33 @@ Tile* PictureLayerTiling::TileAt(int i, int j) const {
 }
 
 void PictureLayerTiling::CreateTile(int i, int j) {
-  gfx::Rect tile_rect = tiling_data_.TileBoundsWithBorder(i, j);
-  tile_rect.set_size(tiling_data_.max_texture_size());
   TileMapKey key(i, j);
   DCHECK(tiles_.find(key) == tiles_.end());
-  scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
+
+  gfx::Rect paint_rect = tiling_data_.TileBoundsWithBorder(i, j);

[enne (OOO), 2013/03/27 16:16:27]

I don't understand this paint_rect code at all.  Within a given tiling, no tiles
overlap, except on the borders.  It is guaranteed that no two tiles at different
indices will contain the other, considering either the bounds or bounds with
border.

A tile at the same index in a different tiling will have the exact same tile
bounds with border.  This seems like something you shouldn't need to store and
certainly something you should never need to compare against in this code.

[whunt, 2013/03/27 17:42:02]

The paint rect has to do with layers that change size.  When we create a tile,
we give store the portion of that tile that's covered by the layers bounds.  If
we shrink the layers bounds this tile is still good but if we grow the layers
bounds this tile is no longer good (assuming it was only partially covered by
the layer to begin with).  When we create tiles for areas in the difference of
two interest rects we may be able to re-use the old tile.  paint_rect is used to
determine if this is possible. 

[enne (OOO), 2013/03/27 18:03:20]

Ah, I see.  Storing a paint rect on every tile still seems like overkill to me.

I think a more elegant way to handle this might be to add synthetic
invalidations to the invalidation region when the layer bounds grows.  Then a
difference of layer bounds sizes between layer twins would be handled by the
general invalidation case.

+  gfx::Rect tile_rect = paint_rect;
+  tile_rect.set_size(tiling_data_.max_texture_size());
+
+  TileMap::iterator found = tiles_.find(key);
+  if (found != tiles_.end()) {
+    if (found->second->paint_rect().Contains(paint_rect))
+      return;
+    tiles_.erase(found);
+  }
+
+  const PictureLayerTiling* sibling = client_->GetSibling(this);
+  if (sibling) {
+    Tile* candidate_tile = sibling->TileAt(i, j);
+    if (candidate_tile &&
+        candidate_tile->paint_rect().Contains(paint_rect) &&
+        !client_->GetInvalidation()->Intersects(paint_rect)) {
+      tiles_[key] = candidate_tile;
+      return;
+    }
+  }
+
+  scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect,
+                                                 paint_rect);
   if (tile)
     tiles_[key] = tile;
 }
@@ -78,80 +113,31 @@ void PictureLayerTiling::SetLayerBounds(gfx::Size layer_bounds) {
 
   gfx::Size old_layer_bounds = layer_bounds_;
   layer_bounds_ = layer_bounds;
-  gfx::Size old_content_bounds = tiling_data_.total_size();
-  gfx::Size content_bounds =
-      gfx::ToCeiledSize(gfx::ScaleSize(layer_bounds_, contents_scale_));
 
-  tiling_data_.SetTotalSize(content_bounds);
+  // Handle the case where the new bounds are empty.
   if (layer_bounds_.IsEmpty()) {

[enne (OOO), 2013/03/27 16:16:27]

I don't think you need to special case this.  The code below does all of these
things if layer_bounds_.IsEmpty(), except for changing the interest_rect (which
I don't think is necessary).

[whunt, 2013/03/27 17:42:02]

I agree, it's not entirely necessary.  The check was in there before as an
early-out and I left it.

     tiles_.clear();
+    interest_rect_ = gfx::Rect();
+    tiling_data_.SetTotalSize(gfx::Size());
     return;
   }
 
+  // Calculate new content bounds.
+  gfx::Size old_content_bounds = tiling_data_.total_size();
+  gfx::Size content_bounds =
+      gfx::ToCeiledSize(gfx::ScaleSize(layer_bounds_, contents_scale_));
+  tiling_data_.SetTotalSize(content_bounds);
   gfx::Size tile_size = client_->CalculateTileSize(
       tiling_data_.max_texture_size(),
       content_bounds);
+
+  // Handle the case where the size of our tiling has changed.
   if (tile_size != tiling_data_.max_texture_size()) {
     tiling_data_.SetMaxTextureSize(tile_size);
     tiles_.clear();
+    interest_rect_ = gfx::Rect();

[enne (OOO), 2013/03/27 16:16:27]

Why does this need to happen?

[whunt, 2013/03/27 17:42:02]

It shouldn't happen.  This line was left over from before and can be deleted.

     CreateTilesFromLayerRect(gfx::Rect(layer_bounds_));
-    return;
   }
-
-  // Any tiles outside our new bounds are invalid and should be dropped.

[enne (OOO), 2013/03/27 16:16:27]

I think you still need this block in the SetLayerBounds function.  When you
change the TilingData's total size to something smaller, you will never get tile
indices outside of that total size, and so you will never be able to reach the
keys of these now-invalid tiles with any iteration over TilingData keys.

[whunt, 2013/03/27 17:42:02]

The old interest rect still contains those tiles.

[enne (OOO), 2013/03/27 18:03:20]

It does, but when you change TilingData's total size to something smaller you
will never get those tile indices that are outside the bounds of that total
size.  These tiles are unreachable via the iterators below.

-  if (old_content_bounds.width() > content_bounds.width() ||
-      old_content_bounds.height() > content_bounds.height()) {
-    int right =
-        tiling_data_.TileXIndexFromSrcCoord(content_bounds.width() - 1);
-    int bottom =
-        tiling_data_.TileYIndexFromSrcCoord(content_bounds.height() - 1);
-
-    std::vector<TileMapKey> invalid_tile_keys;
-    for (TileMap::const_iterator it = tiles_.begin();
-         it != tiles_.end(); ++it) {
-      if (it->first.first > right || it->first.second > bottom)
-        invalid_tile_keys.push_back(it->first);
-    }
-    for (size_t i = 0; i < invalid_tile_keys.size(); ++i)
-      tiles_.erase(invalid_tile_keys[i]);
-  }
-
-  // Create tiles for newly exposed areas.
-  Region layer_region((gfx::Rect(layer_bounds_)));
-  layer_region.Subtract(gfx::Rect(old_layer_bounds));
-  for (Region::Iterator iter(layer_region); iter.has_rect(); iter.next()) {
-    Invalidate(iter.rect());
-    CreateTilesFromLayerRect(iter.rect());
-  }
-}
-
-void PictureLayerTiling::Invalidate(const Region& layer_invalidation) {
-  std::vector<TileMapKey> new_tiles;
-
-  for (Region::Iterator region_iter(layer_invalidation);
-       region_iter.has_rect();
-       region_iter.next()) {
-
-    gfx::Rect layer_invalidation = region_iter.rect();
-    layer_invalidation.Intersect(gfx::Rect(layer_bounds_));
-    gfx::Rect rect =
-        gfx::ToEnclosingRect(ScaleRect(layer_invalidation, contents_scale_));
-
-    for (PictureLayerTiling::Iterator tile_iter(this, contents_scale_, rect);
-         tile_iter;
-         ++tile_iter) {
-      TileMapKey key(tile_iter.tile_i_, tile_iter.tile_j_);
-      TileMap::iterator found = tiles_.find(key);
-      if (found == tiles_.end())
-        continue;
-
-      tiles_.erase(found);
-      new_tiles.push_back(key);
-    }
-  }
-
-  for (size_t i = 0; i < new_tiles.size(); ++i)
-    CreateTile(new_tiles[i].first, new_tiles[i].second);
 }
 
 void PictureLayerTiling::CreateTilesFromLayerRect(gfx::Rect layer_rect) {
@@ -171,6 +157,34 @@ void PictureLayerTiling::CreateTilesFromContentRect(gfx::Rect content_rect) {
   }
 }
 
+PictureLayerTiling::SimpleIterator::SimpleIterator(
+    const PictureLayerTiling* tiling,
+    const gfx::Rect& rect)
+    : key_(0, 0),
+      left_(0),
+      right_(0),
+      bottom_(0) {
+  DCHECK(tiling);
+  if (rect.IsEmpty())
+    return;
+
+  const TilingData& tiling_data = tiling->tiling_data_;
+  key_.first = tiling_data.TileXIndexFromSrcCoord(rect.x());
+  key_.second = tiling_data.TileYIndexFromSrcCoord(rect.y());
+  right_ = tiling_data.TileXIndexFromSrcCoord(rect.right() - 1) + 1;
+  bottom_ = tiling_data.TileYIndexFromSrcCoord(rect.bottom() - 1) + 1;
+  left_ = key_.first;
+}
+
+PictureLayerTiling::SimpleIterator& PictureLayerTiling::SimpleIterator::operator++() {
+  ++key_.first;
+  if (key_.first == right_) {
+    key_.first = left_;
+    ++key_.second;
+  }
+  return *this;
+}
+
 PictureLayerTiling::Iterator::Iterator()
     : tiling_(NULL),
       current_tile_(NULL),
@@ -336,6 +350,26 @@ void PictureLayerTiling::UpdateTilePriorities(
   if (ContentRect().IsEmpty())
     return;
 
+  gfx::Rect viewport_in_content_space =
+      gfx::ToEnclosingRect(gfx::ScaleRect(viewport_in_layer_space,
+                                          contents_scale_));
+
+  gfx::Size tile_size = tiling_data_.max_texture_size();
+  int64 interest_rect_area =
+      TilePriority::kNumTilesToCoverWithInflatedViewportRectForPrioritization *
+      tile_size.width() * tile_size.height();
+
+  gfx::Rect interest_rect = ExpandRectEquallyToAreaBoundedBy(
+      viewport_in_content_space,
+      interest_rect_area,
+      ContentRect());
+  DCHECK(ContentRect().Contains(interest_rect));
+
+  ManageTiles(interest_rect_, interest_rect);
+//  ManageTiles(gfx::Rect(), interest_rect);

[enne (OOO), 2013/03/27 16:16:27]

?

[whunt, 2013/03/27 17:42:02]

Old debugging code, will be deleted.

+//  ManageTiles(gfx::Rect(), viewport_in_content_space);
+  interest_rect_ = interest_rect;

[enne (OOO), 2013/03/27 16:16:27]

I don't think you need to store this.  Is last_interest_rect_ not sufficient?

[whunt, 2013/03/27 17:42:02]

Not sure yet.

+
   bool first_update_in_new_source_frame =
       current_source_frame_number != last_source_frame_number_;
 
@@ -352,30 +386,16 @@ void PictureLayerTiling::UpdateTilePriorities(
   if (!first_update_in_new_impl_frame && !first_update_in_new_source_frame)
     return;
 
-  double time_delta = 0;
-  if (last_impl_frame_time_ != 0 && last_layer_bounds == current_layer_bounds)
+  double time_delta = 0.0;
+  if (last_impl_frame_time_ != 0.0 &&
+      last_layer_bounds == current_layer_bounds)
     time_delta = current_frame_time - last_impl_frame_time_;
 
-  gfx::Rect viewport_in_content_space =
-      gfx::ToEnclosingRect(gfx::ScaleRect(viewport_in_layer_space,
-                                          contents_scale_));
-
-  gfx::Size tile_size = tiling_data_.max_texture_size();
-  int64 prioritized_rect_area =
-      TilePriority::kNumTilesToCoverWithInflatedViewportRectForPrioritization *
-      tile_size.width() * tile_size.height();
-
-  gfx::Rect prioritized_rect = ExpandRectEquallyToAreaBoundedBy(
-      viewport_in_content_space,
-      prioritized_rect_area,
-      ContentRect());
-  DCHECK(ContentRect().Contains(prioritized_rect));
-
   // Iterate through all of the tiles that were live last frame but will
   // not be live this frame, and mark them as being dead.
   for (TilingData::DifferenceIterator iter(&tiling_data_,
-                                           last_prioritized_rect_,
-                                           prioritized_rect);
+                                           last_interest_rect_,
+                                           interest_rect);
        iter;
        ++iter) {
     TileMap::iterator find = tiles_.find(iter.index());
@@ -387,7 +407,7 @@ void PictureLayerTiling::UpdateTilePriorities(
     Tile* tile = find->second.get();
     tile->SetPriority(tree, priority);
   }
-  last_prioritized_rect_ = prioritized_rect;
+  last_interest_rect_ = interest_rect;
 
   gfx::Rect view_rect(device_viewport);
   float current_scale = current_layer_contents_scale / contents_scale_;
@@ -404,7 +424,7 @@ void PictureLayerTiling::UpdateTilePriorities(
         last_screen_transform.matrix().get(0, 3),
         last_screen_transform.matrix().get(1, 3));
 
-    for (TilingData::Iterator iter(&tiling_data_, prioritized_rect);
+    for (TilingData::Iterator iter(&tiling_data_, interest_rect);
          iter; ++iter) {
       TileMap::iterator find = tiles_.find(iter.index());
       if (find == tiles_.end())
@@ -437,7 +457,7 @@ void PictureLayerTiling::UpdateTilePriorities(
       tile->SetPriority(tree, priority);
     }
   } else {
-    for (TilingData::Iterator iter(&tiling_data_, prioritized_rect);
+    for (TilingData::Iterator iter(&tiling_data_, interest_rect);
          iter; ++iter) {
       TileMap::iterator find = tiles_.find(iter.index());
       if (find == tiles_.end())
@@ -485,6 +505,38 @@ void PictureLayerTiling::UpdateTilePriorities(
   last_impl_frame_time_ = current_frame_time;
 }
 
+void PictureLayerTiling::ManageTiles(const gfx::Rect& old_interest_rect,
+                                     const gfx::Rect& new_interest_rect) {
+  if (old_interest_rect == new_interest_rect)
+    return;
+
+  Region free_region(old_interest_rect);
+  free_region.Subtract(new_interest_rect);
+
+  // Iterate to delete all tiles outside of our new interest rect.
+  for (Region::Iterator iter(free_region); iter.has_rect(); iter.next()) {

[enne (OOO), 2013/03/27 16:16:27]

Both of these loops look like a job for TilingData::DifferenceIterator.

[whunt, 2013/03/27 17:42:02]

I'll give that a look.

+    for (SimpleIterator tile_iter(this, iter.rect()); tile_iter; ++tile_iter) {
+      TileMapKey key = *tile_iter;
+      TileMap::iterator found = tiles_.find(key);
+      if (found != tiles_.end() &&

[enne (OOO), 2013/03/27 16:16:27]

Did you mean || here?  I think this intersects condition would be better done in
the SetLayerBounds function (the block I suggested not removing), and then you
could just intersect last and current interest rects with the content bounds
freely.

[whunt, 2013/03/27 17:42:02]

No, I don't mean ||.  In this case we're looking for tiles that both exist and
are not in the new interest rect.

Iterating over the content bounds is going to be less efficient than iterating
over the last interest rect and require extra code in set-layer-bounds.

+          !new_interest_rect.Intersects(found->second->content_rect()))
+       tiles_.erase(found);
+     }
+  }
+
+  Region alloc_region(new_interest_rect);
+  alloc_region.Subtract(old_interest_rect);
+
+  // Iterate to allocate new tiles for all regions with newly exposed area.
+  for (Region::Iterator iter(alloc_region); iter.has_rect(); iter.next()) {
+    for (SimpleIterator tile_iter(this, iter.rect()); tile_iter; ++tile_iter) {
+      TileMapKey key = *tile_iter;
+      TileMap::iterator found = tiles_.find(key);
+      CreateTile(key.first, key.second);
+    }
+  }
+}
+
 void PictureLayerTiling::DidBecomeActive() {
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
     it->second->SetPriority(ACTIVE_TREE, it->second->priority(PENDING_TREE));