cc: Use StackVector of gfx::Rect for discardable image positions.

With [1], we switched to use Region instead of gfx::Rect for tracking
the position of images during discardable image analysis. This causes
a regression in record time for the common case where a single instance
of image exists. Use a StackVector<gfx::Rect, 1> instead to avoid a
heap allocation in these cases.

[1]: https://chromium-review.googlesource.com/c/720245/

R=enne@chromium.org

Bug: 778170
Cq-Include-Trybots: master.tryserver.blink:linux_trusty_blink_rel
Change-Id: Ifc8646f768c5d04ea0ae32e5dd9a9466448a29d9
Reviewed-on: https://chromium-review.googlesource.com/738677
Reviewed-by: enne <enne@chromium.org>
Commit-Queue: Khushal <khushalsagar@chromium.org>
Cr-Commit-Position: refs/heads/master@{#511941}

Author: khushalsagar

cc/base/invalidation_region.cc

@@ -35,15 +35,6 @@ void InvalidationRegion::Union(const gfx::Rect& rect) {
     pending_rects_.push_back(rect);
 }
 
-void InvalidationRegion::Union(const Region& region) {
-  if (region_.GetRegionComplexity() + region.GetRegionComplexity() >
-      kMaxInvalidationRectCount) {
-    region_ = gfx::UnionRects(region_.bounds(), region.bounds());
-  } else {
-    region_.Union(region);
-  }
-}
-
 void InvalidationRegion::FinalizePendingRects() {
   if (pending_rects_.empty())
     return;

cc/base/invalidation_region.h

@@ -24,7 +24,6 @@ class CC_BASE_EXPORT InvalidationRegion {
   void Swap(Region* region);
   void Clear();
   void Union(const gfx::Rect& rect);
-  void Union(const Region& region);
   bool IsEmpty() const { return pending_rects_.empty() && region_.IsEmpty(); }
 
  private:

cc/layers/picture_layer_impl.cc

@@ -834,10 +834,17 @@ bool PictureLayerImpl::ShouldAnimate(PaintImage::Id paint_image_id) const {
   //
   //  Additionally only animate images which are on-screen, animations are
   //  paused once they are not visible.
-  return HasValidTilePriorities() && raster_source_->GetDisplayItemList()
-                                         ->discardable_image_map()
-                                         .GetRegionForImage(paint_image_id)
-                                         .Intersects(visible_layer_rect());
+  if (!HasValidTilePriorities())
+    return false;
+
+  const auto& rects = raster_source_->GetDisplayItemList()
+                          ->discardable_image_map()
+                          .GetRectsForImage(paint_image_id);
+  for (const auto& r : rects.container()) {
+    if (r.Intersects(visible_layer_rect()))
+      return true;
+  }
+  return false;
 }
 
 gfx::Size PictureLayerImpl::CalculateTileSize(
@@ -1578,9 +1585,11 @@ void PictureLayerImpl::InvalidateRegionForImages(
 
   InvalidationRegion image_invalidation;
   for (auto image_id : images_to_invalidate) {
-    image_invalidation.Union(raster_source_->GetDisplayItemList()
-                                 ->discardable_image_map()
-                                 .GetRegionForImage(image_id));
+    const auto& rects = raster_source_->GetDisplayItemList()
+                            ->discardable_image_map()
+                            .GetRectsForImage(image_id);
+    for (const auto& r : rects.container())
+      image_invalidation.Union(r);
   }
   Region invalidation;
   image_invalidation.Swap(&invalidation);

cc/paint/discardable_image_map.cc

@@ -20,7 +20,7 @@
 
 namespace cc {
 namespace {
-const int kMaxRegionComplexity = 256;
+const int kMaxRectsSize = 256;
 
 SkRect MapRect(const SkMatrix& matrix, const SkRect& src) {
   SkRect dst;
@@ -89,8 +89,9 @@ class DiscardableImageGenerator {
   std::vector<std::pair<DrawImage, gfx::Rect>> TakeImages() {
     return std::move(image_set_);
   }
-  base::flat_map<PaintImage::Id, Region> TakeImageIdToRegionMap() {
-    return std::move(image_id_to_region_);
+  base::flat_map<PaintImage::Id, DiscardableImageMap::Rects>
+  TakeImageIdToRectsMap() {
+    return std::move(image_id_to_rects_);
   }
   base::flat_map<PaintImage::Id, PaintImage::DecodingMode>
   TakeDecodingModeMap() {
@@ -295,10 +296,11 @@ class DiscardableImageGenerator {
       color_stats_srgb_image_count_++;
     }
 
-    auto& region = image_id_to_region_[paint_image.stable_id()];
-    region.Union(image_rect);
-    if (region.GetRegionComplexity() >= kMaxRegionComplexity)
-      region = region.bounds();
+    auto& rects = image_id_to_rects_[paint_image.stable_id()];
+    if (rects->size() >= kMaxRectsSize)
+      rects->back().Union(image_rect);
+    else
+      rects->push_back(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
@@ -323,7 +325,7 @@ class DiscardableImageGenerator {
   }
 
   std::vector<std::pair<DrawImage, gfx::Rect>> image_set_;
-  base::flat_map<PaintImage::Id, Region> image_id_to_region_;
+  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_;
@@ -351,7 +353,7 @@ void DiscardableImageMap::Generate(const PaintOpBuffer* paint_op_buffer,
   DiscardableImageGenerator generator(bounds.right(), bounds.bottom(),
                                       paint_op_buffer);
   generator.RecordColorHistograms();
-  image_id_to_region_ = generator.TakeImageIdToRegionMap();
+  image_id_to_rects_ = generator.TakeImageIdToRectsMap();
   animated_images_metadata_ = generator.TakeAnimatedImagesMetadata();
   decoding_mode_map_ = generator.TakeDecodingModeMap();
   all_images_are_srgb_ = generator.all_images_are_srgb();
@@ -375,16 +377,16 @@ void DiscardableImageMap::GetDiscardableImagesInRect(
   *images = images_rtree_.SearchRefs(rect);
 }
 
-const Region& DiscardableImageMap::GetRegionForImage(
+const DiscardableImageMap::Rects& DiscardableImageMap::GetRectsForImage(
     PaintImage::Id image_id) const {
-  static const Region kEmptyRegion;
-  auto it = image_id_to_region_.find(image_id);
-  return it == image_id_to_region_.end() ? kEmptyRegion : it->second;
+  static const Rects kEmptyRects;
+  auto it = image_id_to_rects_.find(image_id);
+  return it == image_id_to_rects_.end() ? kEmptyRects : it->second;
 }
 
 void DiscardableImageMap::Reset() {
-  image_id_to_region_.clear();
-  image_id_to_region_.shrink_to_fit();
+  image_id_to_rects_.clear();
+  image_id_to_rects_.shrink_to_fit();
   images_rtree_.Reset();
 }
 

cc/paint/discardable_image_map.h

@@ -9,7 +9,7 @@
 #include <vector>
 
 #include "base/containers/flat_map.h"
-#include "cc/base/region.h"
+#include "base/containers/stack_container.h"
 #include "cc/base/rtree.h"
 #include "cc/paint/draw_image.h"
 #include "cc/paint/image_animation_count.h"
@@ -31,6 +31,8 @@ class PaintOpBuffer;
 // rect and get back a list of DrawImages in that rect.
 class CC_PAINT_EXPORT DiscardableImageMap {
  public:
+  using Rects = base::StackVector<gfx::Rect, 1>;
+
   struct CC_PAINT_EXPORT AnimatedImageMetadata {
     AnimatedImageMetadata(
         PaintImage::Id paint_image_id,
@@ -51,10 +53,10 @@ class CC_PAINT_EXPORT DiscardableImageMap {
   DiscardableImageMap();
   ~DiscardableImageMap();
 
-  bool empty() const { return image_id_to_region_.empty(); }
+  bool empty() const { return image_id_to_rects_.empty(); }
   void GetDiscardableImagesInRect(const gfx::Rect& rect,
                                   std::vector<const DrawImage*>* images) const;
-  const Region& GetRegionForImage(PaintImage::Id image_id) const;
+  const Rects& GetRectsForImage(PaintImage::Id image_id) const;
   bool all_images_are_srgb() const { return all_images_are_srgb_; }
   const std::vector<AnimatedImageMetadata>& animated_images_metadata() const {
     return animated_images_metadata_;
@@ -77,7 +79,7 @@ class CC_PAINT_EXPORT DiscardableImageMap {
       std::vector<std::pair<DrawImage, gfx::Rect>> images,
       base::flat_map<PaintImage::Id, gfx::Rect> image_id_to_rect);
 
-  base::flat_map<PaintImage::Id, Region> image_id_to_region_;
+  base::flat_map<PaintImage::Id, Rects> image_id_to_rects_;
   std::vector<AnimatedImageMetadata> animated_images_metadata_;
   base::flat_map<PaintImage::Id, PaintImage::DecodingMode> decoding_mode_map_;
   bool all_images_are_srgb_ = false;

cc/paint/discardable_image_map_unittest.cc

@@ -68,7 +68,8 @@ class DiscardableImageMapTest : public testing::Test {
     for (DrawImage& image : image_map.images_rtree_.Search(rect)) {
       auto image_id = image.paint_image().stable_id();
       position_draw_images.push_back(PositionScaleDrawImage(
-          image.paint_image(), image_map.GetRegionForImage(image_id).bounds(),
+          image.paint_image(),
+          ImageRectsToRegion(image_map.GetRectsForImage(image_id)).bounds(),
           image.scale()));
     }
 
@@ -265,7 +266,7 @@ TEST_F(DiscardableImageMapTest, GetDiscardableImagesInRectNonZeroLayer) {
   // Image not present in the list.
   {
     PaintImage image = CreateDiscardablePaintImage(gfx::Size(500, 500));
-    EXPECT_TRUE(image_map.GetRegionForImage(image.stable_id()).IsEmpty());
+    EXPECT_EQ(image_map.GetRectsForImage(image.stable_id())->size(), 0u);
   }
 }
 
@@ -915,7 +916,8 @@ TEST_F(DiscardableImageMapTest, TracksImageRegions) {
     expected_region.Union(rect);
   }
 
-  EXPECT_EQ(image_map.GetRegionForImage(image.stable_id()), expected_region);
+  EXPECT_EQ(ImageRectsToRegion(image_map.GetRectsForImage(image.stable_id())),
+            expected_region);
 }
 
 class DiscardableImageMapColorSpaceTest

cc/test/skia_common.cc

@@ -46,6 +46,13 @@ bool AreDisplayListDrawingResultsSame(const gfx::Rect& layer_rect,
   return !memcmp(pixels_a.get(), pixels_b.get(), pixel_size);
 }
 
+Region ImageRectsToRegion(const DiscardableImageMap::Rects& rects) {
+  Region region;
+  for (const auto& r : rects.container())
+    region.Union(r);
+  return region;
+}
+
 sk_sp<PaintImageGenerator> CreatePaintImageGenerator(const gfx::Size& size) {
   return sk_make_sp<FakePaintImageGenerator>(
       SkImageInfo::MakeN32Premul(size.width(), size.height()));

cc/test/skia_common.h

@@ -8,6 +8,8 @@
 #include <memory>
 
 #include "base/memory/ref_counted.h"
+#include "cc/base/region.h"
+#include "cc/paint/discardable_image_map.h"
 #include "cc/paint/draw_image.h"
 #include "cc/paint/image_animation_count.h"
 #include "cc/paint/paint_image.h"
@@ -31,6 +33,8 @@ bool AreDisplayListDrawingResultsSame(const gfx::Rect& layer_rect,
                                       const DisplayItemList* list_a,
                                       const DisplayItemList* list_b);
 
+Region ImageRectsToRegion(const DiscardableImageMap::Rects& rects);
+
 sk_sp<PaintImageGenerator> CreatePaintImageGenerator(const gfx::Size& size);
 
 PaintImage CreateDiscardablePaintImage(

cc/trees/layer_tree_host_impl_unittest.cc

@@ -12916,10 +12916,10 @@ TEST_F(LayerTreeHostImplTest, CheckerImagingTileInvalidation) {
     else
       EXPECT_FALSE(tile->HasRasterTask());
   }
-  const auto& expected_invalidation =
-      raster_source->GetDisplayItemList()
-          ->discardable_image_map()
-          .GetRegionForImage(checkerable_image.stable_id());
+  const auto expected_invalidation =
+      ImageRectsToRegion(raster_source->GetDisplayItemList()
+                             ->discardable_image_map()
+                             .GetRectsForImage(checkerable_image.stable_id()));
   EXPECT_EQ(expected_invalidation, *(root->GetPendingInvalidation()));
 }