cc: Do GatherPixelRefs from skia at record time

cc/resources/picture.cc

 // 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.
 
 #include "cc/resources/picture.h"
 
+#include <algorithm>
+#include <limits>
+#include <set>
+
 #include "base/base64.h"
 #include "base/debug/trace_event.h"
 #include "cc/debug/rendering_stats.h"
 #include "cc/layers/content_layer_client.h"
 #include "skia/ext/analysis_canvas.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkData.h"
 #include "third_party/skia/include/core/SkDrawFilter.h"
 #include "third_party/skia/include/core/SkPaint.h"
 #include "third_party/skia/include/core/SkStream.h"
 #include "third_party/skia/include/utils/SkPictureUtils.h"
 #include "ui/gfx/rect_conversions.h"
 #include "ui/gfx/skia_util.h"
 
 namespace {
 // URI label for a lazily decoded SkPixelRef.
 const char kLabelLazyDecoded[] = "lazy";
+
 // Version ID; to be used in serialization.
 const int kPictureVersion = 1;
+
 // Minimum size of a decoded stream that we need.
 // 4 bytes for version, 4 * 4 for each of the 2 rects.
 const unsigned int kMinPictureSizeBytes = 36;
 
+int ToMultiple(int value, int multiple) {

[enne (OOO), 2013/04/23 01:59:39]

Should this be moved to cc/base/util.h as RoundDown, equivalent to RoundUp?

[vmpstr, 2013/04/23 20:02:23]

Done.

+  return (value < 0) ?
+      (value / multiple - 1) * multiple
+      : (value / multiple) * multiple;
+}
+
 class DisableLCDTextFilter : public SkDrawFilter {
  public:
   // SkDrawFilter interface.
   virtual bool filter(SkPaint* paint, SkDrawFilter::Type type) OVERRIDE {
     if (type != SkDrawFilter::kText_Type)
       return true;
 
     paint->setLCDRenderText(false);
     return true;
   }
@@ skipping 54 matching lines @@
                            opaque_rect_y,
                            opaque_rect_width,
                            opaque_rect_height);
 
   // Read the picture. This creates an empty picture on failure.
   picture_ = skia::AdoptRef(new SkPicture(&stream, success, NULL));
 }
 
 Picture::Picture(const skia::RefPtr<SkPicture>& picture,
                  gfx::Rect layer_rect,
-                 gfx::Rect opaque_rect) :
+                 gfx::Rect opaque_rect,
+                 const PixelRefMap& lazy_pixel_refs) :
     layer_rect_(layer_rect),
     opaque_rect_(opaque_rect),
