cc: Don't perform image analysis if the DisplayItemList has no images.

Now that we use the PaintOpBuffer, analyze whether a DisplayItemList
has any discardable images to skip metadata generation if unnecessary.

CQ_INCLUDE_TRYBOTS=master.tryserver.blink:linux_trusty_blink_rel

Review-Url: https://codereview.chromium.org/2830243002
Cr-Commit-Position: refs/heads/master@{#469509}

cc/paint/display_item_list.cc

@@ -526,6 +526,8 @@ DisplayItemList::CreateTracedValue(bool include_items) const {
 void DisplayItemList::GenerateDiscardableImagesMetadata() {
   // This should be only called once.
   DCHECK(image_map_.empty());
+  if (!has_discardable_images_)
+    return;
 
   gfx::Rect bounds = rtree_.GetBounds();
   DiscardableImageMap::ScopedMetadataGenerator generator(

cc/paint/display_item_list.h

@@ -18,6 +18,7 @@
 #include "cc/base/rtree.h"
 #include "cc/paint/discardable_image_map.h"
 #include "cc/paint/display_item.h"
+#include "cc/paint/drawing_display_item.h"
 #include "cc/paint/image_id.h"
 #include "cc/paint/paint_export.h"
 #include "third_party/skia/include/core/SkPicture.h"
@@ -119,7 +120,10 @@ class CC_PAINT_EXPORT DisplayItemList
     visual_rects_.push_back(visual_rect);
     GrowCurrentBeginItemVisualRect(visual_rect);
 
-    return AllocateAndConstruct<DisplayItemType>(std::forward<Args>(args)...);
+    const auto& item =
+        AllocateAndConstruct<DisplayItemType>(std::forward<Args>(args)...);
+    has_discardable_images_ |= item.picture->HasDiscardableImages();
+    return item;
   }
 
   // Called after all items are appended, to process the items and, if
@@ -160,6 +164,8 @@ class CC_PAINT_EXPORT DisplayItemList
     return items_.end();
   }
 
+  bool has_discardable_images() const { return has_discardable_images_; }
+
  private:
   FRIEND_TEST_ALL_PREFIXES(DisplayItemListTest, AsValueWithNoItems);
   FRIEND_TEST_ALL_PREFIXES(DisplayItemListTest, AsValueWithItems);
@@ -198,6 +204,7 @@ class CC_PAINT_EXPORT DisplayItemList
   // For testing purposes only. Whether to keep visual rects across calls to
   // Finalize().
   bool retain_visual_rects_ = false;
+  bool has_discardable_images_ = false;
 
   friend class base::RefCountedThreadSafe<DisplayItemList>;
   FRIEND_TEST_ALL_PREFIXES(DisplayItemListTest, ApproximateMemoryUsage);

cc/paint/paint_op_buffer.cc

@@ -502,6 +502,10 @@ size_t DrawDisplayItemListOp::AdditionalBytesUsed() const {
   return list->ApproximateMemoryUsage();
 }
 
+bool DrawDisplayItemListOp::HasDiscardableImages() const {
+  return list->has_discardable_images();
+}
+
 DrawDisplayItemListOp::DrawDisplayItemListOp(const DrawDisplayItemListOp& op) =
     default;
 
@@ -519,6 +523,12 @@ DrawImageOp::DrawImageOp(const PaintImage& image,
       left(left),
       top(top) {}
 
+bool DrawImageOp::HasDiscardableImages() const {
+  // TODO(khushalsagar): Callers should not be able to change the lazy generated
+  // state for a PaintImage.
+  return image.sk_image()->isLazyGenerated();
+}
+
 DrawImageOp::~DrawImageOp() = default;
 
 DrawImageRectOp::DrawImageRectOp(const PaintImage& image,
@@ -532,6 +542,10 @@ DrawImageRectOp::DrawImageRectOp(const PaintImage& image,
       dst(dst),
       constraint(constraint) {}
 
+bool DrawImageRectOp::HasDiscardableImages() const {
+  return image.sk_image()->isLazyGenerated();
+}
+
 DrawImageRectOp::~DrawImageRectOp() = default;
 
 DrawPosTextOp::DrawPosTextOp(size_t bytes,
@@ -550,6 +564,10 @@ size_t DrawRecordOp::AdditionalBytesUsed() const {
   return record->approximateBytesUsed();
 }
 
+bool DrawRecordOp::HasDiscardableImages() const {
+  return record->HasDiscardableImages();
+}
+
 DrawTextBlobOp::DrawTextBlobOp(sk_sp<SkTextBlob> blob,
                                SkScalar x,
                                SkScalar y,

cc/paint/paint_op_buffer.h

@@ -92,6 +92,9 @@ struct CC_PAINT_EXPORT PaintOp {
   int CountSlowPaths() const { return 0; }
   int CountSlowPathsFromFlags() const { return 0; }
 
+  bool HasDiscardableImages() const { return false; }
+  bool HasDiscardableImagesFromFlags() const { return false; }
+
   // Returns the number of bytes used by this op in referenced sub records
   // and display lists.  This doesn't count other objects like paths or blobs.
   size_t AdditionalBytesUsed() const { return 0; }
@@ -107,6 +110,14 @@ struct CC_PAINT_EXPORT PaintOpWithFlags : PaintOp {
   explicit PaintOpWithFlags(const PaintFlags& flags) : flags(flags) {}
 
   int CountSlowPathsFromFlags() const { return flags.getPathEffect() ? 1 : 0; }
+  bool HasDiscardableImagesFromFlags() const {
+    if (!IsDrawOp())
+      return false;
+
+    SkShader* shader = flags.getShader();
+    SkImage* image = shader ? shader->isAImage(nullptr, nullptr) : nullptr;
+    return image && image->isLazyGenerated();
+  }
 
   // Subclasses should provide a static RasterWithFlags() method which is called
   // from the Raster() method. The RasterWithFlags() should use the PaintFlags
@@ -364,6 +375,7 @@ struct CC_PAINT_EXPORT DrawDisplayItemListOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
   size_t AdditionalBytesUsed() const;
+  bool HasDiscardableImages() const;
   // TODO(enne): DisplayItemList should know number of slow paths.
 
   scoped_refptr<DisplayItemList> list;
@@ -409,6 +421,7 @@ struct CC_PAINT_EXPORT DrawImageOp final : PaintOpWithFlags {
                               const PaintFlags* flags,
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
+  bool HasDiscardableImages() const;
 
   PaintImage image;
   SkScalar left;
@@ -434,6 +447,7 @@ struct CC_PAINT_EXPORT DrawImageRectOp final : PaintOpWithFlags {
                               const PaintFlags* flags,
                               SkCanvas* canvas,
                               const SkMatrix& original_ctm);
+  bool HasDiscardableImages() const;
 
   PaintImage image;
   SkRect src;
@@ -558,6 +572,7 @@ struct CC_PAINT_EXPORT DrawRecordOp final : PaintOp {
                      SkCanvas* canvas,
                      const SkMatrix& original_ctm);
   size_t AdditionalBytesUsed() const;
+  bool HasDiscardableImages() const;
 
   sk_sp<const PaintRecord> record;
 };
@@ -771,6 +786,7 @@ class CC_PAINT_EXPORT PaintOpBuffer : public SkRefCnt {
     return sizeof(*this) + reserved_ + subrecord_bytes_used_;
   }
   int numSlowPaths() const { return num_slow_paths_; }
+  bool HasDiscardableImages() const { return has_discardable_images_; }
 
   // Resize the PaintOpBuffer to exactly fit the current amount of used space.
   void ShrinkToFit();
@@ -933,6 +949,9 @@ class CC_PAINT_EXPORT PaintOpBuffer : public SkRefCnt {
     num_slow_paths_ += op->CountSlowPathsFromFlags();
     num_slow_paths_ += op->CountSlowPaths();
 
+    has_discardable_images_ |= op->HasDiscardableImages();
+    has_discardable_images_ |= op->HasDiscardableImagesFromFlags();
+
     subrecord_bytes_used_ += op->AdditionalBytesUsed();
   }
 
@@ -948,6 +967,7 @@ class CC_PAINT_EXPORT PaintOpBuffer : public SkRefCnt {
   int num_slow_paths_ = 0;
   // Record additional bytes used by referenced sub-records and display lists.
   size_t subrecord_bytes_used_ = 0;
+  bool has_discardable_images_ = false;
   SkRect cull_rect_;
 
   DISALLOW_COPY_AND_ASSIGN(PaintOpBuffer);

cc/paint/paint_op_buffer_unittest.cc

@@ -3,6 +3,8 @@
 // found in the LICENSE file.
 
 #include "cc/paint/paint_op_buffer.h"
+#include "cc/paint/display_item_list.h"
+#include "cc/test/skia_common.h"
 #include "cc/test/test_skcanvas.h"
 #include "testing/gtest/include/gtest/gtest.h"
 
@@ -417,4 +419,60 @@ TEST(PaintOpBufferTest, SaveDrawRestore_SingleOpRecordWithSingleNonDrawOp) {
   EXPECT_EQ(1, canvas.restore_count_);
 }
 
+TEST(PaintOpBufferTest, DiscardableImagesTracking_EmptyBuffer) {
+  PaintOpBuffer buffer;
+  EXPECT_FALSE(buffer.HasDiscardableImages());
+}
+
+TEST(PaintOpBufferTest, DiscardableImagesTracking_NoImageOp) {
+  PaintOpBuffer buffer;
+  PaintFlags flags;
+  buffer.push<DrawRectOp>(SkRect::MakeWH(100, 100), flags);
+  EXPECT_FALSE(buffer.HasDiscardableImages());
+}
+
+TEST(PaintOpBufferTest, DiscardableImagesTracking_DrawImage) {
+  PaintOpBuffer buffer;
+  PaintImage image = PaintImage(CreateDiscardableImage(gfx::Size(100, 100)));
+  buffer.push<DrawImageOp>(image, SkIntToScalar(0), SkIntToScalar(0), nullptr);
+  EXPECT_TRUE(buffer.HasDiscardableImages());
+}
+
+TEST(PaintOpBufferTest, DiscardableImagesTracking_DrawImageRect) {
+  PaintOpBuffer buffer;
+  PaintImage image = PaintImage(CreateDiscardableImage(gfx::Size(100, 100)));
+  buffer.push<DrawImageRectOp>(
+      image, SkRect::MakeWH(100, 100), SkRect::MakeWH(100, 100), nullptr,
+      PaintCanvas::SrcRectConstraint::kFast_SrcRectConstraint);
+  EXPECT_TRUE(buffer.HasDiscardableImages());
+}
+
+TEST(PaintOpBufferTest, DiscardableImagesTracking_NestedDrawOp) {
+  sk_sp<PaintRecord> record = sk_make_sp<PaintRecord>();
+  PaintImage image = PaintImage(CreateDiscardableImage(gfx::Size(100, 100)));
+  record->push<DrawImageOp>(image, SkIntToScalar(0), SkIntToScalar(0), nullptr);
+
+  PaintOpBuffer buffer;
+  buffer.push<DrawRecordOp>(record);
+  EXPECT_TRUE(buffer.HasDiscardableImages());
+
+  scoped_refptr<DisplayItemList> list = new DisplayItemList;
+  list->CreateAndAppendDrawingItem<DrawingDisplayItem>(
+      gfx::Rect(0, 0, 100, 100), record);
+  list->Finalize();
+  PaintOpBuffer new_buffer;
+  new_buffer.push<DrawDisplayItemListOp>(list);
+  EXPECT_TRUE(new_buffer.HasDiscardableImages());
+}
+
+TEST(PaintOpBufferTest, DiscardableImagesTracking_OpWithFlags) {
+  PaintOpBuffer buffer;
+  PaintFlags flags;
+  sk_sp<SkImage> image = CreateDiscardableImage(gfx::Size(100, 100));
+  flags.setShader(
+      image->makeShader(SkShader::kClamp_TileMode, SkShader::kClamp_TileMode));
+  buffer.push<DrawRectOp>(SkRect::MakeWH(100, 100), flags);
+  EXPECT_TRUE(buffer.HasDiscardableImages());
+}
+
 }  // namespace cc

cc/paint/transform_display_item.cc

@@ -8,7 +8,11 @@ namespace cc {
 
 TransformDisplayItem::TransformDisplayItem(const gfx::Transform& transform)
 
-    : DisplayItem(TRANSFORM), transform(transform) {}
+    : DisplayItem(TRANSFORM), transform(transform) {
+  // The underlying SkMatrix in gfx::Transform is not thread-safe, unless
+  // getType() has been called.
+  this->transform.matrix().getType();
+}
 
 TransformDisplayItem::~TransformDisplayItem() = default;