cc: Start using raster/eviction iterators.

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 ee17dc8f9b10810e297a2aef47ac414eceb56890..d8a2b8ad2406562fb8273838b9f4987f1eb39e59 100644
--- a/cc/resources/picture_layer_tiling.cc
+++ b/cc/resources/picture_layer_tiling.cc
@@ -54,13 +54,6 @@ class TileEvictionOrder {
   TreePriority tree_priority_;
 };
 
-void ReleaseTile(Tile* tile, WhichTree tree) {
-  // Reset priority as tile is ref-counted and might still be used
-  // even though we no longer hold a reference to it here anymore.
-  tile->SetPriority(tree, TilePriority());
-  tile->set_shared(false);
-}
-
 }  // namespace
 
 scoped_ptr<PictureLayerTiling> PictureLayerTiling::Create(
@@ -81,6 +74,8 @@ PictureLayerTiling::PictureLayerTiling(float contents_scale,
       client_(client),
       tiling_data_(gfx::Size(), gfx::Size(), true),
       last_impl_frame_time_in_seconds_(0.0),
+      content_to_screen_scale_(1.0f),

[danakj, 2014/10/08 18:46:25]

can this be 0 to indicate it is set before we use it?

[vmpstr, 2014/10/08 22:38:34]

Done.

+      can_require_tiles_for_activation_(true),

[danakj, 2014/10/08 18:46:26]

can this be false? we should set it before we use it right?

[vmpstr, 2014/10/08 22:38:34]

Done.

       has_visible_rect_tiles_(false),
       has_skewport_rect_tiles_(false),
       has_soon_border_rect_tiles_(false),
@@ -107,7 +102,7 @@ PictureLayerTiling::PictureLayerTiling(float contents_scale,
 
 PictureLayerTiling::~PictureLayerTiling() {
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it)
-    ReleaseTile(it->second.get(), client_->GetTree());
+    it->second->set_shared(false);
 }
 
 void PictureLayerTiling::SetClient(PictureLayerTilingClient* client) {
@@ -133,6 +128,8 @@ Tile* PictureLayerTiling::CreateTile(int i,
           gfx::ScaleToEnclosingRect(paint_rect, 1.0f / contents_scale_);
       if (!client_->GetInvalidation()->Intersects(rect)) {
         DCHECK(!candidate_tile->is_shared());
+        DCHECK_EQ(i, candidate_tile->tiling_i_index());
+        DCHECK_EQ(j, candidate_tile->tiling_j_index());
         candidate_tile->set_shared(true);
         tiles_[key] = candidate_tile;
         return candidate_tile;
@@ -144,6 +141,7 @@ Tile* PictureLayerTiling::CreateTile(int i,
   scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
   if (tile.get()) {
     DCHECK(!tile->is_shared());
+    tile->set_tiling_index(i, j);
     tiles_[key] = tile;
   }
   return tile.get();
@@ -466,7 +464,7 @@ bool PictureLayerTiling::RemoveTileAt(int i,
   TileMap::iterator found = tiles_.find(TileMapKey(i, j));
   if (found == tiles_.end())
     return false;
-  ReleaseTile(found->second.get(), client_->GetTree());
+  found->second->set_shared(false);
   tiles_.erase(found);
   if (recycled_twin) {
     // Recycled twin does not also have a recycled twin, so pass NULL.
@@ -479,7 +477,7 @@ void PictureLayerTiling::Reset() {
   live_tiles_rect_ = gfx::Rect();
   PictureLayerTiling* recycled_twin = client_->GetRecycledTwinTiling(this);
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    ReleaseTile(it->second.get(), client_->GetTree());
+    it->second->set_shared(false);
     if (recycled_twin)
       recycled_twin->RemoveTileAt(it->first.first, it->first.second, NULL);
   }
@@ -534,7 +532,7 @@ gfx::Rect PictureLayerTiling::ComputeSkewport(
   return skewport;
 }
 
-void PictureLayerTiling::UpdateTilePriorities(
+void PictureLayerTiling::ComputeTilePriorityRects(
     WhichTree tree,
     const gfx::Rect& viewport_in_layer_space,
     float ideal_contents_scale,
@@ -555,16 +553,18 @@ void PictureLayerTiling::UpdateTilePriorities(
     return;
   }
 
+  // Calculate the skewport.
+  gfx::Rect skewport = ComputeSkewport(current_frame_time_in_seconds,
+                                       visible_rect_in_content_space);
+  DCHECK(skewport.Contains(visible_rect_in_content_space));
+
+  // Calculate the eventually/live tiles rect.
   size_t max_tiles_for_interest_area = client_->GetMaxTilesForInterestArea();
 
   gfx::Size tile_size = tiling_data_.max_texture_size();
   int64 eventually_rect_area =
       max_tiles_for_interest_area * tile_size.width() * tile_size.height();
 
-  gfx::Rect skewport = ComputeSkewport(current_frame_time_in_seconds,
-                                       visible_rect_in_content_space);
-  DCHECK(skewport.Contains(visible_rect_in_content_space));
-
   gfx::Rect eventually_rect =
       ExpandRectEquallyToAreaBoundedBy(visible_rect_in_content_space,
                                        eventually_rect_area,
@@ -576,6 +576,13 @@ void PictureLayerTiling::UpdateTilePriorities(
       << "tiling_size: " << tiling_size().ToString()
       << " eventually_rect: " << eventually_rect.ToString();
 
+  // Calculate the soon border rect.
+  content_to_screen_scale_ = ideal_contents_scale / contents_scale_;
+  gfx::Rect soon_border_rect = visible_rect_in_content_space;
+  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale_;
+  soon_border_rect.Inset(-border, -border, -border, -border);
+
+  // Update the tiling state.
   SetLiveTilesRect(eventually_rect);
 
   last_impl_frame_time_in_seconds_ = current_frame_time_in_seconds;
@@ -583,104 +590,20 @@ void PictureLayerTiling::UpdateTilePriorities(
 
   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 = 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();
-
-    tile->SetPriority(tree, now_priority);
-
-    // Set whether tile is occluded or not.
-    gfx::Rect tile_query_rect = ScaleToEnclosingRect(
-        IntersectRects(tile->content_rect(), visible_rect_in_content_space),
-        1.0f / contents_scale_);
-    bool is_occluded = occlusion_in_layer_space.IsOccluded(tile_query_rect);
-    tile->set_is_occluded(tree, is_occluded);
-  }
-
-  // Assign soon priority to skewport tiles.
-  has_skewport_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, skewport, visible_rect_in_content_space);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_skewport_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
-
-    float distance_to_visible =
-        visible_rect_in_content_space.ManhattanInternalDistance(tile_bounds) *
-        content_to_screen_scale;
-
-    TilePriority priority(resolution_, TilePriority::SOON, distance_to_visible);
-    tile->SetPriority(tree, priority);
-  }
-
-  // Assign eventually priority to interest rect tiles.
-  has_eventually_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, eventually_rect, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_eventually_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
-
-    float distance_to_visible =
-        visible_rect_in_content_space.ManhattanInternalDistance(tile_bounds) *
-        content_to_screen_scale;
-    TilePriority priority(
-        resolution_, TilePriority::EVENTUALLY, distance_to_visible);
-    tile->SetPriority(tree, priority);
-  }
-
-  // Upgrade the priority on border tiles to be SOON.
-  gfx::Rect soon_border_rect = visible_rect_in_content_space;
-  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale;
-  soon_border_rect.Inset(-border, -border, -border, -border);
-  has_soon_border_rect_tiles_ = false;
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, soon_border_rect, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    has_soon_border_rect_tiles_ = true;
-    Tile* tile = find->second.get();
-
-    TilePriority priority(resolution_,
-                          TilePriority::SOON,
-                          tile->priority(tree).distance_to_visible);
-    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;
+  current_occlusion_in_layer_space_ = occlusion_in_layer_space;
+
+  // Update has_*_tiles state.
+  gfx::Rect tiling_rect(tiling_size());
+
+  has_visible_rect_tiles_ = tiling_rect.Intersects(current_visible_rect_);
+  has_skewport_rect_tiles_ = tiling_rect.Intersects(current_skewport_rect_);
+  has_soon_border_rect_tiles_ =
+      tiling_rect.Intersects(current_soon_border_rect_);
+  has_eventually_rect_tiles_ = tiling_rect.Intersects(current_eventually_rect_);
 }
 
 void PictureLayerTiling::SetLiveTilesRect(
@@ -741,29 +664,119 @@ void PictureLayerTiling::VerifyLiveTilesRect() {
 #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());
+bool PictureLayerTiling::IsTileOccluded(const Tile* tile) const {
+  DCHECK(tile);
+  // TODO(vmpstr): Skip this work if occlusion is empty (this needs a bool

[danakj, 2014/10/08 18:46:25]

This bool function exists now right?

[vmpstr, 2014/10/08 22:38:34]

Done.

+  // function on occlusion).
+  gfx::Rect tile_query_rect =
+      IntersectRects(tile->content_rect(), current_visible_rect_);

[danakj, 2014/10/08 18:46:25]

nit: gfx::IntersectRects

[vmpstr, 2014/10/08 22:38:34]

Done.

+
+  if (contents_scale_ != 1.f) {
+    tile_query_rect =
+        ScaleToEnclosingRect(tile_query_rect, 1.0f / contents_scale_);

[danakj, 2014/10/08 18:46:26]

nit: gfx::ScaleToE..

[vmpstr, 2014/10/08 22:38:34]

Done.

   }
+
+  // Explicitly check if the tile is outside the viewport. If so, we need to
+  // return false, since occlusion for this tile is unknown.
+  // TODO(vmpstr): Since the current visible rect is really a viewport in
+  // layer space, we should probably clip tile query rect to tiling bounds
+  // or live tiles rect.
+  if (tile_query_rect.IsEmpty())

[danakj, 2014/10/08 18:46:25]

nit: do this before scaling, scaling shouldn't make it empty, and we don't need
to scale if it is empty

[vmpstr, 2014/10/08 22:38:34]

Done.

+    return false;
+  return current_occlusion_in_layer_space_.IsOccluded(tile_query_rect);
 }
 
-void PictureLayerTiling::DidBecomeActive() {
-  PicturePileImpl* active_pile = client_->GetPile();
-  for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    it->second->SetPriority(ACTIVE_TREE, it->second->priority(PENDING_TREE));
-    it->second->SetPriority(PENDING_TREE, TilePriority());
-
-    // Tile holds a ref onto a picture pile. If the tile never gets invalidated
-    // and recreated, then that picture pile ref could exist indefinitely.  To
-    // prevent this, ask the client to update the pile to its own ref.  This
-    // will cause PicturePileImpls to get deleted once the corresponding
-    // PictureLayerImpl and any in flight raster jobs go out of scope.
-    it->second->set_picture_pile(active_pile);
+bool PictureLayerTiling::IsTileRequiredForActivation(const Tile* tile) const {

[danakj, 2014/10/08 18:46:25]

can you add a comment somewhere here saying this is only called for visible
tiles? (or a dcheck for bonus points if that's easy to do)

[vmpstr, 2014/10/08 22:38:34]

Done.

+  DCHECK_EQ(PENDING_TREE, client_->GetTree());
+
+  // If we are not allowed to mark tiles as required for activation, then don't
+  // do it.
+  if (!can_require_tiles_for_activation_)
+    return false;
+
+  if (resolution_ != HIGH_RESOLUTION)
+    return false;
+
+  if (IsTileOccluded(tile))
+    return false;
+
+  if (client_->RequiresHighResToDraw())
+    return true;
+
+  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+  if (!twin_tiling)
+    return true;
+
+  if (twin_tiling->layer_bounds() != layer_bounds())
+    return true;
+
+  if (twin_tiling->current_visible_rect_ != current_visible_rect_)
+    return true;
+
+  Tile* twin_tile =
+      twin_tiling->TileAt(tile->tiling_i_index(), tile->tiling_j_index());
+  // If twin tile is missing, it might not have a recording, so we don't need
+  // this tile to be required for activation.
+  if (!twin_tile)
+    return false;
+
+  return true;
+}
+
+void PictureLayerTiling::UpdateTileAndTwinPriority(Tile* tile) const {
+  UpdateTilePriority(tile);
+
+  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+  if (!tile->is_shared() || !twin_tiling) {
+    WhichTree tree = client_->GetTree();
+    WhichTree twin_tree = tree == ACTIVE_TREE ? PENDING_TREE : ACTIVE_TREE;
+    tile->SetPriority(twin_tree, TilePriority());
+    tile->set_is_occluded(twin_tree, false);
+    if (twin_tree == PENDING_TREE)

[danakj, 2014/10/08 18:46:25]

nit: it's probly cheaper to just set_required all the time than to branch, and
it's harmless to do so right?

[vmpstr, 2014/10/08 22:38:34]

Not really harmless in general, since we have to ensure that required for
activation property is only touched from the pending tree... The active tree
might make a wrong decision and since this is a bool on tile, it would override
the previous value that the pending tree has set. As a follow-up when I clean up
tiles, I think I might make this a per tree setting on tile, since then the code
here is easier and we just have to ensure to read off the right thing when we
need it.

+      tile->set_required_for_activation(false);
+    return;
   }
+
+  twin_tiling->UpdateTilePriority(tile);
+}
+
+void PictureLayerTiling::UpdateTilePriority(Tile* tile) const {
+  // TODO(vmpstr): This code should return the priority instead of setting it on
+  // the tile. This should be a part of the change to move tile priority from
+  // tiles into iterators.
+  WhichTree tree = client_->GetTree();
+
+  if (tree == PENDING_TREE)

[danakj, 2014/10/08 18:46:26]

same here, can just set false always?

[vmpstr, 2014/10/08 22:38:34]

We have ensure that we don't set that from the active tree

Same as above.

+    tile->set_required_for_activation(false);
+  tile->set_is_occluded(tree, false);

[danakj, 2014/10/08 18:46:26]

move these below the early out so we don't set these values twice?

[vmpstr, 2014/10/08 22:38:34]

Done.

+
+  DCHECK_EQ(TileAt(tile->tiling_i_index(), tile->tiling_j_index()), tile);
+  gfx::Rect tile_bounds =
+      tiling_data_.TileBounds(tile->tiling_i_index(), tile->tiling_j_index());
+
+  if (current_visible_rect_.Intersects(tile_bounds)) {
+    tile->SetPriority(tree, TilePriority(resolution_, TilePriority::NOW, 0));
+    if (tree == PENDING_TREE)
+      tile->set_required_for_activation(IsTileRequiredForActivation(tile));
+    tile->set_is_occluded(tree, IsTileOccluded(tile));
+    return;
+  }
+
+  float distance_to_visible =
+      current_visible_rect_.ManhattanInternalDistance(tile_bounds) *
+      content_to_screen_scale_;
+
+  if (current_soon_border_rect_.Intersects(tile_bounds) ||
+      current_skewport_rect_.Intersects(tile_bounds)) {
+    tile->SetPriority(
+        tree,
+        TilePriority(resolution_, TilePriority::SOON, distance_to_visible));
+    return;
+  }
+
+  tile->SetPriority(
+      tree,
+      TilePriority(resolution_, TilePriority::EVENTUALLY, distance_to_visible));
 }
 
 void PictureLayerTiling::GetAllTilesForTracing(
@@ -954,9 +967,8 @@ void PictureLayerTiling::UpdateEvictionCacheIfNeeded(
   eviction_tiles_eventually_and_required_for_activation_.clear();
 
   for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    // TODO(vmpstr): This should update the priority if UpdateTilePriorities
-    // changes not to do this.
     Tile* tile = it->second.get();
+    UpdateTileAndTwinPriority(tile);
     const TilePriority& priority =
         tile->priority_for_tree_priority(tree_priority);
     switch (priority.priority_bin) {
@@ -1031,9 +1043,8 @@ PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator()
     : tiling_(NULL), current_tile_(NULL) {}
 
 PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator(
-    PictureLayerTiling* tiling,
-    WhichTree tree)
-    : tiling_(tiling), phase_(VISIBLE_RECT), tree_(tree), current_tile_(NULL) {
+    PictureLayerTiling* tiling)
+    : tiling_(tiling), phase_(VISIBLE_RECT), current_tile_(NULL) {
   if (!tiling_->has_visible_rect_tiles_) {
     AdvancePhase();
     return;
@@ -1049,8 +1060,11 @@ PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator(
 
   current_tile_ =
       tiling_->TileAt(visible_iterator_.index_x(), visible_iterator_.index_y());
-  if (!current_tile_ || !TileNeedsRaster(current_tile_))
+  if (!current_tile_ || !TileNeedsRaster(current_tile_)) {
     ++(*this);
+    return;
+  }
+  tiling_->UpdateTileAndTwinPriority(current_tile_);

[danakj, 2014/10/08 18:46:26]

while iterating you always update the priority of the tile before returning, but
here you don't if the first tile doesn't need raster. should this be consistent?
could this cause bugs?

[vmpstr, 2014/10/08 22:38:34]

The priority is only updated for returned values. That's true of the rest of the
code here as well. For instance, operator++ has something along the lines of

while (!current_tile_ || !TileNeedsRaster(current_tile_)) {
 ...
}

if (current_tile_)
  tiling_->UpdateTileAndTwinPriority(current_tile_);

[danakj, 2014/10/08 23:38:01]

Right, but if the first tile in the iterator didn't need raster, wouldn't we
have a current_tile_ (ie operator* would have a tile) but it's priorities
wouldn't be updated?

[danakj, 2014/10/08 23:39:27]

Or would operator bool be false? I'm not sure why it would?

[vmpstr, 2014/10/09 00:01:42]

operator* can only return a tile that needs raster for this iterator.. That is,
we skip all tiles that don't need raster silently. If we run out of tiles,
current_tile_ becomes NULL, which is what operator bool checks. 

So basically, we can't have a situation where we would return a tile that
doesn't have its priorities updated and at the same time we can't return a tile
that doesn't need raster. All other cases are consistent in that we don't update
the tile priorities... At least that's the design.

[danakj, 2014/10/09 00:03:26]

Oh, ok ++ above would end up with a NULL tile in that case then I guess. OK
thanks!

 }
 
 PictureLayerTiling::TilingRasterTileIterator::~TilingRasterTileIterator() {}
@@ -1111,6 +1125,9 @@ void PictureLayerTiling::TilingRasterTileIterator::AdvancePhase() {
       break;
     }
   } while (!spiral_iterator_);
+
+  if (current_tile_)
+    tiling_->UpdateTileAndTwinPriority(current_tile_);
 }
 
 PictureLayerTiling::TilingRasterTileIterator&
@@ -1148,6 +1165,9 @@ operator++() {
     }
     current_tile_ = tiling_->TileAt(next_index.first, next_index.second);
   }
+
+  if (current_tile_)
+    tiling_->UpdateTileAndTwinPriority(current_tile_);
   return *this;
 }