From 5af08d0fb2b8c3776ee418c69df0e6ee83ff6ab6 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jul 2019 16:04:56 -0700 Subject: [PATCH 01/14] Let pushColorFilter accept all types of ColorFilters --- flow/layers/color_filter_layer.cc | 6 ++-- flow/layers/color_filter_layer.h | 7 +++-- .../compositor/layer_scene_builder.dart | 2 +- lib/stub_ui/lib/src/ui/compositing.dart | 2 +- lib/ui/compositing.dart | 26 +++++++++++++++-- lib/ui/compositing/scene_builder.cc | 28 ++++++++++++++++++- lib/ui/compositing/scene_builder.h | 4 +++ lib/ui/painting/matrix.cc | 15 ++++++++++ lib/ui/painting/matrix.h | 7 +++++ lib/ui/painting/paint.cc | 16 ++--------- testing/dart/compositing_test.dart | 8 +++++- 11 files changed, 95 insertions(+), 26 deletions(-) diff --git a/flow/layers/color_filter_layer.cc b/flow/layers/color_filter_layer.cc index 81579991916da..f838b0612b2e5 100644 --- a/flow/layers/color_filter_layer.cc +++ b/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/flow/layers/color_filter_layer.h b/flow/layers/color_filter_layer.h index 5ba67ab5218f6..cf1de4cb610fc 100644 --- a/flow/layers/color_filter_layer.h +++ b/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/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart b/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart index 31769a5c968e9..b8c1aad60e6c1 100644 --- a/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart +++ b/lib/stub_ui/lib/src/engine/compositor/layer_scene_builder.dart @@ -116,7 +116,7 @@ class LayerSceneBuilder implements ui.SceneBuilder { } @override - ui.ColorFilterEngineLayer pushColorFilter(ui.Color color, ui.BlendMode blendMode, + ui.ColorFilterEngineLayer pushColorFilter(ui.ColorFilter filter, {ui.ColorFilterEngineLayer oldLayer}) { throw new UnimplementedError(); } diff --git a/lib/stub_ui/lib/src/ui/compositing.dart b/lib/stub_ui/lib/src/ui/compositing.dart index d70865673b303..2bdeab1fdeb71 100644 --- a/lib/stub_ui/lib/src/ui/compositing.dart +++ b/lib/stub_ui/lib/src/ui/compositing.dart @@ -257,7 +257,7 @@ class SceneBuilder { /// blend mode. /// /// See [pop] for details about the operation stack. - ColorFilterEngineLayer pushColorFilter(Color color, BlendMode blendMode, { ColorFilterEngineLayer oldLayer }) { + ColorFilterEngineLayer pushColorFilter(ColorFilter filter, { ColorFilterEngineLayer oldLayer }) { throw new UnimplementedError(); } diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 357952087e2ff..3462b3b9d0c6c 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -387,13 +387,35 @@ 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(filter._type != null && filter._type != ColorFilter._TypeNone); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); - final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(color.value, blendMode.index)); + EngineLayer engineLayer; + switch(filter._type) { + case ColorFilter._TypeMode: + engineLayer = _pushColorFilter(filter._color.value, filter._blendMode.index); + break; + case ColorFilter._TypeMatrix: + engineLayer = _pushColorFilterMatrix(filter._matrix); + break; + case ColorFilter._TypeLinearToSrgbGamma: + engineLayer = _pushColorFilterLinearToSrgbGamma(); + break; + case ColorFilter._TypeSrgbToLinearGamma: + engineLayer = _pushColorFilterSrgbToLinearGamma(); + break; + default: + assert(false); + } + final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; + EngineLayer _pushColorFilterMatrix(Float32List matrix) native 'SceneBuilder_pushColorFilterMatrix'; + EngineLayer _pushColorFilterLinearToSrgbGamma() native 'SceneBuilder_pushColorFilterLinearToSrgbGamma'; + EngineLayer _pushColorFilterSrgbToLinearGamma() native 'SceneBuilder_pushColorFilterSrgbToLinearGamma'; /// Pushes a backdrop filter operation onto the operation stack. /// diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index bdbd9218c10c2..e1d214d12d6d1 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -51,6 +51,9 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, SceneBuilder); V(SceneBuilder, pushClipPath) \ V(SceneBuilder, pushOpacity) \ V(SceneBuilder, pushColorFilter) \ + V(SceneBuilder, pushColorFilterMatrix) \ + V(SceneBuilder, pushColorFilterLinearToSrgbGamma) \ + V(SceneBuilder, pushColorFilterSrgbToLinearGamma) \ V(SceneBuilder, pushBackdropFilter) \ V(SceneBuilder, pushShaderMask) \ V(SceneBuilder, pushPhysicalShape) \ @@ -143,8 +146,31 @@ fml::RefPtr SceneBuilder::pushOpacity(int alpha, fml::RefPtr SceneBuilder::pushColorFilter(int color, int blendMode) { + auto layer = + std::make_shared(SkColorFilters::Blend( + static_cast(color), static_cast(blendMode))); + PushLayer(layer); + return EngineLayer::MakeRetained(layer); +} + +fml::RefPtr SceneBuilder::pushColorFilterMatrix( + const tonic::Float32List& matrix) { + auto layer = std::make_shared( + MakeColorMatrixFilter255(matrix)); + PushLayer(layer); + return EngineLayer::MakeRetained(layer); +} + +fml::RefPtr SceneBuilder::pushColorFilterLinearToSrgbGamma() { + auto layer = std::make_shared( + SkColorFilters::LinearToSRGBGamma()); + PushLayer(layer); + return EngineLayer::MakeRetained(layer); +} + +fml::RefPtr SceneBuilder::pushColorFilterSrgbToLinearGamma() { auto layer = std::make_shared( - static_cast(color), static_cast(blendMode)); + SkColorFilters::SRGBToLinearGamma()); PushLayer(layer); return EngineLayer::MakeRetained(layer); } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 76767d7d759f0..4495159a03c0e 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -49,6 +49,10 @@ class SceneBuilder : public RefCountedDartWrappable { int clipBehavior); fml::RefPtr pushOpacity(int alpha, double dx = 0, double dy = 0); fml::RefPtr pushColorFilter(int color, int blendMode); + fml::RefPtr pushColorFilterMatrix( + const tonic::Float32List& matrix); + fml::RefPtr pushColorFilterLinearToSrgbGamma(); + fml::RefPtr pushColorFilterSrgbToLinearGamma(); fml::RefPtr pushBackdropFilter(ImageFilter* filter); fml::RefPtr pushShaderMask(Shader* shader, double maskRectLeft, diff --git a/lib/ui/painting/matrix.cc b/lib/ui/painting/matrix.cc index 52ba981ebda9d..8c3b629fdc306 100644 --- a/lib/ui/painting/matrix.cc +++ b/lib/ui/painting/matrix.cc @@ -38,4 +38,19 @@ tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix) { return matrix4; } +sk_sp MakeColorMatrixFilter255( + const tonic::Float32List& color_matrix) { + return MakeColorMatrixFilter255(color_matrix.data()); +} + +sk_sp MakeColorMatrixFilter255(const float color_matrix[20]) { + float tmp[20]; + memcpy(tmp, color_matrix, sizeof(tmp)); + tmp[4] *= 1.0f / 255; + tmp[9] *= 1.0f / 255; + tmp[14] *= 1.0f / 255; + tmp[19] *= 1.0f / 255; + return SkColorFilters::Matrix(tmp); +} + } // namespace flutter diff --git a/lib/ui/painting/matrix.h b/lib/ui/painting/matrix.h index bda531472263e..3b7fa9a855f2d 100644 --- a/lib/ui/painting/matrix.h +++ b/lib/ui/painting/matrix.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_LIB_UI_PAINTING_MATRIX_H_ #define FLUTTER_LIB_UI_PAINTING_MATRIX_H_ +#include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkMatrix.h" #include "third_party/tonic/typed_data/typed_list.h" @@ -13,6 +14,12 @@ namespace flutter { SkMatrix ToSkMatrix(const tonic::Float64List& matrix4); tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix); +// Flutter still defines the matrix to be biased by 255 in the last column +// (translate). skia is normalized, treating the last column as 0...1, so we +// post-scale here before calling the skia factory. +sk_sp MakeColorMatrixFilter255( + const tonic::Float32List& color_matrix); +sk_sp MakeColorMatrixFilter255(const float color_matrix[20]); } // namespace flutter #endif // FLUTTER_LIB_UI_PAINTING_MATRIX_H_ diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index 718b9ae61089d..aba05bf2c2975 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -6,6 +6,7 @@ #include "flutter/fml/logging.h" #include "flutter/lib/ui/painting/image_filter.h" +#include "flutter/lib/ui/painting/matrix.h" #include "flutter/lib/ui/painting/shader.h" #include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkImageFilter.h" @@ -75,19 +76,6 @@ enum ColorFilterType { SRGBToLinearGamma }; -// Flutter still defines the matrix to be biased by 255 in the last column -// (translate). skia is normalized, treating the last column as 0...1, so we -// post-scale here before calling the skia factory. -static sk_sp MakeColorMatrixFilter255(const float array[20]) { - float tmp[20]; - memcpy(tmp, array, sizeof(tmp)); - tmp[4] *= 1.0f / 255; - tmp[9] *= 1.0f / 255; - tmp[14] *= 1.0f / 255; - tmp[19] *= 1.0f / 255; - return SkColorFilters::Matrix(tmp); -} - sk_sp ExtractColorFilter(const uint32_t* uint_data, Dart_Handle* values) { switch (uint_data[kColorFilterIndex]) { @@ -108,7 +96,7 @@ sk_sp ExtractColorFilter(const uint32_t* uint_data, FML_CHECK(length == 20); tonic::Float32List decoded(matrixHandle); - return MakeColorMatrixFilter255(decoded.data()); + return MakeColorMatrixFilter255(decoded); } return nullptr; } diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 478f0aef6b35f..64cc3711ba317 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -274,7 +274,13 @@ void main() { 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); + return builder.pushColorFilter( + ColorFilter.mode( + const Color.fromARGB(0, 0, 0, 0), + BlendMode.color, + ), + oldLayer: oldLayer, + ); }); testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushBackdropFilter(ImageFilter.blur(), oldLayer: oldLayer); From 059bf0465b4c55cedf6f5062a7405a7edb73e63f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 1 Jul 2019 17:30:49 -0700 Subject: [PATCH 02/14] release typed data, check matrix len --- lib/ui/compositing.dart | 2 +- lib/ui/compositing/scene_builder.cc | 7 +++++-- lib/ui/compositing/scene_builder.h | 3 +-- lib/ui/painting.dart | 4 +++- lib/ui/painting/matrix.cc | 5 ----- lib/ui/painting/matrix.h | 3 +-- lib/ui/painting/paint.cc | 2 +- 7 files changed, 12 insertions(+), 14 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 3462b3b9d0c6c..530237199a02d 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -397,7 +397,7 @@ class SceneBuilder extends NativeFieldWrapperClass2 { engineLayer = _pushColorFilter(filter._color.value, filter._blendMode.index); break; case ColorFilter._TypeMatrix: - engineLayer = _pushColorFilterMatrix(filter._matrix); + engineLayer = _pushColorFilterMatrix(Float32List.fromList(filter._matrix)); break; case ColorFilter._TypeLinearToSrgbGamma: engineLayer = _pushColorFilterLinearToSrgbGamma(); diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index e1d214d12d6d1..8abbad9737992 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -154,9 +154,12 @@ fml::RefPtr SceneBuilder::pushColorFilter(int color, } fml::RefPtr SceneBuilder::pushColorFilterMatrix( - const tonic::Float32List& matrix) { + tonic::Float32List& matrix) { + FML_DCHECK(matrix.num_elements() == 25); + auto layer = std::make_shared( - MakeColorMatrixFilter255(matrix)); + MakeColorMatrixFilter255(matrix.data())); + matrix.Release(); PushLayer(layer); return EngineLayer::MakeRetained(layer); } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 4495159a03c0e..7c9e32e7b807b 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/lib/ui/compositing/scene_builder.h @@ -49,8 +49,7 @@ class SceneBuilder : public RefCountedDartWrappable { int clipBehavior); fml::RefPtr pushOpacity(int alpha, double dx = 0, double dy = 0); fml::RefPtr pushColorFilter(int color, int blendMode); - fml::RefPtr pushColorFilterMatrix( - const tonic::Float32List& matrix); + fml::RefPtr pushColorFilterMatrix(tonic::Float32List& matrix); fml::RefPtr pushColorFilterLinearToSrgbGamma(); fml::RefPtr pushColorFilterSrgbToLinearGamma(); fml::RefPtr pushBackdropFilter(ImageFilter* filter); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 9e8bc77fe6196..159c759f6036b 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2529,7 +2529,9 @@ class ColorFilter { /// matrix is in row-major order and the translation column is specified in /// unnormalized, 0...255, space. const ColorFilter.matrix(List matrix) - : _color = null, + : assert(matrix != null, 'Color Matrix argument was null.'), + assert(matrix.length == 25, 'Color Matrix must have 25 entries.'), + _color = null, _blendMode = null, _matrix = matrix, _type = _TypeMatrix; diff --git a/lib/ui/painting/matrix.cc b/lib/ui/painting/matrix.cc index 8c3b629fdc306..85d3c798493cc 100644 --- a/lib/ui/painting/matrix.cc +++ b/lib/ui/painting/matrix.cc @@ -38,11 +38,6 @@ tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix) { return matrix4; } -sk_sp MakeColorMatrixFilter255( - const tonic::Float32List& color_matrix) { - return MakeColorMatrixFilter255(color_matrix.data()); -} - sk_sp MakeColorMatrixFilter255(const float color_matrix[20]) { float tmp[20]; memcpy(tmp, color_matrix, sizeof(tmp)); diff --git a/lib/ui/painting/matrix.h b/lib/ui/painting/matrix.h index 3b7fa9a855f2d..d4659a3dd9313 100644 --- a/lib/ui/painting/matrix.h +++ b/lib/ui/painting/matrix.h @@ -17,9 +17,8 @@ tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix); // Flutter still defines the matrix to be biased by 255 in the last column // (translate). skia is normalized, treating the last column as 0...1, so we // post-scale here before calling the skia factory. -sk_sp MakeColorMatrixFilter255( - const tonic::Float32List& color_matrix); sk_sp MakeColorMatrixFilter255(const float color_matrix[20]); + } // namespace flutter #endif // FLUTTER_LIB_UI_PAINTING_MATRIX_H_ diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index aba05bf2c2975..96c4c32a823c2 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -96,7 +96,7 @@ sk_sp ExtractColorFilter(const uint32_t* uint_data, FML_CHECK(length == 20); tonic::Float32List decoded(matrixHandle); - return MakeColorMatrixFilter255(decoded); + return MakeColorMatrixFilter255(decoded.data()); } return nullptr; } From b7b4aa8d5ad308503f445d768faece186a54a21a Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 2 Jul 2019 08:36:28 -0700 Subject: [PATCH 03/14] 20 entries, not 25 --- lib/ui/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 159c759f6036b..22542897ec968 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2530,7 +2530,7 @@ class ColorFilter { /// unnormalized, 0...255, space. const ColorFilter.matrix(List matrix) : assert(matrix != null, 'Color Matrix argument was null.'), - assert(matrix.length == 25, 'Color Matrix must have 25 entries.'), + assert(matrix.length == 20, 'Color Matrix must have 20 entries.'), _color = null, _blendMode = null, _matrix = matrix, From a1e93198314f684b0f6992fccc71352213716f1f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 2 Jul 2019 08:37:01 -0700 Subject: [PATCH 04/14] 20 entries, not 25 --- lib/ui/compositing/scene_builder.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 8abbad9737992..1382f2b7e2ede 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -155,7 +155,7 @@ fml::RefPtr SceneBuilder::pushColorFilter(int color, fml::RefPtr SceneBuilder::pushColorFilterMatrix( tonic::Float32List& matrix) { - FML_DCHECK(matrix.num_elements() == 25); + FML_DCHECK(matrix.num_elements() == 20); auto layer = std::make_shared( MakeColorMatrixFilter255(matrix.data())); From ed6fa6d9ac6c24cc165c800b57f79b4e423d13e7 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 2 Jul 2019 10:21:19 -0700 Subject: [PATCH 05/14] move const to right spot --- testing/dart/compositing_test.dart | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 64cc3711ba317..2b7af48fbc38d 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -275,8 +275,8 @@ void main() { }); testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushColorFilter( - ColorFilter.mode( - const Color.fromARGB(0, 0, 0, 0), + const ColorFilter.mode( + Color.fromARGB(0, 0, 0, 0), BlendMode.color, ), oldLayer: oldLayer, From a3b9075b90155b3f2f6f74218dbfba28e9512e6f Mon Sep 17 00:00:00 2001 From: Dan Field Date: Tue, 2 Jul 2019 18:57:07 -0700 Subject: [PATCH 06/14] tests --- testing/dart/compositing_test.dart | 42 +++++++++++++++++++++++------- 1 file changed, 33 insertions(+), 9 deletions(-) diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index 2b7af48fbc38d..dfdbee8551ba2 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -254,6 +254,39 @@ void main() { testRetainsParentOfOldLayer(pushFunction); } + test('SceneBuilder pushColorFilter handles all four types', () { + 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(), + ); + }); + testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { + return builder.pushColorFilter( + const ColorFilter.srgbToLinearGamma(), + ); + }); + }); + test('SceneBuilder does not share a layer between addRetained and push*', () { testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushOffset(0, 0, oldLayer: oldLayer); @@ -273,15 +306,6 @@ void main() { testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushOpacity(100, 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.pushBackdropFilter(ImageFilter.blur(), oldLayer: oldLayer); }); From ff4ed6b111bbcabd909a8a454260c24487e326fa Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 3 Jul 2019 15:06:20 -0700 Subject: [PATCH 07/14] Refactor ColorFilter to have a native wrapper --- ci/licenses_golden/licenses_flutter | 2 + lib/ui/BUILD.gn | 2 + lib/ui/dart_ui.cc | 2 + lib/ui/painting.dart | 126 +++++++++++++++++----------- lib/ui/painting/color_filter.cc | 70 ++++++++++++++++ lib/ui/painting/color_filter.h | 52 ++++++++++++ lib/ui/painting/paint.cc | 101 +++++----------------- 7 files changed, 227 insertions(+), 128 deletions(-) create mode 100644 lib/ui/painting/color_filter.cc create mode 100644 lib/ui/painting/color_filter.h diff --git a/ci/licenses_golden/licenses_flutter b/ci/licenses_golden/licenses_flutter index dc374fe6e5d77..0fc0177eb95af 100644 --- a/ci/licenses_golden/licenses_flutter +++ b/ci/licenses_golden/licenses_flutter @@ -333,6 +333,8 @@ FILE: ../../../flutter/lib/ui/painting/canvas.cc FILE: ../../../flutter/lib/ui/painting/canvas.h FILE: ../../../flutter/lib/ui/painting/codec.cc FILE: ../../../flutter/lib/ui/painting/codec.h +FILE: ../../../flutter/lib/ui/painting/color_filter.cc +FILE: ../../../flutter/lib/ui/painting/color_filter.h FILE: ../../../flutter/lib/ui/painting/engine_layer.cc FILE: ../../../flutter/lib/ui/painting/engine_layer.h FILE: ../../../flutter/lib/ui/painting/frame_info.cc diff --git a/lib/ui/BUILD.gn b/lib/ui/BUILD.gn index fb8526c06cdff..e7cc9a66bdc4d 100644 --- a/lib/ui/BUILD.gn +++ b/lib/ui/BUILD.gn @@ -24,6 +24,8 @@ source_set("ui") { "painting/canvas.h", "painting/codec.cc", "painting/codec.h", + "painting/color_filter.cc", + "painting/color_filter.h", "painting/engine_layer.cc", "painting/engine_layer.h", "painting/frame_info.cc", diff --git a/lib/ui/dart_ui.cc b/lib/ui/dart_ui.cc index 8605d7a960095..5b3189dfa80f8 100644 --- a/lib/ui/dart_ui.cc +++ b/lib/ui/dart_ui.cc @@ -11,6 +11,7 @@ #include "flutter/lib/ui/isolate_name_server/isolate_name_server_natives.h" #include "flutter/lib/ui/painting/canvas.h" #include "flutter/lib/ui/painting/codec.h" +#include "flutter/lib/ui/painting/color_filter.h" #include "flutter/lib/ui/painting/engine_layer.h" #include "flutter/lib/ui/painting/frame_info.h" #include "flutter/lib/ui/painting/gradient.h" @@ -75,6 +76,7 @@ void DartUI::InitForGlobal() { CanvasPath::RegisterNatives(g_natives); CanvasPathMeasure::RegisterNatives(g_natives); Codec::RegisterNatives(g_natives); + ColorFilter::RegisterNatives(g_natives); DartRuntimeHooks::RegisterNatives(g_natives); EngineLayer::RegisterNatives(g_natives); FontCollection::RegisterNatives(g_natives); diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 9e8bc77fe6196..93a57b211197f 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -1056,13 +1056,10 @@ class Paint { static const int _kStrokeJoinIndex = 6; static const int _kStrokeMiterLimitIndex = 7; static const int _kFilterQualityIndex = 8; - static const int _kColorFilterIndex = 9; - static const int _kColorFilterColorIndex = 10; - static const int _kColorFilterBlendModeIndex = 11; - static const int _kMaskFilterIndex = 12; - static const int _kMaskFilterBlurStyleIndex = 13; - static const int _kMaskFilterSigmaIndex = 14; - static const int _kInvertColorIndex = 15; + static const int _kMaskFilterIndex = 9; + static const int _kMaskFilterBlurStyleIndex = 10; + static const int _kMaskFilterSigmaIndex = 11; + static const int _kInvertColorIndex = 12; static const int _kIsAntiAliasOffset = _kIsAntiAliasIndex << 2; static const int _kColorOffset = _kColorIndex << 2; @@ -1073,20 +1070,17 @@ class Paint { static const int _kStrokeJoinOffset = _kStrokeJoinIndex << 2; static const int _kStrokeMiterLimitOffset = _kStrokeMiterLimitIndex << 2; static const int _kFilterQualityOffset = _kFilterQualityIndex << 2; - static const int _kColorFilterOffset = _kColorFilterIndex << 2; - static const int _kColorFilterColorOffset = _kColorFilterColorIndex << 2; - static const int _kColorFilterBlendModeOffset = _kColorFilterBlendModeIndex << 2; static const int _kMaskFilterOffset = _kMaskFilterIndex << 2; static const int _kMaskFilterBlurStyleOffset = _kMaskFilterBlurStyleIndex << 2; static const int _kMaskFilterSigmaOffset = _kMaskFilterSigmaIndex << 2; static const int _kInvertColorOffset = _kInvertColorIndex << 2; // If you add more fields, remember to update _kDataByteCount. - static const int _kDataByteCount = 75; + static const int _kDataByteCount = 52; // Binary format must match the deserialization code in paint.cc. List _objects; static const int _kShaderIndex = 0; - static const int _kColorFilterMatrixIndex = 1; + static const int _kColorFilterIndex = 1; static const int _kImageFilterIndex = 2; static const int _kObjectCount = 3; // Must be one larger than the largest index. @@ -1342,48 +1336,23 @@ class Paint { /// /// When a shape is being drawn, [colorFilter] overrides [color] and [shader]. ColorFilter get colorFilter { - switch (_data.getInt32(_kColorFilterOffset, _kFakeHostEndian)) { - case ColorFilter._TypeNone: - return null; - case ColorFilter._TypeMode: - return ColorFilter.mode( - Color(_data.getInt32(_kColorFilterColorOffset, _kFakeHostEndian)), - BlendMode.values[_data.getInt32(_kColorFilterBlendModeOffset, _kFakeHostEndian)], - ); - case ColorFilter._TypeMatrix: - return ColorFilter.matrix(_objects[_kColorFilterMatrixIndex]); - case ColorFilter._TypeLinearToSrgbGamma: - return const ColorFilter.linearToSrgbGamma(); - case ColorFilter._TypeSrgbToLinearGamma: - return const ColorFilter.srgbToLinearGamma(); + if (_objects == null || _objects[_kColorFilterIndex] == null) { + return null; } - - return null; + return _objects[_kColorFilterIndex].creator; } set colorFilter(ColorFilter value) { if (value == null) { - _data.setInt32(_kColorFilterOffset, ColorFilter._TypeNone, _kFakeHostEndian); - _data.setInt32(_kColorFilterColorOffset, 0, _kFakeHostEndian); - _data.setInt32(_kColorFilterBlendModeOffset, 0, _kFakeHostEndian); - if (_objects != null) { - _objects[_kColorFilterMatrixIndex] = null; + _objects[_kColorFilterIndex] = null; } } else { - _data.setInt32(_kColorFilterOffset, value._type, _kFakeHostEndian); - - if (value._type == ColorFilter._TypeMode) { - assert(value._color != null); - assert(value._blendMode != null); - - _data.setInt32(_kColorFilterColorOffset, value._color.value, _kFakeHostEndian); - _data.setInt32(_kColorFilterBlendModeOffset, value._blendMode.index, _kFakeHostEndian); - } else if (value._type == ColorFilter._TypeMatrix) { - assert(value._matrix != null); - - _objects ??= List(_kObjectCount); - _objects[_kColorFilterMatrixIndex] = Float32List.fromList(value._matrix); + if (_objects == null) { + _objects = List(_kObjectCount); + _objects[_kColorFilterIndex] = value._toNativeColorFilter(); + } else if (_objects[_kColorFilterIndex]?.creator != value) { + _objects[_kColorFilterIndex] = value._toNativeColorFilter(); } } } @@ -2520,7 +2489,9 @@ class ColorFilter { /// to the [Paint.blendMode], using the output of this filter as the source /// and the background as the destination. const ColorFilter.mode(Color color, BlendMode blendMode) - : _color = color, + : assert(color != null), + assert(blendMode != null), + _color = color, _blendMode = blendMode, _matrix = null, _type = _TypeMode; @@ -2529,7 +2500,9 @@ class ColorFilter { /// matrix is in row-major order and the translation column is specified in /// unnormalized, 0...255, space. const ColorFilter.matrix(List matrix) - : _color = null, + : assert(matrix != null), + assert(matrix.length == 20), + _color = null, _blendMode = null, _matrix = matrix, _type = _TypeMatrix; @@ -2580,6 +2553,21 @@ class ColorFilter { return _color == typedOther._color && _blendMode == typedOther._blendMode; } + _ColorFilter _toNativeColorFilter() { + switch (_type) { + case _TypeMode: + return _ColorFilter.mode(this); + case _TypeMatrix: + return _ColorFilter.matrix(this); + case _TypeLinearToSrgbGamma: + return _ColorFilter.linearToSrgbGamma(this); + case _TypeSrgbToLinearGamma: + return _ColorFilter.srgbToLinearGamma(this); + default: + throw StateError('Unknown mode $_type for ColorFilter.'); + } + } + @override int get hashCode => hashValues(_color, _blendMode, hashList(_matrix), _type); @@ -2600,6 +2588,48 @@ class ColorFilter { } } +/// A [ColorFilter] that is backed by a native SkColorFilter. +// We could not make [ColorFilter] be this, because it would no longer be const +// constructible and would complicate the == and hashCode implementations. +class _ColorFilter extends NativeFieldWrapperClass2 { + _ColorFilter.mode(this.creator) + : assert(creator != null), + assert(creator._type == ColorFilter._TypeMode) { + _constructor(); + _initMode(creator._color.value, creator._blendMode.index); + } + + _ColorFilter.matrix(this.creator) + : assert(creator != null), + assert(creator._type == ColorFilter._TypeMatrix) { + _constructor(); + _initMatrix(Float32List.fromList(creator._matrix)); + } + _ColorFilter.linearToSrgbGamma(this.creator) + : assert(creator != null), + assert(creator._type == ColorFilter._TypeLinearToSrgbGamma) { + _constructor(); + _initLinearToSrgbGamma(); + } + + _ColorFilter.srgbToLinearGamma(this.creator) + : assert(creator != null), + assert(creator._type == ColorFilter._TypeSrgbToLinearGamma) { + _constructor(); + _initSrgbToLinearGamma(); + } + + /// The original Dart object that created the native wrapper, which retains + /// the values used for the filter. + final ColorFilter creator; + + void _constructor() native 'ColorFilter_constructor'; + void _initMode(int color, int blendMode) native 'ColorFilter_initMode'; + void _initMatrix(Float32List matrix) native 'ColorFilter_initMatrix'; + void _initLinearToSrgbGamma() native 'ColorFilter_initLinearToSrgbGamma'; + void _initSrgbToLinearGamma() native 'ColorFilter_initSrgToLinearGamma'; +} + /// A filter operation to apply to a raster image. /// /// See also: diff --git a/lib/ui/painting/color_filter.cc b/lib/ui/painting/color_filter.cc new file mode 100644 index 0000000000000..8d8dbc60ce9b9 --- /dev/null +++ b/lib/ui/painting/color_filter.cc @@ -0,0 +1,70 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#include "flutter/lib/ui/painting/color_filter.h" + +#include "third_party/tonic/converter/dart_converter.h" +#include "third_party/tonic/dart_args.h" +#include "third_party/tonic/dart_binding_macros.h" +#include "third_party/tonic/dart_library_natives.h" + +namespace flutter { + +static void ColorFilter_constructor(Dart_NativeArguments args) { + DartCallConstructor(&ColorFilter::Create, args); +} + +IMPLEMENT_WRAPPERTYPEINFO(ui, ColorFilter); + +#define FOR_EACH_BINDING(V) \ + V(ColorFilter, initMode) \ + V(ColorFilter, initMatrix) \ + V(ColorFilter, initSrgbToLinearGamma) \ + V(ColorFilter, initLinearToSrgbGamma) + +FOR_EACH_BINDING(DART_NATIVE_CALLBACK) + +void ColorFilter::RegisterNatives(tonic::DartLibraryNatives* natives) { + natives->Register( + {{"ColorFilter_constructor", ColorFilter_constructor, 1, true}, + FOR_EACH_BINDING(DART_REGISTER_NATIVE)}); +} + +fml::RefPtr ColorFilter::Create() { + return fml::MakeRefCounted(); +} + +void ColorFilter::initMode(int color, int blend_mode) { + filter_ = SkColorFilters::Blend(static_cast(color), + static_cast(blend_mode)); +} + +sk_sp ColorFilter::MakeColorMatrixFilter255( + const float array[20]) { + float tmp[20]; + memcpy(tmp, array, sizeof(tmp)); + tmp[4] *= 1.0f / 255; + tmp[9] *= 1.0f / 255; + tmp[14] *= 1.0f / 255; + tmp[19] *= 1.0f / 255; + return SkColorFilters::Matrix(tmp); +} + +void ColorFilter::initMatrix(const tonic::Float32List& color_matrix) { + FML_CHECK(color_matrix.num_elements() == 20); + + filter_ = MakeColorMatrixFilter255(color_matrix.data()); +} + +void ColorFilter::initLinearToSrgbGamma() { + filter_ = SkColorFilters::LinearToSRGBGamma(); +} + +void ColorFilter::initSrgbToLinearGamma() { + filter_ = SkColorFilters::SRGBToLinearGamma(); +} + +ColorFilter::~ColorFilter() = default; + +} // namespace flutter diff --git a/lib/ui/painting/color_filter.h b/lib/ui/painting/color_filter.h new file mode 100644 index 0000000000000..e73a89ed84915 --- /dev/null +++ b/lib/ui/painting/color_filter.h @@ -0,0 +1,52 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +#ifndef FLUTTER_LIB_UI_COLOR_FILTER_H_ +#define FLUTTER_LIB_UI_COLOR_FILTER_H_ + +#include "flutter/lib/ui/dart_wrapper.h" +#include "third_party/skia/include/core/SkColorFilter.h" +#include "third_party/tonic/typed_data/typed_list.h" + +using tonic::DartPersistentValue; + +namespace tonic { +class DartLibraryNatives; +} // namespace tonic + +namespace flutter { + +// A handle to an SkCodec object. +// +// Doesn't mirror SkCodec's API but provides a simple sequential access API. +class ColorFilter : public RefCountedDartWrappable { + DEFINE_WRAPPERTYPEINFO(); + FML_FRIEND_MAKE_REF_COUNTED(ColorFilter); + + public: + static fml::RefPtr Create(); + + // Flutter still defines the matrix to be biased by 255 in the last column + // (translate). skia is normalized, treating the last column as 0...1, so we + // post-scale here before calling the skia factory. + static sk_sp MakeColorMatrixFilter255(const float array[20]); + + void initMode(int color, int blend_mode); + void initMatrix(const tonic::Float32List& color_matrix); + void initSrgbToLinearGamma(); + void initLinearToSrgbGamma(); + + ~ColorFilter() override; + + sk_sp filter() const { return filter_; } + + static void RegisterNatives(tonic::DartLibraryNatives* natives); + + private: + sk_sp filter_; +}; + +} // namespace flutter + +#endif // FLUTTER_LIB_UI_COLOR_FILTER_H_ diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index 718b9ae61089d..d63b2ebd63db6 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -5,6 +5,7 @@ #include "flutter/lib/ui/painting/paint.h" #include "flutter/fml/logging.h" +#include "flutter/lib/ui/painting/color_filter.h" #include "flutter/lib/ui/painting/image_filter.h" #include "flutter/lib/ui/painting/shader.h" #include "third_party/skia/include/core/SkColorFilter.h" @@ -27,18 +28,15 @@ constexpr int kStrokeCapIndex = 5; constexpr int kStrokeJoinIndex = 6; constexpr int kStrokeMiterLimitIndex = 7; constexpr int kFilterQualityIndex = 8; -constexpr int kColorFilterIndex = 9; -constexpr int kColorFilterColorIndex = 10; -constexpr int kColorFilterBlendModeIndex = 11; -constexpr int kMaskFilterIndex = 12; -constexpr int kMaskFilterBlurStyleIndex = 13; -constexpr int kMaskFilterSigmaIndex = 14; -constexpr int kInvertColorIndex = 15; -constexpr size_t kDataByteCount = 75; // 4 * (last index + 1) +constexpr int kMaskFilterIndex = 9; +constexpr int kMaskFilterBlurStyleIndex = 10; +constexpr int kMaskFilterSigmaIndex = 11; +constexpr int kInvertColorIndex = 12; +constexpr size_t kDataByteCount = 52; // 4 * (last index + 1) // Indices for objects. constexpr int kShaderIndex = 0; -constexpr int kColorFilterMatrixIndex = 1; +constexpr int kColorFilterIndex = 1; constexpr int kImageFilterIndex = 2; constexpr int kObjectCount = 3; // One larger than largest object index. @@ -66,64 +64,6 @@ constexpr float invert_colors[20] = { // Must be kept in sync with the MaskFilter private constants in painting.dart. enum MaskFilterType { Null, Blur }; -// Must be kept in sync with the ColorFilter private constants in painting.dart. -enum ColorFilterType { - None, - Mode, - Matrix, - LinearToSRGBGamma, - SRGBToLinearGamma -}; - -// Flutter still defines the matrix to be biased by 255 in the last column -// (translate). skia is normalized, treating the last column as 0...1, so we -// post-scale here before calling the skia factory. -static sk_sp MakeColorMatrixFilter255(const float array[20]) { - float tmp[20]; - memcpy(tmp, array, sizeof(tmp)); - tmp[4] *= 1.0f / 255; - tmp[9] *= 1.0f / 255; - tmp[14] *= 1.0f / 255; - tmp[19] *= 1.0f / 255; - return SkColorFilters::Matrix(tmp); -} - -sk_sp ExtractColorFilter(const uint32_t* uint_data, - Dart_Handle* values) { - switch (uint_data[kColorFilterIndex]) { - case Mode: { - SkColor color = uint_data[kColorFilterColorIndex]; - SkBlendMode blend_mode = - static_cast(uint_data[kColorFilterBlendModeIndex]); - - return SkColorFilters::Blend(color, blend_mode); - } - case Matrix: { - Dart_Handle matrixHandle = values[kColorFilterMatrixIndex]; - if (!Dart_IsNull(matrixHandle)) { - FML_DCHECK(Dart_IsList(matrixHandle)); - intptr_t length = 0; - Dart_ListLength(matrixHandle, &length); - - FML_CHECK(length == 20); - - tonic::Float32List decoded(matrixHandle); - return MakeColorMatrixFilter255(decoded.data()); - } - return nullptr; - } - case LinearToSRGBGamma: { - return SkColorFilters::LinearToSRGBGamma(); - } - case SRGBToLinearGamma: { - return SkColorFilters::SRGBToLinearGamma(); - } - default: - FML_DLOG(ERROR) << "Out of range value received for kColorFilterIndex."; - return nullptr; - } -} - Paint::Paint(Dart_Handle paint_objects, Dart_Handle paint_data) { is_null_ = Dart_IsNull(paint_data); if (is_null_) @@ -145,6 +85,13 @@ Paint::Paint(Dart_Handle paint_objects, Dart_Handle paint_data) { paint_.setShader(decoded->shader()); } + Dart_Handle color_filter = values[kColorFilterIndex]; + if (!Dart_IsNull(color_filter)) { + ColorFilter* decoded_color_filter = + tonic::DartConverter::FromDart(color_filter); + paint_.setColorFilter(decoded_color_filter->filter()); + } + Dart_Handle image_filter = values[kImageFilterIndex]; if (!Dart_IsNull(image_filter)) { ImageFilter* decoded = @@ -197,20 +144,14 @@ Paint::Paint(Dart_Handle paint_objects, Dart_Handle paint_data) { if (filter_quality) paint_.setFilterQuality(static_cast(filter_quality)); - if (uint_data[kColorFilterIndex] && uint_data[kInvertColorIndex]) { - sk_sp color_filter = ExtractColorFilter(uint_data, values); - if (color_filter) { - sk_sp invert_filter = - MakeColorMatrixFilter255(invert_colors); - paint_.setColorFilter(invert_filter->makeComposed(color_filter)); - } - } else if (uint_data[kInvertColorIndex]) { - paint_.setColorFilter(MakeColorMatrixFilter255(invert_colors)); - } else if (uint_data[kColorFilterIndex]) { - sk_sp color_filter = ExtractColorFilter(uint_data, values); - if (color_filter) { - paint_.setColorFilter(color_filter); + if (uint_data[kInvertColorIndex]) { + sk_sp invert_filter = + ColorFilter::MakeColorMatrixFilter255(invert_colors); + SkColorFilter* current_filter = paint_.getColorFilter(); + if (current_filter) { + invert_filter = invert_filter->makeComposed(sk_sp(current_filter)); } + paint_.setColorFilter(invert_filter); } switch (uint_data[kMaskFilterIndex]) { From 4b820edf7a6f61db44b3e95ab2ca1bb85f488db5 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Wed, 3 Jul 2019 16:02:10 -0700 Subject: [PATCH 08/14] comment --- lib/ui/painting.dart | 7 +++++-- 1 file changed, 5 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 93a57b211197f..75a09761d3afb 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2589,8 +2589,11 @@ class ColorFilter { } /// A [ColorFilter] that is backed by a native SkColorFilter. -// We could not make [ColorFilter] be this, because it would no longer be const -// constructible and would complicate the == and hashCode implementations. +/// +/// This is a private class, rather than being the implementation of the public +/// ColorFilter, because we want ColorFilter to be const constructible and +/// efficiently comparable, so that widgets can check for ColorFilter equality to +// avoid repainting. class _ColorFilter extends NativeFieldWrapperClass2 { _ColorFilter.mode(this.creator) : assert(creator != null), From 8935830cfb898c9e9fa7ac1613bc2f63275c5de9 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 13:59:18 -0700 Subject: [PATCH 09/14] tests --- testing/dart/color_filter_test.dart | 96 +++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 testing/dart/color_filter_test.dart diff --git a/testing/dart/color_filter_test.dart b/testing/dart/color_filter_test.dart new file mode 100644 index 0000000000000..a2c80cebd213e --- /dev/null +++ b/testing/dart/color_filter_test.dart @@ -0,0 +1,96 @@ +// Copyright 2013 The Flutter Authors. All rights reserved. +// Use of this source code is governed by a BSD-style license that can be +// found in the LICENSE file. + +import 'dart:typed_data'; +import 'dart:ui'; + +import 'package:test/test.dart'; + +const Color red = Color(0xFFAA0000); +const Color green = Color(0xFF00AA00); + +const int greenRedColorBlend = 0xFF3131DB; +const int greenRedColorBlendInverted = 0xFFCECE24; +const int greenGreyscaled = 0xFF7A7A7A; +const int greenInvertedGreyscaled = 0xFF858585; + +const int greenLinearToSrgbGamma = 0xFF00D500; +const int greenLinearToSrgbGammaInverted = 0xFFFF2AFF; + +const int greenSrgbToLinearGamma = 0xFF006700; +const int greenSrgbToLinearGammaInverted = 0xFFFF98FF; + +const List greyscaleColorMatrix = [ + 0.2126, 0.7152, 0.0722, 0, 0, // + 0.2126, 0.7152, 0.0722, 0, 0, // + 0.2126, 0.7152, 0.0722, 0, 0, // + 0, 0, 0, 1, 0, // +]; + +void main() { + Future getBytesForPaint(Paint paint, {int width = 1, int height = 1}) async { + final PictureRecorder recorder = PictureRecorder(); + final Canvas recorderCanvas = Canvas(recorder); + recorderCanvas.drawPaint(paint); + final Picture picture = recorder.endRecording(); + final Image image = await picture.toImage(width, height); + final ByteData bytes = await image.toByteData(); + + expect(bytes.lengthInBytes, width * height * 4); + return bytes.buffer.asUint32List(); + } + + test('ColorFilter - mode', () async { + final Paint paint = Paint() + ..color = green + ..colorFilter = ColorFilter.mode(red, BlendMode.color); + + Uint32List bytes = await getBytesForPaint(paint); + expect(bytes[0], greenRedColorBlend); + + paint.invertColors = true; + bytes = await getBytesForPaint(paint); + expect(bytes[0], greenRedColorBlendInverted); + }); + + test('ColorFilter - matrix', () async { + final Paint paint = Paint() + ..color = green + ..colorFilter = ColorFilter.matrix(greyscaleColorMatrix); + + Uint32List bytes = await getBytesForPaint(paint); + expect(bytes[0], greenGreyscaled); + + paint.invertColors = true; + bytes = await getBytesForPaint(paint); + expect(bytes[0], greenInvertedGreyscaled); + }); + + test('ColorFilter - linearToSrgbGamma', () async { + final Paint paint = Paint() + ..color = green + ..colorFilter = ColorFilter.linearToSrgbGamma(); + + Uint32List bytes = await getBytesForPaint(paint); + expect(bytes[0], greenLinearToSrgbGamma); + + paint.invertColors = true; + bytes = await getBytesForPaint(paint); + expect(bytes[0], greenLinearToSrgbGammaInverted); + }); + + test('ColorFilter - srgbToLinearGamma', () async { + final Paint paint = Paint() + ..color = green + ..colorFilter = ColorFilter.srgbToLinearGamma(); + + Uint32List bytes = await getBytesForPaint(paint); + expect(bytes[0], greenSrgbToLinearGamma); + + paint.invertColors = true; + bytes = await getBytesForPaint(paint); + expect(bytes[0], greenSrgbToLinearGammaInverted); + }); + +} From 1bfc97c588ce806898417f5257e2c90c61718dc1 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 14:19:17 -0700 Subject: [PATCH 10/14] typo --- lib/ui/painting.dart | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/lib/ui/painting.dart b/lib/ui/painting.dart index 75a09761d3afb..0ce48da711729 100644 --- a/lib/ui/painting.dart +++ b/lib/ui/painting.dart @@ -2630,7 +2630,7 @@ class _ColorFilter extends NativeFieldWrapperClass2 { void _initMode(int color, int blendMode) native 'ColorFilter_initMode'; void _initMatrix(Float32List matrix) native 'ColorFilter_initMatrix'; void _initLinearToSrgbGamma() native 'ColorFilter_initLinearToSrgbGamma'; - void _initSrgbToLinearGamma() native 'ColorFilter_initSrgToLinearGamma'; + void _initSrgbToLinearGamma() native 'ColorFilter_initSrgbToLinearGamma'; } /// A filter operation to apply to a raster image. From f10ecfd88de7941825b58e3d9960d45758754254 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 14:56:37 -0700 Subject: [PATCH 11/14] merge --- lib/ui/compositing.dart | 23 ++----------------- lib/ui/compositing/scene_builder.cc | 35 +++-------------------------- lib/ui/compositing/scene_builder.h | 6 ++--- 3 files changed, 7 insertions(+), 57 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index 530237199a02d..271e2d52e29e4 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -391,31 +391,12 @@ class SceneBuilder extends NativeFieldWrapperClass2 { assert(filter != null); assert(filter._type != null && filter._type != ColorFilter._TypeNone); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); - EngineLayer engineLayer; - switch(filter._type) { - case ColorFilter._TypeMode: - engineLayer = _pushColorFilter(filter._color.value, filter._blendMode.index); - break; - case ColorFilter._TypeMatrix: - engineLayer = _pushColorFilterMatrix(Float32List.fromList(filter._matrix)); - break; - case ColorFilter._TypeLinearToSrgbGamma: - engineLayer = _pushColorFilterLinearToSrgbGamma(); - break; - case ColorFilter._TypeSrgbToLinearGamma: - engineLayer = _pushColorFilterSrgbToLinearGamma(); - break; - default: - assert(false); - } + final EngineLayer engineLayer = _pushColorFilter(filter._toNativeColorFilter()); final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(engineLayer); assert(_debugPushLayer(layer)); return layer; } - EngineLayer _pushColorFilter(int color, int blendMode) native 'SceneBuilder_pushColorFilter'; - EngineLayer _pushColorFilterMatrix(Float32List matrix) native 'SceneBuilder_pushColorFilterMatrix'; - EngineLayer _pushColorFilterLinearToSrgbGamma() native 'SceneBuilder_pushColorFilterLinearToSrgbGamma'; - EngineLayer _pushColorFilterSrgbToLinearGamma() native 'SceneBuilder_pushColorFilterSrgbToLinearGamma'; + EngineLayer _pushColorFilter(_ColorFilter filter) native 'SceneBuilder_pushColorFilter'; /// Pushes a backdrop filter operation onto the operation stack. /// diff --git a/lib/ui/compositing/scene_builder.cc b/lib/ui/compositing/scene_builder.cc index 1382f2b7e2ede..5b7148803c2c2 100644 --- a/lib/ui/compositing/scene_builder.cc +++ b/lib/ui/compositing/scene_builder.cc @@ -51,9 +51,6 @@ IMPLEMENT_WRAPPERTYPEINFO(ui, SceneBuilder); V(SceneBuilder, pushClipPath) \ V(SceneBuilder, pushOpacity) \ V(SceneBuilder, pushColorFilter) \ - V(SceneBuilder, pushColorFilterMatrix) \ - V(SceneBuilder, pushColorFilterLinearToSrgbGamma) \ - V(SceneBuilder, pushColorFilterSrgbToLinearGamma) \ V(SceneBuilder, pushBackdropFilter) \ V(SceneBuilder, pushShaderMask) \ V(SceneBuilder, pushPhysicalShape) \ @@ -144,36 +141,10 @@ fml::RefPtr SceneBuilder::pushOpacity(int alpha, return EngineLayer::MakeRetained(layer); } -fml::RefPtr SceneBuilder::pushColorFilter(int color, - int blendMode) { +fml::RefPtr SceneBuilder::pushColorFilter( + const ColorFilter* color_filter) { auto layer = - std::make_shared(SkColorFilters::Blend( - static_cast(color), static_cast(blendMode))); - PushLayer(layer); - return EngineLayer::MakeRetained(layer); -} - -fml::RefPtr SceneBuilder::pushColorFilterMatrix( - tonic::Float32List& matrix) { - FML_DCHECK(matrix.num_elements() == 20); - - auto layer = std::make_shared( - MakeColorMatrixFilter255(matrix.data())); - matrix.Release(); - PushLayer(layer); - return EngineLayer::MakeRetained(layer); -} - -fml::RefPtr SceneBuilder::pushColorFilterLinearToSrgbGamma() { - auto layer = std::make_shared( - SkColorFilters::LinearToSRGBGamma()); - PushLayer(layer); - return EngineLayer::MakeRetained(layer); -} - -fml::RefPtr SceneBuilder::pushColorFilterSrgbToLinearGamma() { - auto layer = std::make_shared( - SkColorFilters::SRGBToLinearGamma()); + std::make_shared(color_filter->filter()); PushLayer(layer); return EngineLayer::MakeRetained(layer); } diff --git a/lib/ui/compositing/scene_builder.h b/lib/ui/compositing/scene_builder.h index 7c9e32e7b807b..d378876d960a2 100644 --- a/lib/ui/compositing/scene_builder.h +++ b/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,10 +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 pushColorFilterMatrix(tonic::Float32List& matrix); - fml::RefPtr pushColorFilterLinearToSrgbGamma(); - fml::RefPtr pushColorFilterSrgbToLinearGamma(); + fml::RefPtr pushColorFilter(const ColorFilter* color_filter); fml::RefPtr pushBackdropFilter(ImageFilter* filter); fml::RefPtr pushShaderMask(Shader* shader, double maskRectLeft, From 884f0e3fd1ec9911a1f7e9ffc46df062987d60dc Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 15:04:52 -0700 Subject: [PATCH 12/14] Use right accessor --- lib/ui/painting/paint.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index d63b2ebd63db6..00006c3f989e9 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -147,9 +147,9 @@ Paint::Paint(Dart_Handle paint_objects, Dart_Handle paint_data) { if (uint_data[kInvertColorIndex]) { sk_sp invert_filter = ColorFilter::MakeColorMatrixFilter255(invert_colors); - SkColorFilter* current_filter = paint_.getColorFilter(); + sk_sp current_filter = paint_.refColorFilter(); if (current_filter) { - invert_filter = invert_filter->makeComposed(sk_sp(current_filter)); + invert_filter = invert_filter->makeComposed(current_filter); } paint_.setColorFilter(invert_filter); } From b6a06a9ad2c22ed04ba8669fd31b419d8e72b759 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 16:10:02 -0700 Subject: [PATCH 13/14] merge! --- lib/ui/compositing.dart | 1 - testing/dart/compositing_test.dart | 65 +++++++++++++++--------------- 2 files changed, 32 insertions(+), 34 deletions(-) diff --git a/lib/ui/compositing.dart b/lib/ui/compositing.dart index ce07f4368e242..5f8fdbfe876c0 100644 --- a/lib/ui/compositing.dart +++ b/lib/ui/compositing.dart @@ -389,7 +389,6 @@ class SceneBuilder extends NativeFieldWrapperClass2 { /// See [pop] for details about the operation stack. ColorFilterEngineLayer pushColorFilter(ColorFilter filter, { ColorFilterEngineLayer oldLayer }) { assert(filter != null); - assert(filter._type != null && filter._type != ColorFilter._TypeNone); assert(_debugCheckCanBeUsedAsOldLayer(oldLayer, 'pushColorFilter')); final ColorFilterEngineLayer layer = ColorFilterEngineLayer._(_pushColorFilter(filter._toNativeColorFilter())); assert(_debugPushLayer(layer)); diff --git a/testing/dart/compositing_test.dart b/testing/dart/compositing_test.dart index dfdbee8551ba2..a7b8e9a7c8f47 100644 --- a/testing/dart/compositing_test.dart +++ b/testing/dart/compositing_test.dart @@ -254,39 +254,6 @@ void main() { testRetainsParentOfOldLayer(pushFunction); } - test('SceneBuilder pushColorFilter handles all four types', () { - 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(), - ); - }); - testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { - return builder.pushColorFilter( - const ColorFilter.srgbToLinearGamma(), - ); - }); - }); - test('SceneBuilder does not share a layer between addRetained and push*', () { testNoSharing((SceneBuilder builder, EngineLayer oldLayer) { return builder.pushOffset(0, 0, oldLayer: oldLayer); @@ -324,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, + ); + }); }); } From 75a88a593280cf0553feaebb34915fcfc9496896 Mon Sep 17 00:00:00 2001 From: Dan Field Date: Mon, 8 Jul 2019 16:13:08 -0700 Subject: [PATCH 14/14] remove dead code --- lib/ui/painting/matrix.cc | 10 ---------- lib/ui/painting/matrix.h | 6 ------ lib/ui/painting/paint.cc | 1 - 3 files changed, 17 deletions(-) diff --git a/lib/ui/painting/matrix.cc b/lib/ui/painting/matrix.cc index 85d3c798493cc..52ba981ebda9d 100644 --- a/lib/ui/painting/matrix.cc +++ b/lib/ui/painting/matrix.cc @@ -38,14 +38,4 @@ tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix) { return matrix4; } -sk_sp MakeColorMatrixFilter255(const float color_matrix[20]) { - float tmp[20]; - memcpy(tmp, color_matrix, sizeof(tmp)); - tmp[4] *= 1.0f / 255; - tmp[9] *= 1.0f / 255; - tmp[14] *= 1.0f / 255; - tmp[19] *= 1.0f / 255; - return SkColorFilters::Matrix(tmp); -} - } // namespace flutter diff --git a/lib/ui/painting/matrix.h b/lib/ui/painting/matrix.h index d4659a3dd9313..bda531472263e 100644 --- a/lib/ui/painting/matrix.h +++ b/lib/ui/painting/matrix.h @@ -5,7 +5,6 @@ #ifndef FLUTTER_LIB_UI_PAINTING_MATRIX_H_ #define FLUTTER_LIB_UI_PAINTING_MATRIX_H_ -#include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkMatrix.h" #include "third_party/tonic/typed_data/typed_list.h" @@ -14,11 +13,6 @@ namespace flutter { SkMatrix ToSkMatrix(const tonic::Float64List& matrix4); tonic::Float64List ToMatrix4(const SkMatrix& sk_matrix); -// Flutter still defines the matrix to be biased by 255 in the last column -// (translate). skia is normalized, treating the last column as 0...1, so we -// post-scale here before calling the skia factory. -sk_sp MakeColorMatrixFilter255(const float color_matrix[20]); - } // namespace flutter #endif // FLUTTER_LIB_UI_PAINTING_MATRIX_H_ diff --git a/lib/ui/painting/paint.cc b/lib/ui/painting/paint.cc index 9442757a014d7..00006c3f989e9 100644 --- a/lib/ui/painting/paint.cc +++ b/lib/ui/painting/paint.cc @@ -7,7 +7,6 @@ #include "flutter/fml/logging.h" #include "flutter/lib/ui/painting/color_filter.h" #include "flutter/lib/ui/painting/image_filter.h" -#include "flutter/lib/ui/painting/matrix.h" #include "flutter/lib/ui/painting/shader.h" #include "third_party/skia/include/core/SkColorFilter.h" #include "third_party/skia/include/core/SkImageFilter.h"