Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[Impeller] adds a plus advanced blend for f16 pixel formats #51589

Merged
merged 25 commits into from
Mar 27, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
25 commits
Select commit Hold shift + click to select a range
ed6da6a
[Impeller] adds wide gamut golden tests
gaaclarke Mar 15, 2024
b51b9a2
updated golden
gaaclarke Mar 18, 2024
55135b8
moved to a magic name
gaaclarke Mar 26, 2024
509f20b
[Impeller] fixes alpha blend for plus blend mode
gaaclarke Mar 15, 2024
47498e1
[Impeller] made plus an advanced blend
gaaclarke Mar 21, 2024
c2a9d68
fixed the blending math (jasons test passes now)
gaaclarke Mar 22, 2024
d3e714e
fixed ColorBlendReturnsExpectedResults test
gaaclarke Mar 25, 2024
85e93d7
refactored the geometry tests to not be dependent on order
gaaclarke Mar 25, 2024
dacb1e4
removed single plus test, renamed the overlapping one
gaaclarke Mar 25, 2024
418c811
fixed Play/AiksTest.PaintBlendModeIsRespected/OpenGLES
gaaclarke Mar 25, 2024
c3aa8cc
moved the test to the new magic name, removed the blending test
gaaclarke Mar 26, 2024
5b5e6ca
gave golden image tests the magic name feature
gaaclarke Mar 26, 2024
7953bd7
resurrected the old blend
gaaclarke Mar 26, 2024
eb03810
added switch for the advancedblend
gaaclarke Mar 26, 2024
9a8ee6f
added tests for the magic test name
gaaclarke Mar 27, 2024
7ca79f6
renamed some variables
gaaclarke Mar 27, 2024
337876d
moved things to match how they were
gaaclarke Mar 27, 2024
669a096
factored out the shared blend function
gaaclarke Mar 27, 2024
cc8ab6a
factored out shared function
gaaclarke Mar 27, 2024
90330f4
added skip for non metal tests
gaaclarke Mar 27, 2024
2650ee0
fixed opengl run
gaaclarke Mar 27, 2024
afa7908
updated goldens
gaaclarke Mar 27, 2024
7081ef7
updated goldens
gaaclarke Mar 27, 2024
5025ec1
added support for colorfilters
gaaclarke Mar 27, 2024
202f56d
format
gaaclarke Mar 27, 2024
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion display_list/testing/dl_test_surface_metal.cc
Original file line number Diff line number Diff line change
Expand Up @@ -115,7 +115,7 @@ sk_sp<DlImage> DlMetalSurfaceProvider::MakeImpellerImage(

void DlMetalSurfaceProvider::InitScreenShotter() const {
if (!snapshotter_) {
snapshotter_.reset(new MetalScreenshotter());
snapshotter_.reset(new MetalScreenshotter(/*enable_wide_gamut=*/false));
auto typographer = impeller::TypographerContextSkia::Make();
aiks_context_.reset(new impeller::AiksContext(
snapshotter_->GetPlayground().GetContext(), typographer));
Expand Down
70 changes: 70 additions & 0 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1101,6 +1101,76 @@ TEST_P(AiksTest, PaintBlendModeIsRespected) {
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

// This makes sure the WideGamut named tests use 16bit float pixel format.
TEST_P(AiksTest, F16WideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
EXPECT_FALSE(IsAlphaClampedToOne(
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
}

TEST_P(AiksTest, NotF16) {
EXPECT_TRUE(IsAlphaClampedToOne(
GetContext()->GetCapabilities()->GetDefaultColorFormat()));
}

// Bug: https://github.com/flutter/flutter/issues/142549
TEST_P(AiksTest, BlendModePlusAlphaWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
auto texture = CreateTextureForFixture("airplane.jpg",
/*enable_mipmapping=*/true);

Canvas canvas;
canvas.Scale(GetContentScale());
canvas.DrawPaint({.color = Color(0.9, 1.0, 0.9, 1.0)});
canvas.SaveLayer({});
Paint paint;
paint.blend_mode = BlendMode::kPlus;
paint.color = Color::Red();
canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint);
paint.color = Color::White();
canvas.DrawImageRect(
std::make_shared<Image>(texture), Rect::MakeSize(texture->GetSize()),
Rect::MakeXYWH(100, 100, 400, 400).Expand(-100, -100), paint);
canvas.Restore();
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

// Bug: https://github.com/flutter/flutter/issues/142549
TEST_P(AiksTest, BlendModePlusAlphaColorFilterWideGamut) {
if (GetParam() != PlaygroundBackend::kMetal) {
GTEST_SKIP_("This backend doesn't yet support wide gamut.");
}
EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
PixelFormat::kR16G16B16A16Float);
auto texture = CreateTextureForFixture("airplane.jpg",
/*enable_mipmapping=*/true);

Canvas canvas;
canvas.Scale(GetContentScale());
canvas.DrawPaint({.color = Color(0.1, 0.2, 0.1, 1.0)});
canvas.SaveLayer({
.color_filter =
ColorFilter::MakeBlend(BlendMode::kPlus, Color(Vector4{1, 0, 0, 1})),
});
Paint paint;
paint.color = Color::Red();
canvas.DrawRect(Rect::MakeXYWH(100, 100, 400, 400), paint);
paint.color = Color::White();
canvas.DrawImageRect(
std::make_shared<Image>(texture), Rect::MakeSize(texture->GetSize()),
Rect::MakeXYWH(100, 100, 400, 400).Expand(-100, -100), paint);
canvas.Restore();
ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
}

TEST_P(AiksTest, ColorWheel) {
// Compare with https://fiddle.skia.org/c/@BlendModes

Expand Down
8 changes: 8 additions & 0 deletions impeller/compiler/shader_lib/impeller/color.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -46,4 +46,12 @@ f16vec4 IPHalfPremultiply(f16vec4 color) {
return f16vec4(color.rgb * color.a, color.a);
}

/// Performs the plus blend on `src` and `dst` which are premultiplied colors.
//`max` determines the values the results are clamped to.
f16vec4 IPHalfPlusBlend(f16vec4 src, f16vec4 dst) {
float16_t min = 0.0hf;
float16_t max = 1.0hf;
return clamp(dst + src, min, max);
}

#endif
8 changes: 8 additions & 0 deletions impeller/core/formats.h
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,14 @@ constexpr bool IsStencilWritable(PixelFormat format) {
}
}

/// Returns `true` if the pixel format has an implicit `clamp(x, 0, 1)` in the
/// pixel format. This is important for example when performing the `Plus` blend
/// where we don't want alpha values over 1.0.
constexpr bool IsAlphaClampedToOne(PixelFormat pixel_format) {
return !(pixel_format == PixelFormat::kR32G32B32A32Float ||
pixel_format == PixelFormat::kR16G16B16A16Float);
}

constexpr const char* PixelFormatToString(PixelFormat format) {
switch (format) {
case PixelFormat::kUnknown:
Expand Down
9 changes: 9 additions & 0 deletions impeller/entity/contents/content_context.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,6 +124,8 @@ void ContentContextOptions::ApplyToPipelineDescriptor(
color0.src_color_blend_factor = BlendFactor::kOneMinusDestinationAlpha;
break;
case BlendMode::kPlus:
// The kPlusAdvanced should be used instead.
FML_DCHECK(IsAlphaClampedToOne(color_attachment_pixel_format));
color0.dst_alpha_blend_factor = BlendFactor::kOne;
color0.dst_color_blend_factor = BlendFactor::kOne;
color0.src_alpha_blend_factor = BlendFactor::kOne;
Expand Down Expand Up @@ -324,6 +326,10 @@ ContentContext::ContentContext(
framebuffer_blend_lighten_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kLighten), supports_decal});
framebuffer_blend_plus_advanced_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kPlusAdvanced),
supports_decal});
framebuffer_blend_luminosity_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kLuminosity), supports_decal});
Expand Down Expand Up @@ -371,6 +377,9 @@ ContentContext::ContentContext(
blend_lighten_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kLighten), supports_decal});
blend_plus_advanced_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kPlusAdvanced), supports_decal});
blend_luminosity_pipelines_.CreateDefault(
*context_, options_trianglestrip,
{static_cast<Scalar>(BlendSelectValues::kLuminosity), supports_decal});
Expand Down
19 changes: 19 additions & 0 deletions impeller/entity/contents/content_context.h
Original file line number Diff line number Diff line change
Expand Up @@ -197,6 +197,8 @@ using BlendHuePipeline =
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
using BlendLightenPipeline =
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
using BlendPlusAdvancedPipeline =
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
using BlendLuminosityPipeline =
RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
using BlendMultiplyPipeline =
Expand Down Expand Up @@ -237,6 +239,9 @@ using FramebufferBlendHuePipeline =
using FramebufferBlendLightenPipeline =
RenderPipelineT<FramebufferBlendVertexShader,
FramebufferBlendFragmentShader>;
using FramebufferBlendPlusAdvancedPipeline =
RenderPipelineT<FramebufferBlendVertexShader,
FramebufferBlendFragmentShader>;
using FramebufferBlendLuminosityPipeline =
RenderPipelineT<FramebufferBlendVertexShader,
FramebufferBlendFragmentShader>;
Expand Down Expand Up @@ -640,6 +645,11 @@ class ContentContext {
return GetPipeline(blend_lighten_pipelines_, opts);
}

