Skip to content

Commit 1bb228d

Browse files
authored
[Impeller] Track clip coverage per-pass when not collapsing. (flutter#46597)
Resolves flutter#135916. Big credits to @gaaclarke for the repros and investigation around this. Moves back to tracking clip coverage per-pass when not collapsing. This is especially important for SaveLayers with MatrixImageFilters, which can transform the rendered layer texture and thereby move elements inside or outside of existing parent clips. This should have little to no performance impact, since we should already be generating correct minimal subpass textures based on correct subpass coverage computation. Before: <img width="679" alt="Screenshot 2023-10-05 at 1 25 29 PM" src="https://github.com/flutter/engine/assets/919017/fe63808a-6353-4969-8225-120fd5ee0949"> https://github.com/flutter/engine/assets/919017/9284f055-bee1-40cd-8b7e-f478b00d01da After: <img width="679" alt="Screenshot 2023-10-05 at 1 24 17 PM" src="https://github.com/flutter/engine/assets/919017/066b1e25-9611-4e14-a383-cc3a866dbe36"> https://github.com/flutter/engine/assets/919017/0fbd1f96-8920-472c-bb0e-4414187fc72d
1 parent 59b6b94 commit 1bb228d

4 files changed

Lines changed: 67 additions & 16 deletions

File tree

impeller/aiks/aiks_unittests.cc

Lines changed: 38 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -3619,15 +3619,46 @@ TEST_P(AiksTest, MatrixImageFilterMagnify) {
36193619
canvas.Translate({600, -200});
36203620
canvas.SaveLayer({
36213621
.image_filter = std::make_shared<MatrixImageFilter>(
3622-
Matrix{
3623-
2, 0, 0, 0, //
3624-
0, 2, 0, 0, //
3625-
0, 0, 2, 0, //
3626-
0, 0, 0, 1 //
3627-
},
3622+
Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}),
3623+
});
3624+
canvas.DrawImage(image, {0, 0},
3625+
Paint{.color = Color::White().WithAlpha(0.5)});
3626+
canvas.Restore();
3627+
3628+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3629+
}
3630+
3631+
// Render a white circle at the top left corner of the screen.
3632+
TEST_P(AiksTest, MatrixImageFilterDoesntCullWhenTranslatedFromOffscreen) {
3633+
Canvas canvas;
3634+
canvas.Scale(GetContentScale());
3635+
canvas.Translate({100, 100});
3636+
// Draw a circle in a SaveLayer at -300, but move it back on-screen with a
3637+
// +300 translation applied by a SaveLayer image filter.
3638+
canvas.SaveLayer({
3639+
.image_filter = std::make_shared<MatrixImageFilter>(
3640+
Matrix::MakeTranslation({300, 0}), SamplerDescriptor{}),
3641+
});
3642+
canvas.DrawCircle({-300, 0}, 100, {.color = Color::Green()});
3643+
canvas.Restore();
3644+
3645+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
3646+
}
3647+
3648+
// Render a white circle at the top left corner of the screen.
3649+
TEST_P(AiksTest,
3650+
MatrixImageFilterDoesntCullWhenScaledAndTranslatedFromOffscreen) {
3651+
Canvas canvas;
3652+
canvas.Scale(GetContentScale());
3653+
canvas.Translate({100, 100});
3654+
// Draw a circle in a SaveLayer at -300, but move it back on-screen with a
3655+
// +300 translation applied by a SaveLayer image filter.
3656+
canvas.SaveLayer({
3657+
.image_filter = std::make_shared<MatrixImageFilter>(
3658+
Matrix::MakeTranslation({300, 0}) * Matrix::MakeScale({2, 2, 2}),
36283659
SamplerDescriptor{}),
36293660
});
3630-
canvas.DrawImage(image, {0, 0}, Paint{.color = Color(1.0, 1.0, 1.0, 0.5)});
3661+
canvas.DrawCircle({-150, 0}, 50, {.color = Color::Green()});
36313662
canvas.Restore();
36323663

36333664
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));

