Reverts "[Impeller] adds a plus advanced blend for f16 pixel formats (#51589)" (#51741)

Reverts: #51589
Initiated by: jonahwilliams
Reason for reverting: draw vertices devicelab test is crashing due to SIGABRT in blend contents

![image](https://github.com/flutter/engine/assets/8975114/8bfaec63-29e9-43c2-8954-181d0ad1c413)

Original PR Author: gaaclarke

Reviewed By: {jonahwilliams}

This change reverts the following previous change:
fixes flutter/flutter#142549

[C++, Objective-C, Java style guides]: https://github.com/flutter/engine/blob/main/CONTRIBUTING.md#style

Author: auto-submit[bot]

display_list/testing/dl_test_surface_metal.cc

@@ -115,7 +115,7 @@ sk_sp<DlImage> DlMetalSurfaceProvider::MakeImpellerImage(
 
 void DlMetalSurfaceProvider::InitScreenShotter() const {
   if (!snapshotter_) {
-    snapshotter_.reset(new MetalScreenshotter(/*enable_wide_gamut=*/false));
+    snapshotter_.reset(new MetalScreenshotter());
     auto typographer = impeller::TypographerContextSkia::Make();
     aiks_context_.reset(new impeller::AiksContext(
         snapshotter_->GetPlayground().GetContext(), typographer));

impeller/aiks/aiks_unittests.cc

@@ -1102,76 +1102,6 @@ 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
 

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

@@ -197,8 +197,6 @@ using BlendHuePipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendLightenPipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
-using BlendPlusAdvancedPipeline =
-    RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendLuminosityPipeline =
     RenderPipelineT<AdvancedBlendVertexShader, AdvancedBlendFragmentShader>;
 using BlendMultiplyPipeline =
@@ -239,9 +237,6 @@ using FramebufferBlendHuePipeline =
 using FramebufferBlendLightenPipeline =
     RenderPipelineT<FramebufferBlendVertexShader,
                     FramebufferBlendFragmentShader>;
-using FramebufferBlendPlusAdvancedPipeline =
-    RenderPipelineT<FramebufferBlendVertexShader,
-                    FramebufferBlendFragmentShader>;
 using FramebufferBlendLuminosityPipeline =
     RenderPipelineT<FramebufferBlendVertexShader,
                     FramebufferBlendFragmentShader>;
@@ -645,11 +640,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);
@@ -735,12 +725,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());
@@ -1005,7 +989,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_;
@@ -1031,8 +1014,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

@@ -338,9 +338,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;
@@ -586,7 +583,6 @@ void BlendFilterContents::SetBlendMode(BlendMode blend_mode) {
       BLEND_CASE(Hue)
       BLEND_CASE(Saturation)
       BLEND_CASE(Color)
-      BLEND_CASE(PlusAdvanced)
       BLEND_CASE(Luminosity)
       default:
         FML_UNREACHABLE();
@@ -615,26 +611,19 @@ 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) {
-    return PipelineBlend(inputs, renderer, entity, coverage, blend_mode,
+  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());
   }

impeller/entity/contents/framebuffer_blend_contents.cc

@@ -118,10 +118,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

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