Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Commit 813ad2d

Browse files
author
Jonah Williams
authored
[Impeller] add herustic for ignoring coverage limit w/ image filters. (#55030)
Switching to save_layer_utils regressed perf/memory usage on several devicelab benchmarks, see https://flutter-flutter-perf.skia.org/e/?queries=device_type%3DPixel_7_Pro%26sub_result%3D90th_percentile_gpu_memory_mb%26sub_result%3Daverage_gpu_memory_mb%26test%3Dnew_gallery_impeller_old_zoom__transition_perf&selected=commit%3D42418%26name%3D%252Carch%253Dintel%252Cbranch%253Dmaster%252Cconfig%253Ddefault%252Cdevice_type%253DPixel_7_Pro%252Cdevice_version%253Dnone%252Chost_type%253Dlinux%252Csub_result%253D90th_percentile_gpu_memory_mb%252Ctest%253Dnew_gallery_impeller_old_zoom__transition_perf%252C The reason is that the source_coverage_limit [intersection](https://github.com/flutter/engine/blob/main/impeller/entity/save_layer_utils.cc#L67-L68) is highly unstable from frame to frame if there is an animated blur and/or image filter. This change mostly matches the old entity pass behavior. . In cases where the difference between the intersected coverage result is small (< 20% in both dimensions), we just use the transformed content coverage. The old behavior would _always_ use the transformed content coverage even if it was substantially large than the coverage limit. This posed problems if folks drew content larger than the max texture size. This herustic attempts to prevent that by allowing the source coverage limit to kick in once the difference is larger than 20% in either direction
1 parent 2ae1c76 commit 813ad2d

File tree

2 files changed

+84
-2
lines changed

2 files changed

+84
-2
lines changed

impeller/entity/save_layer_utils.cc

Lines changed: 28 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,13 @@
66

77
namespace impeller {
88

9+
namespace {
10+
bool SizeDifferenceUnderThreshold(Size a, Size b, Scalar threshold) {
11+
return (std::abs(a.width - b.width) / b.width) < threshold &&
12+
(std::abs(a.height - b.height) / b.height) < threshold;
13+
}
14+
} // namespace
15+
916
std::optional<Rect> ComputeSaveLayerCoverage(
1017
const Rect& content_coverage,
1118
const Matrix& effect_transform,
@@ -64,8 +71,27 @@ std::optional<Rect> ComputeSaveLayerCoverage(
6471
return source_coverage_limit;
6572
}
6673

67-
return coverage.TransformBounds(effect_transform)
68-
.Intersection(source_coverage_limit.value());
74+
// Trimming the content coverage by the coverage limit can reduce memory
75+
// coverage. limit can reduce memory bandwith. But in cases where there are
76+
// animated matrix filters, such as in the framework's zoom transition, the
77+
// changing scale values continually change the source_coverage_limit.
78+
// Intersecting the source_coverage_limit with the coverage may result in
79+
// slightly different texture sizes each frame of the animation. This leads
80+
// to non-optimal allocation patterns as differently sized textures cannot
81+
// be reused. Hence the following herustic: If the coverage is within a
82+
// semi-arbitrary percentage of the intersected coverage, then just use the
83+
// transformed coverage. In other cases, use the intersection.
84+
auto transformed_coverage = coverage.TransformBounds(effect_transform);
85+
auto intersected_coverage =
86+
transformed_coverage.Intersection(source_coverage_limit.value());
87+
if (intersected_coverage.has_value() &&
88+
SizeDifferenceUnderThreshold(transformed_coverage.GetSize(),
89+
intersected_coverage->GetSize(), 0.2)) {
90+
// Returning the transformed coverage is always correct, it just may
91+
// be larger than the clip area or onscreen texture.
92+
return transformed_coverage;
93+
}
94+
return intersected_coverage;
6995
}
7096

7197
// If the input coverage is maximum, just return the coverage limit that

impeller/entity/save_layer_utils_unittests.cc

Lines changed: 56 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -224,6 +224,62 @@ TEST(SaveLayerUtilsTest,
224224
ASSERT_FALSE(coverage.has_value());
225225
}
226226

227+
TEST(
228+
SaveLayerUtilsTest,
229+
CoverageLimitIgnoredIfIntersectedValueIsCloseToActualCoverageSmallerWithImageFilter) {
230+
// Create an image filter that slightly shrinks the coverage limit
231+
auto image_filter = FilterContents::MakeMatrixFilter(
232+
FilterInput::Make(Rect()), Matrix::MakeScale({1.1, 1.1, 1}), {});
233+
234+
auto coverage = ComputeSaveLayerCoverage(
235+
/*content_coverage=*/Rect::MakeLTRB(0, 0, 100, 100), //
236+
/*effect_transform=*/{}, //
237+
/*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), //
238+
/*image_filter=*/image_filter //
239+
);
240+
241+
ASSERT_TRUE(coverage.has_value());
242+
// The transfomed coverage limit is ((0, 0), (90.9091, 90.9091)).
243+
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 100, 100));
244+
}
245+
246+
TEST(
247+
SaveLayerUtilsTest,
248+
CoverageLimitIgnoredIfIntersectedValueIsCloseToActualCoverageLargerWithImageFilter) {
249+
// Create an image filter that slightly stretches the coverage limit. Even
250+
// without the special logic for using the original content coverage, we
251+
// verify that we don't introduce any artifacts from the intersection.
252+
auto image_filter = FilterContents::MakeMatrixFilter(
253+
FilterInput::Make(Rect()), Matrix::MakeScale({0.9, 0.9, 1}), {});
254+
255+
auto coverage = ComputeSaveLayerCoverage(
256+
/*content_coverage=*/Rect::MakeLTRB(0, 0, 100, 100), //
257+
/*effect_transform=*/{}, //
258+
/*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), //
259+
/*image_filter=*/image_filter //
260+
);
261+
262+
ASSERT_TRUE(coverage.has_value());
263+
// The transfomed coverage limit is ((0, 0), (111.111, 111.111)).
264+
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 100, 100));
265+
}
266+
267+
TEST(SaveLayerUtilsTest,
268+
CoverageLimitRespectedIfSubstantiallyDifferentFromContentCoverge) {
269+
auto image_filter = FilterContents::MakeMatrixFilter(
270+
FilterInput::Make(Rect()), Matrix::MakeScale({2, 2, 1}), {});
271+
272+
auto coverage = ComputeSaveLayerCoverage(
273+
/*content_coverage=*/Rect::MakeLTRB(0, 0, 1000, 1000), //
274+
/*effect_transform=*/{}, //
275+
/*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), //
276+
/*image_filter=*/image_filter //
277+
);
278+
279+
ASSERT_TRUE(coverage.has_value());
280+
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50));
281+
}
282+
227283
} // namespace testing
228284
} // namespace impeller
229285

0 commit comments

Comments
 (0)