From d3ed232dab379fa4c5906171df682ca60d304aa6 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 16 Oct 2024 19:30:01 -0700 Subject: [PATCH] [Impeller] flood input coverage with destructive color filter. (flutter/engine#55758) Fixes https://github.com/flutter/flutter/issues/126389 Fixes https://github.com/flutter/flutter/issues/137090 Fixes https://github.com/flutter/flutter/issues/154035 If this bit is set, the output of a color filter should flood to the input of a filter to parent clip. We're not checking this right now and so all color filter content is treated as bounded. This needs to flood the input as the image filter (which occurs afterwards) should process the flooded input. --- .../display_list/aiks_dl_blend_unittests.cc | 17 +++++++++++++++++ .../impeller/display_list/aiks_dl_unittests.cc | 6 ++++++ .../src/flutter/impeller/display_list/canvas.cc | 4 +++- .../flutter/impeller/entity/save_layer_utils.cc | 7 ++----- .../testing/impeller_golden_tests_output.txt | 3 +++ 5 files changed, 31 insertions(+), 6 deletions(-) diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_blend_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_blend_unittests.cc index 41c57e8514e..f4691f63cc8 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_blend_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_blend_unittests.cc @@ -888,5 +888,22 @@ TEST_P(AiksTest, ColorWheel) { ASSERT_TRUE(OpenPlaygroundHere(callback)); } +TEST_P(AiksTest, DestructiveBlendColorFilterFloodsClip) { + DisplayListBuilder builder; + + DlPaint paint; + paint.setColor(DlColor::kBlue()); + builder.DrawPaint(paint); + + DlPaint save_paint; + save_paint.setColorFilter( + DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kSrc)); + builder.SaveLayer(nullptr, &save_paint); + builder.Restore(); + + // Should be solid red as the destructive color filter floods the clip. + ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/display_list/aiks_dl_unittests.cc b/engine/src/flutter/impeller/display_list/aiks_dl_unittests.cc index 236345fc725..279b7bc00a4 100644 --- a/engine/src/flutter/impeller/display_list/aiks_dl_unittests.cc +++ b/engine/src/flutter/impeller/display_list/aiks_dl_unittests.cc @@ -160,12 +160,15 @@ TEST_P(AiksTest, TranslucentSaveLayerWithBlendColorFilterDrawsCorrectly) { paint.setColor(DlColor::kBlack().withAlpha(128)); paint.setColorFilter( DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kDstOver)); + builder.Save(); + builder.ClipRect(SkRect::MakeXYWH(100, 500, 300, 300)); builder.SaveLayer(nullptr, &paint); DlPaint draw_paint; draw_paint.setColor(DlColor::kBlue()); builder.DrawRect(SkRect::MakeXYWH(100, 500, 300, 300), draw_paint); builder.Restore(); + builder.Restore(); ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } @@ -203,12 +206,15 @@ TEST_P(AiksTest, TranslucentSaveLayerWithColorAndImageFilterDrawsCorrectly) { save_paint.setColor(DlColor::kBlack().withAlpha(128)); save_paint.setColorFilter( DlBlendColorFilter::Make(DlColor::kRed(), DlBlendMode::kDstOver)); + builder.Save(); + builder.ClipRect(SkRect::MakeXYWH(100, 500, 300, 300)); builder.SaveLayer(nullptr, &save_paint); DlPaint draw_paint; draw_paint.setColor(DlColor::kBlue()); builder.DrawRect(SkRect::MakeXYWH(100, 500, 300, 300), draw_paint); builder.Restore(); + builder.Restore(); ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } diff --git a/engine/src/flutter/impeller/display_list/canvas.cc b/engine/src/flutter/impeller/display_list/canvas.cc index d14db69dc0d..dedb9c31cf2 100644 --- a/engine/src/flutter/impeller/display_list/canvas.cc +++ b/engine/src/flutter/impeller/display_list/canvas.cc @@ -1015,7 +1015,9 @@ void Canvas::SaveLayer(const Paint& paint, filter_contents, // /*flood_output_coverage=*/ Entity::IsBlendModeDestructive(paint.blend_mode), // - /*flood_input_coverage=*/!!backdrop_filter // + /*flood_input_coverage=*/!!backdrop_filter || + (paint.color_filter && + paint.color_filter->modifies_transparent_black()) // ); if (!maybe_subpass_coverage.has_value()) { diff --git a/engine/src/flutter/impeller/entity/save_layer_utils.cc b/engine/src/flutter/impeller/entity/save_layer_utils.cc index 6c9e528ef15..304990240ec 100644 --- a/engine/src/flutter/impeller/entity/save_layer_utils.cc +++ b/engine/src/flutter/impeller/entity/save_layer_utils.cc @@ -25,11 +25,8 @@ std::optional ComputeSaveLayerCoverage( // first is the presence of a backdrop filter on the saveLayer. The second is // the presence of a color filter that effects transparent black on the // saveLayer. The last is the presence of unbounded content within the - // saveLayer (such as a drawPaint, bdf, et cetera). Note that currently - // only the presence of a backdrop filter impacts this flag, while color - // filters are not yet handled - // (https://github.com/flutter/flutter/issues/154035) and unbounded coverage - // is handled in the display list dispatcher. + // saveLayer (such as a drawPaint, bdf, et cetera). Note that unbounded + // coverage is handled in the display list dispatcher. // // Backdrop filters apply before the saveLayer is restored. The presence of // a backdrop filter causes the content coverage of the saveLayer to be diff --git a/engine/src/flutter/testing/impeller_golden_tests_output.txt b/engine/src/flutter/testing/impeller_golden_tests_output.txt index 8952a7430a6..ef3b466533e 100644 --- a/engine/src/flutter/testing/impeller_golden_tests_output.txt +++ b/engine/src/flutter/testing/impeller_golden_tests_output.txt @@ -545,6 +545,9 @@ impeller_Play_AiksTest_CoordinateConversionsAreCorrect_Vulkan.png impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Metal.png impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_OpenGLES.png impeller_Play_AiksTest_CoverageOriginShouldBeAccountedForInSubpasses_Vulkan.png +impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Metal.png +impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_OpenGLES.png +impeller_Play_AiksTest_DestructiveBlendColorFilterFloodsClip_Vulkan.png impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Metal.png impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_OpenGLES.png impeller_Play_AiksTest_DispatcherDoesNotCullPerspectiveTransformedChildDisplayLists_Vulkan.png