impeller/entity/contents/filters/matrix_filter_contents.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -45,7 +45,7 @@ std::optional<Entity> MatrixFilterContents::RenderFilter(
4545
// scaled up, then translations applied by the matrix should be magnified
4646
// accordingly.
4747
//
48-
// To accomplish this, we sandwitch the filter's matrix within the CTM in both
48+
// To accomplish this, we sandwich the filter's matrix within the CTM in both
4949
// cases. But notice that for the subpass backdrop filter case, we use the
5050
// "effect transform" instead of the Entity's transform!
5151
//

impeller/entity/entity_pass.cc

Lines changed: 13 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -198,12 +198,10 @@ std::optional<Rect> EntityPass::GetSubpassCoverage(
198198
std::shared_ptr<FilterContents> image_filter =
199199
subpass.delegate_->WithImageFilter(Rect(), subpass.xformation_);
200200

201-
// If the filter graph transforms the basis of the subpass, then its space
202-
// has deviated too much from the parent pass to safely intersect with the
203-
// pass coverage limit.
204-
coverage_limit =
205-
(image_filter && !image_filter->IsTranslationOnly() ? std::nullopt
206-
: coverage_limit);
201+
// If the subpass has an image filter, then its coverage space may deviate
202+
// from the parent pass and make intersecting with the pass coverage limit
203+
// unsafe.
204+
coverage_limit = image_filter ? std::nullopt : coverage_limit;
207205

208206
auto entities_coverage = subpass.GetElementsCoverage(coverage_limit);
209207
// The entities don't cover anything. There is nothing to do.
@@ -641,6 +639,14 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
641639
auto subpass_capture = capture.CreateChild("EntityPass");
642640
subpass_capture.AddRect("Coverage", *subpass_coverage, {.readonly = true});
643641

642+
// Start non-collapsed subpasses with a fresh clip coverage stack limited by
643+
// the subpass coverage. This is important because image filters applied to
644+
// save layers may transform the subpass texture after it's rendered,
645+
// causing parent clip coverage to get misaligned with the actual area that
646+
// the subpass will affect in the parent pass.
647+
ClipCoverageStack subpass_clip_coverage_stack = {ClipCoverageLayer{
648+
.coverage = subpass_coverage, .clip_depth = subpass->clip_depth_}};
649+
644650
// Stencil textures aren't shared between EntityPasses (as much of the
645651
// time they are transient).
646652
if (!subpass->OnRender(
@@ -652,7 +658,7 @@ EntityPass::EntityResult EntityPass::GetEntityForElement(
652658
subpass_coverage->origin -
653659
global_pass_position, // local_pass_position
654660
++pass_depth, // pass_depth
655-
clip_coverage_stack, // clip_coverage_stack
661+
subpass_clip_coverage_stack, // clip_coverage_stack
656662
subpass->clip_depth_, // clip_depth_floor
657663
subpass_backdrop_filter_contents // backdrop_filter_contents
658664
)) {

impeller/entity/entity_pass.h

Lines changed: 15 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -142,7 +142,21 @@ class EntityPass {
142142
void SetEnableOffscreenCheckerboard(bool enabled);
143143

144144
//----------------------------------------------------------------------------
145-
/// @brief Get the coverage of an unfiltered subpass.
145+
/// @brief Computes the coverage of a given subpass. This is used to
146+
/// determine the texture size of a given subpass before it's rendered
147+
/// to and passed through the subpass ImageFilter, if any.
148+
///
149+
/// @param[in] subpass The EntityPass for which to compute
150+
/// pre-filteredcoverage.
151+
/// @param[in] coverage_limit Confines coverage to a specified area. This
152+
/// hint is used to trim coverage to the root
153+
/// framebuffer area. `std::nullopt` means there
154+
/// is no limit.
155+
///
156+
/// @return The screen space pixel area that the subpass contents will render
157+
/// into, prior to being transformed by the subpass ImageFilter, if
158+
/// any. `std::nullopt` means rendering the subpass will have no
159+
/// effect on the color attachment.
146160
///
147161
std::optional<Rect> GetSubpassCoverage(
148162
const EntityPass& subpass,

0 commit comments

Comments
 (0)