From c9a121b15747d5f4dd7ea792cb3e246fcf37ad14 Mon Sep 17 00:00:00 2001 From: gaaclarke <30870216+gaaclarke@users.noreply.github.com> Date: Tue, 26 Sep 2023 12:14:08 -0700 Subject: [PATCH] [Impeller] fixes behavior for blurred rounded rect clear (#46167) fixes: https://github.com/flutter/flutter/issues/132849 I decided to mimic skia's blend function for clear only in the case for blurred rounded rects since that fixes the listed issue. I'm not sure if there are are other cases where we'd want this behavior. I've added golden tests for the other clear cases too to make sure we didn't break them. ## Pre-launch Checklist - [x] I read the [Contributor Guide] and followed the process outlined there for submitting PRs. - [x] I read the [Tree Hygiene] wiki page, which explains my responsibilities. - [x] I read and followed the [Flutter Style Guide] and the [C++, Objective-C, Java style guides]. - [x] I listed at least one issue that this PR fixes in the description above. - [x] I added new tests to check the change I am making or feature I am adding, or the PR is [test-exempt]. See [testing the engine] for instructions on writing and running engine tests. - [x] I updated/added relevant documentation (doc comments with `///`). - [x] I signed the [CLA]. - [x] All existing and new tests are passing. If you need help, consider asking for advice on the #hackers-new channel on [Discord]. [Contributor Guide]: https://github.com/flutter/flutter/wiki/Tree-hygiene#overview [Tree Hygiene]: https://github.com/flutter/flutter/wiki/Tree-hygiene [test-exempt]: https://github.com/flutter/flutter/wiki/Tree-hygiene#tests [Flutter Style Guide]: https://github.com/flutter/flutter/wiki/Style-guide-for-Flutter-repo [C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style [testing the engine]: https://github.com/flutter/flutter/wiki/Testing-the-engine [CLA]: https://cla.developers.google.com/ [flutter/tests]: https://github.com/flutter/tests [breaking change policy]: https://github.com/flutter/flutter/wiki/Tree-hygiene#handling-breaking-changes [Discord]: https://github.com/flutter/flutter/wiki/Chat --- impeller/aiks/aiks_unittests.cc | 32 +++++++++++++++++++ impeller/entity/contents/content_context.cc | 17 +++++++--- impeller/entity/contents/content_context.h | 12 ++++--- .../contents/solid_rrect_blur_contents.cc | 9 ++++-- 4 files changed, 59 insertions(+), 11 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index bf6bf0e5c2a81..7cf8b69fbc51d 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -3581,5 +3581,37 @@ TEST_P(AiksTest, VerticesGeometryUVPositionData) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +TEST_P(AiksTest, ClearBlendWithBlur) { + Canvas canvas; + Paint white; + white.color = Color::Blue(); + canvas.DrawRect(Rect::MakeXYWH(0, 0, 600.0, 600.0), white); + + Paint clear; + clear.blend_mode = BlendMode::kClear; + clear.mask_blur_descriptor = Paint::MaskBlurDescriptor{ + .style = FilterContents::BlurStyle::kNormal, + .sigma = Sigma(20), + }; + + canvas.DrawCircle(Point::MakeXY(300.0, 300.0), 200.0, clear); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + +TEST_P(AiksTest, ClearBlend) { + Canvas canvas; + Paint white; + white.color = Color::Blue(); + canvas.DrawRect(Rect::MakeXYWH(0, 0, 600.0, 600.0), white); + + Paint clear; + clear.blend_mode = BlendMode::kClear; + + canvas.DrawCircle(Point::MakeXY(300.0, 300.0), 200.0, clear); + + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + } // namespace testing } // namespace impeller diff --git a/impeller/entity/contents/content_context.cc b/impeller/entity/contents/content_context.cc index 1a4985db5a07e..1461e1b9cd9cf 100644 --- a/impeller/entity/contents/content_context.cc +++ b/impeller/entity/contents/content_context.cc @@ -38,10 +38,19 @@ void ContentContextOptions::ApplyToPipelineDescriptor( switch (pipeline_blend) { case BlendMode::kClear: - color0.dst_alpha_blend_factor = BlendFactor::kZero; - color0.dst_color_blend_factor = BlendFactor::kZero; - color0.src_alpha_blend_factor = BlendFactor::kZero; - color0.src_color_blend_factor = BlendFactor::kZero; + if (is_for_rrect_blur_clear) { + color0.alpha_blend_op = BlendOperation::kReverseSubtract; + color0.color_blend_op = BlendOperation::kReverseSubtract; + color0.dst_alpha_blend_factor = BlendFactor::kOne; + color0.dst_color_blend_factor = BlendFactor::kOne; + color0.src_alpha_blend_factor = BlendFactor::kDestinationColor; + color0.src_color_blend_factor = BlendFactor::kDestinationColor; + } else { + color0.dst_alpha_blend_factor = BlendFactor::kZero; + color0.dst_color_blend_factor = BlendFactor::kZero; + color0.src_alpha_blend_factor = BlendFactor::kZero; + color0.src_color_blend_factor = BlendFactor::kZero; + } break; case BlendMode::kSource: color0.blending_enabled = false; diff --git a/impeller/entity/contents/content_context.h b/impeller/entity/contents/content_context.h index 2778fec4f72c2..4c4c17c43b194 100644 --- a/impeller/entity/contents/content_context.h +++ b/impeller/entity/contents/content_context.h @@ -308,13 +308,14 @@ struct ContentContextOptions { PixelFormat color_attachment_pixel_format = PixelFormat::kUnknown; bool has_stencil_attachment = true; bool wireframe = false; + bool is_for_rrect_blur_clear = false; struct Hash { constexpr std::size_t operator()(const ContentContextOptions& o) const { - return fml::HashCombine(o.sample_count, o.blend_mode, o.stencil_compare, - o.stencil_operation, o.primitive_type, - o.color_attachment_pixel_format, - o.has_stencil_attachment, o.wireframe); + return fml::HashCombine( + o.sample_count, o.blend_mode, o.stencil_compare, o.stencil_operation, + o.primitive_type, o.color_attachment_pixel_format, + o.has_stencil_attachment, o.wireframe, o.is_for_rrect_blur_clear); } }; @@ -329,7 +330,8 @@ struct ContentContextOptions { lhs.color_attachment_pixel_format == rhs.color_attachment_pixel_format && lhs.has_stencil_attachment == rhs.has_stencil_attachment && - lhs.wireframe == rhs.wireframe; + lhs.wireframe == rhs.wireframe && + lhs.is_for_rrect_blur_clear == rhs.is_for_rrect_blur_clear; } }; diff --git a/impeller/entity/contents/solid_rrect_blur_contents.cc b/impeller/entity/contents/solid_rrect_blur_contents.cc index d8893b285e12c..ffb190cf7a271 100644 --- a/impeller/entity/contents/solid_rrect_blur_contents.cc +++ b/impeller/entity/contents/solid_rrect_blur_contents.cc @@ -88,8 +88,13 @@ bool SolidRRectBlurContents::Render(const ContentContext& renderer, Command cmd; DEBUG_COMMAND_INFO(cmd, "RRect Shadow"); - auto opts = OptionsFromPassAndEntity(pass, entity); + ContentContextOptions opts = OptionsFromPassAndEntity(pass, entity); opts.primitive_type = PrimitiveType::kTriangle; + Color color = color_; + if (entity.GetBlendMode() == BlendMode::kClear) { + opts.is_for_rrect_blur_clear = true; + color = Color::White(); + } cmd.pipeline = renderer.GetRRectBlurPipeline(opts); cmd.stencil_reference = entity.GetStencilDepth(); @@ -102,7 +107,7 @@ bool SolidRRectBlurContents::Render(const ContentContext& renderer, VS::BindFrameInfo(cmd, pass.GetTransientsBuffer().EmplaceUniform(frame_info)); FS::FragInfo frag_info; - frag_info.color = color_; + frag_info.color = color; frag_info.blur_sigma = blur_sigma; frag_info.rect_size = Point(positive_rect.size); frag_info.corner_radius =