Skip to content
This repository has been archived by the owner on Feb 25, 2025. It is now read-only.

Commit

Permalink
[Impeller] fixes behavior for blurred rounded rect clear (#46167)
Browse files Browse the repository at this point in the history
fixes: flutter/flutter#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].

<!-- Links -->
[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
  • Loading branch information
gaaclarke authored Sep 26, 2023
1 parent c7b3bc5 commit c9a121b
Show file tree
Hide file tree
Showing 4 changed files with 59 additions and 11 deletions.
32 changes: 32 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
17 changes: 13 additions & 4 deletions impeller/entity/contents/content_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
12 changes: 7 additions & 5 deletions impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -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);
}
};

Expand All @@ -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;
}
};

Expand Down
9 changes: 7 additions & 2 deletions impeller/entity/contents/solid_rrect_blur_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand All @@ -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 =
Expand Down

0 comments on commit c9a121b

Please sign in to comment.