From 182e1189873bbec33d96b59f20d3b365e7aaf9ba Mon Sep 17 00:00:00 2001
From: Dan Field <dnfield@google.com>
Date: Fri, 28 Jul 2023 08:18:54 -0700
Subject: [PATCH] [Impeller] Avoid culling when the current matrix has
 perspective. (#44089)

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

Before this patch, the test would fail to render anything because the culling would decide it fell outside the cull rect when the transform has perspective sometimes. We should fix our Rect::TransformBounds implementation, but I can't quite seem to get it happy enough here so I'm just bailing out on culling if there is perspective instead, which is safe. This also makes the patch a bit easier/safer to cherry pick since it's a simple de-optimization when perspective is involved for the sake of fidelity, instead of a larger change that may have other side effects.

Filed https://github.com/flutter/flutter/issues/131445 to track the perspective issue.
---
 impeller/display_list/dl_dispatcher.cc |  4 ++-
 impeller/display_list/dl_unittests.cc  | 37 ++++++++++++++++++++++++++
 impeller/geometry/matrix.h             |  4 +++
 3 files changed, 44 insertions(+), 1 deletion(-)

diff --git a/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc
index 9964e9e71eaf9..9e71de28e1886 100644
--- a/impeller/display_list/dl_dispatcher.cc
+++ b/impeller/display_list/dl_dispatcher.cc
@@ -1074,7 +1074,9 @@ void DlDispatcher::drawDisplayList(
     canvas_.SaveLayer(save_paint);
   }
 
-  if (display_list->has_rtree()) {
+  // TODO(131445): Remove this restriction if we can correctly cull with
+  // perspective transforms.
+  if (display_list->has_rtree() && !initial_matrix_.HasPerspective()) {
     // The canvas remembers the screen-space culling bounds clipped by
     // the surface and the history of clip calls. DisplayList can cull
     // the ops based on a rectangle expressed in its "destination bounds"
diff --git a/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc
index 98ddf75f7c9ea..a902668baa277 100644
--- a/impeller/display_list/dl_unittests.cc
+++ b/impeller/display_list/dl_unittests.cc
@@ -21,6 +21,7 @@
 #include "impeller/display_list/dl_dispatcher.h"
 #include "impeller/display_list/dl_image_impeller.h"
 #include "impeller/display_list/dl_playground.h"
+#include "impeller/entity/contents/solid_color_contents.h"
 #include "impeller/entity/contents/solid_rrect_blur_contents.h"
 #include "impeller/geometry/constants.h"
 #include "impeller/geometry/point.h"
@@ -854,6 +855,42 @@ TEST_P(DisplayListTest, CanDrawShadow) {
   ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
 }
 
+TEST_P(DisplayListTest,
+       DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists) {
+  // Regression test for https://github.com/flutter/flutter/issues/130613
+  flutter::DisplayListBuilder sub_builder(true);
+  sub_builder.DrawRect(SkRect::MakeXYWH(0, 0, 50, 50),
+                       flutter::DlPaint(flutter::DlColor::kRed()));
+  auto display_list = sub_builder.Build();
+
+  DlDispatcher dispatcher(Rect::MakeLTRB(0, 0, 2400, 1800));
+  dispatcher.scale(2.0, 2.0);
+  dispatcher.translate(-93.0, 0.0);
+  // clang-format off
+  dispatcher.transformFullPerspective(
+     0.8, -0.2, -0.1, -0.0,
+     0.0,  1.0,  0.0,  0.0,
+     1.4,  1.3,  1.0,  0.0,
+    63.2, 65.3, 48.6,  1.1
+  );
+  // clang-format on
+  dispatcher.translate(35.0, 75.0);
+  dispatcher.drawDisplayList(display_list, 1.0f);
+  auto picture = dispatcher.EndRecordingAsPicture();
+
+  bool found = false;
+  picture.pass->IterateAllEntities([&found](Entity& entity) {
+    if (std::static_pointer_cast<SolidColorContents>(entity.GetContents())
+            ->GetColor() == Color::Red()) {
+      found = true;
+      return false;
+    }
+
+    return true;
+  });
+  EXPECT_TRUE(found);
+}
+
 TEST_P(DisplayListTest, TransparentShadowProducesCorrectColor) {
   DlDispatcher dispatcher;
   dispatcher.save();
diff --git a/impeller/geometry/matrix.h b/impeller/geometry/matrix.h
index 6938cf930fffa..119702f6130a8 100644
--- a/impeller/geometry/matrix.h
+++ b/impeller/geometry/matrix.h
@@ -313,6 +313,10 @@ struct Matrix {
             m[9] == 0 && m[10] == 1 && m[11] == 0 && m[14] == 0 && m[15] == 1);
   }
 
+  constexpr bool HasPerspective() const {
+    return m[3] != 0 || m[7] != 0 || m[11] != 0 || m[15] != 1;
+  }
+
   constexpr bool IsAligned(Scalar tolerance = 0) const {
     int v[] = {!ScalarNearlyZero(m[0], tolerance),  //
                !ScalarNearlyZero(m[1], tolerance),  //