[Impeller] Don't cull readbacks outside the damage rect. (#46705)

Resolves flutter/flutter#136058.

The damage rect properly expands to include the backdrop filter readback area to a certain extent, but it suddenly gets excluded once it no longer partially overlaps with the damage rect. This caused the illusion of EntityPass improperly culling the backdrop entity, leading me into a bit of a wild goose chase.

Before:

https://github.com/flutter/engine/assets/919017/94b8c077-0945-4a2c-96e0-27230d980c38

After:

https://github.com/flutter/engine/assets/919017/f1c78365-6e9b-46cb-9e69-33472d488831

Author: bdero

flow/diff_context.cc

@@ -123,12 +123,16 @@ Damage DiffContext::ComputeDamage(const SkIRect& accumulated_buffer_damage,
   SkRect frame_damage(damage_);
 
   for (const auto& r : readbacks_) {
-    SkRect rect = SkRect::Make(r.rect);
-    if (rect.intersects(frame_damage)) {
-      frame_damage.join(rect);
-    }
-    if (rect.intersects(buffer_damage)) {
-      buffer_damage.join(rect);
+    SkRect paint_rect = SkRect::Make(r.paint_rect);
+    SkRect readback_rect = SkRect::Make(r.readback_rect);
+    // Changes either in readback or paint rect require repainting both readback
+    // and paint rect.
+    if (paint_rect.intersects(frame_damage) ||
+        readback_rect.intersects(frame_damage)) {
+      frame_damage.join(readback_rect);
+      frame_damage.join(paint_rect);
+      buffer_damage.join(readback_rect);
+      buffer_damage.join(paint_rect);
     }
   }
 
@@ -220,9 +224,11 @@ void DiffContext::AddExistingPaintRegion(const PaintRegion& region) {
   }
 }
 
-void DiffContext::AddReadbackRegion(const SkIRect& rect) {
+void DiffContext::AddReadbackRegion(const SkIRect& paint_rect,
+                                    const SkIRect& readback_rect) {
   Readback readback;
-  readback.rect = rect;
+  readback.paint_rect = paint_rect;
+  readback.readback_rect = readback_rect;
   readback.position = rects_->size();
   // Push empty rect as a placeholder for position in current subtree
   rects_->push_back(SkRect::MakeEmpty());

flow/diff_context.h

@@ -123,8 +123,12 @@ class DiffContext {
   // The idea of readback region is that if any part of the readback region
   // needs to be repainted, then the whole readback region must be repainted;
   //
-  // Readback rect is in screen coordinates.
-  void AddReadbackRegion(const SkIRect& rect);
+  // paint_rect - rectangle where the filter paints contents (in screen
+  //              coordinates)
+  // readback_rect - rectangle where the filter samples from (in screen
+  //                 coordinates)
+  void AddReadbackRegion(const SkIRect& paint_rect,
+                         const SkIRect& readback_rect);
 
   // Returns the paint region for current subtree; Each rect in paint region is
   // in screen coordinates; Once a layer accumulates the paint regions of its
@@ -261,8 +265,11 @@ class DiffContext {
     // determine if subtree has any readback
     size_t position;
 
-    // readback area, in screen coordinates
-    SkIRect rect;
+    // Paint region of the filter performing readback, in screen coordinates.
+    SkIRect paint_rect;
+
+    // Readback area of the filter, in screen coordinates.
+    SkIRect readback_rect;
   };
 
   std::vector<Readback> readbacks_;

flow/layers/backdrop_filter_layer.cc

@@ -31,7 +31,7 @@ void BackdropFilterLayer::Diff(DiffContext* context, const Layer* old_layer) {
     SkIRect filter_input_bounds;  // in screen coordinates
     filter_->get_input_device_bounds(
         filter_target_bounds, context->GetTransform3x3(), filter_input_bounds);
-    context->AddReadbackRegion(filter_input_bounds);
+    context->AddReadbackRegion(filter_target_bounds, filter_input_bounds);
   }
 
   DiffChildren(context, prev);

flow/layers/backdrop_filter_layer_unittests.cc

@@ -469,7 +469,6 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) {
   auto path1 = SkPath().addRect(SkRect::MakeLTRB(180, 180, 190, 190));
   l4.root()->Add(std::make_shared<MockLayer>(path1));
   damage = DiffLayerTree(l4, l3);
-
   EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(180, 180, 190, 190));
 
   MockLayerTree l5;
@@ -482,6 +481,31 @@ TEST_F(BackdropLayerDiffTest, BackdropLayer) {
   EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(0, 0, 190, 190));
 }
 
+TEST_F(BackdropLayerDiffTest, ReadbackOutsideOfPaintArea) {
+  auto filter = DlMatrixImageFilter(SkMatrix::Translate(50, 50),
+                                    DlImageSampling::kLinear);
+
+  MockLayerTree l1(SkISize::Make(100, 100));
+
+  auto clip = std::make_shared<ClipRectLayer>(SkRect::MakeLTRB(60, 60, 80, 80),
+                                              Clip::hardEdge);
+  clip->Add(std::make_shared<BackdropFilterLayer>(filter.shared(),
+                                                  DlBlendMode::kSrcOver));
+  l1.root()->Add(clip);
+  auto damage = DiffLayerTree(l1, MockLayerTree(SkISize::Make(100, 100)));
+
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(60 - 50, 60 - 50, 80, 80));
+
+  MockLayerTree l2(SkISize::Make(100, 100));
+  // path inside readback area must trigger whole readback repaint + filter
+  // repaint.
+  auto path2 = SkPath().addRect(SkRect::MakeXYWH(60 - 50, 60 - 50, 10, 10));
+  l2.root()->Add(clip);
+  l2.root()->Add(std::make_shared<MockLayer>(path2));
+  damage = DiffLayerTree(l2, l1);
+  EXPECT_EQ(damage.frame_damage, SkIRect::MakeLTRB(60 - 50, 60 - 50, 80, 80));
+}
+
 TEST_F(BackdropLayerDiffTest, BackdropLayerInvalidTransform) {
   auto filter = DlBlurImageFilter(10, 10, DlTileMode::kClamp);