From eba4f6b442d3fd7c6962512aa1fb0b32b53cb83f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Sep 2024 14:44:15 -0700 Subject: [PATCH 1/2] [Impeller] add herustic for ignoring coverage limit w/ image filters. --- impeller/entity/save_layer_utils.cc | 25 ++++++++- impeller/entity/save_layer_utils_unittests.cc | 56 +++++++++++++++++++ 2 files changed, 79 insertions(+), 2 deletions(-) diff --git a/impeller/entity/save_layer_utils.cc b/impeller/entity/save_layer_utils.cc index 68abbf7a9c2a9..c41d9e5ae396f 100644 --- a/impeller/entity/save_layer_utils.cc +++ b/impeller/entity/save_layer_utils.cc @@ -64,8 +64,29 @@ std::optional ComputeSaveLayerCoverage( return source_coverage_limit; } - return coverage.TransformBounds(effect_transform) - .Intersection(source_coverage_limit.value()); + // Returning the transformed coverage is always correct, it just may + // be larger than the clip area or onscreen - trimming it via the coverage + // limit can reduce memory bandwith. In cases where there are animated + // matrix filters, such as in the framework's zoom transition, the changing + // scale values continually change the source_coverage_limit. Thus + // intersecting the source_coverage_limit with the coverage may result in + // slightly different texture sizes each frame of the animation. This leads + // to non-optimal allocation patterns as differently sized textures cannot + // be reused. Hence the following herustic: If the coverage is within a + // semi-arbitrary percentage of the intersected coverage, then just use the + // transformed coverage. In other cases, use the intersection. + auto transformed_coverage = coverage.TransformBounds(effect_transform); + auto intersected_coverage = + transformed_coverage.Intersection(source_coverage_limit.value()); + if ((std::abs(transformed_coverage.GetWidth() - + intersected_coverage->GetWidth()) / + intersected_coverage->GetWidth()) <= 0.2 && + (std::abs(transformed_coverage.GetHeight() - + intersected_coverage->GetHeight()) / + intersected_coverage->GetHeight()) <= 0.2) { + return transformed_coverage; + } + return intersected_coverage; } // If the input coverage is maximum, just return the coverage limit that diff --git a/impeller/entity/save_layer_utils_unittests.cc b/impeller/entity/save_layer_utils_unittests.cc index aa5dfd92cae25..b72ac33ae768b 100644 --- a/impeller/entity/save_layer_utils_unittests.cc +++ b/impeller/entity/save_layer_utils_unittests.cc @@ -224,6 +224,62 @@ TEST(SaveLayerUtilsTest, ASSERT_FALSE(coverage.has_value()); } +TEST( + SaveLayerUtilsTest, + CoverageLimitIgnoredIfIntersectedValueIsCloseToActualCoverageSmallerWithImageFilter) { + // Create an image filter that slightly shrinks the coverage limit + auto image_filter = FilterContents::MakeMatrixFilter( + FilterInput::Make(Rect()), Matrix::MakeScale({1.1, 1.1, 1}), {}); + + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/image_filter // + ); + + ASSERT_TRUE(coverage.has_value()); + // The transfomed coverage limit is ((0, 0), (90.9091, 90.9091)). + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 100, 100)); +} + +TEST( + SaveLayerUtilsTest, + CoverageLimitIgnoredIfIntersectedValueIsCloseToActualCoverageLargerWithImageFilter) { + // Create an image filter that slightly stretches the coverage limit. Even + // without the special logic for using the original content coverage, we + // verify that we don't introduce any artifacts from the intersection. + auto image_filter = FilterContents::MakeMatrixFilter( + FilterInput::Make(Rect()), Matrix::MakeScale({0.9, 0.9, 1}), {}); + + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/image_filter // + ); + + ASSERT_TRUE(coverage.has_value()); + // The transfomed coverage limit is ((0, 0), (111.111, 111.111)). + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 100, 100)); +} + +TEST(SaveLayerUtilsTest, + CoverageLimitRespectedIfSubstantiallyDifferentFromContentCoverge) { + auto image_filter = FilterContents::MakeMatrixFilter( + FilterInput::Make(Rect()), Matrix::MakeScale({2, 2, 1}), {}); + + auto coverage = ComputeSaveLayerCoverage( + /*content_coverage=*/Rect::MakeLTRB(0, 0, 1000, 1000), // + /*effect_transform=*/{}, // + /*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), // + /*image_filter=*/image_filter // + ); + + ASSERT_TRUE(coverage.has_value()); + EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50)); +} + } // namespace testing } // namespace impeller From f30a407f1a6725e20764f6f7e9d4458f58b6fc27 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 7 Sep 2024 15:53:34 -0700 Subject: [PATCH 2/2] ++ --- impeller/entity/save_layer_utils.cc | 29 +++++++++++++++++------------ 1 file changed, 17 insertions(+), 12 deletions(-) diff --git a/impeller/entity/save_layer_utils.cc b/impeller/entity/save_layer_utils.cc index c41d9e5ae396f..fa8d7d712138b 100644 --- a/impeller/entity/save_layer_utils.cc +++ b/impeller/entity/save_layer_utils.cc @@ -6,6 +6,13 @@ namespace impeller { +namespace { +bool SizeDifferenceUnderThreshold(Size a, Size b, Scalar threshold) { + return (std::abs(a.width - b.width) / b.width) < threshold && + (std::abs(a.height - b.height) / b.height) < threshold; +} +} // namespace + std::optional ComputeSaveLayerCoverage( const Rect& content_coverage, const Matrix& effect_transform, @@ -64,12 +71,11 @@ std::optional ComputeSaveLayerCoverage( return source_coverage_limit; } - // Returning the transformed coverage is always correct, it just may - // be larger than the clip area or onscreen - trimming it via the coverage - // limit can reduce memory bandwith. In cases where there are animated - // matrix filters, such as in the framework's zoom transition, the changing - // scale values continually change the source_coverage_limit. Thus - // intersecting the source_coverage_limit with the coverage may result in + // Trimming the content coverage by the coverage limit can reduce memory + // coverage. limit can reduce memory bandwith. But in cases where there are + // animated matrix filters, such as in the framework's zoom transition, the + // changing scale values continually change the source_coverage_limit. + // Intersecting the source_coverage_limit with the coverage may result in // slightly different texture sizes each frame of the animation. This leads // to non-optimal allocation patterns as differently sized textures cannot // be reused. Hence the following herustic: If the coverage is within a @@ -78,12 +84,11 @@ std::optional ComputeSaveLayerCoverage( auto transformed_coverage = coverage.TransformBounds(effect_transform); auto intersected_coverage = transformed_coverage.Intersection(source_coverage_limit.value()); - if ((std::abs(transformed_coverage.GetWidth() - - intersected_coverage->GetWidth()) / - intersected_coverage->GetWidth()) <= 0.2 && - (std::abs(transformed_coverage.GetHeight() - - intersected_coverage->GetHeight()) / - intersected_coverage->GetHeight()) <= 0.2) { + if (intersected_coverage.has_value() && + SizeDifferenceUnderThreshold(transformed_coverage.GetSize(), + intersected_coverage->GetSize(), 0.2)) { + // Returning the transformed coverage is always correct, it just may + // be larger than the clip area or onscreen texture. return transformed_coverage; } return intersected_coverage;