cc: Use reverse spiral iterator in tiling eviction.

cc/resources/picture_layer_tiling.h

Index: cc/resources/picture_layer_tiling.h
diff --git a/cc/resources/picture_layer_tiling.h b/cc/resources/picture_layer_tiling.h
index 17da548c916ae4e8e9903642f953a8f70aa5335d..c55cd5a6e1691318fd555e0f91bb85f6dcc32f07 100644
--- a/cc/resources/picture_layer_tiling.h
+++ b/cc/resources/picture_layer_tiling.h
@@ -64,9 +64,7 @@ class CC_EXPORT PictureLayerTiling {
 
   enum EvictionCategory {
     EVENTUALLY,
-    EVENTUALLY_AND_REQUIRED_FOR_ACTIVATION,
     SOON,
-    SOON_AND_REQUIRED_FOR_ACTIVATION,
     NOW,
     NOW_AND_REQUIRED_FOR_ACTIVATION
   };
@@ -122,7 +120,6 @@ class CC_EXPORT PictureLayerTiling {
    public:
     TilingEvictionTileIterator();
     TilingEvictionTileIterator(PictureLayerTiling* tiling,
-                               TreePriority tree_priority,
                                EvictionCategory category);
     ~TilingEvictionTileIterator();
 
@@ -132,8 +129,22 @@ class CC_EXPORT PictureLayerTiling {
     TilingEvictionTileIterator& operator++();
 
    private:
-    const std::vector<Tile*>* eviction_tiles_;
-    size_t current_eviction_tiles_index_;
+    void AdvanceEventually(bool first_run);
+    void AdvanceSoon(bool first_run);
+    void AdvanceNow(bool first_run);

[reveman, 2014/11/11 00:13:27]

If I understand the code correctly, which one of these functions that we use
depends on on the value of eviction_category_ and we would never use two. I
found that a bit confusing. I expected each one to be used over the lifetime of
an iterator when I first looked at this code.

If these different categories are so significantly different that they need
different functions to advance, should they be implemented as different
iterators?

[vmpstr, 2014/11/11 22:02:53]

Correct.
This is basically to avoid having a vector in the layer level iterators. For
raster, we have a similar problem that that we solve by persisting the HIGH and
LOW res iterator and picking one or the other depending on the next tile
returned. The problem with eviction is that we can have any number of these
tilings so in order to persist iterators, we'd need a vector. 

The current solution is to "instantiate" the iterator with what eviction
category we need. This worked fine in the approach where we sort vectors,
because it simply meant we pick one of these vectors based on the category.
However, with the spiral approach we don't have a vector to pick, we have to
iterate the category that's given to us (which happens to have slightly
different iteration requirements, hence the several functions).

We can have different iterators here, but that would complicate the layer level
iterator, unless we put a virtual base here. However, if we put a virtual base
with a ::Create call that takes a category, we'll have to create these iterators
on the heap. I think that's a more correct direction to go, to be honest,
because it allows us to be a lot more flexible with having different types of
these iterators (for ganesh, for testing, etc), and maybe call them something
other than iterators (queues?). 

That's something that I feel we should consider separately and measure
performance impact (I imagine that sort of change would have to be made for the
raster iterator as well). Wdyt?

+
+    PictureLayerTiling* tiling_;
+    EvictionCategory eviction_category_;
+
+    bool processing_occluded_now_tiles_;
+    bool processing_soon_border_rect_;
+    std::vector<Tile*> unoccluded_now_tiles_;

[reveman, 2014/11/11 00:13:27]

can you do two passes instead of having this vector?

[vmpstr, 2014/11/11 22:54:51]

Done.

+    size_t unoccluded_now_tiles_index_;
+
+    Tile* current_tile_;
+
+    TilingData::Iterator visible_iterator_;
+    TilingData::ReverseSpiralDifferenceIterator spiral_iterator_;
   };
 
   ~PictureLayerTiling();
@@ -320,10 +331,6 @@ class CC_EXPORT PictureLayerTiling {
                             const gfx::Rect& visible_rect_in_content_space)
       const;
 
-  void UpdateEvictionCacheIfNeeded(TreePriority tree_priority);
-  const std::vector<Tile*>* GetEvictionTiles(TreePriority tree_priority,
-                                             EvictionCategory category);
-
   void Invalidate(const Region& layer_region);
 
   void DoInvalidate(const Region& layer_region,
@@ -364,19 +371,6 @@ class CC_EXPORT PictureLayerTiling {
 
   Occlusion current_occlusion_in_layer_space_;
 
-  // TODO(reveman): Remove this in favour of an array of eviction_tiles_ when we
-  // change all enums to have a consistent way of getting the count/last
-  // element.
-  std::vector<Tile*> eviction_tiles_now_;
-  std::vector<Tile*> eviction_tiles_now_and_required_for_activation_;
-  std::vector<Tile*> eviction_tiles_soon_;
-  std::vector<Tile*> eviction_tiles_soon_and_required_for_activation_;
-  std::vector<Tile*> eviction_tiles_eventually_;
-  std::vector<Tile*> eviction_tiles_eventually_and_required_for_activation_;
-
-  bool eviction_tiles_cache_valid_;
-  TreePriority eviction_cache_tree_priority_;
-
  private:
   DISALLOW_ASSIGN(PictureLayerTiling);