From e38fb755f3513d964902eab613d18a13f69f123c Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 10 Jul 2019 12:06:58 -0700 Subject: [PATCH] Let pushColorFilter accept all types of ColorFilters (flutter/engine#9641) --- .../flutter/flow/layers/color_filter_layer.cc | 6 ++-- .../flutter/flow/layers/color_filter_layer.h | 7 ++-- engine/src/flutter/lib/ui/compositing.dart | 7 ++-- .../lib/ui/compositing/scene_builder.cc | 8 ++--- .../lib/ui/compositing/scene_builder.h | 3 +- engine/src/flutter/lib/ui/painting.dart | 4 +-- .../testing/dart/compositing_test.dart | 35 +++++++++++++++++-- 7 files changed, 51 insertions(+), 19 deletions(-) diff --git a/engine/src/flutter/flow/layers/color_filter_layer.cc b/engine/src/flutter/flow/layers/color_filter_layer.cc index 8157999191..f838b0612b 100644 --- a/engine/src/flutter/flow/layers/color_filter_layer.cc +++ b/engine/src/flutter/flow/layers/color_filter_layer.cc @@ -6,8 +6,8 @@ namespace flutter { -ColorFilterLayer::ColorFilterLayer(SkColor color, SkBlendMode blend_mode) - : color_(color), blend_mode_(blend_mode) {} +ColorFilterLayer::ColorFilterLayer(sk_sp filter) + : filter_(std::move(filter)) {} ColorFilterLayer::~ColorFilterLayer() = default; @@ -16,7 +16,7 @@ void ColorFilterLayer::Paint(PaintContext& context) const { FML_DCHECK(needs_painting()); SkPaint paint; - paint.setColorFilter(SkColorFilters::Blend(color_, blend_mode_)); + paint.setColorFilter(filter_); Layer::AutoSaveLayer save = Layer::AutoSaveLayer::Create(context, paint_bounds(), &paint); diff --git a/engine/src/flutter/flow/layers/color_filter_layer.h b/engine/src/flutter/flow/layers/color_filter_layer.h index 5ba67ab521..cf1de4cb61 100644 --- a/engine/src/flutter/flow/layers/color_filter_layer.h +++ b/engine/src/flutter/flow/layers/color_filter_layer.h @@ -7,18 +7,19 @@ #include "flutter/flow/layers/container_layer.h" +#include "third_party/skia/include/core/SkColorFilter.h" + namespace flutter { class ColorFilterLayer : public ContainerLayer { public: - ColorFilterLayer(SkColor color, SkBlendMode blend_mode); + ColorFilterLayer(sk_sp filter); ~ColorFilterLayer() override; void Paint(PaintContext& context) const override; private: - SkColor color_; - SkBlendMode blend_mode_; + sk_sp filter_; FML_DISALLOW_COPY_AND_ASSIGN(ColorFilterLayer); }; diff --git a/engine/src/flutter/lib/ui/compositing.dart b/engine/src/flutter/lib/ui/compositing.dart index 357952087e..5f8fdbfe87 100644 --- a/engine/src/flutter/lib/ui/compositing.dart +++ b/engine/src/flutter/lib/ui/compositing.dart @@ -387,13 +387,14 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// {@macro dart.ui.sceneBuilder.oldLayerVsRetained} /// /// See [pop] for details about the operation stack. - ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { + ColorFilterEngineLayer pushColorFilter(ColorFilter filter, { ColorFilterEngineLayer oldLayer }) { + assert(filter != null); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); - final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); + final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(filter._toNativeColorFilter())); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; + EngineLayer _pushColorFilter(_ColorFilter filter) native 'SceneBuilder_pushColorFilter'; /// Pushes a backdrop filter operation onto the operation stack. /// diff --git a/engine/src/flutter/lib/ui/compositing/scene_builder.cc b/engine/src/flutter/lib/ui/compositing/scene_builder.cc index bdbd9218c1..5b7148803c 100644 --- a/engine/src/flutter/lib/ui/compositing/scene_builder.cc +++ b/engine/src/flutter/lib/ui/compositing/scene_builder.cc @@ -141,10 +141,10 @@ fml::RefPtr SceneBuilder::pushOpacity(int alpha, return EngineLayer::MakeRetained(layer); } -fml::RefPtr SceneBuilder::pushColorFilter(int color, - int blendMode) { - auto layer = std::make_shared( - static_cast(color), static_cast(blendMode)); +fml::RefPtr SceneBuilder::pushColorFilter( + const ColorFilter* color_filter) { + auto layer = + std::make_shared(color_filter->filter()); PushLayer(layer); return EngineLayer::MakeRetained(layer); } diff --git a/engine/src/flutter/lib/ui/compositing/scene_builder.h b/engine/src/flutter/lib/ui/compositing/scene_builder.h index 76767d7d75..d378876d96 100644 --- a/engine/src/flutter/lib/ui/compositing/scene_builder.h +++ b/engine/src/flutter/lib/ui/compositing/scene_builder.h @@ -12,6 +12,7 @@ #include "flutter/lib/ui/compositing/scene.h" #include "flutter/lib/ui/dart_wrapper.h" +#include "flutter/lib/ui/painting/color_filter.h" #include "flutter/lib/ui/painting/engine_layer.h" #include "flutter/lib/ui/painting/image_filter.h" #include "flutter/lib/ui/painting/path.h" @@ -48,7 +49,7 @@ class SceneBuilder : public RefCountedDartWrappable { fml::RefPtr pushClipPath(const CanvasPath* path, int clipBehavior); fml::RefPtr pushOpacity(int alpha, double dx = 0, double dy = 0); - fml::RefPtr pushColorFilter(int color, int blendMode); + fml::RefPtr pushColorFilter(const ColorFilter* color_filter); fml::RefPtr pushBackdropFilter(ImageFilter* filter); fml::RefPtr pushShaderMask(Shader* shader, double maskRectLeft, diff --git a/engine/src/flutter/lib/ui/painting.dart b/engine/src/flutter/lib/ui/painting.dart index 0ce48da711..bafb0b293a 100644 --- a/engine/src/flutter/lib/ui/painting.dart +++ b/engine/src/flutter/lib/ui/painting.dart @@ -2500,8 +2500,8 @@ class ColorFilter { /// matrix is in row-major order and the translation column is specified in /// unnormalized, 0...255, space. const ColorFilter.matrix(List matrix) - : assert(matrix != null), - assert(matrix.length == 20), + : assert(matrix != null, 'Color Matrix argument was null.'), + assert(matrix.length == 20, 'Color Matrix must have 20 entries.'), _color = null, _blendMode = null, _matrix = matrix, diff --git a/engine/src/flutter/testing/dart/compositing_test.dart b/engine/src/flutter/testing/dart/compositing_test.dart index 478f0aef6b..a7b8e9a7c8 100644 --- a/engine/src/flutter/testing/dart/compositing_test.dart +++ b/engine/src/flutter/testing/dart/compositing_test.dart @@ -273,9 +273,6 @@ void main() { testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushOpacity(100, oldLayer: oldLayer); }); - testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { - return builder.pushColorFilter(const Color.fromARGB(0, 0, 0, 0), BlendMode.color, oldLayer: oldLayer); - }); testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushBackdropFilter(ImageFilter.blur(), oldLayer: oldLayer); }); @@ -294,6 +291,38 @@ void main() { testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushPhysicalShape(path: Path(), color: const Color.fromARGB(0, 0, 0, 0), oldLayer: oldLayer); }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter( + const ColorFilter.mode( + Color.fromARGB(0, 0, 0, 0), + BlendMode.color, + ), + oldLayer: oldLayer, + ); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter( + const ColorFilter.matrix([ + 1, 0, 0, 0, 0, + 0, 1, 0, 0, 0, + 0, 0, 1, 0, 0, + 0, 0, 0, 1, 0, + ]), + oldLayer: oldLayer, + ); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter( + const ColorFilter.linearToSrgbGamma(), + oldLayer: oldLayer, + ); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter( + const ColorFilter.srgbToLinearGamma(), + oldLayer: oldLayer, + ); + }); }); }