cc: Plumb decoding mode state to checker image tracker.

This patch plumbs PaintImage decoding state from recorded paint images to checker image tracker and merges them into a map. R=khushalsagar@chromium.org Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel Change-Id: Ic6ed4462c82e4ac5917f5d5bc0abdc9df2756150 Reviewed-on: https://chromium-review.googlesource.com/723587 Commit-Queue: vmpstr <vmpstr@chromium.org> Reviewed-by: Khushal <khushalsagar@chromium.org> Cr-Commit-Position: refs/heads/master@{#509879}
qweri0p · Oct 18, 2017 · 9167e1c · 9167e1c
1 parent 33d89dc
commit 9167e1c
Show file tree

Hide file tree

Showing 18 changed files with 420 additions and 4 deletions.
diff --git a/cc/layers/picture_layer_impl.cc b/cc/layers/picture_layer_impl.cc
@@ -693,6 +693,10 @@ void PictureLayerImpl::UpdateRasterSource(
     tilings_->UpdateTilingsToCurrentRasterSourceForCommit(
         raster_source_, invalidation_, MinimumContentsScale(),
         MaximumContentsScale());
+    // We're in a commit, make sure to update the state of the checker image
+    // tracker with the new async attribute data.
+    layer_tree_impl()->UpdateImageDecodingHints(
+        raster_source_->TakeDecodingModeMap());
   }
 }
 

