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

[Impeller] simplify render target size rounding up heuristics. #57043

Merged
auto-submit merged 4 commits into from
Dec 10, 2024
Merged

[Impeller] simplify render target size rounding up heuristics. #57043

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
ca03f8c
simplify rounding up heuristics.
Dec 7, 2024
b667d59
update testing.
Dec 7, 2024
745ab80
flar feedback.
Dec 9, 2024
951bc02
add more test cases.
Dec 9, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
40 changes: 12 additions & 28 deletions impeller/entity/save_layer_utils.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,6 +12,8 @@ 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;
}

static constexpr Scalar kDefaultSizeThreshold = 0.3;
} // namespace

std::optional<Rect> ComputeSaveLayerCoverage(
Expand Down Expand Up @@ -84,7 +86,8 @@ std::optional<Rect> ComputeSaveLayerCoverage(
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;
Expand Down Expand Up @@ -113,35 +116,16 @@ std::optional<Rect> ComputeSaveLayerCoverage(
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
72 changes: 28 additions & 44 deletions impeller/entity/save_layer_utils_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -280,77 +280,61 @@ TEST(SaveLayerUtilsTest,
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50));
}

TEST(SaveLayerUtilsTest,
Copy link
Contributor

@flar flar Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are all of these other test cases deleted? I think it's important to verify that we are rounding up in reasonable cases but not in "unreasonable" cases (and though our policy on unreasonable might change over time, not having any test for it means it could change unintentionally)

Copy link
Contributor Author

@jonahwilliams jonahwilliams Dec 9, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a few more test cases that cover one or both of the dimensions being too different for the round up.

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,
PartiallyIntersectingCoverageIgnoresOriginWithSlideSemanticsY) {
// Y varies, translation is performed on coverage.
TEST(SaveLayerUtilsTest, DontRoundUpCoverageWhenNotCloseToCoverageLimitWidth) {
// 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, 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, 5, 100, 105));
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 90));
}

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.
TEST(SaveLayerUtilsTest, DontRoundUpCoverageWhenNotCloseToCoverageLimitHeight) {
// X varies, translation is performed on coverage.
auto coverage = ComputeSaveLayerCoverage(
/*content_coverage=*/Rect::MakeLTRB(0, 80, 200, 200), //
/*effect_transform=*/{}, //
/*coverage_limit=*/Rect::MakeLTRB(0, 0, 100, 100), //
/*image_filter=*/nullptr //
/*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, 80, 100, 100));
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 90, 50));
}

TEST(SaveLayerUtilsTest, PartiallyIntersectingCoverageWithOveriszedThresholdX) {
// X varies, translation is not performed on coverage because it is too far
// offscreen.
TEST(SaveLayerUtilsTest,
DontRoundUpCoverageWhenNotCloseToCoverageLimitWidthHeight) {
// X varies, translation is performed on coverage.
auto coverage = ComputeSaveLayerCoverage(
/*content_coverage=*/Rect::MakeLTRB(80, 0, 200, 200), //
/*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(80, 0, 100, 100));
EXPECT_EQ(coverage.value(), Rect::MakeLTRB(0, 0, 50, 50));
}

} // namespace testing
Expand Down
Loading