cc: Disable decoding of PaintRecord shaders in cc.

For PaintRecord backed shaders, skia performs the rasterization of the
shader's record at the required scale before tiling. In order to decode
images in these records using cc's decode cache, we create a temporary
SkPicture with images replaced with decodes from cc's cache and use
that in a new SkPictureShader each time the PaintShader is rasterized.

Skia internally maintains a cache of rasterized pictures used in these
shaders, keyed on the SkPictureShader's uniqueID. Since a new
SkPictureShader is created for each raster call, we don't get a cache
hit in skia and end up re-rasterizing the shader's record.

The ideal solution for this would be to maintain a PaintShaderCache
which caches rasterized records. But given the rarity of these cases,
its not worth duplicating skia's picture cache. However, if any of
the images in these records are animated, they must go through cc's
decode stack to update them and use cached animation frames. The
change ensures that we only decode shaders in cc if they have animated
images. Note that the performance hit of re-rasterization of records
in these cases will still exist.

R=chrishtr@chromium.org, enne@chromium.org, ericrk@chromium.org

Bug: 817640
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel;master.tryserver.chromium.android:android_optional_gpu_tests_rel;master.tryserver.chromium.linux:linux_layout_tests_slimming_paint_v2;master.tryserver.chromium.linux:linux_optional_gpu_tests_rel;master.tryserver.chromium.mac:mac_optional_gpu_tests_rel;master.tryserver.chromium.win:win_optional_gpu_tests_rel
Change-Id: Ibd1f3db9902f21fca20c99be1cf9e36b54cd7085
Reviewed-on: https://chromium-review.googlesource.com/952529
Commit-Queue: Khushal <khushalsagar@chromium.org>
Reviewed-by: Eric Karl <ericrk@chromium.org>
Reviewed-by: enne <enne@chromium.org>
Reviewed-by: Chris Harrelson <chrishtr@chromium.org>
Reviewed-by: Antoine Labour <piman@chromium.org>
Cr-Commit-Position: refs/heads/master@{#542570}

cc/paint/discardable_image_map.cc

@@ -9,6 +9,7 @@
 #include <algorithm>
 #include <limits>
 
+#include "base/auto_reset.h"
 #include "base/containers/adapters.h"
 #include "base/memory/ptr_util.h"
 #include "base/metrics/histogram_macros.h"
@@ -270,8 +271,19 @@ class DiscardableImageGenerator {
       canvas.setMatrix(SkMatrix::MakeRectToRect(flags.getShader()->tile(),
                                                 scaled_tile_rect,
                                                 SkMatrix::kFill_ScaleToFit));
+      base::AutoReset<bool> auto_reset(&iterating_record_shaders_, true);
+      size_t prev_image_set_size = image_set_.size();
       GatherDiscardableImages(flags.getShader()->paint_record().get(), &op_rect,
                               &canvas);
+
+      // We only track animated images for PaintShaders. If we added any entry
+      // to the |image_set_|, this shader any has animated images.
+      // Note that it is thread-safe to set the |has_animated_images| bit on
+      // PaintShader here since the analysis is done on the main thread, before
+      // the PaintOpBuffer is used for rasterization.
+      DCHECK_GE(image_set_.size(), prev_image_set_size);
+      if (image_set_.size() > prev_image_set_size)
+        const_cast<PaintShader*>(flags.getShader())->set_has_animated_images();
     }
   }
 
@@ -319,16 +331,24 @@ class DiscardableImageGenerator {
           paint_image.reset_animation_sequence_id());
     }
 
-    image_set_.emplace_back(
-        DrawImage(std::move(paint_image), src_irect, filter_quality, matrix),
-        image_rect);
+    // If we are iterating images in a record shader, only track them if they
+    // are animated. We defer decoding of images in record shaders to skia, but
+    // we still need to track animated images to invalidate and advance the
+    // animation in cc.
+    bool add_image = !iterating_record_shaders_ || paint_image.ShouldAnimate();
+    if (add_image) {
+      image_set_.emplace_back(
+          DrawImage(std::move(paint_image), src_irect, filter_quality, matrix),
+          image_rect);
+    }
   }
 
   std::vector<std::pair<DrawImage, gfx::Rect>> image_set_;
   base::flat_map<PaintImage::Id, DiscardableImageMap::Rects> image_id_to_rects_;
   std::vector<DiscardableImageMap::AnimatedImageMetadata>
       animated_images_metadata_;
   base::flat_map<PaintImage::Id, PaintImage::DecodingMode> decoding_mode_map_;
+  bool iterating_record_shaders_ = false;
 
   // Statistics about the number of images and pixels that will require color
   // conversion if the target color space is not sRGB.

