From 0d4c7dbe07cce8df9f3bdeed8cb43ac629ff8ec3 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 25 Nov 2024 15:48:05 -0800 Subject: [PATCH] [Impeller] better handle allocation herustics of Android slide in page transition. (flutter/engine#56762) For https://github.com/flutter/flutter/pull/158881 The new Android m3 page transition animates a saveLayer w/ opacity + slide in animation. See example video from linked PR: https://github.com/user-attachments/assets/1382374a-4e0c-4a0e-9d70-948ce12e6298 On each frame, we intersect the coverage rect of this saveLayer contents with the screen rect, and shrink it to a partial rectangle. But this changes each frame, which forces us to re-allocate a large texture each frame, causing performance issues. Why not ignore the cull rect entirely? We sometimes ignore the cull rect for the image filter layer for similar reasons, but from observation, the sizes of these saveLayer can be slightly larger than the screen for flutter gallery. Instead, I attempted to use the cull rect size to shrink the saveLayer by shifting the origin before intersecting. I think this should be safe to do, as I believe it could only leave the coverage as larger than it would have been and not smaller. --- .../impeller/entity/save_layer_utils.cc | 38 +++++++++- .../entity/save_layer_utils_unittests.cc | 73 +++++++++++++++++++ 2 files changed, 109 insertions(+), 2 deletions(-) diff --git a/engine/src/flutter/impeller/entity/save_layer_utils.cc b/engine/src/flutter/impeller/entity/save_layer_utils.cc index 304990240ec..8d10181198b 100644 --- a/engine/src/flutter/impeller/entity/save_layer_utils.cc +++ b/engine/src/flutter/impeller/entity/save_layer_utils.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/entity/save_layer_utils.h" +#include "impeller/geometry/scalar.h" namespace impeller { @@ -106,8 +107,41 @@ std::optional ComputeSaveLayerCoverage( // Transform the input coverage into the global coordinate space before // computing the bounds limit intersection. - return coverage.TransformBounds(effect_transform) - .Intersection(coverage_limit); + Rect transformed_coverage = coverage.TransformBounds(effect_transform); + std::optional intersection = + transformed_coverage.Intersection(coverage_limit); + if (!intersection.has_value()) { + return std::nullopt; + } + + // Sometimes a saveLayer is only slightly shifted outside of the cull rect, + // but is being animated in. This is common for the Android slide in page + // transitions. In these cases, computing a cull that is too tight can cause + // thrashing of the texture cache. Instead, we try to determine the + // intersection using only the sizing by shifting the coverage rect into the + // cull rect origin. + Point delta = coverage_limit.GetOrigin() - transformed_coverage.GetOrigin(); + + // This herustic is limited to perfectly vertical or horizontal transitions + // that slide in, limited to a fixed threshold of ~30%. This value is based on + // the Android slide in page transition which experimental has threshold + // values of up to 28%. + static constexpr Scalar kThresholdLimit = 0.3; + + if (ScalarNearlyEqual(delta.y, 0) || ScalarNearlyEqual(delta.x, 0)) { + Scalar threshold = std::max(std::abs(delta.x / coverage_limit.GetWidth()), + std::abs(delta.y / coverage_limit.GetHeight())); + if (threshold < kThresholdLimit) { + std::optional shifted_intersected_value = + transformed_coverage.Shift(delta).Intersection(coverage_limit); + + if (shifted_intersected_value.has_value()) { + return shifted_intersected_value.value().Shift(-delta); + } + } + } + + return intersection; } } // namespace impeller diff --git a/engine/src/flutter/impeller/entity/save_layer_utils_unittests.cc b/engine/src/flutter/impeller/entity/save_layer_utils_unittests.cc index b72ac33ae76..d86c8209ab6 100644 --- a/engine/src/flutter/impeller/entity/save_layer_utils_unittests.cc +++ b/engine/src/flutter/impeller/entity/save_layer_utils_unittests.cc @@ -280,6 +280,79 @@ TEST(SaveLayerUtilsTest, EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50)); } +TEST(SaveLayerUtilsTest, + PartiallyIntersectingCoverageIgnoresOriginWithSlideSemanticsX) { + // X varies, translation is performed on coverage. + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(5, 0, 210, 210), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/nullptr // + ); + + ASSERT_TRUE(coverage.has_value()); + // Size that matches coverage limit + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(5, 0, 105, 100)); +} + +TEST(SaveLayerUtilsTest, + PartiallyIntersectingCoverageIgnoresOriginWithSlideSemanticsY) { + // Y varies, translation is performed on coverage. + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(0, 5, 210, 210), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/nullptr // + ); + + ASSERT_TRUE(coverage.has_value()); + // Size that matches coverage limit + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 5, 100, 105)); +} + +TEST(SaveLayerUtilsTest, PartiallyIntersectingCoverageIgnoresOriginBothXAndY) { + // Both X and Y vary, no transation is performed. + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(5, 5, 210, 210), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/nullptr // + ); + + ASSERT_TRUE(coverage.has_value()); + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(5, 5, 100, 100)); +} + +TEST(SaveLayerUtilsTest, PartiallyIntersectingCoverageWithOveriszedThresholdY) { + // Y varies, translation is not performed on coverage because it is too far + // offscreen. + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(0, 80, 200, 200), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/nullptr // + ); + + ASSERT_TRUE(coverage.has_value()); + // Size that matches coverage limit + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 80, 100, 100)); +} + +TEST(SaveLayerUtilsTest, PartiallyIntersectingCoverageWithOveriszedThresholdX) { + // X varies, translation is not performed on coverage because it is too far + // offscreen. + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(80, 0, 200, 200), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/nullptr // + ); + + ASSERT_TRUE(coverage.has_value()); + // Size that matches coverage limit + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(80, 0, 100, 100)); +} + } // namespace testing } // namespace impeller