-    picture_(picture) {
+    picture_(picture),
+    lazy_pixel_refs_(lazy_pixel_refs) {
 }
 
 Picture::~Picture() {
 }
 
 scoped_refptr<Picture> Picture::GetCloneForDrawingOnThread(
     unsigned thread_index) const {
   // SkPicture is not thread-safe to rasterize with, this returns a clone
   // to rasterize with on a specific thread.
   CHECK_GT(clones_.size(), thread_index);
   return clones_[thread_index];
 }
 
 void Picture::CloneForDrawing(int num_threads) {
   TRACE_EVENT1("cc", "Picture::CloneForDrawing", "num_threads", num_threads);
 
   DCHECK(picture_);
   scoped_ptr<SkPicture[]> clones(new SkPicture[num_threads]);
   picture_->clone(&clones[0], num_threads);
 
   clones_.clear();
   for (int i = 0; i < num_threads; i++) {
     scoped_refptr<Picture> clone = make_scoped_refptr(
         new Picture(skia::AdoptRef(new SkPicture(clones[i])),
                     layer_rect_,
-                    opaque_rect_));
+                    opaque_rect_,
+                    lazy_pixel_refs_));
     clones_.push_back(clone);
   }
 }
 
 void Picture::Record(ContentLayerClient* painter,
                      RenderingStats* stats,
                      const SkTileGridPicture::TileGridInfo& tile_grid_info) {
   TRACE_EVENT2("cc", "Picture::Record",
                "width", layer_rect_.width(), "height", layer_rect_.height());
 
@@ skipping 31 matching lines @@
   if (stats) {
     stats->total_paint_time += base::TimeTicks::Now() - begin_paint_time;
     stats->total_pixels_painted +=
         layer_rect_.width() * layer_rect_.height();
   }
 
   canvas->restore();
   picture_->endRecording();
 
   opaque_rect_ = gfx::ToEnclosedRect(opaque_layer_rect);
+
+  cell_size_ = gfx::Size(

[enne (OOO), 2013/04/23 01:59:39]

For what it's worth, this isn't going to line up with the tile grid.  You're
going to be off-by-one, and so will hit four tile grid cells for every one cell
here.

That said, I don't think it really matters.  Honestly, any resolution that is
about the average size of images will probably work out ok.  Maybe you'll decode
something you don't need, but most of the time some other high priority tile
will need it and it won't be a waste.

[reveman, 2013/04/23 14:33:24]

I would vote for getting this to line up properly in this patch so we only hit
one cell when using an identity transform.

I think it's worth getting the common case as fast as possible on the cc thread
as we'll be iterating over a large set of tiles and all their pixel refs inside
AssignGpuMemoryToTiles in the near future.

[vmpstr, 2013/04/23 20:02:23]

I think I fixed this. Right now, when we do gather pixels from skia, we do it
for a rect x,y w by h. When computing the indecies, we can just use x,y w-1 by
h-1 rect. That way, if the only overlap is the right/bottom boundary, then we
will only check one index (but still capture refs in w by h). If the overlap is
bigger than one pixel, then we need to (correctly) check more indecies.

+      tile_grid_info.fTileInterval.width() +
+          2 * tile_grid_info.fMargin.width(),
+      tile_grid_info.fTileInterval.height() +
+          2 * tile_grid_info.fMargin.height());
+  DCHECK_GT(cell_size_.width(), 0);
+  DCHECK_GT(cell_size_.height(), 0);
+  GatherAllPixelRefs();
 }
 
 void Picture::Raster(
     SkCanvas* canvas,
     gfx::Rect content_rect,
     float contents_scale,
     bool enable_lcd_text) {
   TRACE_EVENT2("cc", "Picture::Raster",
                "layer width", layer_rect_.width(),
                "layer height", layer_rect_.height());
   DCHECK(picture_);
 
   DisableLCDTextFilter disable_lcd_text_filter;
 
   canvas->save();
   canvas->clipRect(gfx::RectToSkRect(content_rect));
   canvas->scale(contents_scale, contents_scale);
   canvas->translate(layer_rect_.x(), layer_rect_.y());
   // Pictures by default have LCD text enabled.
   if (!enable_lcd_text)
     canvas->setDrawFilter(&disable_lcd_text_filter);
   canvas->drawPicture(*picture_);
   canvas->restore();
 }
 
