Skip to content

Commit

Permalink
Reland: [Impeller] adds a plus advanced blend for f16 pixel formats (#…
Browse files Browse the repository at this point in the history
…51756)

Relands #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
  • Loading branch information
gaaclarke authored Mar 28, 2024
1 parent c176d11 commit b50789f
Show file tree
Hide file tree
Showing 31 changed files with 393 additions and 205 deletions.
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 @@ -1102,6 +1102,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
75 changes: 45 additions & 30 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 @@ -546,7 +549,7 @@ static std::optional<Entity> PipelineBlend(

#define BLEND_CASE(mode) \
case BlendMode::k##mode: \
advanced_blend_proc_ = \
return \
[](const FilterInput::Vector& inputs, const ContentContext& renderer, \
const Entity& entity, const Rect& coverage, BlendMode blend_mode, \
std::optional<Color> fg_color, \
Expand All @@ -559,35 +562,39 @@ static std::optional<Entity> PipelineBlend(
}; \
break;

namespace {
BlendFilterContents::AdvancedBlendProc GetAdvancedBlendProc(
BlendMode blend_mode) {
switch (blend_mode) {
BLEND_CASE(Screen)
BLEND_CASE(Overlay)
BLEND_CASE(Darken)
BLEND_CASE(Lighten)
BLEND_CASE(ColorDodge)
BLEND_CASE(ColorBurn)
BLEND_CASE(HardLight)
BLEND_CASE(SoftLight)
BLEND_CASE(Difference)
BLEND_CASE(Exclusion)
BLEND_CASE(Multiply)
BLEND_CASE(Hue)
BLEND_CASE(Saturation)
BLEND_CASE(Color)
BLEND_CASE(PlusAdvanced)
BLEND_CASE(Luminosity)
default:
FML_UNREACHABLE();
}
}
} // namespace

void BlendFilterContents::SetBlendMode(BlendMode blend_mode) {
if (blend_mode > Entity::kLastAdvancedBlendMode) {
VALIDATION_LOG << "Invalid blend mode " << static_cast<int>(blend_mode)
<< " assigned to BlendFilterContents.";
}

blend_mode_ = blend_mode;

if (blend_mode > Entity::kLastPipelineBlendMode) {
switch (blend_mode) {
BLEND_CASE(Screen)
BLEND_CASE(Overlay)
BLEND_CASE(Darken)
BLEND_CASE(Lighten)
BLEND_CASE(ColorDodge)
BLEND_CASE(ColorBurn)
BLEND_CASE(HardLight)
BLEND_CASE(SoftLight)
BLEND_CASE(Difference)
BLEND_CASE(Exclusion)
BLEND_CASE(Multiply)
BLEND_CASE(Hue)
BLEND_CASE(Saturation)
BLEND_CASE(Color)
BLEND_CASE(Luminosity)
default:
FML_UNREACHABLE();
}
}
}

void BlendFilterContents::SetForegroundColor(std::optional<Color> color) {
Expand All @@ -611,21 +618,29 @@ 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_,
foreground_color_, GetAbsorbOpacity(),
GetAlpha());
AdvancedBlendProc advanced_blend_proc = GetAdvancedBlendProc(blend_mode);
return advanced_blend_proc(inputs, renderer, entity, coverage, blend_mode,
foreground_color_, GetAbsorbOpacity(),
GetAlpha());
}

FML_UNREACHABLE();
Expand Down
1 change: 0 additions & 1 deletion impeller/entity/contents/filters/blend_filter_contents.h
Original file line number Diff line number Diff line change
Expand Up @@ -79,7 +79,6 @@ class BlendFilterContents : public ColorFilterContents {
ColorFilterContents::AbsorbOpacity absorb_opacity) const;

BlendMode blend_mode_ = BlendMode::kSourceOver;
AdvancedBlendProc advanced_blend_proc_;
std::optional<Color> foreground_color_;

BlendFilterContents(const BlendFilterContents&) = delete;
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 @@ -970,6 +970,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
Loading

0 comments on commit b50789f

Please sign in to comment.