Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
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
38 changes: 36 additions & 2 deletions impeller/entity/save_layer_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "impeller/entity/save_layer_utils.h"
#include "impeller/geometry/scalar.h"

namespace impeller {

Expand Down Expand Up @@ -106,8 +107,41 @@ std::optional<Rect> 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<Rect> 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)) {
Copy link
Contributor

Choose a reason for hiding this comment

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

Not sure if this needs a code comment. My first reading of this line was "wait, we only do this if there is very nearly no delta?" before I realized it was looking for horizontal or vertical transitions.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added more documentation

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);
}
}
}

return intersection;
}

} // namespace impeller
73 changes: 73 additions & 0 deletions impeller/entity/save_layer_utils_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Perhaps add a test where the threshold is too large in X and another in Y.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done


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

Expand Down