ImageFilterLayer should adjust child layer bounds during diffing (flutter/engine#29317)

engine/src/flutter/flow/diff_context.cc

@@ -22,6 +22,7 @@ DiffContext::DiffContext(SkISize frame_size,
 void DiffContext::BeginSubtree() {
   state_stack_.push_back(state_);
   state_.rect_index_ = rects_->size();
+  state_.has_filter_bounds_adjustment = false;
   if (state_.transform_override) {
     state_.transform = *state_.transform_override;
     state_.transform_override = std::nullopt;
@@ -30,12 +31,18 @@ void DiffContext::BeginSubtree() {
 
 void DiffContext::EndSubtree() {
   FML_DCHECK(!state_stack_.empty());
+  if (state_.has_filter_bounds_adjustment) {
+    filter_bounds_adjustment_stack_.pop_back();
+  }
   state_ = std::move(state_stack_.back());
   state_stack_.pop_back();
 }
 
 DiffContext::State::State()
-    : dirty(false), cull_rect(kGiantRect), rect_index_(0) {}
+    : dirty(false),
+      cull_rect(kGiantRect),
+      rect_index_(0),
+      has_filter_bounds_adjustment(false) {}
 
 void DiffContext::PushTransform(const SkMatrix& transform) {
   state_.transform.preConcat(transform);
@@ -45,6 +52,21 @@ void DiffContext::SetTransform(const SkMatrix& transform) {
   state_.transform_override = transform;
 }
 
+void DiffContext::PushFilterBoundsAdjustment(FilterBoundsAdjustment filter) {
+  FML_DCHECK(state_.has_filter_bounds_adjustment == false);
+  state_.has_filter_bounds_adjustment = true;
+  filter_bounds_adjustment_stack_.push_back(filter);
+}
+
+SkRect DiffContext::ApplyFilterBoundsAdjustment(SkRect rect) const {
+  // Apply filter bounds adjustment in reverse order
+  for (auto i = filter_bounds_adjustment_stack_.rbegin();
+       i != filter_bounds_adjustment_stack_.rend(); ++i) {
+    rect = (*i)(rect);
+  }
+  return rect;
+}
+
 Damage DiffContext::ComputeDamage(
     const SkIRect& accumulated_buffer_damage) const {
   SkRect buffer_damage = SkRect::Make(accumulated_buffer_damage);
@@ -100,10 +122,12 @@ void DiffContext::AddLayerBounds(const SkRect& rect) {
   // During painting we cull based on non-overriden transform and then
   // override the transform right before paint. Do the same thing here to get
   // identical paint rect.
-  auto transformed_rect = state_.transform.mapRect(rect);
+  auto transformed_rect =
+      ApplyFilterBoundsAdjustment(state_.transform.mapRect(rect));
   if (transformed_rect.intersects(state_.cull_rect)) {
     auto paint_rect = state_.transform_override
-                          ? state_.transform_override->mapRect(rect)
+                          ? ApplyFilterBoundsAdjustment(
+                                state_.transform_override->mapRect(rect))
                           : transformed_rect;
     rects_->push_back(paint_rect);
     if (IsSubtreeDirty()) {

engine/src/flutter/flow/diff_context.h

@@ -5,6 +5,7 @@
 #ifndef FLUTTER_FLOW_DIFF_CONTEXT_H_
 #define FLUTTER_FLOW_DIFF_CONTEXT_H_
 
+#include <functional>
 #include <map>
 #include <optional>
 #include <vector>
@@ -75,6 +76,14 @@ class DiffContext {
   // Pushes cull rect for current subtree
   bool PushCullRect(const SkRect& clip);
 
+  // Function that adjusts layer bounds (in device coordinates) depending
+  // on filter.
+  using FilterBoundsAdjustment = std::function<SkRect(SkRect)>;
+
+  // Pushes filter bounds adjustment to current subtree. Every layer in this
+  // subtree will have bounds adjusted by this function.
+  void PushFilterBoundsAdjustment(FilterBoundsAdjustment filter);
+
   // Returns transform matrix for current subtree
   const SkMatrix& GetTransform() const { return state_.transform; }
 
@@ -191,6 +200,10 @@ class DiffContext {
     SkMatrix transform;
     std::optional<SkMatrix> transform_override;
     size_t rect_index_;
+
+    // Whether this subtree has filter bounds adjustment function. If so,
+    // it will need to be removed from stack when subtree is closed.
+    bool has_filter_bounds_adjustment;
   };
 
   std::shared_ptr<std::vector<SkRect>> rects_;
@@ -198,6 +211,11 @@ class DiffContext {
   SkISize frame_size_;
   double frame_device_pixel_ratio_;
   std::vector<State> state_stack_;
+  std::vector<FilterBoundsAdjustment> filter_bounds_adjustment_stack_;
+
+  // Applies the filter bounds adjustment stack on provided rect.
+  // Rect must be in device coordinates.
+  SkRect ApplyFilterBoundsAdjustment(SkRect rect) const;
 
   SkRect damage_ = SkRect::MakeEmpty();
 

engine/src/flutter/flow/layers/image_filter_layer.cc

@@ -23,20 +23,18 @@ void ImageFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
     }
   }
 
-  DiffChildren(context, prev);
-
-  SkMatrix inverse;
-  if (context->GetTransform().invert(&inverse)) {
-    auto screen_bounds = context->CurrentSubtreeRegion().ComputeBounds();
-
+  if (filter_) {
     auto filter = filter_->makeWithLocalMatrix(context->GetTransform());
-
-    auto filter_bounds =
-        filter->filterBounds(screen_bounds.roundOut(), SkMatrix::I(),
-                             SkImageFilter::kForward_MapDirection);
-    context->AddLayerBounds(inverse.mapRect(SkRect::Make(filter_bounds)));
+    if (filter) {
+      // This transform will be applied to every child rect in the subtree
+      context->PushFilterBoundsAdjustment([filter](SkRect rect) {
+        return SkRect::Make(
+            filter->filterBounds(rect.roundOut(), SkMatrix::I(),
+                                 SkImageFilter::kForward_MapDirection));
+      });
+    }
   }
-
+  DiffChildren(context, prev);
   context->SetLayerPaintRegion(this, context->CurrentSubtreeRegion());
 }
 

engine/src/flutter/flow/layers/image_filter_layer_unittests.cc

@@ -385,6 +385,46 @@ TEST_F(ImageFilterLayerDiffTest, ImageFilterLayer) {
   EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(130, 130, 141, 141));
 }
 
+TEST_F(ImageFilterLayerDiffTest, ImageFilterLayerInflatestChildSize) {
+  auto filter = SkImageFilters::Blur(10, 10, SkTileMode::kClamp, nullptr);
+
+  {
+    // tests later assume 30px paint area, fail early if that's not the case
+    auto paint_rect =
+        filter->filterBounds(SkIRect::MakeWH(10, 10), SkMatrix::I(),
+                             SkImageFilter::kForward_MapDirection);
+    EXPECT_EQ(paint_rect, SkIRect::MakeLTRB(-30, -30, 40, 40));
+  }
+
+  MockLayerTree l1;
+
+  // Use nested filter layers to check if both contribute to child bounds
+  auto filter_layer_1_1 = std::make_shared<ImageFilterLayer>(filter);
+  auto filter_layer_1_2 = std::make_shared<ImageFilterLayer>(filter);
+  filter_layer_1_1->Add(filter_layer_1_2);
+  auto path = SkPath().addRect(SkRect::MakeLTRB(100, 100, 110, 110));
+  filter_layer_1_2->Add(
+      std::make_shared<MockLayer>(path, SkPaint(SkColors::kYellow)));
+  l1.root()->Add(filter_layer_1_1);
+
+  // second layer tree with identical filter layers but different child layer
+  MockLayerTree l2;
+  auto filter_layer2_1 = std::make_shared<ImageFilterLayer>(filter);
+  filter_layer2_1->AssignOldLayer(filter_layer_1_1.get());
+  auto filter_layer2_2 = std::make_shared<ImageFilterLayer>(filter);
+  filter_layer2_2->AssignOldLayer(filter_layer_1_2.get());
+  filter_layer2_1->Add(filter_layer2_2);
+  filter_layer2_2->Add(
+      std::make_shared<MockLayer>(path, SkPaint(SkColors::kRed)));
+  l2.root()->Add(filter_layer2_1);
+
+  DiffLayerTree(l1, MockLayerTree());
+  auto damage = DiffLayerTree(l2, l1);
+
+  // ensure that filter properly inflated child size
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(40, 40, 170, 170));
+}
+
 #endif
 
 }  // namespace testing