cc/paint/discardable_image_map.h

@@ -17,6 +17,7 @@
 #include "cc/paint/paint_export.h"
 #include "cc/paint/paint_flags.h"
 #include "cc/paint/paint_image.h"
+#include "cc/paint/paint_shader.h"
 #include "third_party/skia/include/core/SkCanvas.h"
 #include "third_party/skia/include/core/SkRefCnt.h"
 #include "ui/gfx/geometry/rect.h"

cc/paint/discardable_image_map_unittest.cc

@@ -733,8 +733,15 @@ TEST_F(DiscardableImageMapTest, CapturesImagesInPaintRecordShaders) {
   // Create the record to use in the shader.
   auto shader_record = sk_make_sp<PaintOpBuffer>();
   shader_record->push<ScaleOp>(2.0f, 2.0f);
-  PaintImage paint_image = CreateDiscardablePaintImage(gfx::Size(100, 100));
-  shader_record->push<DrawImageOp>(paint_image, 0.f, 0.f, nullptr);
+
+  PaintImage static_image = CreateDiscardablePaintImage(gfx::Size(100, 100));
+  shader_record->push<DrawImageOp>(static_image, 0.f, 0.f, nullptr);
+
+  std::vector<FrameMetadata> frames = {
+      FrameMetadata(true, base::TimeDelta::FromMilliseconds(1)),
+      FrameMetadata(true, base::TimeDelta::FromMilliseconds(1))};
+  PaintImage animated_image = CreateAnimatedImage(gfx::Size(100, 100), frames);
+  shader_record->push<DrawImageOp>(animated_image, 0.f, 0.f, nullptr);
 
   gfx::Rect visible_rect(500, 500);
   scoped_refptr<DisplayItemList> display_list = new DisplayItemList();
@@ -749,15 +756,18 @@ TEST_F(DiscardableImageMapTest, CapturesImagesInPaintRecordShaders) {
   display_list->EndPaintOfUnpaired(visible_rect);
   display_list->Finalize();
 
+  EXPECT_FALSE(flags.getShader()->has_animated_images());
   display_list->GenerateDiscardableImagesMetadata();
+  EXPECT_TRUE(flags.getShader()->has_animated_images());
   const auto& image_map = display_list->discardable_image_map();
 
-  // The image rect is set to the rect for the DrawRectOp.
+  // The image rect is set to the rect for the DrawRectOp, and only animated
+  // images in a shader are tracked.
   std::vector<PositionScaleDrawImage> draw_images =
       GetDiscardableImagesInRect(image_map, visible_rect);
   std::vector<gfx::Rect> inset_rects = InsetImageRects(draw_images);
   ASSERT_EQ(draw_images.size(), 1u);
-  EXPECT_EQ(draw_images[0].image, paint_image);
+  EXPECT_EQ(draw_images[0].image, animated_image);
   // The position of the image is the position of the DrawRectOp that uses the
   // shader.
   EXPECT_EQ(gfx::Rect(400, 400), inset_rects[0]);
@@ -766,6 +776,40 @@ TEST_F(DiscardableImageMapTest, CapturesImagesInPaintRecordShaders) {
   EXPECT_EQ(SkSize::Make(4.f, 4.f), draw_images[0].scale);
 }
 
+TEST_F(DiscardableImageMapTest, EmbeddedShaderWithAnimatedImages) {
+  // Create the record with animated image to use in the shader.
+  SkRect tile = SkRect::MakeWH(100, 100);
+  auto shader_record = sk_make_sp<PaintOpBuffer>();
+  std::vector<FrameMetadata> frames = {
+      FrameMetadata(true, base::TimeDelta::FromMilliseconds(1)),
+      FrameMetadata(true, base::TimeDelta::FromMilliseconds(1))};
+  PaintImage animated_image = CreateAnimatedImage(gfx::Size(100, 100), frames);
+  shader_record->push<DrawImageOp>(animated_image, 0.f, 0.f, nullptr);
+  auto shader_with_image = PaintShader::MakePaintRecord(
+      shader_record, tile, SkShader::TileMode::kClamp_TileMode,
+      SkShader::TileMode::kClamp_TileMode, nullptr);
+
+  // Create a second shader which uses the shader above.
+  auto second_shader_record = sk_make_sp<PaintOpBuffer>();
+  PaintFlags flags;
+  flags.setShader(shader_with_image);
+  second_shader_record->push<DrawRectOp>(SkRect::MakeWH(200, 200), flags);
+  auto shader_with_shader_with_image = PaintShader::MakePaintRecord(
+      second_shader_record, tile, SkShader::TileMode::kClamp_TileMode,
+      SkShader::TileMode::kClamp_TileMode, nullptr);
+
+  gfx::Rect visible_rect(500, 500);
+  scoped_refptr<DisplayItemList> display_list = new DisplayItemList();
+  display_list->StartPaint();
+  flags.setShader(shader_with_shader_with_image);
+  display_list->push<DrawRectOp>(SkRect::MakeWH(200, 200), flags);
+  display_list->EndPaintOfUnpaired(visible_rect);
+  display_list->Finalize();
+  display_list->GenerateDiscardableImagesMetadata();
+  EXPECT_TRUE(shader_with_image->has_animated_images());
+  EXPECT_TRUE(shader_with_shader_with_image->has_animated_images());
+}
+
 TEST_F(DiscardableImageMapTest, DecodingModeHintsBasic) {
   gfx::Rect visible_rect(100, 100);
   PaintImage unspecified_image =

cc/paint/paint_op_buffer_unittest.cc

@@ -3102,6 +3102,7 @@ TEST(PaintOpBufferTest, RecordShadersSerializeScaledImages) {
       record_buffer, SkRect::MakeWH(10.f, 10.f),
       SkShader::TileMode::kRepeat_TileMode,
       SkShader::TileMode::kRepeat_TileMode, nullptr);
+  shader->set_has_animated_images();
   auto buffer = sk_make_sp<PaintOpBuffer>();
   buffer->push<ScaleOp>(0.5f, 0.8f);
   PaintFlags flags;

cc/paint/paint_shader.h

@@ -106,6 +106,9 @@ class CC_PAINT_EXPORT PaintShader : public SkRefCnt {
 
   ~PaintShader() override;
 
+  void set_has_animated_images() { has_animated_images_ = true; }
+  bool has_animated_images() const { return has_animated_images_; }
+
   SkMatrix GetLocalMatrix() const {
     return local_matrix_ ? *local_matrix_ : SkMatrix::I();
   }
@@ -197,6 +200,8 @@ class CC_PAINT_EXPORT PaintShader : public SkRefCnt {
   // accesses to it are thread-safe.
   sk_sp<SkShader> cached_shader_;
 
+  bool has_animated_images_ = false;
+
   DISALLOW_COPY_AND_ASSIGN(PaintShader);
 };
 

cc/paint/paint_shader_unittest.cc

@@ -84,6 +84,7 @@ TEST(PaintShaderTest, DecodePaintRecord) {
   auto record_shader = PaintShader::MakePaintRecord(
       record, SkRect::MakeWH(100, 100), SkShader::TileMode::kClamp_TileMode,
       SkShader::TileMode::kClamp_TileMode, &local_matrix);
+  record_shader->set_has_animated_images();
 
   PaintOpBuffer buffer;
   PaintFlags flags;

cc/paint/scoped_raster_flags.cc

@@ -17,6 +17,9 @@ ScopedRasterFlags::ScopedRasterFlags(const PaintFlags* flags,
   if (flags->HasDiscardableImages() && image_provider) {
     DCHECK(flags->HasShader());
 
+    // TODO(khushalsagar): The decoding of images in PaintFlags here is a bit of
+    // a mess. We decode image shaders at the correct scale but ignore that
+    // during serialization and just use the original image.
     decode_stashing_image_provider_.emplace(image_provider);
     if (flags->getShader()->shader_type() == PaintShader::Type::kImage) {
       DecodeImageShader(ctm);
@@ -97,6 +100,11 @@ void ScopedRasterFlags::DecodeImageShader(const SkMatrix& ctm) {
 
 void ScopedRasterFlags::DecodeRecordShader(const SkMatrix& ctm,
                                            bool create_skia_shader) {
+  // TODO(khushalsagar): For OOP, we have to decode everything during
+  // serialization. This will force us to use original sized decodes.
+  if (!flags()->getShader()->has_animated_images())
+    return;
+
   auto decoded_shader = flags()->getShader()->CreateDecodedPaintRecord(
       ctm, &*decode_stashing_image_provider_);
   if (!decoded_shader) {

cc/paint/scoped_raster_flags_unittest.cc

@@ -54,6 +54,7 @@ TEST(ScopedRasterFlagsTest, KeepsDecodesAlive) {
   auto record_shader = PaintShader::MakePaintRecord(
       record, SkRect::MakeWH(100, 100), SkShader::TileMode::kClamp_TileMode,
       SkShader::TileMode::kClamp_TileMode, &SkMatrix::I());
+  record_shader->set_has_animated_images();
 
   MockImageProvider provider;
   PaintFlags flags;

cc/trees/layer_tree_host_unittest.cc

@@ -8484,6 +8484,8 @@ class LayerTreeHostTestImageAnimation : public LayerTreeHostTest {
  public:
   void BeginTest() override { PostSetNeedsCommitToMainThread(); }
 
+  virtual void AddImageOp(const PaintImage& image) = 0;
+
   void SetupTree() override {
     gfx::Size layer_size(1000, 500);
     content_layer_client_.set_bounds(layer_size);
@@ -8502,7 +8504,7 @@ class LayerTreeHostTestImageAnimation : public LayerTreeHostTest {
             .set_animation_type(PaintImage::AnimationType::ANIMATED)
             .set_repetition_count(kAnimationLoopOnce)
             .TakePaintImage();
-    content_layer_client_.add_draw_image(image, gfx::Point(0, 0), PaintFlags());
+    AddImageOp(image);
 
     layer_tree_host()->SetRootLayer(
         FakePictureLayer::Create(&content_layer_client_));
@@ -8549,16 +8551,51 @@ class LayerTreeHostTestImageAnimation : public LayerTreeHostTest {
     EXPECT_EQ(generator_->frames_decoded().size(), 3u);
   }
 
- private:
+ protected:
   FakeContentLayerClient content_layer_client_;
   sk_sp<FakePaintImageGenerator> generator_;
   int draw_count_ = 0;
 };
 
-MULTI_THREAD_TEST_F(LayerTreeHostTestImageAnimation);
+class LayerTreeHostTestImageAnimationDrawImage
+    : public LayerTreeHostTestImageAnimation {
+  void AddImageOp(const PaintImage& image) override {
+    content_layer_client_.add_draw_image(image, gfx::Point(0, 0), PaintFlags());
+  }
+};
 
-class LayerTreeHostTestImageAnimationSynchronousScheduling
+MULTI_THREAD_TEST_F(LayerTreeHostTestImageAnimationDrawImage);
+
+class LayerTreeHostTestImageAnimationDrawImageShader
+    : public LayerTreeHostTestImageAnimation {
+  void AddImageOp(const PaintImage& image) override {
+    PaintFlags flags;
+    flags.setShader(
+        PaintShader::MakeImage(image, SkShader::TileMode::kRepeat_TileMode,
+                               SkShader::TileMode::kRepeat_TileMode, nullptr));
+    content_layer_client_.add_draw_rect(gfx::Rect(500, 500), flags);
+  }
+};
+
+MULTI_THREAD_TEST_F(LayerTreeHostTestImageAnimationDrawImageShader);
+
+class LayerTreeHostTestImageAnimationDrawRecordShader
     : public LayerTreeHostTestImageAnimation {
+  void AddImageOp(const PaintImage& image) override {
+    auto record = sk_make_sp<PaintOpBuffer>();
+    record->push<DrawImageOp>(image, 0.f, 0.f, nullptr);
+    PaintFlags flags;
+    flags.setShader(PaintShader::MakePaintRecord(
+        record, SkRect::MakeWH(500, 500), SkShader::TileMode::kClamp_TileMode,
+        SkShader::TileMode::kClamp_TileMode, nullptr));
+    content_layer_client_.add_draw_rect(gfx::Rect(500, 500), flags);
+  }
+};
+
+MULTI_THREAD_TEST_F(LayerTreeHostTestImageAnimationDrawRecordShader);
+
+class LayerTreeHostTestImageAnimationSynchronousScheduling
+    : public LayerTreeHostTestImageAnimationDrawImage {
  public:
   void InitializeSettings(LayerTreeSettings* settings) override {
     LayerTreeHostTestImageAnimation::InitializeSettings(settings);

third_party/WebKit/LayoutTests/SlowTests

@@ -135,6 +135,9 @@ crbug.com/789111 virtual/pwa-full-code-cache/http/tests/devtools/service-workers
 crbug.com/763197 [ Linux Mac ] virtual/gpu-rasterization/images/color-profile-border-radius.html [ Slow ]
 crbug.com/763197 [ Linux Mac ] virtual/gpu-rasterization/images/color-profile-image-canvas-svg.html [ Slow ]
 crbug.com/763197 [ Linux Mac ] virtual/gpu-rasterization/images/color-profile-image-svg-resource-url.html [ Slow ]
+crbug.com/763197 [ Mac ] virtual/gpu-rasterization/images/color-profile-background-image-space.html [ Slow ]
+crbug.com/763197 [ Mac ] virtual/gpu-rasterization/images/color-profile-svg-fill-text.html [ Slow ]
+crbug.com/763197 [ Mac ] virtual/gpu-rasterization/images/color-profile-svg.html [ Slow ]
 
 crbug.com/510337 cssom/cssvalue-comparison.html [ Slow ]