cc: Update discardable image metadata generation to get PaintImages.

cc/paint/discardable_image_map.cc

Index: cc/paint/discardable_image_map.cc
diff --git a/cc/paint/discardable_image_map.cc b/cc/paint/discardable_image_map.cc
index c1a3b57ccb532bfc4affac0e30c903625af7c0ca..ff58be848e472d78510b7d1e6548f8eb1550c2fb 100644
--- a/cc/paint/discardable_image_map.cc
+++ b/cc/paint/discardable_image_map.cc
@@ -12,6 +12,7 @@
 #include "base/containers/adapters.h"
 #include "base/memory/ptr_util.h"
 #include "cc/base/math_util.h"
+#include "cc/paint/discardable_image_store.h"
 #include "cc/paint/display_item_list.h"
 #include "third_party/skia/include/core/SkPath.h"
 #include "third_party/skia/include/utils/SkNWayCanvas.h"
@@ -19,126 +20,88 @@
 #include "ui/gfx/skia_util.h"
 
 namespace cc {
-
+namespace {
 SkRect MapRect(const SkMatrix& matrix, const SkRect& src) {
   SkRect dst;
   matrix.mapRect(&dst, src);
   return dst;
 }
 
-// Returns a rect clamped to |max_size|. Note that |paint_rect| should intersect
-// or be contained by a rect defined by (0, 0) and |max_size|.
-gfx::Rect SafeClampPaintRectToSize(const SkRect& paint_rect,
-                                   const gfx::Size& max_size) {
-  // bounds_rect.x() + bounds_rect.width() (aka bounds_rect.right()) might
-  // overflow integer bounds, so do custom intersect, since gfx::Rect::Intersect
-  // uses bounds_rect.right().
-  gfx::RectF bounds_rect = gfx::SkRectToRectF(paint_rect);
-  float x_offset_if_negative = bounds_rect.x() < 0.f ? bounds_rect.x() : 0.f;
-  float y_offset_if_negative = bounds_rect.y() < 0.f ? bounds_rect.y() : 0.f;
-  bounds_rect.set_x(std::max(0.f, bounds_rect.x()));
-  bounds_rect.set_y(std::max(0.f, bounds_rect.y()));
-
-  // Verify that the rects intersect or that bound_rect is contained by
-  // max_size.
-  DCHECK_GE(bounds_rect.width(), -x_offset_if_negative);
-  DCHECK_GE(bounds_rect.height(), -y_offset_if_negative);
-  DCHECK_GE(max_size.width(), bounds_rect.x());
-  DCHECK_GE(max_size.height(), bounds_rect.y());
-
-  bounds_rect.set_width(std::min(bounds_rect.width() + x_offset_if_negative,
-                                 max_size.width() - bounds_rect.x()));
-  bounds_rect.set_height(std::min(bounds_rect.height() + y_offset_if_negative,
-                                  max_size.height() - bounds_rect.y()));
-  return gfx::ToEnclosingRect(bounds_rect);
-}
-
-namespace {
-
 // We're using an NWay canvas with no added canvases, so in effect
 // non-overridden functions are no-ops.
-class DiscardableImagesMetadataCanvas : public SkNWayCanvas {
+class DiscardableImagesMetadataStore : public DiscardableImageStore,
+                                       public SkNWayCanvas {

[vmpstr, 2017/04/27 16:28:53]

Why is this still deriving from SkNWayCanvas? Can we make SkNoDrawCanvas a
member instead?

[Khushal, 2017/04/27 22:06:02]

Done.

  public:
-  DiscardableImagesMetadataCanvas(
+  DiscardableImagesMetadataStore(
       int width,
       int height,
       std::vector<std::pair<DrawImage, gfx::Rect>>* image_set,
       std::unordered_map<ImageId, gfx::Rect>* image_id_to_rect)
       : SkNWayCanvas(width, height),
         image_set_(image_set),
-        image_id_to_rect_(image_id_to_rect),
-        canvas_bounds_(SkRect::MakeIWH(width, height)),
-        canvas_size_(width, height) {}
+        image_id_to_rect_(image_id_to_rect) {}
 
- protected:
-  // we need to "undo" the behavior of SkNWayCanvas, which will try to forward
-  // it.
-  void onDrawPicture(const SkPicture* picture,
-                     const SkMatrix* matrix,
-                     const SkPaint* paint) override {
-    SkCanvas::onDrawPicture(picture, matrix, paint);
-  }
+  SkCanvas* NoDrawCanvas() override { return this; }
 
-  void onDrawImage(const SkImage* image,
-                   SkScalar x,
-                   SkScalar y,
-                   const SkPaint* paint) override {
-    const SkMatrix& ctm = getTotalMatrix();
-    AddImage(
-        sk_ref_sp(image), SkRect::MakeIWH(image->width(), image->height()),
-        MapRect(ctm, SkRect::MakeXYWH(x, y, image->width(), image->height())),
-        ctm, paint);
-  }
+  void AddImage(const PaintImage& paint_image,

[vmpstr, 2017/04/27 16:28:53]

This function is unchanged, right?

[Khushal, 2017/04/27 22:06:02]

Nope. It includes a fix for using the clip rect to clamp the paint rect instead
of the canvas_bounds_ which we were using right now.

I think the rect mapping using the paints might also be incorrect, because we
should be mapping the rect first using the ctm in the save operations (that's
what SkRecordDraw does). I was planning on doing this in a subsequent patch.

+                const SkRect& src_rect,
+                const SkRect& rect,
+                const SkMatrix* local_matrix,
+                const PaintFlags& flags) override {
+    sk_sp<const SkImage> sk_image = paint_image.sk_image();
+    if (!sk_image->isLazyGenerated())
+      return;
 
-  void onDrawImageRect(const SkImage* image,
-                       const SkRect* src,
-                       const SkRect& dst,
-                       const SkPaint* paint,
-                       SrcRectConstraint) override {
+    const SkRect clip_rect = SkRect::Make(getDeviceClipBounds());
     const SkMatrix& ctm = getTotalMatrix();
-    SkRect src_storage;
-    if (!src) {
-      src_storage = SkRect::MakeIWH(image->width(), image->height());
-      src = &src_storage;
+
+    SkRect paint_rect = MapRect(ctm, rect);
+    bool computed_paint_bounds =
+        ComputePaintBounds(paint_rect, ToSkPaint(&flags), &paint_rect);
+    if (!computed_paint_bounds) {
+      // TODO(vmpstr): UMA this case.
+      paint_rect = clip_rect;
     }
-    SkMatrix matrix;
-    matrix.setRectToRect(*src, dst, SkMatrix::kFill_ScaleToFit);
-    matrix.preConcat(ctm);
-    AddImage(sk_ref_sp(image), *src, MapRect(ctm, dst), matrix, paint);
-  }
 
-  void onDrawImageNine(const SkImage* image,
-                       const SkIRect& center,
-                       const SkRect& dst,
-                       const SkPaint* paint) override {
-    // No cc embedder issues image nine calls.
-    NOTREACHED();
-  }
+    // Clamp the image rect by the current clip rect.
+    if (!paint_rect.intersect(clip_rect))
+      return;
 
-  void onDrawRect(const SkRect& r, const SkPaint& paint) override {
-    AddPaintImage(r, paint);
-  }
+    SkFilterQuality filter_quality = flags.getFilterQuality();
 
-  void onDrawPath(const SkPath& path, const SkPaint& paint) override {
-    AddPaintImage(path.getBounds(), paint);
-  }
+    SkIRect src_irect;
+    src_rect.roundOut(&src_irect);
+    gfx::Rect image_rect = gfx::ToEnclosingRect(gfx::SkRectToRectF(paint_rect));
 
-  void onDrawOval(const SkRect& r, const SkPaint& paint) override {
-    AddPaintImage(r, paint);
-  }
+    // During raster, we use the device clip bounds on the canvas, which outsets
+    // the actual clip by 1 due to the possibility of antialiasing. Account for
+    // this here by outsetting the image rect by 1. Note that this only affects
+    // queries into the rtree, which will now return images that only touch the
+    // bounds of the query rect.
+    //
+    // Note that it's not sufficient for us to inset the device clip bounds at
+    // raster time, since we might be sending a larger-than-one-item display
+    // item to skia, which means that skia will internally determine whether to
+    // raster the picture (using device clip bounds that are outset).
+    image_rect.Inset(-1, -1);
 
-  void onDrawArc(const SkRect& r,
-                 SkScalar start_angle,
-                 SkScalar sweep_angle,
-                 bool use_center,
-                 const SkPaint& paint) override {
-    AddPaintImage(r, paint);
-  }
+    // The true target color space will be assigned when it is known, in
+    // GetDiscardableImagesInRect.
+    gfx::ColorSpace target_color_space;
+
+    SkMatrix matrix = ctm;
+    if (local_matrix)
+      matrix.postConcat(*local_matrix);
 
-  void onDrawRRect(const SkRRect& rr, const SkPaint& paint) override {
-    AddPaintImage(rr.rect(), paint);
+    // TODO(khushalsagar): Keep PaintImage in DrawImage.
+    (*image_id_to_rect_)[sk_image->uniqueID()].Union(image_rect);
+    image_set_->push_back(
+        std::make_pair(DrawImage(std::move(sk_image), src_irect, filter_quality,
+                                 matrix, target_color_space),
+                       image_rect));
   }
 
+ protected:
   SaveLayerStrategy getSaveLayerStrategy(const SaveLayerRec& rec) override {
     saved_paints_.push_back(rec.fPaint ? *rec.fPaint : SkPaint());
     return SkNWayCanvas::getSaveLayerStrategy(rec);
@@ -176,80 +139,8 @@ class DiscardableImagesMetadataCanvas : public SkNWayCanvas {
     return true;
   }
 
-  void AddImage(sk_sp<const SkImage> image,
-                const SkRect& src_rect,
-                const SkRect& rect,
-                const SkMatrix& matrix,
-                const SkPaint* paint) {
-    if (!image->isLazyGenerated())
-      return;
-
-    SkRect paint_rect;
-    bool computed_paint_bounds = ComputePaintBounds(rect, paint, &paint_rect);
-    if (!computed_paint_bounds) {
-      // TODO(vmpstr): UMA this case.
-      paint_rect = canvas_bounds_;
-    }
-
-    if (!paint_rect.intersects(canvas_bounds_))
-      return;
-
-    SkFilterQuality filter_quality = kNone_SkFilterQuality;
-    if (paint) {
-      filter_quality = paint->getFilterQuality();
-    }
-
-    SkIRect src_irect;
-    src_rect.roundOut(&src_irect);
-    gfx::Rect image_rect = SafeClampPaintRectToSize(paint_rect, canvas_size_);
-
-    // During raster, we use the device clip bounds on the canvas, which outsets
-    // the actual clip by 1 due to the possibility of antialiasing. Account for
-    // this here by outsetting the image rect by 1. Note that this only affects
-    // queries into the rtree, which will now return images that only touch the
-    // bounds of the query rect.
-    //
-    // Note that it's not sufficient for us to inset the device clip bounds at
-    // raster time, since we might be sending a larger-than-one-item display
-    // item to skia, which means that skia will internally determine whether to
-    // raster the picture (using device clip bounds that are outset).
-    image_rect.Inset(-1, -1);
-
-    // The true target color space will be assigned when it is known, in
-    // GetDiscardableImagesInRect.
-    gfx::ColorSpace target_color_space;
-
-    (*image_id_to_rect_)[image->uniqueID()].Union(image_rect);
-    image_set_->push_back(
-        std::make_pair(DrawImage(std::move(image), src_irect, filter_quality,
-                                 matrix, target_color_space),
-                       image_rect));
-  }
-
-  // Currently this function only handles extracting images from SkImageShaders
-  // embedded in SkPaints. Other embedded image cases, such as SkPictures,
-  // are not yet handled.
-  void AddPaintImage(const SkRect& rect, const SkPaint& paint) {
-    SkShader* shader = paint.getShader();
-    if (shader) {
-      SkMatrix matrix;
-      SkShader::TileMode xy[2];
-      SkImage* image = shader->isAImage(&matrix, xy);
-      if (image) {
-        const SkMatrix& ctm = getTotalMatrix();
-        matrix.postConcat(ctm);
-        // TODO(ericrk): Handle cases where we only need a sub-rect from the
-        // image. crbug.com/671821
-        AddImage(sk_ref_sp(image), SkRect::MakeFromIRect(image->bounds()),
-                 MapRect(ctm, rect), matrix, &paint);
-      }
-    }
-  }
-
   std::vector<std::pair<DrawImage, gfx::Rect>>* image_set_;
   std::unordered_map<ImageId, gfx::Rect>* image_id_to_rect_;
-  const SkRect canvas_bounds_;
-  const gfx::Size canvas_size_;
   std::vector<SkPaint> saved_paints_;
 };
 
@@ -259,10 +150,10 @@ DiscardableImageMap::DiscardableImageMap() {}
 
 DiscardableImageMap::~DiscardableImageMap() {}
 
-std::unique_ptr<SkCanvas> DiscardableImageMap::BeginGeneratingMetadata(
-    const gfx::Size& bounds) {
+std::unique_ptr<DiscardableImageStore>
+DiscardableImageMap::BeginGeneratingMetadata(const gfx::Size& bounds) {
   DCHECK(all_images_.empty());
-  return base::MakeUnique<DiscardableImagesMetadataCanvas>(
+  return base::MakeUnique<DiscardableImagesMetadataStore>(
       bounds.width(), bounds.height(), &all_images_, &image_id_to_rect_);
 }
 
@@ -296,7 +187,7 @@ DiscardableImageMap::ScopedMetadataGenerator::ScopedMetadataGenerator(
     DiscardableImageMap* image_map,
     const gfx::Size& bounds)
     : image_map_(image_map),
-      metadata_canvas_(image_map->BeginGeneratingMetadata(bounds)) {}
+      image_store_(image_map->BeginGeneratingMetadata(bounds)) {}
 
 DiscardableImageMap::ScopedMetadataGenerator::~ScopedMetadataGenerator() {
   image_map_->EndGeneratingMetadata();