From aa1118f0cc6bcd93a336f65ab4413daab6bff8b3 Mon Sep 17 00:00:00 2001
From: Jim Graham <flar@google.com>
Date: Mon, 27 Nov 2023 13:26:45 -0800
Subject: [PATCH] SurfaceFrame root DisplayLists will no longer prepare an
 RTree

---
 display_list/display_list.cc        |  7 +-----
 flow/embedded_views.cc              |  1 +
 flow/surface_frame.cc               |  9 +++++--
 flow/surface_frame_unittests.cc     | 38 +++++++++++++++++++++--------
 lib/ui/painting/picture_recorder.cc |  6 +++--
 5 files changed, 41 insertions(+), 20 deletions(-)

diff --git a/display_list/display_list.cc b/display_list/display_list.cc
index e06bcf246d3d4..dfacba0c114f0 100644
--- a/display_list/display_list.cc
+++ b/display_list/display_list.cc
@@ -153,17 +153,12 @@ void DisplayList::Dispatch(DlOpReceiver& receiver,
   if (cull_rect.isEmpty()) {
     return;
   }
-  if (cull_rect.contains(bounds())) {
+  if (!has_rtree() || cull_rect.contains(bounds())) {
     Dispatch(receiver);
     return;
   }
   const DlRTree* rtree = this->rtree().get();
   FML_DCHECK(rtree != nullptr);
-  if (rtree == nullptr) {
-    FML_LOG(ERROR) << "dispatched with culling rect on DL with no rtree";
-    Dispatch(receiver);
-    return;
-  }
   uint8_t* ptr = storage_.get();
   std::vector<int> rect_indices;
   rtree->search(cull_rect, &rect_indices);
diff --git a/flow/embedded_views.cc b/flow/embedded_views.cc
index 1bf6e6c1d2911..e1936d1af7d20 100644
--- a/flow/embedded_views.cc
+++ b/flow/embedded_views.cc
@@ -18,6 +18,7 @@ DlCanvas* DisplayListEmbedderViewSlice::canvas() {
 
 void DisplayListEmbedderViewSlice::end_recording() {
   display_list_ = builder_->Build();
+  FML_DCHECK(display_list_->has_rtree());
   builder_ = nullptr;
 }
 
diff --git a/flow/surface_frame.cc b/flow/surface_frame.cc
index dd1b98d33b8b2..7f7e8a8da8773 100644
--- a/flow/surface_frame.cc
+++ b/flow/surface_frame.cc
@@ -31,8 +31,13 @@ SurfaceFrame::SurfaceFrame(sk_sp<SkSurface> surface,
     canvas_ = &adapter_;
   } else if (display_list_fallback) {
     FML_DCHECK(!frame_size.isEmpty());
-    dl_builder_ =
-        sk_make_sp<DisplayListBuilder>(SkRect::Make(frame_size), true);
+    // The root frame of a surface will be filled by the layer_tree which
+    // performs branch culling so it will be unlikely to need an rtree for
+    // further culling during `DisplayList::Dispatch`. Further, this canvas
+    // will live underneath any platform views so we do not need to compute
+    // exact coverage to describe "pixel ownership" to the platform.
+    dl_builder_ = sk_make_sp<DisplayListBuilder>(SkRect::Make(frame_size),
+                                                 /*prepare_rtree=*/false);
     canvas_ = dl_builder_.get();
   }
 }
diff --git a/flow/surface_frame_unittests.cc b/flow/surface_frame_unittests.cc
index 0f6f9ba3081fc..81516ad266dda 100644
--- a/flow/surface_frame_unittests.cc
+++ b/flow/surface_frame_unittests.cc
@@ -11,27 +11,45 @@ namespace flutter {
 
 TEST(FlowTest, SurfaceFrameDoesNotSubmitInDtor) {
   SurfaceFrame::FramebufferInfo framebuffer_info;
+  auto callback = [](const SurfaceFrame&, DlCanvas*) {
+    EXPECT_FALSE(true);
+    return true;
+  };
   auto surface_frame = std::make_unique<SurfaceFrame>(
-      /*surface=*/nullptr, framebuffer_info,
-      /*submit_callback=*/
-      [](const SurfaceFrame&, DlCanvas*) {
-        EXPECT_FALSE(true);
-        return true;
-      },
-      SkISize::Make(800, 600));
+      /*surface=*/nullptr,
+      /*framebuffer_info=*/framebuffer_info,
+      /*submit_callback=*/callback,
+      /*frame_size=*/SkISize::Make(800, 600));
   surface_frame.reset();
 }
 
 TEST(FlowTest, SurfaceFrameDoesNotHaveEmptyCanvas) {
   SurfaceFrame::FramebufferInfo framebuffer_info;
+  auto callback = [](const SurfaceFrame&, DlCanvas*) { return true; };
   SurfaceFrame frame(
-      /*surface=*/nullptr, framebuffer_info,
-      /*submit_callback=*/[](const SurfaceFrame&, DlCanvas*) { return true; },
+      /*surface=*/nullptr,
+      /*framebuffer_info=*/framebuffer_info,
+      /*submit_callback=*/callback,
       /*frame_size=*/SkISize::Make(800, 600),
-      /*context_result=*/nullptr, /*display_list_fallback=*/true);
+      /*context_result=*/nullptr,
+      /*display_list_fallback=*/true);
 
   EXPECT_FALSE(frame.Canvas()->GetLocalClipBounds().isEmpty());
   EXPECT_FALSE(frame.Canvas()->QuickReject(SkRect::MakeLTRB(10, 10, 50, 50)));
 }
 
+TEST(FlowTest, SurfaceFrameDoesNotPrepareRtree) {
+  SurfaceFrame::FramebufferInfo framebuffer_info;
+  auto callback = [](const SurfaceFrame&, DlCanvas*) { return true; };
+  auto surface_frame = std::make_unique<SurfaceFrame>(
+      /*surface=*/nullptr,
+      /*framebuffer_info=*/framebuffer_info,
+      /*submit_callback=*/callback,
+      /*frame_size=*/SkISize::Make(800, 600),
+      /*context_result=*/nullptr,
+      /*display_list_fallback=*/true);
+  surface_frame->Canvas()->DrawRect(SkRect::MakeWH(100, 100), DlPaint());
+  EXPECT_FALSE(surface_frame->BuildDisplayList()->has_rtree());
+}
+
 }  // namespace flutter
diff --git a/lib/ui/painting/picture_recorder.cc b/lib/ui/painting/picture_recorder.cc
index 6a24e9346992d..58d9d86632a7d 100644
--- a/lib/ui/painting/picture_recorder.cc
+++ b/lib/ui/painting/picture_recorder.cc
@@ -36,10 +36,12 @@ void PictureRecorder::endRecording(Dart_Handle dart_picture) {
     return;
   }
 
-  Picture::CreateAndAssociateWithDartWrapper(dart_picture,
-                                             display_list_builder_->Build());
+  auto display_list = display_list_builder_->Build();
   display_list_builder_ = nullptr;
 
+  FML_DCHECK(display_list->has_rtree());
+  Picture::CreateAndAssociateWithDartWrapper(dart_picture, display_list);
+
   canvas_->Invalidate();
   canvas_ = nullptr;
   ClearDartWrapper();