From 64d713b2fd2b685f98a8bf4b8851f298b91ef93a Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 19 Sep 2023 20:55:02 -0700 Subject: [PATCH 1/2] [Impeller] Ensure that reused textures are cleared before getting sampled by backdrop filters. --- impeller/aiks/aiks_unittests.cc | 24 ++++++++++++++++++++++++ impeller/entity/entity_pass.cc | 8 +------- 2 files changed, 25 insertions(+), 7 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index be7b50c8cb347..44a27de5286ec 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -2264,6 +2264,30 @@ TEST_P(AiksTest, DrawPaintAbsorbsClears) { ASSERT_EQ(render_pass->GetCommands().size(), 0llu); } +// This is important to enforce with texture reuse, since cached textures need +// to be cleared before reuse. +TEST_P(AiksTest, + ParentSaveLayerCreatesRenderPassWhenChildBackdropFilterIsPresent) { + Canvas canvas; + canvas.SaveLayer({}, std::nullopt, ImageFilter::MakeMatrix(Matrix(), {})); + canvas.DrawPaint({.color = Color::Red(), .blend_mode = BlendMode::kSource}); + canvas.DrawPaint({.color = Color::CornflowerBlue().WithAlpha(0.75), + .blend_mode = BlendMode::kSourceOver}); + canvas.Restore(); + + Picture picture = canvas.EndRecordingAsPicture(); + + std::shared_ptr spy = ContextSpy::Make(); + std::shared_ptr real_context = GetContext(); + std::shared_ptr mock_context = spy->MakeContext(real_context); + AiksContext renderer(mock_context, nullptr); + std::shared_ptr image = picture.ToImage(renderer, {300, 300}); + + ASSERT_EQ(spy->render_passes_.size(), 3llu); + std::shared_ptr render_pass = spy->render_passes_[0]; + ASSERT_EQ(render_pass->GetCommands().size(), 0llu); +} + TEST_P(AiksTest, DrawRectAbsorbsClears) { Canvas canvas; canvas.DrawRect({0, 0, 300, 300}, diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 6b4017ad0a19c..3ffc56a1010bb 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -537,6 +537,7 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( // The subpass will need to read from the current pass texture when // rendering the backdrop, so if there's an active pass, end it prior to // rendering the subpass. + pass_context.GetRenderPass(pass_depth); pass_context.EndPass(); } @@ -680,13 +681,6 @@ bool EntityPass::OnRender( return false; } - if (!collapsed_parent_pass && - !GetClearColor(root_pass_size).IsTransparent()) { - // Force the pass context to create at least one new pass if the clear color - // is present. - pass_context.GetRenderPass(pass_depth); - } - auto render_element = [&stencil_depth_floor, &pass_context, &pass_depth, &renderer, &stencil_coverage_stack, &global_pass_position](Entity& element_entity) { From 530277d348ec12c520826ef457e9ecd20a712455 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Tue, 19 Sep 2023 21:44:00 -0700 Subject: [PATCH 2/2] Fix build failure and add comment --- impeller/entity/entity_pass.cc | 16 +++++++++++++++- 1 file changed, 15 insertions(+), 1 deletion(-) diff --git a/impeller/entity/entity_pass.cc b/impeller/entity/entity_pass.cc index 3ffc56a1010bb..563bcf4ef7abb 100644 --- a/impeller/entity/entity_pass.cc +++ b/impeller/entity/entity_pass.cc @@ -534,10 +534,17 @@ EntityPass::EntityResult EntityPass::GetEntityForElement( proc(FilterInput::Make(std::move(texture)), subpass->xformation_.Basis(), Entity::RenderingMode::kSubpass); + // If the very first thing we render in this EntityPass is a subpass that + // happens to have a backdrop filter, than that backdrop filter will end + // may wind up sampling from the raw, uncleared texture that came straight + // out of the texture cache. By calling `pass_context.GetRenderPass` here, + // we force the texture to pass through at least one RenderPass with the + // correct clear configuration before any sampling occurs. + pass_context.GetRenderPass(pass_depth); + // The subpass will need to read from the current pass texture when // rendering the backdrop, so if there's an active pass, end it prior to // rendering the subpass. - pass_context.GetRenderPass(pass_depth); pass_context.EndPass(); } @@ -681,6 +688,13 @@ bool EntityPass::OnRender( return false; } + if (!collapsed_parent_pass && + !GetClearColor(root_pass_size).IsTransparent()) { + // Force the pass context to create at least one new pass if the clear color + // is present. + pass_context.GetRenderPass(pass_depth); + } + auto render_element = [&stencil_depth_floor, &pass_context, &pass_depth, &renderer, &stencil_coverage_stack, &global_pass_position](Entity& element_entity) {