[Impeller] simplify invert colors flag by supporting composed color filters. (flutter/engine#46391)

Fixes https://github.com/flutter/flutter/issues/135699

To avoid the ugly complicated code for conditionally applying both colorfilters, make the dl dispatcher handle merging these filters together in a defined order. The original bug is caused by not checking for invert colors in a CPU fast path.

engine/src/flutter/impeller/aiks/aiks_unittests.cc

@@ -13,6 +13,7 @@
 #include "flutter/testing/testing.h"
 #include "impeller/aiks/aiks_playground.h"
 #include "impeller/aiks/canvas.h"
+#include "impeller/aiks/color_filter.h"
 #include "impeller/aiks/image.h"
 #include "impeller/aiks/image_filter.h"
 #include "impeller/aiks/paint_pass_delegate.h"
@@ -123,16 +124,47 @@ TEST_P(AiksTest, CanRenderImage) {
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
 }
 
-TEST_P(AiksTest, CanRenderInvertedImage) {
+constexpr ColorMatrix kColorInversion = {.array = {
+                                             -1.0, 0,    0,    1.0, 0,  //
+                                             0,    -1.0, 0,    1.0, 0,  //
+                                             0,    0,    -1.0, 1.0, 0,  //
+                                             1.0,  1.0,  1.0,  1.0, 0   //
+                                         }};
+
+TEST_P(AiksTest, CanRenderMergedColorFilterImage) {
   Canvas canvas;
   Paint paint;
   auto image = std::make_shared<Image>(CreateTextureForFixture("kalimba.jpg"));
   paint.color = Color::Red();
-  paint.invert_colors = true;
+  paint.color_filter = ColorFilter::MakeComposed(
+      ColorFilter::MakeMatrix(kColorInversion),
+      ColorFilter::MakeBlend(BlendMode::kSourceOver, Color::Yellow()));
   canvas.DrawImage(image, Point::MakeXY(100.0, 100.0), paint);
   ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
 }
 
+TEST_P(AiksTest, CanRenderMergedColorFilter) {
+  Canvas canvas;
+  Paint paint;
+  paint.color = Color::Red();
+  paint.color_filter = ColorFilter::MakeComposed(
+      ColorFilter::MakeMatrix(kColorInversion),
+      ColorFilter::MakeBlend(BlendMode::kSourceOver, Color::Yellow()));
+  canvas.DrawRect(Rect::MakeLTRB(0, 0, 100, 100), paint);
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
+TEST_P(AiksTest, CanRenderMergedColorFilterDrawPaint) {
+  Canvas canvas;
+  Paint paint;
+  paint.color = Color::Red();
+  paint.color_filter = ColorFilter::MakeComposed(
+      ColorFilter::MakeMatrix(kColorInversion),
+      ColorFilter::MakeBlend(BlendMode::kSourceOver, Color::Yellow()));
+  canvas.DrawPaint(paint);
+  ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture()));
+}
+
 namespace {
 bool GenerateMipmap(const std::shared_ptr<Context>& context,
                     std::shared_ptr<Texture> texture,

engine/src/flutter/impeller/aiks/canvas.cc

@@ -4,7 +4,6 @@
 
 #include "impeller/aiks/canvas.h"
 
-#include <algorithm>
 #include <optional>
 #include <utility>
 

engine/src/flutter/impeller/aiks/color_filter.cc

@@ -3,7 +3,10 @@
 // found in the LICENSE file.
 
 #include "impeller/aiks/color_filter.h"
+
+#include <utility>
 #include "impeller/entity/contents/filters/color_filter_contents.h"
+#include "impeller/entity/contents/filters/filter_contents.h"
 #include "impeller/entity/contents/filters/inputs/filter_input.h"
 #include "impeller/geometry/color.h"
 
@@ -34,6 +37,12 @@ std::shared_ptr<ColorFilter> ColorFilter::MakeLinearToSrgb() {
   return std::make_shared<LinearToSrgbColorFilter>();
 }
 
+std::shared_ptr<ColorFilter> ColorFilter::MakeComposed(
+    const std::shared_ptr<ColorFilter>& outer,
+    const std::shared_ptr<ColorFilter>& inner) {
+  return std::make_shared<ComposedColorFilter>(outer, inner);
+}
+
 /*******************************************************************************
  ******* BlendColorFilter
  ******************************************************************************/
@@ -142,4 +151,40 @@ std::shared_ptr<ColorFilter> LinearToSrgbColorFilter::Clone() const {
   return std::make_shared<LinearToSrgbColorFilter>(*this);
 }
 
+/*******************************************************************************
+ ******* ComposedColorFilter
+ ******************************************************************************/
+
+ComposedColorFilter::ComposedColorFilter(
+    const std::shared_ptr<ColorFilter>& outer,
+    const std::shared_ptr<ColorFilter>& inner)
+    : outer_(outer), inner_(inner) {}
+
+ComposedColorFilter::~ComposedColorFilter() = default;
+
+std::shared_ptr<ColorFilterContents>
+ComposedColorFilter::WrapWithGPUColorFilter(
+    std::shared_ptr<FilterInput> input,
+    ColorFilterContents::AbsorbOpacity absorb_opacity) const {
+  std::shared_ptr<FilterContents> inner = inner_->WrapWithGPUColorFilter(
+      input, ColorFilterContents::AbsorbOpacity::kNo);
+  return outer_->WrapWithGPUColorFilter(FilterInput::Make(inner),
+                                        absorb_opacity);
+}
+
+// |ColorFilter|
+ColorFilter::ColorFilterProc ComposedColorFilter::GetCPUColorFilterProc()
+    const {
+  return [inner = inner_, outer = outer_](Color color) {
+    auto inner_proc = inner->GetCPUColorFilterProc();
+    auto outer_proc = outer->GetCPUColorFilterProc();
+    return outer_proc(inner_proc(color));
+  };
+}
+
+// |ColorFilter|
+std::shared_ptr<ColorFilter> ComposedColorFilter::Clone() const {
+  return std::make_shared<ComposedColorFilter>(outer_, inner_);
+}
+
 }  // namespace impeller

engine/src/flutter/impeller/aiks/color_filter.h

@@ -34,6 +34,10 @@ class ColorFilter {
 
   static std::shared_ptr<ColorFilter> MakeLinearToSrgb();
 
+  static std::shared_ptr<ColorFilter> MakeComposed(
+      const std::shared_ptr<ColorFilter>& outer,
+      const std::shared_ptr<ColorFilter>& inner);
+
   /// @brief  Wraps the given filter input with a GPU-based filter that will
   ///         perform the color operation. The given input will first be
   ///         rendered to a texture and then filtered.
@@ -147,4 +151,28 @@ class LinearToSrgbColorFilter final : public ColorFilter {
   std::shared_ptr<ColorFilter> Clone() const override;
 };
 
+/// @brief Applies color filters as f(g(x)), where x is the input color.
+class ComposedColorFilter final : public ColorFilter {
+ public:
+  ComposedColorFilter(const std::shared_ptr<ColorFilter>& outer,
+                      const std::shared_ptr<ColorFilter>& inner);
+
+  ~ComposedColorFilter() override;
+
+  // |ColorFilter|
+  std::shared_ptr<ColorFilterContents> WrapWithGPUColorFilter(
+      std::shared_ptr<FilterInput> input,
+      ColorFilterContents::AbsorbOpacity absorb_opacity) const override;
+
+  // |ColorFilter|
+  ColorFilterProc GetCPUColorFilterProc() const override;
+
+  // |ColorFilter|
+  std::shared_ptr<ColorFilter> Clone() const override;
+
+ private:
+  std::shared_ptr<ColorFilter> outer_;
+  std::shared_ptr<ColorFilter> inner_;
+};
+
 }  // namespace impeller

engine/src/flutter/impeller/aiks/paint.cc

@@ -59,7 +59,6 @@ std::shared_ptr<Contents> Paint::CreateContentsForGeometry(
 std::shared_ptr<Contents> Paint::WithFilters(
     std::shared_ptr<Contents> input) const {
   input = WithColorFilter(input, ColorFilterContents::AbsorbOpacity::kYes);
-  input = WithInvertFilter(input);
   auto image_filter =
       WithImageFilter(input, Matrix(), Entity::RenderingMode::kDirect);
   if (image_filter) {
@@ -121,33 +120,10 @@ std::shared_ptr<Contents> Paint::WithColorFilter(
   if (input->ApplyColorFilter(color_filter->GetCPUColorFilterProc())) {
     return input;
   }
-
   return color_filter->WrapWithGPUColorFilter(FilterInput::Make(input),
                                               absorb_opacity);
 }
 
-/// A color matrix which inverts colors.
-// clang-format off
-constexpr ColorMatrix kColorInversion = {
-  .array = {
-    -1.0,    0,    0, 1.0, 0, //
-       0, -1.0,    0, 1.0, 0, //
-       0,    0, -1.0, 1.0, 0, //
-     1.0,  1.0,  1.0, 1.0, 0  //
-  }
-};
-// clang-format on
-
-std::shared_ptr<Contents> Paint::WithInvertFilter(
-    std::shared_ptr<Contents> input) const {
-  if (!invert_colors) {
-    return input;
-  }
-
-  return ColorFilterContents::MakeColorMatrix(
-      {FilterInput::Make(std::move(input))}, kColorInversion);
-}
-
 std::shared_ptr<FilterContents> Paint::MaskBlurDescriptor::CreateMaskBlur(
     std::shared_ptr<ColorSourceContents> color_source_contents,
     const std::shared_ptr<ColorFilter>& color_filter) const {

engine/src/flutter/impeller/aiks/paint.h

@@ -61,7 +61,6 @@ struct Paint {
   Scalar stroke_miter = 4.0;
   Style style = Style::kFill;
   BlendMode blend_mode = BlendMode::kSourceOver;
-  bool invert_colors = false;
 
   std::shared_ptr<ImageFilter> image_filter;
   std::shared_ptr<ColorFilter> color_filter;
@@ -108,9 +107,6 @@ struct Paint {
       std::shared_ptr<Contents> input,
       ColorFilterContents::AbsorbOpacity absorb_opacity =
           ColorFilterContents::AbsorbOpacity::kNo) const;
-
-  std::shared_ptr<Contents> WithInvertFilter(
-      std::shared_ptr<Contents> input) const;
 };
 
 }  // namespace impeller

engine/src/flutter/impeller/display_list/dl_dispatcher.cc

@@ -8,7 +8,6 @@
 #include <cstring>
 #include <memory>
 #include <optional>
-#include <unordered_map>
 #include <utility>
 #include <vector>
 
@@ -16,20 +15,13 @@
 #include "flutter/fml/trace_event.h"
 #include "impeller/aiks/color_filter.h"
 #include "impeller/core/formats.h"
-#include "impeller/display_list/dl_image_impeller.h"
 #include "impeller/display_list/dl_vertices_geometry.h"
 #include "impeller/display_list/nine_patch_converter.h"
 #include "impeller/display_list/skia_conversions.h"
-#include "impeller/entity/contents/conical_gradient_contents.h"
 #include "impeller/entity/contents/filters/filter_contents.h"
 #include "impeller/entity/contents/filters/inputs/filter_input.h"
-#include "impeller/entity/contents/linear_gradient_contents.h"
-#include "impeller/entity/contents/radial_gradient_contents.h"
 #include "impeller/entity/contents/runtime_effect_contents.h"
-#include "impeller/entity/contents/sweep_gradient_contents.h"
-#include "impeller/entity/contents/tiled_texture_contents.h"
 #include "impeller/entity/entity.h"
-#include "impeller/entity/geometry/geometry.h"
 #include "impeller/geometry/path.h"
 #include "impeller/geometry/path_builder.h"
 #include "impeller/geometry/scalar.h"
@@ -41,6 +33,18 @@
 
 namespace impeller {
 
+/// A color matrix which inverts colors.
+// clang-format off
+constexpr ColorMatrix kColorInversion = {
+  .array = {
+    -1.0,    0,    0, 1.0, 0, //
+       0, -1.0,    0, 1.0, 0, //
+       0,    0, -1.0, 1.0, 0, //
+     1.0,  1.0,  1.0, 1.0, 0  //
+  }
+};
+// clang-format on
+
 #define UNIMPLEMENTED \
   FML_DLOG(ERROR) << "Unimplemented detail in " << __FUNCTION__;
 
@@ -484,12 +488,24 @@ static std::shared_ptr<ColorFilter> ToColorFilter(
 // |flutter::DlOpReceiver|
 void DlDispatcher::setColorFilter(const flutter::DlColorFilter* filter) {
   // Needs https://github.com/flutter/flutter/issues/95434
-  paint_.color_filter = ToColorFilter(filter);
+  if (paint_.color_filter) {
+    auto color_filter = ToColorFilter(filter);
+    paint_.color_filter =
+        ColorFilter::MakeComposed(paint_.color_filter, color_filter);
+  } else {
+    paint_.color_filter = ToColorFilter(filter);
+  }
 }
 
 // |flutter::DlOpReceiver|
 void DlDispatcher::setInvertColors(bool invert) {
-  paint_.invert_colors = invert;
+  if (paint_.color_filter) {
+    auto invert_filter = ColorFilter::MakeMatrix(kColorInversion);
+    paint_.color_filter =
+        ColorFilter::MakeComposed(invert_filter, paint_.color_filter);
+  } else {
+    paint_.color_filter = ColorFilter::MakeMatrix(kColorInversion);
+  }
 }
 
 // |flutter::DlOpReceiver|