diff --git a/cc/paint/discardable_image_map.cc b/cc/paint/discardable_image_map.cc
@@ -91,6 +91,10 @@ class DiscardableImageGenerator {
   base::flat_map<PaintImage::Id, gfx::Rect> TakeImageIdToRectMap() {
     return std::move(image_id_to_rect_);
   }
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+  TakeDecodingModeMap() {
+    return std::move(decoding_mode_map_);
+  }
   std::vector<DiscardableImageMap::AnimatedImageMetadata>
   TakeAnimatedImagesMetadata() {
     return std::move(animated_images_metadata_);
@@ -291,6 +295,15 @@ class DiscardableImageGenerator {
     }
 
     image_id_to_rect_[paint_image.stable_id()].Union(image_rect);
+    auto decoding_mode_it = decoding_mode_map_.find(paint_image.stable_id());
+    // Use the decoding mode if we don't have one yet, otherwise use the more
+    // conservative one of the two existing ones.
+    if (decoding_mode_it == decoding_mode_map_.end()) {
+      decoding_mode_map_[paint_image.stable_id()] = paint_image.decoding_mode();
+    } else {
+      decoding_mode_it->second = PaintImage::GetConservative(
+          decoding_mode_it->second, paint_image.decoding_mode());
+    }
 
     if (paint_image.ShouldAnimate()) {
       animated_images_metadata_.emplace_back(
@@ -308,6 +321,7 @@ class DiscardableImageGenerator {
   base::flat_map<PaintImage::Id, gfx::Rect> image_id_to_rect_;
   std::vector<DiscardableImageMap::AnimatedImageMetadata>
       animated_images_metadata_;
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode> decoding_mode_map_;
 
   // Statistics about the number of images and pixels that will require color
   // conversion if the target color space is not sRGB.
@@ -334,6 +348,7 @@ void DiscardableImageMap::Generate(const PaintOpBuffer* paint_op_buffer,
   generator.RecordColorHistograms();
   image_id_to_rect_ = generator.TakeImageIdToRectMap();
   animated_images_metadata_ = generator.TakeAnimatedImagesMetadata();
+  decoding_mode_map_ = generator.TakeDecodingModeMap();
   all_images_are_srgb_ = generator.all_images_are_srgb();
   auto images = generator.TakeImages();
   images_rtree_.Build(
@@ -344,6 +359,11 @@ void DiscardableImageMap::Generate(const PaintOpBuffer* paint_op_buffer,
          size_t index) { return items[index].first; });
 }
 
+base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+DiscardableImageMap::TakeDecodingModeMap() {
+  return std::move(decoding_mode_map_);
+}
+
 void DiscardableImageMap::GetDiscardableImagesInRect(
     const gfx::Rect& rect,
     std::vector<const DrawImage*>* images) const {

diff --git a/cc/paint/discardable_image_map.h b/cc/paint/discardable_image_map.h
@@ -62,6 +62,10 @@ class CC_PAINT_EXPORT DiscardableImageMap {
   void Reset();
   void Generate(const PaintOpBuffer* paint_op_buffer, const gfx::Rect& bounds);
 
+  // This should only be called once from the compositor thread at commit time.
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+  TakeDecodingModeMap();
+
  private:
   friend class ScopedMetadataGenerator;
   friend class DiscardableImageMapTest;
@@ -74,6 +78,7 @@ class CC_PAINT_EXPORT DiscardableImageMap {
 
   base::flat_map<PaintImage::Id, gfx::Rect> image_id_to_rect_;
   std::vector<AnimatedImageMetadata> animated_images_metadata_;
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode> decoding_mode_map_;
   bool all_images_are_srgb_ = false;
 
   RTree<DrawImage> images_rtree_;

diff --git a/cc/paint/discardable_image_map_unittest.cc b/cc/paint/discardable_image_map_unittest.cc
@@ -791,6 +791,127 @@ TEST_F(DiscardableImageMapTest, CapturesImagesInPaintRecordShaders) {
   EXPECT_EQ(SkSize::Make(4.f, 4.f), draw_images[0].scale);
 }
 
+TEST_F(DiscardableImageMapTest, DecodingModeHintsBasic) {
+  gfx::Rect visible_rect(100, 100);
+  PaintImage unspecified_image =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(1)
+          .set_decoding_mode(PaintImage::DecodingMode::kUnspecified)
+          .TakePaintImage();
+  PaintImage async_image =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(2)
+          .set_decoding_mode(PaintImage::DecodingMode::kAsync)
+          .TakePaintImage();
+  PaintImage sync_image =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(3)
+          .set_decoding_mode(PaintImage::DecodingMode::kSync)
+          .TakePaintImage();
+
+  FakeContentLayerClient content_layer_client;
+  content_layer_client.set_bounds(visible_rect.size());
+  content_layer_client.add_draw_image(unspecified_image, gfx::Point(0, 0),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(async_image, gfx::Point(10, 10),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(sync_image, gfx::Point(20, 20),
+                                      PaintFlags());
+  scoped_refptr<DisplayItemList> display_list =
+      content_layer_client.PaintContentsToDisplayList(
+          ContentLayerClient::PAINTING_BEHAVIOR_NORMAL);
+  display_list->GenerateDiscardableImagesMetadata();
+  auto decode_hints = display_list->TakeDecodingModeMap();
+  ASSERT_EQ(decode_hints.size(), 3u);
+  ASSERT_TRUE(decode_hints.find(1) != decode_hints.end());
+  ASSERT_TRUE(decode_hints.find(2) != decode_hints.end());
+  ASSERT_TRUE(decode_hints.find(3) != decode_hints.end());
+  EXPECT_EQ(decode_hints[1], PaintImage::DecodingMode::kUnspecified);
+  EXPECT_EQ(decode_hints[2], PaintImage::DecodingMode::kAsync);
+  EXPECT_EQ(decode_hints[3], PaintImage::DecodingMode::kSync);
+
+  decode_hints = display_list->TakeDecodingModeMap();
+  EXPECT_EQ(decode_hints.size(), 0u);
+}
+
+TEST_F(DiscardableImageMapTest, DecodingModeHintsDuplicates) {
+  gfx::Rect visible_rect(100, 100);
+  PaintImage unspecified_image1 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(1)
+          .set_decoding_mode(PaintImage::DecodingMode::kUnspecified)
+          .TakePaintImage();
+  PaintImage async_image1 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(1)
+          .set_decoding_mode(PaintImage::DecodingMode::kAsync)
+          .TakePaintImage();
+
+  PaintImage unspecified_image2 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(2)
+          .set_decoding_mode(PaintImage::DecodingMode::kUnspecified)
+          .TakePaintImage();
+  PaintImage sync_image2 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(2)
+          .set_decoding_mode(PaintImage::DecodingMode::kSync)
+          .TakePaintImage();
+
+  PaintImage async_image3 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(3)
+          .set_decoding_mode(PaintImage::DecodingMode::kAsync)
+          .TakePaintImage();
+  PaintImage sync_image3 =
+      PaintImageBuilder::WithCopy(
+          CreateDiscardablePaintImage(gfx::Size(10, 10)))
+          .set_id(3)
+          .set_decoding_mode(PaintImage::DecodingMode::kSync)
+          .TakePaintImage();
+
+  FakeContentLayerClient content_layer_client;
+  content_layer_client.set_bounds(visible_rect.size());
+  content_layer_client.add_draw_image(unspecified_image1, gfx::Point(0, 0),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(async_image1, gfx::Point(10, 10),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(unspecified_image2, gfx::Point(20, 20),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(sync_image2, gfx::Point(30, 30),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(async_image3, gfx::Point(40, 40),
+                                      PaintFlags());
+  content_layer_client.add_draw_image(sync_image3, gfx::Point(50, 50),
+                                      PaintFlags());
+  scoped_refptr<DisplayItemList> display_list =
+      content_layer_client.PaintContentsToDisplayList(
+          ContentLayerClient::PAINTING_BEHAVIOR_NORMAL);
+  display_list->GenerateDiscardableImagesMetadata();
+  auto decode_hints = display_list->TakeDecodingModeMap();
+  ASSERT_EQ(decode_hints.size(), 3u);
+  ASSERT_TRUE(decode_hints.find(1) != decode_hints.end());
+  ASSERT_TRUE(decode_hints.find(2) != decode_hints.end());
+  ASSERT_TRUE(decode_hints.find(3) != decode_hints.end());
+  // 1 was unspecified and async, so the result should be unspecified.
+  EXPECT_EQ(decode_hints[1], PaintImage::DecodingMode::kUnspecified);
+  // 2 was unspecified and sync, so the result should be sync.
+  EXPECT_EQ(decode_hints[2], PaintImage::DecodingMode::kSync);
+  // 3 was async and sync, so the result should be sync
+  EXPECT_EQ(decode_hints[3], PaintImage::DecodingMode::kSync);
+
+  decode_hints = display_list->TakeDecodingModeMap();
+  EXPECT_EQ(decode_hints.size(), 0u);
+}
+
 class DiscardableImageMapColorSpaceTest
     : public DiscardableImageMapTest,
       public testing::WithParamInterface<gfx::ColorSpace> {};

diff --git a/cc/paint/display_item_list.h b/cc/paint/display_item_list.h
@@ -154,6 +154,10 @@ class CC_PAINT_EXPORT DisplayItemList
   const DiscardableImageMap& discardable_image_map() const {
     return image_map_;
   }
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+  TakeDecodingModeMap() {
+    return image_map_.TakeDecodingModeMap();
+  }
 
   void EmitTraceSnapshot() const;
   void GenerateDiscardableImagesMetadata();

diff --git a/cc/paint/paint_image.cc b/cc/paint/paint_image.cc
@@ -42,10 +42,26 @@ bool PaintImage::operator==(const PaintImage& other) const {
          is_multipart_ == other.is_multipart_;
 }
 
+// static
+PaintImage::DecodingMode PaintImage::GetConservative(DecodingMode one,
+                                                     DecodingMode two) {
+  if (one == two)
+    return one;
+  if (one == DecodingMode::kSync || two == DecodingMode::kSync)
+    return DecodingMode::kSync;
+  if (one == DecodingMode::kUnspecified || two == DecodingMode::kUnspecified)
+    return DecodingMode::kUnspecified;
+  DCHECK_EQ(one, DecodingMode::kAsync);
+  DCHECK_EQ(two, DecodingMode::kAsync);
+  return DecodingMode::kAsync;
+}
+
+// static
 PaintImage::Id PaintImage::GetNextId() {
   return s_next_id_.GetNext();
 }
 
+// static
 PaintImage::ContentId PaintImage::GetNextContentId() {
   return s_next_content_id_.GetNext();
 }

diff --git a/cc/paint/paint_image.h b/cc/paint/paint_image.h
@@ -91,6 +91,9 @@ class CC_PAINT_EXPORT PaintImage {
     kAsync
   };
 
+  // Returns the more conservative mode out of the two given ones.
+  static DecodingMode GetConservative(DecodingMode one, DecodingMode two);
+
   static Id GetNextId();
   static ContentId GetNextContentId();
 

diff --git a/cc/raster/raster_source.cc b/cc/raster/raster_source.cc
@@ -198,6 +198,11 @@ void RasterSource::GetDiscardableImagesInRect(
                                                                     images);
 }
 
+base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+RasterSource::TakeDecodingModeMap() {
+  return display_list_->TakeDecodingModeMap();
+}
+
 gfx::Rect RasterSource::GetRectForImage(PaintImage::Id image_id) const {
   if (!display_list_)
     return gfx::Rect();

diff --git a/cc/raster/raster_source.h b/cc/raster/raster_source.h
@@ -115,6 +115,9 @@ class CC_EXPORT RasterSource : public base::RefCountedThreadSafe<RasterSource> {
 
   SkColor background_color() const { return background_color_; }
 
+  base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
+  TakeDecodingModeMap();
+
  protected:
   // RecordingSource is the only class that can create a raster source.
   friend class RecordingSource;

diff --git a/cc/test/skia_common.cc b/cc/test/skia_common.cc
@@ -62,6 +62,10 @@ PaintImage CreateDiscardablePaintImage(const gfx::Size& size,
       .set_paint_image_generator(sk_make_sp<FakePaintImageGenerator>(
           SkImageInfo::MakeN32Premul(size.width(), size.height(), color_space),
           std::vector<FrameMetadata>{FrameMetadata()}, allocate_encoded_data))
+      // For simplicity, assume that any paint image created for testing is
+      // unspecified decode mode as would be the case with most img tags on the
+      // web.
+      .set_decoding_mode(PaintImage::DecodingMode::kUnspecified)
       .TakePaintImage();
 }