-void Picture::GatherPixelRefs(const gfx::Rect& layer_rect,
-                              std::list<skia::LazyPixelRef*>& pixel_ref_list) {
+void Picture::GatherPixelRefsFromSkia(
+    const gfx::Rect& query_rect,

[enne (OOO), 2013/04/23 01:59:39]

const gfx::Rect& => gfx::Rect

[vmpstr, 2013/04/23 20:02:23]

Done.

+    std::list<skia::LazyPixelRef*>& pixel_ref_list) {

[reveman, 2013/04/23 14:33:24]

nit: while you're here. I think the style guide recommends passing return values
by pointer rather than reference.

[vmpstr, 2013/04/23 20:02:23]

Done.

   DCHECK(picture_);
   SkData* pixel_refs = SkPictureUtils::GatherPixelRefs(
-      picture_.get(), SkRect::MakeXYWH(layer_rect.x(),
-                                       layer_rect.y(),
-                                       layer_rect.width(),
-                                       layer_rect.height()));
+      picture_.get(), SkRect::MakeXYWH(query_rect.x() - layer_rect_.x(),

[enne (OOO), 2013/04/23 01:59:39]

Haha.  Sorry for misreading earlier.  Calling it query_rect helps a lot.

[vmpstr, 2013/04/23 20:02:23]

Yeah, I vote in general to avoid naming completely different things with names
that differ only by an underscore :P

+                                       query_rect.y() - layer_rect_.y(),
+                                       query_rect.width(),
+                                       query_rect.height()));
   if (!pixel_refs)
     return;
 
   void* data = const_cast<void*>(pixel_refs->data());
   if (!data) {
     pixel_refs->unref();
     return;
   }
 
   SkPixelRef** refs = reinterpret_cast<SkPixelRef**>(data);
@@ skipping 27 matching lines @@
   picture_->serialize(&stream);
 
   // Encode the picture as base64.
   size_t serialized_size = stream.bytesWritten();
   scoped_ptr<char[]> serialized_picture(new char[serialized_size]);
   stream.copyTo(serialized_picture.get());
   base::Base64Encode(std::string(serialized_picture.get(), serialized_size),
                      output);
 }
 
