[Impeller] removes advanced plus blending (#52163)

Now that #52019 has landed, we
shouldn't need it anymore.

## 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

Author: gaaclarke

impeller/aiks/aiks_unittests.cc

@@ -941,13 +941,14 @@ TEST_P(AiksTest, CanDrawPaintMultipleTimes) {
 TEST_P(AiksTest, FormatWideGamut) {
   EXPECT_EQ(GetContext()->GetCapabilities()->GetDefaultColorFormat(),
             PixelFormat::kB10G10R10A10XR);
-  EXPECT_TRUE(IsAlphaClampedToOne(
-      GetContext()->GetCapabilities()->GetDefaultColorFormat()));
 }
 
 TEST_P(AiksTest, FormatSRGB) {
-  EXPECT_TRUE(IsAlphaClampedToOne(
-      GetContext()->GetCapabilities()->GetDefaultColorFormat()));
+  PixelFormat pixel_format =
+      GetContext()->GetCapabilities()->GetDefaultColorFormat();
+  EXPECT_TRUE(pixel_format == PixelFormat::kR8G8B8A8UNormInt ||
+              pixel_format == PixelFormat::kB8G8R8A8UNormInt)
+      << "pixel format: " << PixelFormatToString(pixel_format);
 }
 
 TEST_P(AiksTest, TransformMultipliesCorrectly) {

impeller/compiler/shader_lib/impeller/color.glsl

@@ -46,12 +46,4 @@ 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

impeller/core/formats.h

@@ -138,14 +138,6 @@ 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:

impeller/entity/contents/content_context.cc

@@ -124,8 +124,6 @@ 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;
@@ -326,10 +324,6 @@ 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});
@@ -377,9 +371,6 @@ 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});

impeller/entity/contents/content_context.h

@@ -181,8 +181,6 @@ using BlendHuePipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendLightenPipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
-using BlendPlusAdvancedPipeline =
-    RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendLuminosityPipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendMultiplyPipeline =
@@ -223,9 +221,6 @@ using FramebufferBlendHuePipeline =
 using FramebufferBlendLightenPipeline =
     RenderPipelineT<FramebufferBlendVertexShader,
                     FramebufferBlendFragmentShader>;
-using FramebufferBlendPlusAdvancedPipeline =
-    RenderPipelineT<FramebufferBlendVertexShader,
-                    FramebufferBlendFragmentShader>;
 using FramebufferBlendLuminosityPipeline =
     RenderPipelineT<FramebufferBlendVertexShader,
                     FramebufferBlendFragmentShader>;
@@ -609,11 +604,6 @@ 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);
@@ -699,12 +689,6 @@ 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());
@@ -967,7 +951,6 @@ 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_;
@@ -993,8 +976,6 @@ 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>

impeller/entity/contents/filters/blend_filter_contents.cc

@@ -339,9 +339,6 @@ 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;
@@ -665,7 +662,7 @@ static std::optional<Entity> PipelineBlend(
 
 #define BLEND_CASE(mode)                                                      \
   case BlendMode::k##mode:                                                    \
-    return                                                                    \
+    advanced_blend_proc_ =                                                    \
         [](const FilterInput::Vector& inputs, const ContentContext& renderer, \
            const Entity& entity, const Rect& coverage, BlendMode blend_mode,  \
            std::optional<Color> fg_color,                                     \
@@ -678,39 +675,35 @@ 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) {
@@ -734,35 +727,27 @@ std::optional<Entity> BlendFilterContents::RenderFilter(
                          std::nullopt, GetAbsorbOpacity(), GetAlpha());
   }
 
-  BlendMode blend_mode = blend_mode_;
-  if (blend_mode == BlendMode::kPlus &&
-      !IsAlphaClampedToOne(
-          renderer.GetContext()->GetCapabilities()->GetDefaultColorFormat())) {
-    blend_mode = BlendMode::kPlusAdvanced;
-  }
-
-  if (blend_mode <= Entity::kLastPipelineBlendMode) {
+  if (blend_mode_ <= Entity::kLastPipelineBlendMode) {
     if (inputs.size() == 1 && foreground_color_.has_value() &&
         GetAbsorbOpacity() == ColorFilterContents::AbsorbOpacity::kYes) {
       return CreateForegroundPorterDuffBlend(
           inputs[0], renderer, entity, coverage, foreground_color_.value(),
           blend_mode_, GetAlpha(), GetAbsorbOpacity());
     }
-    return PipelineBlend(inputs, renderer, entity, coverage, blend_mode,
+    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());
     }
-    AdvancedBlendProc advanced_blend_proc = GetAdvancedBlendProc(blend_mode);
-    return advanced_blend_proc(inputs, renderer, entity, coverage, blend_mode,
-                               foreground_color_, GetAbsorbOpacity(),
-                               GetAlpha());
+    return advanced_blend_proc_(inputs, renderer, entity, coverage, blend_mode_,
+                                foreground_color_, GetAbsorbOpacity(),
+                                GetAlpha());
   }
 
   FML_UNREACHABLE();

impeller/entity/contents/filters/blend_filter_contents.h

@@ -93,6 +93,7 @@ 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;

impeller/entity/contents/framebuffer_blend_contents.cc

@@ -117,10 +117,6 @@ 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;

impeller/entity/contents/framebuffer_blend_contents.h

@@ -27,7 +27,6 @@ enum class BlendSelectValues {
   kHue,
   kSaturation,
   kColor,
-  kPlusAdvanced,
   kLuminosity,
 };
 

impeller/entity/entity_pass.cc

@@ -967,13 +967,6 @@ 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();