Skip to content

Commit b50789f

Browse files
authored
Reland: [Impeller] adds a plus advanced blend for f16 pixel formats (flutter#51756)
Relands flutter#51589 The fix is in 74397bc. I couldn't figure out how to get a test in the engine to cover it. The test is in the devicelab. Here's what I attempted: ```c++ TEST_P(AiksTest, BlendModePlusAlphaColorFilterAlphaWideGamut) { if (GetParam() != PlaygroundBackend::kMetal) { GTEST_SKIP_("This backend doesn't yet support wide gamut."); } EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(), PixelFormat::kR16G16B16A16Float); Canvas canvas; canvas.Scale(GetContentScale()); canvas.DrawPaint({.color = Color(0.1, 0.2, 0.1, 0.5)}); canvas.SaveLayer({ .color_filter = ColorFilter::MakeBlend(BlendMode::kPlus, Color(Vector4{1, 0, 0, 0.5})), }); Paint paint; paint.color = Color(1, 0, 0, 0.5); canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint); paint.color = Color::White(); canvas.Restore(); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } ``` ## 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
1 parent c176d11 commit b50789f

31 files changed

+393
-205
lines changed

display_list/testing/dl_test_surface_metal.cc

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -115,7 +115,7 @@ sk_sp<DlImage> DlMetalSurfaceProvider::MakeImpellerImage(
115115

116116
void DlMetalSurfaceProvider::InitScreenShotter() const {
117117
if (!snapshotter_) {
118-
snapshotter_.reset(new MetalScreenshotter());
118+
snapshotter_.reset(new MetalScreenshotter(/*enable_wide_gamut=*/false));
119119
auto typographer = impeller::TypographerContextSkia::Make();
120120
aiks_context_.reset(new impeller::AiksContext(
121121
snapshotter_->GetPlayground().GetContext(), typographer));

impeller/aiks/aiks_unittests.cc

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1102,6 +1102,76 @@ TEST_P(AiksTest, PaintBlendModeIsRespected) {
11021102
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
11031103
}
11041104

1105+
// This makes sure the WideGamut named tests use 16bit float pixel format.
1106+
TEST_P(AiksTest, F16WideGamut) {
1107+
if (GetParam() != PlaygroundBackend::kMetal) {
1108+
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
1109+
}
1110+
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
1111+
PixelFormat::kR16G16B16A16Float);
1112+
EXPECT_FALSE(IsAlphaClampedToOne(
1113+
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
1114+
}
1115+
1116+
TEST_P(AiksTest, NotF16) {
1117+
EXPECT_TRUE(IsAlphaClampedToOne(
1118+
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
1119+
}
1120+
1121+
// Bug: https://github.com/flutter/flutter/issues/142549
1122+
TEST_P(AiksTest, BlendModePlusAlphaWideGamut) {
1123+
if (GetParam() != PlaygroundBackend::kMetal) {
1124+
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
1125+
}
1126+
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
1127+
PixelFormat::kR16G16B16A16Float);
1128+
auto texture = CreateTextureForFixture("airplane.jpg",
1129+
/*enable_mipmapping=*/true);
1130+
1131+
Canvas canvas;
1132+
canvas.Scale(GetContentScale());
1133+
canvas.DrawPaint({.color = Color(0.9, 1.0, 0.9, 1.0)});
1134+
canvas.SaveLayer({});
1135+
Paint paint;
1136+
paint.blend_mode = BlendMode::kPlus;
1137+
paint.color = Color::Red();
1138+
canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint);
1139+
paint.color = Color::White();
1140+
canvas.DrawImageRect(
1141+
std::make_shared<Image>(texture), Rect::MakeSize(texture->GetSize()),
1142+
Rect::MakeXYWH(100, 100, 400, 400).Expand(-100, -100), paint);
1143+
canvas.Restore();
1144+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1145+
}
1146+
1147+
// Bug: https://github.com/flutter/flutter/issues/142549
1148+
TEST_P(AiksTest, BlendModePlusAlphaColorFilterWideGamut) {
1149+
if (GetParam() != PlaygroundBackend::kMetal) {
1150+
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
1151+
}
1152+
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
1153+
PixelFormat::kR16G16B16A16Float);
1154+
auto texture = CreateTextureForFixture("airplane.jpg",
1155+
/*enable_mipmapping=*/true);
1156+
1157+
Canvas canvas;
1158+
canvas.Scale(GetContentScale());
1159+
canvas.DrawPaint({.color = Color(0.1, 0.2, 0.1, 1.0)});
1160+
canvas.SaveLayer({
1161+
.color_filter =
1162+
ColorFilter::MakeBlend(BlendMode::kPlus, Color(Vector4{1, 0, 0, 1})),
1163+
});
1164+
Paint paint;
1165+
paint.color = Color::Red();
1166+
canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint);
1167+
paint.color = Color::White();
1168+
canvas.DrawImageRect(
1169+
std::make_shared<Image>(texture), Rect::MakeSize(texture->GetSize()),
1170+
Rect::MakeXYWH(100, 100, 400, 400).Expand(-100, -100), paint);
1171+
canvas.Restore();
1172+
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
1173+
}
1174+
11051175
TEST_P(AiksTest, ColorWheel) {
11061176
// Compare with https://fiddle.skia.org/c/@BlendModes
11071177

impeller/compiler/shader_lib/impeller/color.glsl

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -46,4 +46,12 @@ f16vec4 IPHalfPremultiply(f16vec4 color) {
4646
return f16vec4(color.rgb * color.a, color.a);
4747
}
4848

49+
/// Performs the plus blend on `src` and `dst` which are premultiplied colors.
50+
//`max` determines the values the results are clamped to.
51+
f16vec4 IPHalfPlusBlend(f16vec4 src, f16vec4 dst) {
52+
float16_t min = 0.0hf;
53+
float16_t max = 1.0hf;
54+
return clamp(dst + src, min, max);
55+
}
56+
4957
#endif

impeller/core/formats.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -138,6 +138,14 @@ constexpr bool IsStencilWritable(PixelFormat format) {
138138
}
139139
}
140140

141+
/// Returns `true` if the pixel format has an implicit `clamp(x, 0, 1)` in the
142+
/// pixel format. This is important for example when performing the `Plus` blend
143+
/// where we don't want alpha values over 1.0.
144+
constexpr bool IsAlphaClampedToOne(PixelFormat pixel_format) {
145+
return !(pixel_format == PixelFormat::kR32G32B32A32Float ||
146+
pixel_format == PixelFormat::kR16G16B16A16Float);
147+
}
148+
141149
constexpr const char* PixelFormatToString(PixelFormat format) {
142150
switch (format) {
143151
case PixelFormat::kUnknown:

impeller/entity/contents/content_context.cc

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -124,6 +124,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
124124
color0.src_color_blend_factor = BlendFactor::kOneMinusDestinationAlpha;
125125
break;
126126
case BlendMode::kPlus:
127+
// The kPlusAdvanced should be used instead.
128+
FML_DCHECK(IsAlphaClampedToOne(color_attachment_pixel_format));
127129
color0.dst_alpha_blend_factor = BlendFactor::kOne;
128130
color0.dst_color_blend_factor = BlendFactor::kOne;
129131
color0.src_alpha_blend_factor = BlendFactor::kOne;
@@ -324,6 +326,10 @@ ContentContext::ContentContext(
324326
framebuffer_blend_lighten_pipelines_.CreateDefault(
325327
*context_, options_trianglestrip,
326328
{static_cast<Scalar>(BlendSelectValues::kLighten), supports_decal});
329+
framebuffer_blend_plus_advanced_pipelines_.CreateDefault(
330+
*context_, options_trianglestrip,
331+
{static_cast<Scalar>(BlendSelectValues::kPlusAdvanced),
332+
supports_decal});
327333
framebuffer_blend_luminosity_pipelines_.CreateDefault(
328334
*context_, options_trianglestrip,
329335
{static_cast<Scalar>(BlendSelectValues::kLuminosity), supports_decal});
@@ -371,6 +377,9 @@ ContentContext::ContentContext(
371377
blend_lighten_pipelines_.CreateDefault(
372378
*context_, options_trianglestrip,
373379
{static_cast<Scalar>(BlendSelectValues::kLighten), supports_decal});
380+
blend_plus_advanced_pipelines_.CreateDefault(
381+
*context_, options_trianglestrip,
382+
{static_cast<Scalar>(BlendSelectValues::kPlusAdvanced), supports_decal});
374383
blend_luminosity_pipelines_.CreateDefault(
375384
*context_, options_trianglestrip,
376385
{static_cast<Scalar>(BlendSelectValues::kLuminosity), supports_decal});

impeller/entity/contents/content_context.h

Lines changed: 19 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -197,6 +197,8 @@ using BlendHuePipeline =
197197
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
198198
using BlendLightenPipeline =
199199
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
200+
using BlendPlusAdvancedPipeline =
201+
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
200202
using BlendLuminosityPipeline =
201203
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
202204
using BlendMultiplyPipeline =
@@ -237,6 +239,9 @@ using FramebufferBlendHuePipeline =
237239
using FramebufferBlendLightenPipeline =
238240
RenderPipelineT<FramebufferBlendVertexShader,
239241
FramebufferBlendFragmentShader>;
242+
using FramebufferBlendPlusAdvancedPipeline =
243+
RenderPipelineT<FramebufferBlendVertexShader,
244+
FramebufferBlendFragmentShader>;
240245
using FramebufferBlendLuminosityPipeline =
241246
RenderPipelineT<FramebufferBlendVertexShader,
242247
FramebufferBlendFragmentShader>;
@@ -640,6 +645,11 @@ class ContentContext {
640645
return GetPipeline(blend_lighten_pipelines_, opts);
641646
}
642647

648+
std::shared_ptr<Pipeline<PipelineDescriptor>> GetBlendPlusAdvancedPipeline(
649+
ContentContextOptions opts) const {
650+
return GetPipeline(blend_plus_advanced_pipelines_, opts);
651+
}
652+
643653
std::shared_ptr<Pipeline<PipelineDescriptor>> GetBlendLuminosityPipeline(
644654
ContentContextOptions opts) const {
645655
return GetPipeline(blend_luminosity_pipelines_, opts);
@@ -725,6 +735,12 @@ class ContentContext {
725735
return GetPipeline(framebuffer_blend_lighten_pipelines_, opts);
726736
}
727737

738+
std::shared_ptr<Pipeline<PipelineDescriptor>>
739+
GetFramebufferBlendPlusAdvancedPipeline(ContentContextOptions opts) const {
740+
FML_DCHECK(GetDeviceCapabilities().SupportsFramebufferFetch());
741+
return GetPipeline(framebuffer_blend_plus_advanced_pipelines_, opts);
742+
}
743+
728744
std::shared_ptr<Pipeline<PipelineDescriptor>>
729745
GetFramebufferBlendLuminosityPipeline(ContentContextOptions opts) const {
730746
FML_DCHECK(GetDeviceCapabilities().SupportsFramebufferFetch());
@@ -989,6 +1005,7 @@ class ContentContext {
9891005
mutable Variants<BlendHardLightPipeline> blend_hardlight_pipelines_;
9901006
mutable Variants<BlendHuePipeline> blend_hue_pipelines_;
9911007
mutable Variants<BlendLightenPipeline> blend_lighten_pipelines_;
1008+
mutable Variants<BlendPlusAdvancedPipeline> blend_plus_advanced_pipelines_;
9921009
mutable Variants<BlendLuminosityPipeline> blend_luminosity_pipelines_;
9931010
mutable Variants<BlendMultiplyPipeline> blend_multiply_pipelines_;
9941011
mutable Variants<BlendOverlayPipeline> blend_overlay_pipelines_;
@@ -1014,6 +1031,8 @@ class ContentContext {
10141031
framebuffer_blend_hue_pipelines_;
10151032
mutable Variants<FramebufferBlendLightenPipeline>
10161033
framebuffer_blend_lighten_pipelines_;
1034+
mutable Variants<FramebufferBlendPlusAdvancedPipeline>
1035+
framebuffer_blend_plus_advanced_pipelines_;
10171036
mutable Variants<FramebufferBlendLuminosityPipeline>
10181037
framebuffer_blend_luminosity_pipelines_;
10191038
mutable Variants<FramebufferBlendMultiplyPipeline>

impeller/entity/contents/filters/blend_filter_contents.cc

Lines changed: 45 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -338,6 +338,9 @@ std::optional<Entity> BlendFilterContents::CreateForegroundAdvancedBlend(
338338
case BlendMode::kColor:
339339
pass.SetPipeline(renderer.GetBlendColorPipeline(options));
340340
break;
341+
case BlendMode::kPlusAdvanced:
342+
pass.SetPipeline(renderer.GetBlendPlusAdvancedPipeline(options));
343+
break;
341344
case BlendMode::kLuminosity:
342345
pass.SetPipeline(renderer.GetBlendLuminosityPipeline(options));
343346
break;
@@ -546,7 +549,7 @@ static std::optional<Entity> PipelineBlend(
546549

547550
#define BLEND_CASE(mode) \
548551
case BlendMode::k##mode: \
549-
advanced_blend_proc_ = \
552+
return \
550553
[](const FilterInput::Vector& inputs, const ContentContext& renderer, \
551554
const Entity& entity, const Rect& coverage, BlendMode blend_mode, \
552555
std::optional<Color> fg_color, \
@@ -559,35 +562,39 @@ static std::optional<Entity> PipelineBlend(
559562
}; \
560563
break;
561564

565+
namespace {
566+
BlendFilterContents::AdvancedBlendProc GetAdvancedBlendProc(
567+
BlendMode blend_mode) {
568+
switch (blend_mode) {
569+
BLEND_CASE(Screen)
570+
BLEND_CASE(Overlay)
571+
BLEND_CASE(Darken)
572+
BLEND_CASE(Lighten)
573+
BLEND_CASE(ColorDodge)
574+
BLEND_CASE(ColorBurn)
575+
BLEND_CASE(HardLight)
576+
BLEND_CASE(SoftLight)
577+
BLEND_CASE(Difference)
578+
BLEND_CASE(Exclusion)
579+
BLEND_CASE(Multiply)
580+
BLEND_CASE(Hue)
581+
BLEND_CASE(Saturation)
582+
BLEND_CASE(Color)
583+
BLEND_CASE(PlusAdvanced)
584+
BLEND_CASE(Luminosity)
585+
default:
586+
FML_UNREACHABLE();
587+
}
588+
}
589+
} // namespace
590+
562591
void BlendFilterContents::SetBlendMode(BlendMode blend_mode) {
563592
if (blend_mode > Entity::kLastAdvancedBlendMode) {
564593
VALIDATION_LOG << "Invalid blend mode " << static_cast<int>(blend_mode)
565594
<< " assigned to BlendFilterContents.";
566595
}
567596

568597
blend_mode_ = blend_mode;
569-
570-
if (blend_mode > Entity::kLastPipelineBlendMode) {
571-
switch (blend_mode) {
572-
BLEND_CASE(Screen)
573-
BLEND_CASE(Overlay)
574-
BLEND_CASE(Darken)
575-
BLEND_CASE(Lighten)
576-
BLEND_CASE(ColorDodge)
577-
BLEND_CASE(ColorBurn)
578-
BLEND_CASE(HardLight)
579-
BLEND_CASE(SoftLight)
580-
BLEND_CASE(Difference)
581-
BLEND_CASE(Exclusion)
582-
BLEND_CASE(Multiply)
583-
BLEND_CASE(Hue)
584-
BLEND_CASE(Saturation)
585-
BLEND_CASE(Color)
586-
BLEND_CASE(Luminosity)
587-
default:
588-
FML_UNREACHABLE();
589-
}
590-
}
591598
}
592599

593600
void BlendFilterContents::SetForegroundColor(std::optional<Color> color) {
@@ -611,21 +618,29 @@ std::optional<Entity> BlendFilterContents::RenderFilter(
611618
std::nullopt, GetAbsorbOpacity(), GetAlpha());
612619
}
613620

614-
if (blend_mode_ <= Entity::kLastPipelineBlendMode) {
615-
return PipelineBlend(inputs, renderer, entity, coverage, blend_mode_,
621+
BlendMode blend_mode = blend_mode_;
622+
if (blend_mode == BlendMode::kPlus &&
623+
!IsAlphaClampedToOne(
624+
renderer.GetContext()->GetCapabilities()->GetDefaultColorFormat())) {
625+
blend_mode = BlendMode::kPlusAdvanced;
626+
}
627+
628+
if (blend_mode <= Entity::kLastPipelineBlendMode) {
629+
return PipelineBlend(inputs, renderer, entity, coverage, blend_mode,
616630
foreground_color_, GetAbsorbOpacity(), GetAlpha());
617631
}
618632

619-
if (blend_mode_ <= Entity::kLastAdvancedBlendMode) {
633+
if (blend_mode <= Entity::kLastAdvancedBlendMode) {
620634
if (inputs.size() == 1 && foreground_color_.has_value() &&
621635
GetAbsorbOpacity() == ColorFilterContents::AbsorbOpacity::kYes) {
622636
return CreateForegroundAdvancedBlend(
623637
inputs[0], renderer, entity, coverage, foreground_color_.value(),
624-
blend_mode_, GetAlpha(), GetAbsorbOpacity());
638+
blend_mode, GetAlpha(), GetAbsorbOpacity());
625639
}
626-
return advanced_blend_proc_(inputs, renderer, entity, coverage, blend_mode_,
627-
foreground_color_, GetAbsorbOpacity(),
628-
GetAlpha());
640+
AdvancedBlendProc advanced_blend_proc = GetAdvancedBlendProc(blend_mode);
641+
return advanced_blend_proc(inputs, renderer, entity, coverage, blend_mode,
642+
foreground_color_, GetAbsorbOpacity(),
643+
GetAlpha());
629644
}
630645

631646
FML_UNREACHABLE();

impeller/entity/contents/filters/blend_filter_contents.h

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -79,7 +79,6 @@ class BlendFilterContents : public ColorFilterContents {
7979
ColorFilterContents::AbsorbOpacity absorb_opacity) const;
8080

8181
BlendMode blend_mode_ = BlendMode::kSourceOver;
82-
AdvancedBlendProc advanced_blend_proc_;
8382
std::optional<Color> foreground_color_;
8483

8584
BlendFilterContents(const BlendFilterContents&) = delete;

impeller/entity/contents/framebuffer_blend_contents.cc

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -118,6 +118,10 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer,
118118
case BlendMode::kColor:
119119
pass.SetPipeline(renderer.GetFramebufferBlendColorPipeline(options));
120120
break;
121+
case BlendMode::kPlusAdvanced:
122+
pass.SetPipeline(
123+
renderer.GetFramebufferBlendPlusAdvancedPipeline(options));
124+
break;
121125
case BlendMode::kLuminosity:
122126
pass.SetPipeline(renderer.GetFramebufferBlendLuminosityPipeline(options));
123127
break;

impeller/entity/contents/framebuffer_blend_contents.h

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@ enum class BlendSelectValues {
2727
kHue,
2828
kSaturation,
2929
kColor,
30+
kPlusAdvanced,
3031
kLuminosity,
3132
};
3233

0 commit comments

Comments
 (0)