cc: Use reverse spiral iterator in tiling eviction.

cc/resources/picture_layer_tiling.h

 // Copyright 2012 The Chromium Authors. All rights reserved.
 // Use of this source code is governed by a BSD-style license that can be
 // found in the LICENSE file.
 
 #ifndef CC_RESOURCES_PICTURE_LAYER_TILING_H_
 #define CC_RESOURCES_PICTURE_LAYER_TILING_H_
 
 #include <set>
 #include <utility>
 #include <vector>
@@ skipping 46 matching lines @@
  protected:
   virtual ~PictureLayerTilingClient() {}
 };
 
 class CC_EXPORT PictureLayerTiling {
  public:
   static const int kBorderTexels = 1;
 
   enum EvictionCategory {
     EVENTUALLY,
-    EVENTUALLY_AND_REQUIRED_FOR_ACTIVATION,
     SOON,
-    SOON_AND_REQUIRED_FOR_ACTIVATION,
     NOW,
     NOW_AND_REQUIRED_FOR_ACTIVATION
   };
 
   class CC_EXPORT TilingRasterTileIterator {
    public:
     TilingRasterTileIterator();
     explicit TilingRasterTileIterator(PictureLayerTiling* tiling);
     ~TilingRasterTileIterator();
 
@@ skipping 35 matching lines @@
 
     Tile* current_tile_;
     TilingData::Iterator visible_iterator_;
     TilingData::SpiralDifferenceIterator spiral_iterator_;
   };
 
   class CC_EXPORT TilingEvictionTileIterator {
    public:
     TilingEvictionTileIterator();
     TilingEvictionTileIterator(PictureLayerTiling* tiling,
-                               TreePriority tree_priority,
                                EvictionCategory category);
     ~TilingEvictionTileIterator();
 
     operator bool() const;
     const Tile* operator*() const;
     Tile* operator*();
     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();
 
   // Create a tiling with no tiles.  CreateTiles must be called to add some.
   static scoped_ptr<PictureLayerTiling> Create(
       float contents_scale,
       const gfx::Size& layer_bounds,
       PictureLayerTilingClient* client);
   gfx::Size layer_bounds() const { return layer_bounds_; }
@@ skipping 166 matching lines @@
   bool RemoveTileAt(int i, int j, PictureLayerTiling* recycled_twin);
 
   // Computes a skewport. The calculation extrapolates the last visible
   // rect and the current visible rect to expand the skewport to where it
   // would be in |skewport_target_time| seconds. Note that the skewport
   // is guaranteed to contain the current visible rect.
   gfx::Rect ComputeSkewport(double current_frame_time_in_seconds,
                             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,
                     bool recreate_invalidated_tiles);
 
   void UpdateTileAndTwinPriority(Tile* tile) const;
   void UpdateTilePriority(Tile* tile) const;
 
   // Given properties.
   float contents_scale_;
@@ skipping 20 matching lines @@
   gfx::Rect current_soon_border_rect_;
   gfx::Rect current_eventually_rect_;
 
   bool has_visible_rect_tiles_;
   bool has_skewport_rect_tiles_;
   bool has_soon_border_rect_tiles_;
   bool has_eventually_rect_tiles_;
 
   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);
 
   RectExpansionCache expansion_cache_;
 };
 
 }  // namespace cc
 
 #endif  // CC_RESOURCES_PICTURE_LAYER_TILING_H_