+void Picture::GatherAllPixelRefs() {
+  int min_x = std::numeric_limits<int>::max();
+  int min_y = std::numeric_limits<int>::max();
+  int max_x = 0;
+  int max_y = 0;
+
+  // Capture pixel refs for this picture in a grid
+  // with cell_size_ sized cells.
+  for (int y = layer_rect_.y();
+      y < layer_rect_.bottom();
+      y += cell_size_.height()) {
+    for (int x = layer_rect_.x();
+        x < layer_rect_.right();
+        x += cell_size_.width()) {
+      gfx::Size extent(std::min(cell_size_.width(), layer_rect_.right() - x),

[enne (OOO), 2013/04/23 01:59:39]

I think these lines would be more clearly put as:

gfx::Rect rect(gfx::Point(x, y), cell_size_);
rect.intersect(layer_rect_);

[vmpstr, 2013/04/23 20:02:23]

Done.

+                       std::min(cell_size_.height(), layer_rect_.bottom() - y));
+      gfx::Rect rect(x, y, extent.width(), extent.height());
+
+      std::list<skia::LazyPixelRef*> lazy_pixel_refs;
+      GatherPixelRefsFromSkia(rect, lazy_pixel_refs);
+
+      // Only capture non-empty cells.
+      if (!lazy_pixel_refs.empty()) {
+        lazy_pixel_refs_[PixelRefMapKey(x, y)].swap(lazy_pixel_refs);
+        min_x = std::min(min_x, x);
+        min_y = std::min(min_y, y);
+        max_x = std::max(max_x, x);
+        max_y = std::max(max_y, y);
+      }
+    }
+  }
+
+  min_lazy_pixel_cell_ = gfx::Point(min_x, min_y);
+  max_lazy_pixel_cell_ = gfx::Point(max_x, max_y);
+}
+
+Picture::LazyPixelRefIterator::LazyPixelRefIterator()
+    : picture_(NULL),
+      current_list_(NULL),
+      current_pixel_ref_(NULL),
+      min_point_(-1, -1),
+      max_point_(-1, -1),
+      current_x_(0),
+      current_y_(0) {
+}
+
+Picture::LazyPixelRefIterator::LazyPixelRefIterator(
+    gfx::Rect rect,

[enne (OOO), 2013/04/23 01:59:39]

rect => query_rect?

[vmpstr, 2013/04/23 20:02:23]

Done.

+    const Picture* picture)
+    : picture_(picture),
+      current_list_(NULL),
+      current_pixel_ref_(NULL) {
+  gfx::Rect layer_rect = picture->layer_rect_;
+  gfx::Size cell_size = picture->cell_size_;
+
+  // We have to find a cell_size aligned point that
+  // corresponds to the given rect. First, subtract the layer origin,
+  // then ensure the point is a multiple of cell_size,
+  // and finally, add the layer origin back.
+  min_point_ = gfx::Point(
+      ToMultiple(rect.x() - layer_rect.x(), cell_size.width())
+          + layer_rect.x(),
+      ToMultiple(rect.y() - layer_rect.y(), cell_size.height())
+          + layer_rect.y());
+  max_point_ = gfx::Point(
+      ToMultiple(rect.right() - layer_rect.x(), cell_size.width())
+          + layer_rect.x(),
+      ToMultiple(rect.bottom() - layer_rect.y(), cell_size.height())
+          + layer_rect.y());
+
+  // Limit the points to knows pixel ref boundaries.

[enne (OOO), 2013/04/23 01:59:39]

knows => known?

[vmpstr, 2013/04/23 20:02:23]

Oops. Fixed.

+  min_point_ = gfx::Point(
+      std::max(min_point_.x(), picture->min_lazy_pixel_cell_.x()),
+      std::max(min_point_.y(), picture->min_lazy_pixel_cell_.y()));
+  max_point_ = gfx::Point(
+      std::min(max_point_.x(), picture->max_lazy_pixel_cell_.x()),
+      std::min(max_point_.y(), picture->max_lazy_pixel_cell_.y()));
+
+  // Make the current x be cell_size.width() less than min point, so that
+  // the first increment will point at min_point_.
+  current_x_ = min_point_.x() - cell_size.width();
+  current_y_ = min_point_.y();
+  ++(*this);
+}
+
+Picture::LazyPixelRefIterator::~LazyPixelRefIterator() {
+}
+
+Picture::LazyPixelRefIterator& Picture::LazyPixelRefIterator::operator++() {
+  // If we're not at the end of the list, then just get the next item.
+  if (current_list_ &&
+      current_list_iterator_ != current_list_->end()) {

[reveman, 2013/04/23 14:33:24]

I think this would be cleaner if you used a sentinel node here instead of
allowing current_list_ to be NULL. you can get rid of |current_pixel_ref_| and
|current_list_iterator_| would be the current position instead of being one step
ahead, which I find awkward. should also make this a bit more efficient.

[vmpstr, 2013/04/23 20:02:23]

Done.

+    current_pixel_ref_ = *current_list_iterator_;
+    ++current_list_iterator_;
+    return *this;
+  }
+
+  // If we already passed the max y, do nothing.
+  if (current_y_ > max_point_.y())
+    return *this;
+
+  while (true) {
+    gfx::Size cell_size = picture_->cell_size_;
+
+    // Advance the current grid cell.
+    current_x_ += cell_size.width();
+    if (current_x_ > max_point_.x()) {
+      current_y_ += cell_size.height();
+      current_x_ = min_point_.x();
+      // Min x can be greater than max x if the query rect
+      // does not intersect the layer rect.
+      if (current_y_ > max_point_.y() ||
+          current_x_ > max_point_.x()) {
+        current_pixel_ref_ = NULL;
+        break;
+      }
+    }
+
+    // If there are no pixel refs at this grid cell, keep incrementing.
+    PixelRefMap::const_iterator iter =
+        picture_->lazy_pixel_refs_.find(PixelRefMapKey(current_x_, current_y_));

[reveman, 2013/04/23 14:33:24]

nit: think it might be a bit cleaner to put "PixelRefMapKey key(current_x_,
current_y_)" on a separate line.

[vmpstr, 2013/04/23 20:02:23]

Done.

+    if (iter == picture_->lazy_pixel_refs_.end())
+      continue;
+
+    // We found a non-empty list: store it and get the first pixel ref.
+    current_list_ = &iter->second;
+    current_list_iterator_ = current_list_->begin();
+    current_pixel_ref_ = *current_list_iterator_;
+    ++current_list_iterator_;
+    break;
+  }
+  return *this;
+}
+
 }  // namespace cc