std::shared_ptr<Pipeline<PipelineDescriptor>> GetBlendPlusAdvancedPipeline(
ContentContextOptions opts) const {
return GetPipeline(blend_plus_advanced_pipelines_, opts);
}

std::shared_ptr<Pipeline<PipelineDescriptor>> GetBlendLuminosityPipeline(
ContentContextOptions opts) const {
return GetPipeline(blend_luminosity_pipelines_, opts);
Expand Down Expand Up @@ -725,6 +735,12 @@ class ContentContext {
return GetPipeline(framebuffer_blend_lighten_pipelines_, opts);
}

std::shared_ptr<Pipeline<PipelineDescriptor>>
GetFramebufferBlendPlusAdvancedPipeline(ContentContextOptions opts) const {
FML_DCHECK(GetDeviceCapabilities().SupportsFramebufferFetch());
return GetPipeline(framebuffer_blend_plus_advanced_pipelines_, opts);
}

std::shared_ptr<Pipeline<PipelineDescriptor>>
GetFramebufferBlendLuminosityPipeline(ContentContextOptions opts) const {
FML_DCHECK(GetDeviceCapabilities().SupportsFramebufferFetch());
Expand Down Expand Up @@ -989,6 +1005,7 @@ class ContentContext {
mutable Variants<BlendHardLightPipeline> blend_hardlight_pipelines_;
mutable Variants<BlendHuePipeline> blend_hue_pipelines_;
mutable Variants<BlendLightenPipeline> blend_lighten_pipelines_;
mutable Variants<BlendPlusAdvancedPipeline> blend_plus_advanced_pipelines_;
mutable Variants<BlendLuminosityPipeline> blend_luminosity_pipelines_;
mutable Variants<BlendMultiplyPipeline> blend_multiply_pipelines_;
mutable Variants<BlendOverlayPipeline> blend_overlay_pipelines_;
Expand All @@ -1014,6 +1031,8 @@ class ContentContext {
framebuffer_blend_hue_pipelines_;
mutable Variants<FramebufferBlendLightenPipeline>
framebuffer_blend_lighten_pipelines_;
mutable Variants<FramebufferBlendPlusAdvancedPipeline>
framebuffer_blend_plus_advanced_pipelines_;
mutable Variants<FramebufferBlendLuminosityPipeline>
framebuffer_blend_luminosity_pipelines_;
mutable Variants<FramebufferBlendMultiplyPipeline>
Expand Down
21 changes: 16 additions & 5 deletions impeller/entity/contents/filters/blend_filter_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,6 +338,9 @@ std::optional<Entity> BlendFilterContents::CreateForegroundAdvancedBlend(
case BlendMode::kColor:
pass.SetPipeline(renderer.GetBlendColorPipeline(options));
break;
case BlendMode::kPlusAdvanced:
pass.SetPipeline(renderer.GetBlendPlusAdvancedPipeline(options));
break;
case BlendMode::kLuminosity:
pass.SetPipeline(renderer.GetBlendLuminosityPipeline(options));
break;
Expand Down Expand Up @@ -583,6 +586,7 @@ void BlendFilterContents::SetBlendMode(BlendMode blend_mode) {
BLEND_CASE(Hue)
BLEND_CASE(Saturation)
BLEND_CASE(Color)
BLEND_CASE(PlusAdvanced)
BLEND_CASE(Luminosity)
default:
FML_UNREACHABLE();
Expand Down Expand Up @@ -611,19 +615,26 @@ std::optional<Entity> BlendFilterContents::RenderFilter(
std::nullopt, GetAbsorbOpacity(), GetAlpha());
}

if (blend_mode_ <= Entity::kLastPipelineBlendMode) {
return PipelineBlend(inputs, renderer, entity, coverage, blend_mode_,
BlendMode blend_mode = blend_mode_;
if (blend_mode == BlendMode::kPlus &&
!IsAlphaClampedToOne(
renderer.GetContext()->GetCapabilities()->GetDefaultColorFormat())) {
blend_mode = BlendMode::kPlusAdvanced;
}

if (blend_mode <= Entity::kLastPipelineBlendMode) {
return PipelineBlend(inputs, renderer, entity, coverage, blend_mode,
foreground_color_, GetAbsorbOpacity(), GetAlpha());
}

if (blend_mode_ <= Entity::kLastAdvancedBlendMode) {
if (blend_mode <= Entity::kLastAdvancedBlendMode) {
if (inputs.size() == 1 && foreground_color_.has_value() &&
GetAbsorbOpacity() == ColorFilterContents::AbsorbOpacity::kYes) {
return CreateForegroundAdvancedBlend(
inputs[0], renderer, entity, coverage, foreground_color_.value(),
blend_mode_, GetAlpha(), GetAbsorbOpacity());
blend_mode, GetAlpha(), GetAbsorbOpacity());
}
return advanced_blend_proc_(inputs, renderer, entity, coverage, blend_mode_,
return advanced_blend_proc_(inputs, renderer, entity, coverage, blend_mode,
foreground_color_, GetAbsorbOpacity(),
GetAlpha());
}
Expand Down
4 changes: 4 additions & 0 deletions impeller/entity/contents/framebuffer_blend_contents.cc
Original file line number Diff line number Diff line change
Expand Up @@ -118,6 +118,10 @@ bool FramebufferBlendContents::Render(const ContentContext& renderer,
case BlendMode::kColor:
pass.SetPipeline(renderer.GetFramebufferBlendColorPipeline(options));
break;
case BlendMode::kPlusAdvanced:
pass.SetPipeline(
renderer.GetFramebufferBlendPlusAdvancedPipeline(options));
break;
case BlendMode::kLuminosity:
pass.SetPipeline(renderer.GetFramebufferBlendLuminosityPipeline(options));
break;
Expand Down
1 change: 1 addition & 0 deletions impeller/entity/contents/framebuffer_blend_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ enum class BlendSelectValues {
kHue,
kSaturation,
kColor,
kPlusAdvanced,
kLuminosity,
};

Expand Down
7 changes: 7 additions & 0 deletions impeller/entity/entity_pass.cc
Original file line number Diff line number Diff line change
Expand Up @@ -941,6 +941,13 @@ bool EntityPass::OnRender(
/// Setup advanced blends.
///

if (result.entity.GetBlendMode() == BlendMode::kPlus &&
!IsAlphaClampedToOne(pass_context.GetPassTarget()
.GetRenderTarget()
.GetRenderTargetPixelFormat())) {
result.entity.SetBlendMode(BlendMode::kPlusAdvanced);
}

if (result.entity.GetBlendMode() > Entity::kLastPipelineBlendMode) {
if (renderer.GetDeviceCapabilities().SupportsFramebufferFetch()) {
auto src_contents = result.entity.GetContents();
Expand Down
43 changes: 27 additions & 16 deletions impeller/entity/shaders/blending/advanced_blend.frag
Original file line number Diff line number Diff line change
Expand Up @@ -36,22 +36,33 @@ f16vec4 Sample(f16sampler2D texture_sampler, vec2 texture_coords) {
}

void main() {
f16vec4 dst =
IPHalfUnpremultiply(Sample(texture_sampler_dst, // sampler
v_dst_texture_coords // texture coordinates
));
dst *= blend_info.dst_input_alpha;
f16vec4 src = blend_info.color_factor > 0.0hf
? blend_info.color
: IPHalfUnpremultiply(Sample(
texture_sampler_src, // sampler
v_src_texture_coords // texture coordinates
));
if (blend_info.color_factor == 0.0hf) {
src.a *= blend_info.src_input_alpha;
}
f16vec4 premultiplied_dst =
Sample(texture_sampler_dst, // sampler
v_dst_texture_coords // texture coordinates
);
int nblend_type = int(blend_type);

if (nblend_type == /*BlendSelectValues::kPlusAdvanced*/ 14) {
f16vec4 premultiplied_src =
Sample(texture_sampler_src, // sampler
v_src_texture_coords // texture coordinates
);
frag_color = IPHalfPlusBlend(premultiplied_src, premultiplied_dst);
} else {
f16vec4 dst = IPHalfUnpremultiply(premultiplied_dst);
dst *= blend_info.dst_input_alpha;

f16vec3 blend_result = AdvancedBlend(dst.rgb, src.rgb, int(blend_type));
f16vec4 src = blend_info.color_factor > 0.0hf
? blend_info.color
: IPHalfUnpremultiply(Sample(
texture_sampler_src, // sampler
v_src_texture_coords // texture coordinates
));
if (blend_info.color_factor == 0.0hf) {
src.a *= blend_info.src_input_alpha;
}

frag_color = IPApplyBlendedColor(dst, src, blend_result);
f16vec3 blend_result = AdvancedBlend(dst.rgb, src.rgb, nblend_type);
frag_color = IPApplyBlendedColor(dst, src, blend_result);
}
}
5 changes: 4 additions & 1 deletion impeller/entity/shaders/blending/blend_select.glsl
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,7 @@
// kHue,
// kSaturation,
// kColor,
// kPlusAdvanced,
// kLuminosity,
// Note, this isn't a switch as GLSL ES 1.0 does not support them.
f16vec3 AdvancedBlend(f16vec3 dst, f16vec3 src, int blend_type) {
Expand Down Expand Up @@ -65,7 +66,9 @@ f16vec3 AdvancedBlend(f16vec3 dst, f16vec3 src, int blend_type) {
if (blend_type == 13) {
return IPBlendColor(dst, src);
}
if (blend_type == 14) {
// 14 is `PlusAdvanced`, it's handled in advanced_blend.frag and
// framebuffer_blend.frag.
if (blend_type == 15) {
return IPBlendLuminosity(dst, src);
}
return f16vec3(0.0hf);
Expand Down
23 changes: 15 additions & 8 deletions impeller/entity/shaders/blending/framebuffer_blend.frag
Original file line number Diff line number Diff line change
Expand Up @@ -43,14 +43,21 @@ vec4 Sample(sampler2D texture_sampler, vec2 texture_coords) {
}

void main() {
f16vec4 dst = IPHalfUnpremultiply(f16vec4(ReadDestination()));
f16vec4 src = IPHalfUnpremultiply(
f16vec4 premultiplied_dst = f16vec4(ReadDestination());
f16vec4 premultiplied_src =
f16vec4(Sample(texture_sampler_src, // sampler
v_src_texture_coords // texture coordinates
)));
src.a *= frag_info.src_input_alpha;

f16vec3 blend_result = AdvancedBlend(dst.rgb, src.rgb, int(blend_type));

frag_color = IPApplyBlendedColor(dst, src, blend_result);
));
int nblend_type = int(blend_type);

if (nblend_type == /*BlendSelectValues::kPlusAdvanced*/ 14) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it would be OK to put this in its own shader.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in the middle of still refactoring this. I'll ping you when it's ready to review =)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see benefits and disadvantages both ways. What we shouldn't have is the code duplicated in 2 different parts. I've unified them with a shared function since this comment was made.

frag_color = IPHalfPlusBlend(premultiplied_src, premultiplied_dst);
} else {
f16vec4 dst = IPHalfUnpremultiply(premultiplied_dst);
f16vec4 src = IPHalfUnpremultiply(premultiplied_src);
src.a *= frag_info.src_input_alpha;

f16vec3 blend_result = AdvancedBlend(dst.rgb, src.rgb, nblend_type);
frag_color = IPApplyBlendedColor(dst, src, blend_result);
}
}
2 changes: 2 additions & 0 deletions impeller/geometry/color.cc
Original file line number Diff line number Diff line change
Expand Up @@ -276,6 +276,8 @@ Color Color::Blend(Color src, BlendMode blend_mode) const {
return (src.Premultiply() * (1 - dst.alpha) +
dst.Premultiply() * (1 - src.alpha))
.Unpremultiply();
case BlendMode::kPlusAdvanced:
[[fallthrough]];
case BlendMode::kPlus:
// r = min(s + d, 1)
return (Min(src.Premultiply() + dst.Premultiply(), 1)).Unpremultiply();
Expand Down
Loading