cc: Consolidate the analysis_canvas operations

cc/picture_pile_impl.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_PICTURE_PILE_IMPL_H_
 #define CC_PICTURE_PILE_IMPL_H_
 
 #include <list>
 #include <map>
 
@@ skipping 31 matching lines @@
       std::list<skia::LazyPixelRef*>& pixel_refs);
 
   void PushPropertiesTo(PicturePileImpl* other);
 
   skia::RefPtr<SkPicture> GetFlattenedPicture();
 
   void set_slow_down_raster_scale_factor(int factor) {
     slow_down_raster_scale_factor_for_debug_ = factor;
   }
 
-  bool IsCheapInRect(gfx::Rect content_rect, float contents_scale) const;
+  bool GetColorIfSolidInRect(const gfx::Rect& content_rect,
+                             float contents_scale,
+                             SkColor* color);
+  bool IsCheapInRect(const gfx::Rect& content_rect,
+                     float contents_scale);
+  bool IsTransparentInRect(const gfx::Rect& content_rect,
+                           float contents_scale);
 
  protected:
   friend class PicturePile;
 
   PicturePileImpl();
   virtual ~PicturePileImpl();
 
+  struct Analysis {
+    Analysis();
+
+    bool is_solid_color_;
+    SkColor solid_color_;
+    bool is_transparent_;
+    bool is_cheap_;
+
+    bool is_analyzed_;
+    gfx::Rect analyzed_content_rect_;
+    float analyzed_contents_scale_;
+  } analysis_;

[reveman, 2013/02/28 18:54:55]

is caching of last analysis just so we can call GetColorIfSolidInRect and
IsCheapInRect after each other without another call to AnalyzeInRect or is there
more to it?

If that's it, then lets instead make AnalyzeInRect public and have it return the
result in a "Picture::Analysis*". ManageTileState is a more appropriated place
to store this result. Caching the result and making it seem like
GetColorIfSolidInRect and such are cheap is misleading.

[Tom Hudson, 2013/02/28 19:25:27]

This is really expensive. We really ought to unify it with GatherPixelRefs(),
although that's on a different thread (compositor instead of worker).
I don't really understand this part of the design. Why do we have
ManagedTileState? Why not store the information about the tile/picture pile on
the objects themselves, instead of in a separate object that needs to be kept in
sync?

The comment in the header file for ManagedTileState is pretty tautologically
empty, and doesn't tell me anything about the reasoning behind the class.

Wouldn't moving storage of the data into a different struct mean we couldn't
encapsulate the whole cache management / staleness issue inside PicturePileImpl,
and have to be much more explicit about it?

[reveman, 2013/02/28 19:53:48]

Picture pile knows nothing about tiles. ManagedTileState needs a bit of cleanup
but it's part of the Tile and generally just the separation of the tile state
that is managed by the tile manager. Moving the analysis result to
ManagedTileState means moving it to the tile.
Yes, we have to be more explicit about it which I think is good. Caching the
result at the picture pile level provides no benefits, it's just confusing.

+
+  void AnalyzeInRect(const gfx::Rect& content_rect,
+                     float contents_scale);
+
   typedef std::map<base::PlatformThreadId, scoped_refptr<PicturePileImpl> >
       CloneMap;
   CloneMap clones_;
 
   int slow_down_raster_scale_factor_for_debug_;
 
   DISALLOW_COPY_AND_ASSIGN(PicturePileImpl);
 };
 
 }  // namespace cc
 
 #endif  // CC_PICTURE_PILE_IMPL_H_