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 fb68cbb2fa43fcf1933dfafd8a21f4220781946a..53765fd1f8d9bec6944ac037caa10560bf647582 100644
--- a/cc/resources/picture_layer_tiling.cc
+++ b/cc/resources/picture_layer_tiling.cc
@@ -64,9 +64,7 @@ PictureLayerTiling::PictureLayerTiling(float contents_scale,
       resolution_(NON_IDEAL_RESOLUTION),
       client_(client),
       tiling_data_(gfx::Size(), gfx::Rect(), true),
-      last_impl_frame_time_in_seconds_(0.0),
-      eviction_tiles_cache_valid_(false),
-      eviction_cache_tree_priority_(SAME_PRIORITY_FOR_BOTH_TREES) {
+      last_impl_frame_time_in_seconds_(0.0) {
   gfx::Size content_bounds =
       gfx::ToCeiledSize(gfx::ScaleSize(layer_bounds, contents_scale));
   gfx::Size tile_size = client_->CalculateTileSize(content_bounds);
@@ -118,24 +116,18 @@ Tile* PictureLayerTiling::CreateTile(int i,
 
   // Create a new tile because our twin didn't have a valid one.
   scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
-  if (tile.get())
+  if (tile.get()) {
+    tile->set_tiling_index(i, j);
     tiles_[key] = tile;
+  }
   return tile.get();
 }
 
-void PictureLayerTiling::CreateMissingTilesInLiveTilesRect() {
-  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
-  bool include_borders = true;
-  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);
-  }
+Tile* PictureLayerTiling::GetOrCreateTileAt(int i, int j) {
+  Tile* existing_tile = TileAt(i, j);
+  if (existing_tile)
+    return existing_tile;
+  return CreateTile(i, j, client_->GetTwinTiling(this));
 }
 
 void PictureLayerTiling::SetLayerBounds(const gfx::Size& layer_bounds) {
@@ -169,17 +161,7 @@ void PictureLayerTiling::SetLayerBounds(const gfx::Size& layer_bounds) {
   Invalidate(layer_region);
 }
 
-void PictureLayerTiling::RemoveTilesInRegion(const Region& layer_region) {
-  DoInvalidate(layer_region, false /* recreate_tiles */);
-}
-
 void PictureLayerTiling::Invalidate(const Region& layer_region) {
-  DoInvalidate(layer_region, true /* recreate_tiles */);
-}
-
-void PictureLayerTiling::DoInvalidate(const Region& layer_region,
-                                      bool recreate_tiles) {
-  std::vector<TileMapKey> new_tile_keys;
   gfx::Rect expanded_live_tiles_rect(
       tiling_data_.ExpandRectToTileBoundsWithBorders(live_tiles_rect_));
   for (Region::Iterator iter(layer_region); iter.has_rect(); iter.next()) {
@@ -201,16 +183,44 @@ void PictureLayerTiling::DoInvalidate(const Region& layer_region,
       if (find == tiles_.end())
         continue;
       tiles_.erase(find);
-      if (recreate_tiles)
-        new_tile_keys.push_back(key);
     }
   }
+}
 
-  if (recreate_tiles) {
-    const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
-    for (size_t i = 0; i < new_tile_keys.size(); ++i)
-      CreateTile(new_tile_keys[i].first, new_tile_keys[i].second, twin_tiling);
+void PictureLayerTiling::UpdateTileOcclusion(Tile* tile) {
+  if (!tile)
+    return;
+
+  bool is_occluded = occlusion_set_.count(std::make_pair(
+                         tile->tiling_i_index(), tile->tiling_j_index())) != 0;
+  tile->set_is_occluded(is_occluded);
+}
+
+bool PictureLayerTiling::IsTileRequiredForActivation(Tile* tile) const {

[vmpstr, 2014/07/02 23:51:29]

I tried my best to capture what the previous thing did and what the tests are
testing here...

+  if (tile->is_occluded())
+    return false;
+
+  if (resolution_ != HIGH_RESOLUTION)
+    return false;
+
+  WhichTree tree = client_->GetTree();
+  if (tree != PENDING_TREE)
+    return false;
+
+  if (client_->RequiresHighResToDraw())
+    return true;
+
+  const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+  if (twin_tiling) {
+    if (twin_tiling->layer_bounds() != layer_bounds())
+      return true;
+
+    Tile* twin_tile =
+        twin_tiling->TileAt(tile->tiling_i_index(), tile->tiling_j_index());
+    if (!twin_tile || twin_tile == tile)
+      return false;
   }
+  return true;
 }
 
 PictureLayerTiling::CoverageIterator::CoverageIterator()
@@ -465,99 +475,90 @@ void PictureLayerTiling::UpdateTilePriorities(
   current_visible_rect_in_content_space_ = visible_rect_in_content_space;
   current_skewport_ = skewport;
   current_eventually_rect_ = eventually_rect;
-  eviction_tiles_cache_valid_ = false;
-
-  TilePriority now_priority(resolution_, TilePriority::NOW, 0);
-  float content_to_screen_scale =
-      1.0f / (contents_scale_ * layer_contents_scale);
-
-  // Assign now priority to all visible tiles.
-  bool include_borders = true;
-  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;
-    Tile* tile = find->second.get();
+  content_to_screen_scale_ = 1.0f / (contents_scale_ * layer_contents_scale);
 
-    tile->SetPriority(tree, now_priority);
+  current_soon_border_rect_ = visible_rect_in_content_space;
+  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale_;
+  current_soon_border_rect_.Inset(-border, -border, -border, -border);
 
-    // Set whether tile is occluded or not.
-    bool is_occluded = false;
-    if (occlusion_tracker) {
+  // Compute occluded indecies.

[enne (OOO), 2014/07/07 19:41:59]

I'm not buying the occlusion_set_.  Why can't UpdateTileOcclusion do this work?

[vmpstr, 2014/07/07 23:24:47]

From my understanding of occlusion, we can only really get it from
occlusion_tracker during a front to back walk of the tree. UpdateTilePriorities
happens to be in that walk. However, when we UpdateTileOcclusion, we're not in a
tree walk (and don't have an occlusion tracker for that matter).

+  occlusion_set_.clear();
+  if (occlusion_tracker) {
+    bool include_borders = true;
+    for (TilingData::Iterator iter(
+             &tiling_data_, visible_rect_in_content_space, include_borders);
+         iter;
+         ++iter) {
+      gfx::Rect paint_rect =
+          tiling_data_.TileBoundsWithBorder(iter.index_x(), iter.index_y());
+      gfx::Rect tile_rect = paint_rect;
+      tile_rect.set_size(tiling_data_.max_texture_size());
       gfx::Rect tile_query_rect = ScaleToEnclosingRect(
-          IntersectRects(tile->content_rect(), visible_rect_in_content_space),
+          IntersectRects(tile_rect, visible_rect_in_content_space),
           1.0f / contents_scale_);
-      // TODO(vmpstr): Remove render_target and draw_transform from the
-      // parameters so they can be hidden from the tiling.
-      is_occluded = occlusion_tracker->Occluded(
+      bool is_occluded = occlusion_tracker->Occluded(
           render_target, tile_query_rect, draw_transform);
+      if (!is_occluded)
+        continue;
+
+      occlusion_set_.insert(iter.index());
     }
-    tile->set_is_occluded(is_occluded);
   }
+}
 
-  // Assign soon priority to skewport tiles.
-  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;
-    Tile* tile = find->second.get();
+void PictureLayerTiling::SetTilePriority(Tile* tile, bool include_twin) const {
+  // First update the twin if required.
+  if (include_twin) {
+    const PictureLayerTiling* twin_tiling = client_->GetTwinTiling(this);
+    if (twin_tiling)
+      twin_tiling->SetTilePriority(tile, false);
+  }
 
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
+  WhichTree tree = client_->GetTree();
 
-    float distance_to_visible =
-        visible_rect_in_content_space.ManhattanInternalDistance(tile_bounds) *
-        content_to_screen_scale;
+  // If we're on the pending tree, start by clearing out the required for
+  // activation flag.
+  if (tree == PENDING_TREE)
+    tile->UnsetRequiredForActivation();
 
-    TilePriority priority(resolution_, TilePriority::SOON, distance_to_visible);
-    tile->SetPriority(tree, priority);
+  // If this tiling doesn't have the given tile, then set default priority.
+  if (TileAt(tile->tiling_i_index(), tile->tiling_j_index()) != tile) {
+    tile->SetPriority(tree, TilePriority());
+    return;
   }
 
-  // Assign eventually priority to interest rect tiles.
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, eventually_rect, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    Tile* tile = find->second.get();
+  gfx::Rect tile_bounds = tiling_data_.TileBoundsWithBorder(
+      tile->tiling_i_index(), tile->tiling_j_index());
 
-    gfx::Rect tile_bounds =
-        tiling_data_.TileBounds(iter.index_x(), iter.index_y());
+  // If we're in the now bin, set the priority and return.
+  if (current_visible_rect_in_content_space_.Intersects(tile_bounds)) {
+    tile->SetPriority(tree, TilePriority(resolution_, TilePriority::NOW, 0));
 
-    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);
+    // If necessary, mark this visible tile as required for activation.
+    if (IsTileRequiredForActivation(tile))
+      tile->MarkRequiredForActivation();

[enne (OOO), 2014/07/07 19:41:59]

Does it matter that this can mark active tree only tiles too? That seems a
little sketchy to me.

[vmpstr, 2014/07/07 23:24:47]

IsTileRequiredForActivation has a tree check that returns false if this is not a
pending tree. To pair it with line 521, I could make this more explicit? ie

if (tree == PENDING_TREE && IsTileRequiredForActivation(tile))

wdyt?

+    return;
   }
 
-  // Upgrade the priority on border tiles to be SOON.
-  current_soon_border_rect_ = visible_rect_in_content_space;
-  float border = kSoonBorderDistanceInScreenPixels / content_to_screen_scale;
-  current_soon_border_rect_.Inset(-border, -border, -border, -border);
-  for (TilingData::DifferenceIterator iter(
-           &tiling_data_, current_soon_border_rect_, skewport);
-       iter;
-       ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
-    if (find == tiles_.end())
-      continue;
-    Tile* tile = find->second.get();
-
-    TilePriority priority(resolution_,
-                          TilePriority::SOON,
-                          tile->priority(tree).distance_to_visible);
-    tile->SetPriority(tree, priority);
+  // For non-now bin, we'll need to know distance to visible.
+  float distance_to_visible =
+      current_visible_rect_in_content_space_.ManhattanInternalDistance(
+          tile_bounds) *
+      content_to_screen_scale_;
+
+  // Soon border and skewport tiles get SOON bin, otherwise they get EVENTUALLY
+  // bin.
+  if (current_soon_border_rect_.Intersects(tile_bounds) ||
+      current_skewport_.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::SetLiveTilesRect(
@@ -581,36 +582,14 @@ void PictureLayerTiling::SetLiveTilesRect(
       tiles_.erase(found);
   }
 
-  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;
 }
 
-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());
-  }
-}
-
 void PictureLayerTiling::DidBecomeActive() {
   PicturePileImpl* active_pile = client_->GetPile();
   for (TileMap::const_iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
+    it->second->UnsetRequiredForActivation();
     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
@@ -797,23 +776,18 @@ gfx::Rect PictureLayerTiling::ExpandRectEquallyToAreaBoundedBy(
 
 void PictureLayerTiling::UpdateEvictionCacheIfNeeded(
     TreePriority tree_priority) {
-  if (eviction_tiles_cache_valid_ &&
-      eviction_cache_tree_priority_ == tree_priority)
-    return;
 
   eviction_tiles_cache_.clear();
   eviction_tiles_cache_.reserve(tiles_.size());
   for (TileMap::iterator it = tiles_.begin(); it != tiles_.end(); ++it) {
-    // TODO(vmpstr): This should update the priority if UpdateTilePriorities
-    // changes not to do this.
+    UpdateTileOcclusion(it->second);

[enne (OOO), 2014/07/07 19:41:59]

I feel like there are a lot of paired UpdateTileOcclusion/SetTilePriority.  I
feel a bit like occlusion is part of priority.  Is it possible to combine these?
I know there's one case where you update tile occlusion and then decide you
don't need to raster so you don't update the tile priority, but I kind of feel
like then you have a partially stale priority, which is odd.

[vmpstr, 2014/07/07 23:24:47]

It's possible... I just feel like priority calculation thing is a pretty
expensive operation. Also, current approach amounts to:

UpdateTileOcclusion(tile)
if (TileNeedsRaster(tile))
  SetTilePriority(tile);
  return tile for raster;

I feel like if it was a part of priority then it would be something like

SetTilePriority(tile)
if (TileNeedsRaster(tile))
  return tile for raster;

However, this means that we would update priorities for tiles that are already
done as well, which definitely seems wasteful.

The alternative is

if (TileDoesntNeedsRasterBecauseItHasResources(tile))
  skip
else
  SetTilePriority(tile)
  if (TileDoesntNeedRasterBecauseOfOcclusion(tile))
    skip
return tile for raster;

which only updates priorities if the tile wasn't done yet, but it also seems
more awkward (modulo function names)?

<off topic>
Maybe in general, a tile should be something like the following:

class Tile {
 // common data
 picture pile
 use picture analysis
 managed state (tile versions)
 content rect, content scale

 // per tree data
 TreeSpecificTileData tree_data_[NUM_TREES]
};

and TreeSpecificTileData be something like

class TreeSpecificTileData {
 tile priority
 occlusion
 required for activation
};

I'm just not sure it makes sense to clump some of these values into
TilePriority, since to me priority really is just means distance and bin, the
rest is kind of non-priority per-tree settings. Wdyt? Maybe occlusion is also a
priority thing... I'm just throwing ideas out there for now.

+    SetTilePriority(it->second, true);

[enne (OOO), 2014/07/07 19:41:59]

The boolean parameter is hard to read.  I think callsites would be more readable
if you had SetTilePriority and SetTileAndTwinPriority, even if it did the same
thing underneath.

[vmpstr, 2014/07/07 23:24:46]

Done.

     eviction_tiles_cache_.push_back(it->second);
   }
 
   std::sort(eviction_tiles_cache_.begin(),
             eviction_tiles_cache_.end(),
             TileEvictionOrder(tree_priority));
-  eviction_tiles_cache_valid_ = true;
-  eviction_cache_tree_priority_ = tree_priority;
 }
 
 PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator()
@@ -844,10 +818,13 @@ PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator(
     return;
   }
 
-  current_tile_ =
-      tiling_->TileAt(visible_iterator_.index_x(), visible_iterator_.index_y());
+  current_tile_ = tiling_->GetOrCreateTileAt(visible_iterator_.index_x(),
+                                             visible_iterator_.index_y());
+  tiling_->UpdateTileOcclusion(current_tile_);

[enne (OOO), 2014/07/07 19:41:59]

Does this need to get wrapped by !current_tile_ too?

[vmpstr, 2014/07/07 23:24:47]

Whoops.

   if (!current_tile_ || !TileNeedsRaster(current_tile_))
     ++(*this);
+  else
+    tiling_->SetTilePriority(current_tile_, true);
 }
 
 PictureLayerTiling::TilingRasterTileIterator::~TilingRasterTileIterator() {}
@@ -866,8 +843,9 @@ void PictureLayerTiling::TilingRasterTileIterator::AdvancePhase() {
     }
 
     while (spiral_iterator_) {
-      current_tile_ = tiling_->TileAt(spiral_iterator_.index_x(),
-                                      spiral_iterator_.index_y());
+      current_tile_ = tiling_->GetOrCreateTileAt(spiral_iterator_.index_x(),
+                                                 spiral_iterator_.index_y());
+      tiling_->UpdateTileOcclusion(current_tile_);
       if (current_tile_ && TileNeedsRaster(current_tile_))
         break;
       ++spiral_iterator_;
@@ -878,6 +856,9 @@ void PictureLayerTiling::TilingRasterTileIterator::AdvancePhase() {
       break;
     }
   } while (!spiral_iterator_);
+
+  if (current_tile_)
+    tiling_->SetTilePriority(current_tile_, true);
 }
 
 PictureLayerTiling::TilingRasterTileIterator&
@@ -924,8 +905,13 @@ operator++() {
         next_index = spiral_iterator_.index();
         break;
     }
-    current_tile_ = tiling_->TileAt(next_index.first, next_index.second);
+    current_tile_ =
+        tiling_->GetOrCreateTileAt(next_index.first, next_index.second);
+    tiling_->UpdateTileOcclusion(current_tile_);
   }
+
+  if (current_tile_)
+    tiling_->SetTilePriority(current_tile_, true);
   return *this;
 }