cc: Add tiling raster tile 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 939e0b163d4a459aa50d0437a9aec5b94c2f8230..6916e29ed2ef1f69223e50d366e80b8c64c82aba 100644
--- a/cc/resources/picture_layer_tiling.cc
+++ b/cc/resources/picture_layer_tiling.cc
@@ -10,6 +10,8 @@
 
 #include "base/debug/trace_event.h"
 #include "cc/base/math_util.h"
+#include "cc/resources/tile.h"
+#include "cc/resources/tile_priority.h"
 #include "ui/gfx/point_conversions.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/safe_integer_conversions.h"
@@ -64,16 +66,9 @@ gfx::SizeF PictureLayerTiling::ContentSizeF() const {
   return gfx::ScaleSize(layer_bounds_, contents_scale_);
 }
 
-Tile* PictureLayerTiling::TileAt(int i, int j) const {
-  TileMap::const_iterator iter = tiles_.find(TileMapKey(i, j));
-  if (iter == tiles_.end())
-    return NULL;
-  return iter->second.get();
-}
-
-void PictureLayerTiling::CreateTile(int i,
-                                    int j,
-                                    const PictureLayerTiling* twin_tiling) {
+Tile* PictureLayerTiling::CreateTile(int i,
+                                     int j,
+                                     const PictureLayerTiling* twin_tiling) {
   TileMapKey key(i, j);
   DCHECK(tiles_.find(key) == tiles_.end());
 
@@ -90,7 +85,7 @@ void PictureLayerTiling::CreateTile(int i,
           gfx::ScaleToEnclosingRect(paint_rect, 1.0f / contents_scale_);
       if (!client_->GetInvalidation()->Intersects(rect)) {
         tiles_[key] = candidate_tile;
-        return;
+        return candidate_tile;
       }
     }
   }
@@ -99,6 +94,7 @@ void PictureLayerTiling::CreateTile(int i,
   scoped_refptr<Tile> tile = client_->CreateTile(this, tile_rect);
   if (tile.get())
     tiles_[key] = tile;
+  return tile.get();
 }
 
 Region PictureLayerTiling::OpaqueRegionInContentRect(
@@ -430,12 +426,19 @@ void PictureLayerTiling::UpdateTilePriorities(
   last_impl_frame_time_in_seconds_ = current_frame_time_in_seconds;
   last_visible_rect_in_content_space_ = visible_rect_in_content_space;
 
-  // Assign now priority to all visible tiles.
+  current_skewport_ = skewport;
+  current_eventually_rect_ = eventually_rect;
+  current_visible_rect_in_content_space_ = visible_rect_in_content_space;
+
   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.
   for (TilingData::Iterator iter(&tiling_data_, visible_rect_in_content_space);
        iter;
        ++iter) {
-    TileMap::iterator find = tiles_.find(iter.index());
+    TileMap::const_iterator find = tiles_.find(iter.index());

[enne (OOO), 2014/03/06 02:30:45]

I realize that technically the iterator is const because you're not modifying
the map, you're modifying something the map points to.  It's like using const on
a function that modifies something your object points to.  Sure, it's
technically right, but I think it's a confusing communication to other
developers.

Does const buy you something here?  If not, I preferred the previous code.

[vmpstr, 2014/03/07 18:22:23]

Not really, I just prefer const_iterator when it can be const_iterator to
indicate that the map itself will not mutate. I reverted back to non-const.

     if (find == tiles_.end())
       continue;
     Tile* tile = find->second.get();
@@ -443,14 +446,12 @@ void PictureLayerTiling::UpdateTilePriorities(
     tile->SetPriority(tree, now_priority);
   }
 
-  // Assign soon priority to all tiles in the skewport that are not visible.
-  float content_to_screen_scale =
-      1.0f / (contents_scale_ * layer_contents_scale);
+  // 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());
+    TileMap::const_iterator find = tiles_.find(iter.index());
     if (find == tiles_.end())
       continue;
     Tile* tile = find->second.get();
@@ -466,13 +467,12 @@ void PictureLayerTiling::UpdateTilePriorities(
     tile->SetPriority(tree, priority);
   }
 
-  // Assign eventually priority to all tiles in the eventually rect that are not
-  // in the skewport.
+  // 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());
+    TileMap::const_iterator find = tiles_.find(iter.index());
     if (find == tiles_.end())
       continue;
     Tile* tile = find->second.get();
@@ -556,6 +556,21 @@ void PictureLayerTiling::UpdateTilesToCurrentPile() {
   }
 }
 
+size_t PictureLayerTiling::RequiredGPUMemoryInBytes() const {

[epenner, 2014/03/06 10:21:40]

Could this be replaced with math? If not, or this isn't a significant cost,
maybe just comment why it isn't just math at the moment, for posterity.

[vmpstr, 2014/03/07 18:22:23]

Do you mean something like visible_tiles_count * some_gpu_usage? If so, I don't
think it can. tile->GPUMemoryUsageInBytes is not a constant expression and it
depends on how many tile versions have resources.

If we decide that RequiredGPUMemoryInBytes should just return
visible_tiles_count * one_tile_size, then it can change.


I've actually removed this function for now, since it's not being used
currently.

+  size_t amount = 0;
+  for (TilingData::Iterator iter(&tiling_data_,
+                                 last_visible_rect_in_content_space_);
+       iter;
+       ++iter) {
+    TileMap::const_iterator find = tiles_.find(iter.index());
+    if (find == tiles_.end())
+      continue;
+    Tile* tile = find->second.get();
+    amount += tile->GPUMemoryUsageInBytes();
+  }
+  return amount;
+}
+
 scoped_ptr<base::Value> PictureLayerTiling::AsValue() const {
   scoped_ptr<base::DictionaryValue> state(new base::DictionaryValue());
   state->SetInteger("num_tiles", tiles_.size());
@@ -723,4 +738,94 @@ gfx::Rect PictureLayerTiling::ExpandRectEquallyToAreaBoundedBy(
   return result;
 }
 
+PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator()
+    : tiling_(NULL), current_tile_(NULL) {}
+
+PictureLayerTiling::TilingRasterTileIterator::TilingRasterTileIterator(
+    PictureLayerTiling* tiling,
+    WhichTree tree)
+    : tiling_(tiling),
+      type_(VISIBLE),
+      tree_(tree),
+      current_tile_(NULL),
+      visible_iterator_(&tiling->tiling_data_,
+                        tiling->current_visible_rect_in_content_space_),
+      spiral_iterator_(&tiling->tiling_data_,
+                       tiling->current_skewport_,
+                       tiling->current_visible_rect_in_content_space_,
+                       tiling->current_visible_rect_in_content_space_) {
+  if (!visible_iterator_) {
+    AdvancePhase();
+    return;
+  }
+
+  current_tile_ =
+      tiling_->TileAt(visible_iterator_.index_x(), visible_iterator_.index_y());
+  if (!current_tile_ || !TileNeedsRaster(current_tile_))
+    ++(*this);
+}
+
+PictureLayerTiling::TilingRasterTileIterator::~TilingRasterTileIterator() {}
+
+void PictureLayerTiling::TilingRasterTileIterator::AdvancePhase() {
+  DCHECK_LT(type_, EVENTUALLY);
+
+  do {
+    type_ = static_cast<Type>(type_ + 1);
+    if (type_ == EVENTUALLY) {
+      spiral_iterator_ = TilingData::SpiralDifferenceIterator(
+          &tiling_->tiling_data_,
+          tiling_->current_eventually_rect_,
+          tiling_->current_skewport_,
+          tiling_->current_visible_rect_in_content_space_);
+    }
+
+    while (spiral_iterator_) {

[epenner, 2014/03/06 10:21:40]

This is just for advancing past rastered tiles right? It feels sort of awkward
to be done in several places. Feel free to ignore this suggestion if it doesn't
end up helping, but would it work to have an Advance() function that just
advances the iterator one step (like operator++ would do were it not for
skipping some tiles), and then call that Advance() in a loop in operator++ until
TileNeedsRaster(current_tile)?

[vmpstr, 2014/03/07 18:22:23]

So I was just going to call ++(*this); here in a similar way that Advance would
work. The problem is that any form of advance or ++, should call AdvancePhase
when it's at the end of the current iterator, and I don't really want this to be
recursive. I mean it shouldn't hurt anything, but it's harder to reason about
it.

+      current_tile_ = tiling_->TileAt(spiral_iterator_.index_x(),
+                                      spiral_iterator_.index_y());
+      if (current_tile_ && TileNeedsRaster(current_tile_))
+        break;
+      ++spiral_iterator_;
+    }
+
+    if (!spiral_iterator_ && type_ == EVENTUALLY)
+      break;
+  } while (!spiral_iterator_);
+}
+
+PictureLayerTiling::TilingRasterTileIterator&
+PictureLayerTiling::TilingRasterTileIterator::
+operator++() {
+  current_tile_ = NULL;
+  while (!current_tile_ || !TileNeedsRaster(current_tile_)) {
+    std::pair<int, int> next_index;
+    switch (type_) {
+      case VISIBLE:
+        ++visible_iterator_;
+        if (!visible_iterator_) {
+          AdvancePhase();
+          return *this;
+        }
+        next_index = visible_iterator_.index();
+        break;
+      case SKEWPORT:
+        ++spiral_iterator_;
+        if (!spiral_iterator_) {
+          AdvancePhase();
+          return *this;
+        }
+        next_index = spiral_iterator_.index();
+        break;
+      case EVENTUALLY:
+        ++spiral_iterator_;
+        if (!spiral_iterator_)
+          return *this;
+        next_index = spiral_iterator_.index();
+        break;
+    }
+    current_tile_ = tiling_->TileAt(next_index.first, next_index.second);
+  }
+  return *this;
+}
+
 }  // namespace cc