ImageFilterLayer Reset cache_children; Fix some comments

JsouLiang · JsouLiang · commit 9b68847b8e27 · 2022-06-27T15:11:45.000+08:00
diff --git a/flow/layers/cacheable_layer.cc b/flow/layers/cacheable_layer.cc
@@ -14,18 +14,18 @@ AutoCache::AutoCache(RasterCacheItem* raster_cache_item,
     : raster_cache_item_(raster_cache_item),
       context_(context),
       matrix_(matrix) {
-  if (cacheEnable()) {
+  if (IsCacheEnabled()) {
     raster_cache_item->PrerollSetup(context, matrix);
     current_index_ = context_->raster_cached_entries->size();
   }
 }
 
-bool AutoCache::cacheEnable() {
+bool AutoCache::IsCacheEnabled() {
   return raster_cache_item_ && context_ && context_->raster_cache;
 }
 
 AutoCache::~AutoCache() {
-  if (cacheEnable()) {
+  if (IsCacheEnabled()) {
     raster_cache_item_->PrerollFinalize(context_, matrix_);
   }
 }
diff --git a/flow/layers/cacheable_layer.h b/flow/layers/cacheable_layer.h
@@ -24,7 +24,7 @@ class AutoCache {
   ~AutoCache();
 
  private:
-  inline bool cacheEnable();
+  inline bool IsCacheEnabled();
   int current_index_;
   RasterCacheItem* raster_cache_item_ = nullptr;
   PrerollContext* context_ = nullptr;
diff --git a/flow/layers/clip_path_layer_unittests.cc b/flow/layers/clip_path_layer_unittests.cc
@@ -497,7 +497,7 @@ TEST_F(ClipPathLayerTest, LayerCached) {
   auto layer =
       std::make_shared<ClipPathLayer>(layer_clip, Clip::antiAliasWithSaveLayer);
   layer->Add(mock1);
-  SkPaint paint;
+
   auto initial_transform = SkMatrix::Translate(50.0, 25.5);
   SkMatrix cache_ctm = initial_transform;
   SkCanvas cache_canvas;
@@ -525,6 +525,7 @@ TEST_F(ClipPathLayerTest, LayerCached) {
   EXPECT_EQ(raster_cache()->GetLayerCachedEntriesCount(), (size_t)1);
   EXPECT_EQ(clip_cache_item->cache_state(),
             RasterCacheItem::CacheState::kCurrent);
+  SkPaint paint;
   EXPECT_TRUE(raster_cache()->Draw(clip_cache_item->GetId().value(),
                                    cache_canvas, &paint));
 }
diff --git a/flow/layers/clip_rect_layer_unittests.cc b/flow/layers/clip_rect_layer_unittests.cc
@@ -480,7 +480,6 @@ TEST_F(ClipRectLayerTest, OpacityInheritanceSaveLayerPainting) {
 TEST_F(ClipRectLayerTest, LayerCached) {
   auto path1 = SkPath().addRect({10, 10, 30, 30});
   auto mock1 = MockLayer::MakeOpacityCompatible(path1);
-  SkPaint paint;
   SkRect clip_rect = SkRect::MakeWH(500, 500);
   auto layer =
       std::make_shared<ClipRectLayer>(clip_rect, Clip::antiAliasWithSaveLayer);
@@ -511,6 +510,7 @@ TEST_F(ClipRectLayerTest, LayerCached) {
   EXPECT_EQ(raster_cache()->GetLayerCachedEntriesCount(), (size_t)1);
   EXPECT_EQ(clip_cache_item->cache_state(),
             RasterCacheItem::CacheState::kCurrent);
+  SkPaint paint;
   EXPECT_TRUE(raster_cache()->Draw(clip_cache_item->GetId().value(),
                                    cache_canvas, &paint));
 }
diff --git a/flow/layers/clip_rrect_layer_unittests.cc b/flow/layers/clip_rrect_layer_unittests.cc
@@ -526,7 +526,7 @@ TEST_F(ClipRRectLayerTest, LayerCached) {
 
 TEST_F(ClipRRectLayerTest, NoSaveLayerShouldNotCache) {
   auto path1 = SkPath().addRect({10, 10, 30, 30});
-  SkPaint paint = SkPaint();
+
   auto mock1 = MockLayer::MakeOpacityCompatible(path1);
   SkRect clip_rect = SkRect::MakeWH(500, 500);
   SkRRect clip_r_rect = SkRRect::MakeRectXY(clip_rect, 20, 20);
diff --git a/flow/layers/color_filter_layer_unittests.cc b/flow/layers/color_filter_layer_unittests.cc
@@ -341,17 +341,26 @@ TEST_F(ColorFilterLayerTest, CacheColorFilterLayerSelf) {
   layer->Paint(paint_context());
   // frame 2.
   layer->Preroll(preroll_context(), initial_transform);
+  LayerTree::TryToRasterCache(cacheable_items(), &paint_context());
+  // ColorFilterLayer default cache children.
+  EXPECT_EQ(cacheable_color_filter_item->cache_state(),
+            RasterCacheItem::CacheState::kChildren);
+  EXPECT_TRUE(raster_cache()->Draw(cacheable_color_filter_item->GetId().value(),
+                                   cache_canvas, &paint));
+  EXPECT_FALSE(raster_cache()->Draw(
+      cacheable_color_filter_item->GetId().value(), other_canvas, &paint));
   layer->Paint(paint_context());
+
   // frame 3.
   layer->Preroll(preroll_context(), initial_transform);
   layer->Paint(paint_context());
 
   LayerTree::TryToRasterCache(cacheable_items(), &paint_context());
-  // frame1,2 cache the ImageFilter's children layer, frame3 cache the
-  // ImageFilterLayer
+  // frame1,2 cache the ColorFilterLayer's children layer, frame3 cache the
+  // ColorFilterLayer
   EXPECT_EQ(raster_cache()->GetLayerCachedEntriesCount(), (size_t)2);
 
-  // ImageFilterLayer default cache itself.
+  // ColorFilterLayer default cache itself.
   EXPECT_EQ(cacheable_color_filter_item->cache_state(),
             RasterCacheItem::CacheState::kCurrent);
   EXPECT_EQ(cacheable_color_filter_item->GetId(),
@@ -382,7 +391,7 @@ TEST_F(ColorFilterLayerTest, OpacityInheritance) {
   PrerollContext* context = preroll_context();
   context->subtree_can_inherit_opacity = false;
   color_filter_layer->Preroll(preroll_context(), initial_transform);
-  // ImageFilterLayers can always inherit opacity whether or not their
+  // ColorFilterLayer can always inherit opacity whether or not their
   // children are compatible.
   EXPECT_TRUE(context->subtree_can_inherit_opacity);
 
diff --git a/flow/layers/container_layer.h b/flow/layers/container_layer.h
@@ -28,7 +28,7 @@ class ContainerLayer : public Layer {
   virtual void DiffChildren(DiffContext* context,
                             const ContainerLayer* old_layer);
 
-  void PaintChildren(PaintContext& context) const;
+  void PaintChildren(PaintContext& context) const override;
 
   const ContainerLayer* as_container_layer() const override { return this; }
 
diff --git a/flow/layers/display_list_raster_cache_item.cc b/flow/layers/display_list_raster_cache_item.cc
@@ -170,7 +170,9 @@ bool DisplayListRasterCacheItem::TryToPrepareRasterCache(
   };
   return context.raster_cache->UpdateCacheEntry(
       GetId().value(), r_context,
-      [=](SkCanvas* canvas) { display_list_->RenderTo(canvas); });
+      [display_list = display_list_](SkCanvas* canvas) {
+        display_list->RenderTo(canvas);
+      });
   return false;
 }
 }  // namespace flutter
diff --git a/flow/layers/image_filter_layer.cc b/flow/layers/image_filter_layer.cc
@@ -68,6 +68,10 @@ void ImageFilterLayer::Preroll(PrerollContext* context,
 
   set_paint_bounds(child_bounds);
 
+  // CacheChildren only when the transformed_filter_ doesn't equal null.
+  // So in here we reset the LayerRasterCacheItem cache state.
+  layer_raster_cache_item_->MarkNotCacheChildren();
+
   transformed_filter_ = filter_->makeWithLocalMatrix(matrix);
   if (transformed_filter_) {
     layer_raster_cache_item_->MarkCacheChildren();
diff --git a/flow/layers/layer.h b/flow/layers/layer.h
@@ -302,6 +302,8 @@ class Layer {
 
   virtual void Paint(PaintContext& context) const = 0;
 
+  virtual void PaintChildren(PaintContext& context) const { FML_DCHECK(false); }
+
   bool subtree_has_platform_view() const { return subtree_has_platform_view_; }
   void set_subtree_has_platform_view(bool value) {
     subtree_has_platform_view_ = value;
diff --git a/flow/layers/layer_raster_cache_item.cc b/flow/layers/layer_raster_cache_item.cc
@@ -25,8 +25,7 @@ void LayerRasterCacheItem::PrerollSetup(PrerollContext* context,
   if (context->raster_cache && context->raster_cached_entries) {
     context->raster_cached_entries->push_back(this);
     child_items_ = context->raster_cached_entries->size();
-    auto child_matrix = matrix;
-    matrix_ = child_matrix;
+    matrix_ = matrix;
   }
 }
 
@@ -114,8 +113,7 @@ bool LayerRasterCacheItem::Rasterize(const PaintContext& paint_context,
       layer_->Paint(context);
       break;
     case CacheState::kChildren:
-      FML_DCHECK(layer_->as_container_layer());
-      layer_->as_container_layer()->PaintChildren(context);
+      layer_->PaintChildren(context);
       break;
     case CacheState::kNone:
       FML_DCHECK(cache_state_ != CacheState::kNone);
@@ -144,7 +142,7 @@ bool LayerRasterCacheItem::TryToPrepareRasterCache(const PaintContext& context,
     };
     return context.raster_cache->UpdateCacheEntry(
         GetId().value(), r_context,
-        [=](SkCanvas* canvas) { Rasterize(context, canvas); });
+        [ctx = context, this](SkCanvas* canvas) { Rasterize(ctx, canvas); });
   }
   return false;
 }
diff --git a/flow/layers/layer_raster_cache_item.h b/flow/layers/layer_raster_cache_item.h
@@ -49,6 +49,8 @@ class LayerRasterCacheItem : public RasterCacheItem {
 
   void MarkCacheChildren() { can_cache_children_ = true; }
 
+  void MarkNotCacheChildren() { can_cache_children_ = false; }
+
   bool IsCacheChildren() const { return cache_state_ == CacheState::kChildren; }
 
  protected:
diff --git a/flow/raster_cache.cc b/flow/raster_cache.cc
@@ -43,10 +43,9 @@ void RasterCacheResult::draw(SkCanvas& canvas, const SkPaint* paint) const {
 }
 
 RasterCache::RasterCache(size_t access_threshold,
-                         size_t picture_and_display_list_cache_limit_per_frame)
+                         size_t display_list_cache_limit_per_frame)
     : access_threshold_(access_threshold),
-      picture_and_display_list_cache_limit_per_frame_(
-          picture_and_display_list_cache_limit_per_frame),
+      display_list_cache_limit_per_frame_(display_list_cache_limit_per_frame),
       checkerboard_images_(false) {}
 
 /// @note Procedure doesn't copy all closures.
@@ -102,10 +101,6 @@ bool RasterCache::UpdateCacheEntry(
           display_list_cached_this_frame_++;
           break;
         }
-        case RasterCacheKeyType::kPicture: {
-          picture_cached_this_frame_++;
-          break;
-        }
         default:
           break;
       }
@@ -165,7 +160,6 @@ bool RasterCache::Draw(const RasterCacheKeyID& id,
 }
 
 void RasterCache::PrepareNewFrame() {
-  picture_cached_this_frame_ = 0;
   display_list_cached_this_frame_ = 0;
 }
 
@@ -182,7 +176,7 @@ void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map<Entry>& cache,
     } else if (entry.image) {
       RasterCacheKeyKind kind = it->first.kind();
       switch (kind) {
-        case RasterCacheKeyKind::kPictureMetrics:
+        case RasterCacheKeyKind::kDisplayListMetrics:
           picture_metrics.in_use_count++;
           picture_metrics.in_use_bytes += entry.image->image_bytes();
           break;
@@ -199,7 +193,7 @@ void RasterCache::SweepOneCacheAfterFrame(RasterCacheKey::Map<Entry>& cache,
     if (it->second.image) {
       RasterCacheKeyKind kind = it->first.kind();
       switch (kind) {
-        case RasterCacheKeyKind::kPictureMetrics:
+        case RasterCacheKeyKind::kDisplayListMetrics:
           picture_metrics.eviction_count++;
           picture_metrics.eviction_bytes += it->second.image->image_bytes();
           break;
@@ -241,13 +235,13 @@ size_t RasterCache::GetLayerCachedEntriesCount() const {
 }
 
 size_t RasterCache::GetPictureCachedEntriesCount() const {
-  size_t picture_cached_entries_count = 0;
+  size_t display_list_cached_entries_count = 0;
   for (const auto& item : cache_) {
-    if (item.first.kind() == RasterCacheKeyKind::kPictureMetrics) {
-      picture_cached_entries_count++;
+    if (item.first.kind() == RasterCacheKeyKind::kDisplayListMetrics) {
+      display_list_cached_entries_count++;
     }
   }
-  return picture_cached_entries_count;
+  return display_list_cached_entries_count;
 }
 
 void RasterCache::SetCheckboardCacheImages(bool checkerboard) {
@@ -289,7 +283,7 @@ size_t RasterCache::EstimateLayerCacheByteSize() const {
 size_t RasterCache::EstimatePictureCacheByteSize() const {
   size_t picture_cache_bytes = 0;
   for (const auto& item : cache_) {
-    if (item.first.kind() == RasterCacheKeyKind::kPictureMetrics &&
+    if (item.first.kind() == RasterCacheKeyKind::kDisplayListMetrics &&
         item.second.image) {
       picture_cache_bytes += item.second.image->image_bytes();
     }
diff --git a/flow/raster_cache.h b/flow/raster_cache.h
@@ -186,9 +186,8 @@ class RasterCache {
 
   bool GenerateNewCacheInThisFrame() const {
     // Disabling caching when access_threshold is zero is historic behavior.
-    return access_threshold_ != 0 &&
-           picture_cached_this_frame_ + display_list_cached_this_frame_ <
-               picture_and_display_list_cache_limit_per_frame_;
+    return access_threshold_ != 0 && display_list_cached_this_frame_ <
+                                         display_list_cache_limit_per_frame_;
   }
 
   /**
@@ -217,8 +216,7 @@ class RasterCache {
                                RasterCacheMetrics& layer_metrics);
 
   const size_t access_threshold_;
-  const size_t picture_and_display_list_cache_limit_per_frame_;
-  mutable size_t picture_cached_this_frame_ = 0;
+  const size_t display_list_cache_limit_per_frame_;
   mutable size_t display_list_cached_this_frame_ = 0;
   RasterCacheMetrics layer_metrics_;
   RasterCacheMetrics picture_metrics_;
diff --git a/flow/raster_cache_key.h b/flow/raster_cache_key.h
@@ -18,12 +18,7 @@ namespace flutter {
 
 class Layer;
 
-enum class RasterCacheKeyType {
-  kLayer,
-  kPicture,
-  kDisplayList,
-  kLayerChildren
-};
+enum class RasterCacheKeyType { kLayer, kDisplayList, kLayerChildren };
 
 class RasterCacheKeyID {
  public:
@@ -60,7 +55,7 @@ class RasterCacheKeyID {
   const RasterCacheKeyType type_;
 };
 
-enum class RasterCacheKeyKind { kLayerMetrics, kPictureMetrics };
+enum class RasterCacheKeyKind { kLayerMetrics, kDisplayListMetrics };
 
 class RasterCacheKey {
  public:
@@ -79,9 +74,8 @@ class RasterCacheKey {
 
   RasterCacheKeyKind kind() const {
     switch (id_.type()) {
-      case RasterCacheKeyType::kPicture:
       case RasterCacheKeyType::kDisplayList:
-        return RasterCacheKeyKind::kPictureMetrics;
+        return RasterCacheKeyKind::kDisplayListMetrics;
       case RasterCacheKeyType::kLayer:
       case RasterCacheKeyType::kLayerChildren:
         return RasterCacheKeyKind::kLayerMetrics;
diff --git a/flow/raster_cache_unittests.cc b/flow/raster_cache_unittests.cc
@@ -531,7 +531,6 @@ TEST(RasterCache, RasterCacheKeyHashFunction) {
   SkMatrix matrix = SkMatrix::I();
   uint64_t id = 5;
   RasterCacheKey layer_key(id, RasterCacheKeyType::kLayer, matrix);
-  RasterCacheKey picture_key(id, RasterCacheKeyType::kPicture, matrix);
   RasterCacheKey display_list_key(id, RasterCacheKeyType::kDisplayList, matrix);
   RasterCacheKey layer_children_key(id, RasterCacheKeyType::kLayerChildren,
                                     matrix);
@@ -540,11 +539,6 @@ TEST(RasterCache, RasterCacheKeyHashFunction) {
   auto layer_hash_code = hash_function(layer_key);
   ASSERT_EQ(layer_hash_code, layer_cache_key_id.GetHash());
 
-  auto picture_cache_key_id =
-      RasterCacheKeyID(id, RasterCacheKeyType::kPicture);
-  auto picture_hash_code = hash_function(picture_key);
-  ASSERT_EQ(picture_hash_code, picture_cache_key_id.GetHash());
-
   auto display_list_cache_key_id =
       RasterCacheKeyID(id, RasterCacheKeyType::kDisplayList);
   auto display_list_hash_code = hash_function(display_list_key);
@@ -561,17 +555,14 @@ TEST(RasterCache, RasterCacheKeySameID) {
   SkMatrix matrix = SkMatrix::I();
   uint64_t id = 5;
   RasterCacheKey layer_key(id, RasterCacheKeyType::kLayer, matrix);
-  RasterCacheKey picture_key(id, RasterCacheKeyType::kPicture, matrix);
   RasterCacheKey display_list_key(id, RasterCacheKeyType::kDisplayList, matrix);
   RasterCacheKey layer_children_key(id, RasterCacheKeyType::kLayerChildren,
                                     matrix);
   map[layer_key] = 100;
-  map[picture_key] = 200;
   map[display_list_key] = 300;
   map[layer_children_key] = 400;
 
   ASSERT_EQ(map[layer_key], 100);
-  ASSERT_EQ(map[picture_key], 200);
   ASSERT_EQ(map[display_list_key], 300);
   ASSERT_EQ(map[layer_children_key], 400);
 }
@@ -591,7 +582,7 @@ TEST(RasterCache, RasterCacheKeySameType) {
   ASSERT_EQ(map[layer_second_key], 100);
   ASSERT_EQ(map[layer_third_key], 150);
 
-  type = RasterCacheKeyType::kPicture;
+  type = RasterCacheKeyType::kDisplayList;
   RasterCacheKey picture_first_key(20, type, matrix);
   RasterCacheKey picture_second_key(25, type, matrix);
   RasterCacheKey picture_third_key(30, type, matrix);
diff --git a/flow/testing/mock_raster_cache.cc b/flow/testing/mock_raster_cache.cc
@@ -74,13 +74,13 @@ void MockRasterCache::AddMockPicture(int width, int height) {
       .checkerboard       = preroll_context_.checkerboard_offscreen_layers,
       // clang-format on
   };
-  UpdateCacheEntry(
-      RasterCacheKeyID(display_list->unique_id(), RasterCacheKeyType::kPicture),
-      r_context, [&](SkCanvas* canvas) {
-        SkRect cache_rect = RasterCacheUtil::GetDeviceBounds(
-            r_context.logical_rect, r_context.matrix);
-        return std::make_unique<MockRasterCacheResult>(cache_rect);
-      });
+  UpdateCacheEntry(RasterCacheKeyID(display_list->unique_id(),
+                                    RasterCacheKeyType::kDisplayList),
+                   r_context, [&](SkCanvas* canvas) {
+                     SkRect cache_rect = RasterCacheUtil::GetDeviceBounds(
+                         r_context.logical_rect, r_context.matrix);
+                     return std::make_unique<MockRasterCacheResult>(cache_rect);
+                   });
 }
 
 static std::vector<RasterCacheItem*> raster_cache_items_;

Original file line number	Diff line number	Diff line change
`@@ -14,18 +14,18 @@ AutoCache::AutoCache(RasterCacheItem* raster_cache_item,`
`14`	`14`	`: raster_cache_item_(raster_cache_item),`
`15`	`15`	`context_(context),`
`16`	`16`	`matrix_(matrix) {`
`17`		`- if (cacheEnable()) {`
	`17`	`+ if (IsCacheEnabled()) {`
`18`	`18`	`raster_cache_item->PrerollSetup(context, matrix);`
`19`	`19`	`current_index_ = context_->raster_cached_entries->size();`
`20`	`20`	`}`
`21`	`21`	`}`
`22`	`22`
`23`		`-bool AutoCache::cacheEnable() {`
	`23`	`+bool AutoCache::IsCacheEnabled() {`
`24`	`24`	`return raster_cache_item_ && context_ && context_->raster_cache;`
`25`	`25`	`}`
`26`	`26`
`27`	`27`	`AutoCache::~AutoCache() {`
`28`		`- if (cacheEnable()) {`
	`28`	`+ if (IsCacheEnabled()) {`
`29`	`29`	`raster_cache_item_->PrerollFinalize(context_, matrix_);`
`30`	`30`	`}`
`31`	`31`	`}`