[Impeller] simplify render target size rounding up heuristics. (#57043)
Remove the over-engineered "slide around round up" herusistic for the same heuristic used by the image filters: if the coverage size is within 30% of the cull rect size, just use the cull rect size.
This stabilizes allocations for the new android slide in page transition, but in a less complicated way than the old herustic.
diff --git a/impeller/entity/save_layer_utils.cc b/impeller/entity/save_layer_utils.cc
index 8d10181..f6ad180 100644
--- a/impeller/entity/save_layer_utils.cc
+++ b/impeller/entity/save_layer_utils.cc
@@ -12,6 +12,8 @@
return (std::abs(a.width - b.width) / b.width) < threshold &&
(std::abs(a.height - b.height) / b.height) < threshold;
}
+
+static constexpr Scalar kDefaultSizeThreshold = 0.3;
} // namespace
std::optional<Rect> ComputeSaveLayerCoverage(
@@ -84,7 +86,8 @@
transformed_coverage.Intersection(source_coverage_limit.value());
if (intersected_coverage.has_value() &&
SizeDifferenceUnderThreshold(transformed_coverage.GetSize(),
- intersected_coverage->GetSize(), 0.2)) {
+ intersected_coverage->GetSize(),
+ kDefaultSizeThreshold)) {
// Returning the transformed coverage is always correct, it just may
// be larger than the clip area or onscreen texture.
return transformed_coverage;
@@ -113,35 +116,16 @@
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<Rect> shifted_intersected_value =
- transformed_coverage.Shift(delta).Intersection(coverage_limit);
-
- if (shifted_intersected_value.has_value()) {
- return shifted_intersected_value.value().Shift(-delta);
- }
- }
+ // The the resulting coverage rect is nearly the same as the coverage_limit,
+ // round up to the coverage_limit.
+ Rect intersect_rect = intersection.value();
+ if (SizeDifferenceUnderThreshold(intersect_rect.GetSize(),
+ coverage_limit.GetSize(),
+ kDefaultSizeThreshold)) {
+ return coverage_limit;
}
- return intersection;
+ return intersect_rect;
}
} // namespace impeller
diff --git a/impeller/entity/save_layer_utils_unittests.cc b/impeller/entity/save_layer_utils_unittests.cc
index d86c820..5139204 100644
--- a/impeller/entity/save_layer_utils_unittests.cc
+++ b/impeller/entity/save_layer_utils_unittests.cc
@@ -280,77 +280,61 @@
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50));
}
-TEST(SaveLayerUtilsTest,
- PartiallyIntersectingCoverageIgnoresOriginWithSlideSemanticsX) {
+TEST(SaveLayerUtilsTest, RoundUpCoverageWhenCloseToCoverageLimit) {
// 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 //
+ /*content_coverage=*/Rect::MakeLTRB(0, 0, 90, 90), //
+ /*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));
+ EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 100, 100));
+}
+
+TEST(SaveLayerUtilsTest, DontRoundUpCoverageWhenNotCloseToCoverageLimitWidth) {
+ // X varies, translation is performed on coverage.
+ auto coverage = ComputeSaveLayerCoverage(
+ /*content_coverage=*/Rect::MakeLTRB(0, 0, 50, 90), //
+ /*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, 0, 50, 90));
+}
+
+TEST(SaveLayerUtilsTest, DontRoundUpCoverageWhenNotCloseToCoverageLimitHeight) {
+ // X varies, translation is performed on coverage.
+ auto coverage = ComputeSaveLayerCoverage(
+ /*content_coverage=*/Rect::MakeLTRB(0, 0, 90, 50), //
+ /*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, 0, 90, 50));
}
TEST(SaveLayerUtilsTest,
- PartiallyIntersectingCoverageIgnoresOriginWithSlideSemanticsY) {
- // Y varies, translation is performed on coverage.
+ DontRoundUpCoverageWhenNotCloseToCoverageLimitWidthHeight) {
+ // X 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 //
+ /*content_coverage=*/Rect::MakeLTRB(0, 0, 50, 50), //
+ /*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));
+ EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50));
}
} // namespace testing
