diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 695a7b26a1de2..1a50dbe3e9406 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3619,15 +3619,46 @@ TEST_P(AiksTest, MatrixImageFilterMagnify) { canvas.Translate({600, -200}); canvas.SaveLayer({ .image_filter = std::make_shared( - Matrix{ - 2, 0, 0, 0, // - 0, 2, 0, 0, // - 0, 0, 2, 0, // - 0, 0, 0, 1 // - }, + Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}), + }); + canvas.DrawImage(image, {0, 0}, + Paint{.color = Color::White().WithAlpha(0.5)}); + canvas.Restore(); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + +// Render a white circle at the top left corner of the screen. +TEST_P(AiksTest, MatrixImageFilterDoesntCullWhenTranslatedFromOffscreen) { + Canvas canvas; + canvas.Scale(GetContentScale()); + canvas.Translate({100, 100}); + // Draw a circle in a SaveLayer at -300, but move it back on-screen with a + // +300 translation applied by a SaveLayer image filter. + canvas.SaveLayer({ + .image_filter = std::make_shared( + Matrix::MakeTranslation({300, 0}), SamplerDescriptor{}), + }); + canvas.DrawCircle({-300, 0}, 100, {.color = Color::Green()}); + canvas.Restore(); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + +// Render a white circle at the top left corner of the screen. +TEST_P(AiksTest, + MatrixImageFilterDoesntCullWhenScaledAndTranslatedFromOffscreen) { + Canvas canvas; + canvas.Scale(GetContentScale()); + canvas.Translate({100, 100}); + // Draw a circle in a SaveLayer at -300, but move it back on-screen with a + // +300 translation applied by a SaveLayer image filter. + canvas.SaveLayer({ + .image_filter = std::make_shared( + Matrix::MakeTranslation({300, 0}) * Matrix::MakeScale({2, 2, 2}), SamplerDescriptor{}), }); - canvas.DrawImage(image, {0, 0}, Paint{.color = Color(1.0, 1.0, 1.0, 0.5)}); + canvas.DrawCircle({-150, 0}, 50, {.color = Color::Green()}); canvas.Restore(); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); diff --git a/impeller/entity/contents/filters/matrix_filter_contents.cc b/impeller/entity/contents/filters/matrix_filter_contents.cc index d88edc298e3b9..c16bcdb88d698 100644 --- a/impeller/entity/contents/filters/matrix_filter_contents.cc +++ b/impeller/entity/contents/filters/matrix_filter_contents.cc @@ -45,7 +45,7 @@ std::optional MatrixFilterContents::RenderFilter( // scaled up, then translations applied by the matrix should be magnified // accordingly. // - // To accomplish this, we sandwitch the filter's matrix within the CTM in both + // To accomplish this, we sandwich the filter's matrix within the CTM in both // cases. But notice that for the subpass backdrop filter case, we use the // "effect transform" instead of the Entity's transform! // diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index c6af10254f7d6..bb065b959e146 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -198,12 +198,10 @@ std::optional EntityPass::GetSubpassCoverage( std::shared_ptr image_filter = subpass.delegate_->WithImageFilter(Rect(), subpass.xformation_); - // If the filter graph transforms the basis of the subpass, then its space - // has deviated too much from the parent pass to safely intersect with the - // pass coverage limit. - coverage_limit = - (image_filter && !image_filter->IsTranslationOnly() ? std::nullopt - : coverage_limit); + // If the subpass has an image filter, then its coverage space may deviate + // from the parent pass and make intersecting with the pass coverage limit + // unsafe. + coverage_limit = image_filter ? std::nullopt : coverage_limit; auto entities_coverage = subpass.GetElementsCoverage(coverage_limit); // The entities don't cover anything. There is nothing to do. @@ -641,6 +639,14 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( auto subpass_capture = capture.CreateChild("EntityPass"); subpass_capture.AddRect("Coverage", *subpass_coverage, {.readonly = true}); + // Start non-collapsed subpasses with a fresh clip coverage stack limited by + // the subpass coverage. This is important because image filters applied to + // save layers may transform the subpass texture after it's rendered, + // causing parent clip coverage to get misaligned with the actual area that + // the subpass will affect in the parent pass. + ClipCoverageStack subpass_clip_coverage_stack = {ClipCoverageLayer{ + .coverage = subpass_coverage, .clip_depth = subpass->clip_depth_}}; + // Stencil textures aren't shared between EntityPasses (as much of the // time they are transient). if (!subpass->OnRender( @@ -652,7 +658,7 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( subpass_coverage->origin - global_pass_position, // local_pass_position ++pass_depth, // pass_depth - clip_coverage_stack, // clip_coverage_stack + subpass_clip_coverage_stack, // clip_coverage_stack subpass->clip_depth_, // clip_depth_floor subpass_backdrop_filter_contents // backdrop_filter_contents )) { diff --git a/impeller/entity/entity_pass.h b/impeller/entity/entity_pass.h index 522f4f9562b6e..261d3272b14aa 100644 --- a/impeller/entity/entity_pass.h +++ b/impeller/entity/entity_pass.h @@ -142,7 +142,21 @@ class EntityPass { void SetEnableOffscreenCheckerboard(bool enabled); //---------------------------------------------------------------------------- - /// @brief Get the coverage of an unfiltered subpass. + /// @brief Computes the coverage of a given subpass. This is used to + /// determine the texture size of a given subpass before it's rendered + /// to and passed through the subpass ImageFilter, if any. + /// + /// @param[in] subpass The EntityPass for which to compute + /// pre-filteredcoverage. + /// @param[in] coverage_limit Confines coverage to a specified area. This + /// hint is used to trim coverage to the root + /// framebuffer area. `std::nullopt` means there + /// is no limit. + /// + /// @return The screen space pixel area that the subpass contents will render + /// into, prior to being transformed by the subpass ImageFilter, if + /// any. `std::nullopt` means rendering the subpass will have no + /// effect on the color attachment. /// std::optional GetSubpassCoverage( const EntityPass& subpass,