From f8048be39a7c1a3821d3e648122a5c12278b7610 Mon Sep 17 00:00:00 2001
From: Kaushik Iska <iska.kaushik@gmail.com>
Date: Wed, 9 Nov 2022 05:57:29 -0500
Subject: [PATCH] [Reland] Add rects to accumulator rather than bounds (#37435)
 (#37451)

* [Reland] Add rects to accumulator rather than bounds (#37435)

When the accumulator is an `RTreeBoundsAccumulator` rather than a `RectBoundsAccumulator` just accumulating the bounds results in incorrect results as the `rtree` would need to be aware of the constituent non-overlapping rectangles. This would work fine for `RectBoundsAccumulator` as it would just adjust its bounds based on the passed rects.

Fixes: https://github.com/flutter/flutter/issues/113251

* Add a test for nested display list rtree
---
 display_list/display_list_unittests.cc | 20 ++++++++++++++++++++
 display_list/display_list_utils.cc     | 18 +++++++++++++++++-
 display_list/display_list_utils.h      | 15 +++++++++++++++
 testing/scenario_app/run_ios_tests.sh  |  2 --
 4 files changed, 52 insertions(+), 3 deletions(-)

diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc
index 9ca2877a046a6..2a9e5d7cf7e27 100644
--- a/display_list/display_list_unittests.cc
+++ b/display_list/display_list_unittests.cc
@@ -1650,6 +1650,26 @@ TEST(DisplayList, RTreeOfSaveLayerFilterScene) {
   test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
 }
 
+TEST(DisplayList, NestedDisplayListRTreesAreSparse) {
+  DisplayListBuilder nested_dl_builder;
+  nested_dl_builder.drawRect({10, 10, 20, 20});
+  nested_dl_builder.drawRect({50, 50, 60, 60});
+  auto nested_display_list = nested_dl_builder.Build();
+
+  DisplayListBuilder builder;
+  builder.drawDisplayList(nested_display_list);
+  auto display_list = builder.Build();
+
+  auto rtree = display_list->rtree();
+  std::vector<SkRect> rects = {
+      {10, 10, 20, 20},
+      {50, 50, 60, 60},
+  };
+
+  // Hitting both sub-dl drawRect calls
+  test_rtree(rtree, {19, 19, 51, 51}, rects, {0, 1});
+}
+
 TEST(DisplayList, RemoveUnnecessarySaveRestorePairs) {
   {
     DisplayListBuilder builder;
diff --git a/display_list/display_list_utils.cc b/display_list/display_list_utils.cc
index 391723ccd1d13..ff8ddd534dca5 100644
--- a/display_list/display_list_utils.cc
+++ b/display_list/display_list_utils.cc
@@ -672,7 +672,23 @@ void DisplayListBoundsCalculator::drawPicture(const sk_sp<SkPicture> picture,
 }
 void DisplayListBoundsCalculator::drawDisplayList(
     const sk_sp<DisplayList> display_list) {
-  AccumulateOpBounds(display_list->bounds(), kDrawDisplayListFlags);
+  const SkRect bounds = display_list->bounds();
+  switch (accumulator_.type()) {
+    case BoundsAccumulatorType::kRect:
+      AccumulateOpBounds(bounds, kDrawDisplayListFlags);
+      return;
+    case BoundsAccumulatorType::kRTree:
+      std::list<SkRect> rects =
+          display_list->rtree()->searchNonOverlappingDrawnRects(bounds);
+      for (const SkRect& rect : rects) {
+        // TODO (https://github.com/flutter/flutter/issues/114919): Attributes
+        // are not necessarily `kDrawDisplayListFlags`.
+        AccumulateOpBounds(rect, kDrawDisplayListFlags);
+      }
+      return;
+  }
+
+  FML_UNREACHABLE();
 }
 void DisplayListBoundsCalculator::drawTextBlob(const sk_sp<SkTextBlob> blob,
                                                SkScalar x,
diff --git a/display_list/display_list_utils.h b/display_list/display_list_utils.h
index ff2bad1544fd7..406e852fbcae9 100644
--- a/display_list/display_list_utils.h
+++ b/display_list/display_list_utils.h
@@ -337,6 +337,11 @@ class ClipBoundsDispatchHelper : public virtual Dispatcher,
   void intersect(const SkRect& clipBounds, bool is_aa);
 };
 
+enum class BoundsAccumulatorType {
+  kRect,
+  kRTree,
+};
+
 class BoundsAccumulator {
  public:
   /// function definition for modifying the bounds of a rectangle
@@ -393,6 +398,8 @@ class BoundsAccumulator {
   virtual bool restore(
       std::function<bool(const SkRect& original, SkRect& modified)> map,
       const SkRect* clip = nullptr) = 0;
+
+  virtual BoundsAccumulatorType type() const = 0;
 };
 
 class RectBoundsAccumulator final : public virtual BoundsAccumulator {
@@ -414,6 +421,10 @@ class RectBoundsAccumulator final : public virtual BoundsAccumulator {
     return rect_.bounds();
   }
 
+  BoundsAccumulatorType type() const override {
+    return BoundsAccumulatorType::kRect;
+  }
+
  private:
   class AccumulationRect {
    public:
@@ -455,6 +466,10 @@ class RTreeBoundsAccumulator final : public virtual BoundsAccumulator {
 
   sk_sp<DlRTree> rtree() const;
 
+  BoundsAccumulatorType type() const override {
+    return BoundsAccumulatorType::kRTree;
+  }
+
  private:
   std::vector<SkRect> rects_;
   std::vector<size_t> saved_offsets_;
diff --git a/testing/scenario_app/run_ios_tests.sh b/testing/scenario_app/run_ios_tests.sh
index d917015d35d1d..af3a207ff05cc 100755
--- a/testing/scenario_app/run_ios_tests.sh
+++ b/testing/scenario_app/run_ios_tests.sh
@@ -52,12 +52,10 @@ echo "Running simulator tests with Impeller"
 echo ""
 
 # Skip testFontRenderingWhenSuppliedWithBogusFont: https://github.com/flutter/flutter/issues/113250
-# Skip testOneOverlayAndTwoIntersectingOverlays: https://github.com/flutter/flutter/issues/113251
 set -o pipefail && xcodebuild -sdk iphonesimulator \
   -scheme Scenarios \
   -destination 'platform=iOS Simulator,OS=13.0,name=iPhone 8' \
   clean test \
   FLUTTER_ENGINE="$FLUTTER_ENGINE" \
   -skip-testing "ScenariosUITests/BogusFontTextTest/testFontRenderingWhenSuppliedWithBogusFont" \
-  -skip-testing "ScenariosUITests/UnobstructedPlatformViewTests/testOneOverlayAndTwoIntersectingOverlays" \
   INFOPLIST_FILE="Scenarios/Info_Impeller.plist" # Plist with FLTEnableImpeller=YES