cc: Do GatherPixelRefs from skia at record time

cc/resources/picture.h

Index: cc/resources/picture.h
diff --git a/cc/resources/picture.h b/cc/resources/picture.h
index a29d5afaa6438177def4ca4af6c58e7c629d0c8a..caa62e0e90f93f59378f727e75d0763cd610e52e 100644
--- a/cc/resources/picture.h
+++ b/cc/resources/picture.h
@@ -7,11 +7,14 @@
 
 #include <list>
 #include <string>
+#include <utility>
 #include <vector>
 
 #include "base/basictypes.h"
+#include "base/hash_tables.h"
 #include "base/memory/ref_counted.h"
 #include "cc/base/cc_export.h"
+#include "cc/base/hash_pair.h"
 #include "skia/ext/lazy_pixel_ref.h"
 #include "skia/ext/refptr.h"
 #include "third_party/skia/include/core/SkPixelRef.h"
@@ -30,6 +33,11 @@ struct RenderingStats;
 class CC_EXPORT Picture
     : public base::RefCountedThreadSafe<Picture> {
  public:
+  typedef std::pair<int, int> PixelRefMapKey;
+  typedef base::hash_map<
+      PixelRefMapKey,
+      std::list<skia::LazyPixelRef*> > PixelRefMap;

[reveman, 2013/04/24 14:10:52]

Sorry for changing my mind but as the set of lazy pixel refs is static how about
we store them in a std::vector<skia::LazyPixelRef*> instead? That will be more
compact, a bit more efficient to iterate over and clean up some of the iterator
logic below.

[vmpstr, 2013/04/24 21:11:10]

Changed to vector.

+
   static scoped_refptr<Picture> Create(gfx::Rect layer_rect);
   static scoped_refptr<Picture> CreateFromBase64String(
       const std::string& encoded_string);
@@ -58,12 +66,33 @@ class CC_EXPORT Picture
               float contents_scale,
               bool enable_lcd_text);
 
-  void GatherPixelRefs(
-      const gfx::Rect& layer_rect,
-      std::list<skia::LazyPixelRef*>& pixel_ref_list);
-
   void AsBase64String(std::string* output) const;
 
+  class CC_EXPORT PixelRefIterator {
+   public:
+    PixelRefIterator();
+    PixelRefIterator(const PixelRefIterator& other);
+    PixelRefIterator(gfx::Rect layer_rect, const Picture* picture);
+    ~PixelRefIterator();
+
+    skia::LazyPixelRef* operator->() const { return *current_list_iterator_; }
+    skia::LazyPixelRef* operator*() const { return *current_list_iterator_; }
+    PixelRefIterator& operator++();
+    PixelRefIterator& operator=(const PixelRefIterator& other);
+    operator bool() const { return !!*current_list_iterator_; }
+
+   private:
+    const Picture* picture_;
+    std::list<skia::LazyPixelRef*> sentinel_list_;
+    const std::list<skia::LazyPixelRef*>* current_list_;
+    std::list<skia::LazyPixelRef*>::const_iterator current_list_iterator_;

[reveman, 2013/04/24 14:10:52]

if we use the std::vector idea I mentioned above this becomes something like:
const std::vector<skia::LazyPixelRef*> empty_vector_;
const std::vector<skia::LazyPixelRef*>* pixel_refs_;
unsigned current_position_;

[vmpstr, 2013/04/24 21:11:10]

I kept the current way.. With position, we'd have to index into a vector with
each access. I think it's better, in terms of efficiency, to keep an iterator
instead (although under the covers it's probably the same thing).

[reveman, 2013/04/24 23:01:27]

index or iterator would be the same for std::vector in terms of performance. the
benefit of using an index is that you can set it to -1 and use an empty vector
as sentinel value.

+
+    gfx::Point min_point_;
+    gfx::Point max_point_;
+    int current_x_;
+    int current_y_;
+  };
+
  private:
   explicit Picture(gfx::Rect layer_rect);
   Picture(const std::string& encoded_string, bool* success);
@@ -71,9 +100,15 @@ class CC_EXPORT Picture
   // ownership to this picture.
   Picture(const skia::RefPtr<SkPicture>&,
           gfx::Rect layer_rect,
-          gfx::Rect opaque_rect);
+          gfx::Rect opaque_rect,
+          const PixelRefMap& pixel_refs);
   ~Picture();
 
+  void GatherPixelRefsFromSkia(
+      gfx::Rect query_rect,
+      std::list<skia::LazyPixelRef*>* pixel_ref_list);
+  void GatherAllPixelRefs();
+
   gfx::Rect layer_rect_;
   gfx::Rect opaque_rect_;
   skia::RefPtr<SkPicture> picture_;
@@ -81,7 +116,13 @@ class CC_EXPORT Picture
   typedef std::vector<scoped_refptr<Picture> > PictureVector;
   PictureVector clones_;
 
+  PixelRefMap pixel_refs_;
+  gfx::Point min_pixel_cell_;
+  gfx::Point max_pixel_cell_;
+  gfx::Size cell_size_;
+
   friend class base::RefCountedThreadSafe<Picture>;
+  friend class PixelRefIterator;
   DISALLOW_COPY_AND_ASSIGN(Picture);
 };