From 90709e4e33a71c88272a99c4830e2bd95302ae5d Mon Sep 17 00:00:00 2001
From: Jonah Williams <jonahwilliams@google.com>
Date: Fri, 13 Oct 2023 12:34:55 -0700
Subject: [PATCH] [Impeller] fix clear color optimization for large subpasses.
 (flutter/engine#46887)

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

The cause of this bug is that we use different sizes to compute the clear color value when constructing the render target and when skipping entities:

https://github.com/flutter/engine/blob/main/impeller/entity/entity_pass.cc#L632
https://github.com/flutter/engine/blob/main/impeller/entity/entity_pass.cc#L731 and
https://github.com/flutter/engine/blob/main/impeller/entity/entity_pass.cc#L900C3-L900C3

Either the former should use the root pass size or the later should use the subpass size. This usually isn't an issue because if something covers the root pass size it generally always covers the subpass size. But during the page transition, the scaled subpass ends up slightly bigger than the root pass size and so these conditions are mismatched:

Subpass size: (1550, 3188) root pass size (1440, 3036)

I think subpass size is correct. If the subpass is larger than the parent, then checking the parent size will give incorrect result.
---
 .../flutter/impeller/aiks/aiks_unittests.cc   | 25 ++++++++++++++++++-
 .../flutter/impeller/entity/entity_pass.cc    |  5 ++--
 2 files changed, 27 insertions(+), 3 deletions(-)

diff --git a/engine/src/flutter/impeller/aiks/aiks_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_unittests.cc
index 1a50dbe3e94..3dda081b04a 100644
--- a/engine/src/flutter/impeller/aiks/aiks_unittests.cc
+++ b/engine/src/flutter/impeller/aiks/aiks_unittests.cc
@@ -2134,9 +2134,12 @@ TEST_P(AiksTest, CanRenderClippedLayers) {
     canvas.DrawRect(Rect::MakeSize(Size{400, 400}), {.color = Color::White()});
     // Fill the layer with green, but do so with a color blend that can't be
     // collapsed into the parent pass.
+    // TODO(jonahwilliams): this blend mode was changed from color burn to
+    // hardlight to work around https://github.com/flutter/flutter/issues/136554
+    // .
     canvas.DrawRect(
         Rect::MakeSize(Size{400, 400}),
-        {.color = Color::Green(), .blend_mode = BlendMode::kColorBurn});
+        {.color = Color::Green(), .blend_mode = BlendMode::kHardLight});
   }
 
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
@@ -3664,5 +3667,25 @@ TEST_P(AiksTest,
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
 }
 
+// This should be solid red, if you see a little red box this is broken.
+TEST_P(AiksTest, ClearColorOptimizationWhenSubpassIsBiggerThanParentPass) {
+  SetWindowSize({400, 400});
+  Canvas canvas;
+  canvas.Scale(GetContentScale());
+  canvas.DrawRect(Rect::MakeLTRB(200, 200, 300, 300), {.color = Color::Red()});
+  canvas.SaveLayer({
+      .image_filter = std::make_shared<MatrixImageFilter>(
+          Matrix::MakeScale({2, 2, 1}), SamplerDescriptor{}),
+  });
+  // Draw a rectangle that would fully cover the parent pass size, but not
+  // the subpass that it is rendered in.
+  canvas.DrawRect(Rect::MakeLTRB(0, 0, 400, 400), {.color = Color::Green()});
+  // Draw a bigger rectangle to force the subpass to be bigger.
+  canvas.DrawRect(Rect::MakeLTRB(0, 0, 800, 800), {.color = Color::Red()});
+  canvas.Restore();
+
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
 }  // namespace testing
 }  // namespace impeller
diff --git a/engine/src/flutter/impeller/entity/entity_pass.cc b/engine/src/flutter/impeller/entity/entity_pass.cc
index bb065b959e1..11219ad4937 100644
--- a/engine/src/flutter/impeller/entity/entity_pass.cc
+++ b/engine/src/flutter/impeller/entity/entity_pass.cc
@@ -726,9 +726,10 @@ bool EntityPass::OnRender(
     VALIDATION_LOG << SPrintF("Pass context invalid (Depth=%d)", pass_depth);
     return false;
   }
+  auto clear_color_size = pass_target.GetRenderTarget().GetRenderTargetSize();
 
   if (!collapsed_parent_pass &&
-      !GetClearColor(root_pass_size).IsTransparent()) {
+      !GetClearColor(clear_color_size).IsTransparent()) {
     // Force the pass context to create at least one new pass if the clear color
     // is present.
     pass_context.GetRenderPass(pass_depth);
@@ -897,7 +898,7 @@ bool EntityPass::OnRender(
     // Skip elements that are incorporated into the clear color.
     if (is_collapsing_clear_colors) {
       auto [entity_color, _] =
-          ElementAsBackgroundColor(element, root_pass_size);
+          ElementAsBackgroundColor(element, clear_color_size);
       if (entity_color.has_value()) {
         continue;
       }