From 09c811781d3eb7852083f16e856d1bf13b3b49ca Mon Sep 17 00:00:00 2001 From: Jason Simmons Date: Thu, 31 Oct 2024 18:02:10 -0700 Subject: [PATCH] [Impeller] Fix handling of destination opacity in advanced blends (flutter/engine#56251) The framebuffer blend pipeline needs to support a dst_input_alpha parameter in order to implement the AbsorbOpacity flag. Also, dst_input_alpha should only be applied to the alpha channel of the unpremultiplied destination color. Fixes https://github.com/flutter/flutter/issues/157716 --- .../display_list/aiks_dl_blend_unittests.cc | 20 ++++++++++++++ .../contents/filters/blend_filter_contents.cc | 4 +++ .../contents/framebuffer_blend_contents.cc | 1 + .../shaders/blending/advanced_blend.frag | 2 +- .../shaders/blending/framebuffer_blend.frag | 2 ++ engine/src/flutter/impeller/tools/malioc.json | 26 +++++++++---------- .../testing/impeller_golden_tests_output.txt | 3 +++ 7 files changed, 44 insertions(+), 14 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 37f37e7a90f..3b3a1bf3c82 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 @@ -906,5 +906,25 @@ TEST_P(AiksTest, DestructiveBlendColorFilterFloodsClip) { ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); } +TEST_P(AiksTest, AdvancedBlendColorFilterWithDestinationOpacity) { + DisplayListBuilder builder; + + builder.DrawPaint(DlPaint(DlColor::kWhite())); + + DlPaint save_paint; + save_paint.setOpacity(0.3); + save_paint.setColorFilter(DlBlendColorFilter::Make(DlColor::kTransparent(), + DlBlendMode::kSaturation)); + builder.SaveLayer(nullptr, &save_paint); + builder.DrawRect(SkRect::MakeXYWH(100, 100, 300, 300), + DlPaint(DlColor::kMaroon())); + builder.DrawRect(SkRect::MakeXYWH(200, 200, 300, 300), + DlPaint(DlColor::kBlue())); + 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/entity/contents/filters/blend_filter_contents.cc b/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc index ce35b710af7..b427de9ab76 100644 --- a/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/filters/blend_filter_contents.cc @@ -850,6 +850,10 @@ std::optional BlendFilterContents::CreateFramebufferAdvancedBlend( VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); frag_info.src_input_alpha = 1.0; + frag_info.dst_input_alpha = + absorb_opacity == ColorFilterContents::AbsorbOpacity::kYes + ? dst_snapshot->opacity + : 1.0; FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); return pass.Draw().ok(); diff --git a/engine/src/flutter/impeller/entity/contents/framebuffer_blend_contents.cc b/engine/src/flutter/impeller/entity/contents/framebuffer_blend_contents.cc index 13f7bd61a0d..6597b3ff9ca 100644 --- a/engine/src/flutter/impeller/entity/contents/framebuffer_blend_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/framebuffer_blend_contents.cc @@ -139,6 +139,7 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer, VS::BindFrameInfo(pass, host_buffer.EmplaceUniform(frame_info)); frag_info.src_input_alpha = src_snapshot->opacity; + frag_info.dst_input_alpha = 1.0; FS::BindFragInfo(pass, host_buffer.EmplaceUniform(frag_info)); return pass.Draw().ok(); diff --git a/engine/src/flutter/impeller/entity/shaders/blending/advanced_blend.frag b/engine/src/flutter/impeller/entity/shaders/blending/advanced_blend.frag index db444f522a0..ff553b7c6b2 100644 --- a/engine/src/flutter/impeller/entity/shaders/blending/advanced_blend.frag +++ b/engine/src/flutter/impeller/entity/shaders/blending/advanced_blend.frag @@ -40,7 +40,7 @@ void main() { IPHalfUnpremultiply(Sample(texture_sampler_dst, // sampler v_dst_texture_coords // texture coordinates )); - dst *= blend_info.dst_input_alpha; + dst.a *= blend_info.dst_input_alpha; f16vec4 src = blend_info.color_factor > 0.0hf ? blend_info.color : IPHalfUnpremultiply(Sample( diff --git a/engine/src/flutter/impeller/entity/shaders/blending/framebuffer_blend.frag b/engine/src/flutter/impeller/entity/shaders/blending/framebuffer_blend.frag index 6d03856b4a8..624bb7dc17d 100644 --- a/engine/src/flutter/impeller/entity/shaders/blending/framebuffer_blend.frag +++ b/engine/src/flutter/impeller/entity/shaders/blending/framebuffer_blend.frag @@ -28,6 +28,7 @@ uniform sampler2D texture_sampler_src; uniform FragInfo { float16_t src_input_alpha; + float16_t dst_input_alpha; } frag_info; @@ -44,6 +45,7 @@ vec4 Sample(sampler2D texture_sampler, vec2 texture_coords) { void main() { f16vec4 dst = IPHalfUnpremultiply(f16vec4(ReadDestination())); + dst.a *= frag_info.dst_input_alpha; f16vec4 src = IPHalfUnpremultiply( f16vec4(Sample(texture_sampler_src, // sampler v_src_texture_coords // texture coordinates diff --git a/engine/src/flutter/impeller/tools/malioc.json b/engine/src/flutter/impeller/tools/malioc.json index 787254a4fa0..8dacf095ca2 100644 --- a/engine/src/flutter/impeller/tools/malioc.json +++ b/engine/src/flutter/impeller/tools/malioc.json @@ -20,8 +20,8 @@ "arith_fma" ], "longest_path_cycles": [ - 0.578125, - 0.578125, + 0.53125, + 0.53125, 0.21875, 0.125, 0.0, @@ -42,8 +42,8 @@ "arith_fma" ], "shortest_path_cycles": [ - 0.53125, - 0.53125, + 0.484375, + 0.484375, 0.203125, 0.0625, 0.0, @@ -55,8 +55,8 @@ "arith_fma" ], "total_cycles": [ - 0.578125, - 0.578125, + 0.53125, + 0.53125, 0.296875, 0.125, 0.0, @@ -1114,8 +1114,8 @@ "arith_fma" ], "longest_path_cycles": [ - 0.578125, - 0.578125, + 0.53125, + 0.53125, 0.265625, 0.125, 0.0, @@ -1136,8 +1136,8 @@ "arith_fma" ], "shortest_path_cycles": [ - 0.53125, - 0.53125, + 0.484375, + 0.484375, 0.21875, 0.0625, 0.0, @@ -1149,8 +1149,8 @@ "arith_fma" ], "total_cycles": [ - 0.578125, - 0.578125, + 0.53125, + 0.53125, 0.34375, 0.125, 0.0, @@ -1178,7 +1178,7 @@ "arithmetic" ], "longest_path_cycles": [ - 4.289999961853027, + 3.9600000381469727, 2.0, 2.0 ], diff --git a/engine/src/flutter/testing/impeller_golden_tests_output.txt b/engine/src/flutter/testing/impeller_golden_tests_output.txt index 6a4cd13cf7e..d7d9a085c80 100644 --- a/engine/src/flutter/testing/impeller_golden_tests_output.txt +++ b/engine/src/flutter/testing/impeller_golden_tests_output.txt @@ -1,5 +1,8 @@ digest.json impeller_GoldenTests_ConicalGradient.png +impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_Metal.png +impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_OpenGLES.png +impeller_Play_AiksTest_AdvancedBlendColorFilterWithDestinationOpacity_Vulkan.png impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_Metal.png impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_OpenGLES.png impeller_Play_AiksTest_BackdropRestoreUsesCorrectCoverageForFirstRestoredClip_Vulkan.png