From 5b879530c618cb8a6ff77755d373d5fd7ec57fc6 Mon Sep 17 00:00:00 2001 From: Matan Lurey Date: Wed, 20 Sep 2023 11:35:29 -0700 Subject: [PATCH] Make `dl_color_filter` tidy. --- display_list/effects/dl_color_filter.cc | 4 ++-- display_list/effects/dl_color_filter.h | 20 ++++++++++--------- .../effects/dl_color_filter_unittests.cc | 4 ++-- display_list/effects/dl_color_source.h | 17 +++++++++------- .../testing/dl_rendering_unittests.cc | 4 ++-- display_list/testing/dl_test_snippets.cc | 4 ++-- flow/layers/color_filter_layer_unittests.cc | 14 ++++++------- impeller/display_list/dl_unittests.cc | 6 ++++-- lib/ui/painting/color_filter.cc | 4 ++-- 9 files changed, 42 insertions(+), 35 deletions(-) diff --git a/display_list/effects/dl_color_filter.cc b/display_list/effects/dl_color_filter.cc index 99f4a7cdd8210..686484421561c 100644 --- a/display_list/effects/dl_color_filter.cc +++ b/display_list/effects/dl_color_filter.cc @@ -190,11 +190,11 @@ bool DlMatrixColorFilter::can_commute_with_opacity() const { } const std::shared_ptr - DlSrgbToLinearGammaColorFilter::instance = + DlSrgbToLinearGammaColorFilter::kInstance = std::make_shared(); const std::shared_ptr - DlLinearToSrgbGammaColorFilter::instance = + DlLinearToSrgbGammaColorFilter::kInstance = std::make_shared(); } // namespace flutter diff --git a/display_list/effects/dl_color_filter.h b/display_list/effects/dl_color_filter.h index a96517bb110db..42a4845e0a90c 100644 --- a/display_list/effects/dl_color_filter.h +++ b/display_list/effects/dl_color_filter.h @@ -63,7 +63,7 @@ class DlBlendColorFilter final : public DlColorFilter { : color_(color), mode_(mode) {} DlBlendColorFilter(const DlBlendColorFilter& filter) : DlBlendColorFilter(filter.color_, filter.mode_) {} - DlBlendColorFilter(const DlBlendColorFilter* filter) + explicit DlBlendColorFilter(const DlBlendColorFilter* filter) : DlBlendColorFilter(filter->color_, filter->mode_) {} static std::shared_ptr Make(DlColor color, DlBlendMode mode); @@ -113,12 +113,12 @@ class DlBlendColorFilter final : public DlColorFilter { // pixel data, the necessary pre<->non-pre conversions must be performed. class DlMatrixColorFilter final : public DlColorFilter { public: - DlMatrixColorFilter(const float matrix[20]) { + explicit DlMatrixColorFilter(const float matrix[20]) { memcpy(matrix_, matrix, sizeof(matrix_)); } DlMatrixColorFilter(const DlMatrixColorFilter& filter) : DlMatrixColorFilter(filter.matrix_) {} - DlMatrixColorFilter(const DlMatrixColorFilter* filter) + explicit DlMatrixColorFilter(const DlMatrixColorFilter* filter) : DlMatrixColorFilter(filter->matrix_) {} static std::shared_ptr Make(const float matrix[20]); @@ -155,12 +155,13 @@ class DlMatrixColorFilter final : public DlColorFilter { // gamma curve to the rendered pixels. class DlSrgbToLinearGammaColorFilter final : public DlColorFilter { public: - static const std::shared_ptr instance; + static const std::shared_ptr kInstance; DlSrgbToLinearGammaColorFilter() {} DlSrgbToLinearGammaColorFilter(const DlSrgbToLinearGammaColorFilter& filter) : DlSrgbToLinearGammaColorFilter() {} - DlSrgbToLinearGammaColorFilter(const DlSrgbToLinearGammaColorFilter* filter) + explicit DlSrgbToLinearGammaColorFilter( + const DlSrgbToLinearGammaColorFilter* filter) : DlSrgbToLinearGammaColorFilter() {} DlColorFilterType type() const override { @@ -170,7 +171,7 @@ class DlSrgbToLinearGammaColorFilter final : public DlColorFilter { bool modifies_transparent_black() const override { return false; } bool can_commute_with_opacity() const override { return true; } - std::shared_ptr shared() const override { return instance; } + std::shared_ptr shared() const override { return kInstance; } protected: bool equals_(const DlColorFilter& other) const override { @@ -186,12 +187,13 @@ class DlSrgbToLinearGammaColorFilter final : public DlColorFilter { // to the rendered pixels. class DlLinearToSrgbGammaColorFilter final : public DlColorFilter { public: - static const std::shared_ptr instance; + static const std::shared_ptr kInstance; DlLinearToSrgbGammaColorFilter() {} DlLinearToSrgbGammaColorFilter(const DlLinearToSrgbGammaColorFilter& filter) : DlLinearToSrgbGammaColorFilter() {} - DlLinearToSrgbGammaColorFilter(const DlLinearToSrgbGammaColorFilter* filter) + explicit DlLinearToSrgbGammaColorFilter( + const DlLinearToSrgbGammaColorFilter* filter) : DlLinearToSrgbGammaColorFilter() {} DlColorFilterType type() const override { @@ -201,7 +203,7 @@ class DlLinearToSrgbGammaColorFilter final : public DlColorFilter { bool modifies_transparent_black() const override { return false; } bool can_commute_with_opacity() const override { return true; } - std::shared_ptr shared() const override { return instance; } + std::shared_ptr shared() const override { return kInstance; } protected: bool equals_(const DlColorFilter& other) const override { diff --git a/display_list/effects/dl_color_filter_unittests.cc b/display_list/effects/dl_color_filter_unittests.cc index c82ca6eafef97..157348f3cf3ae 100644 --- a/display_list/effects/dl_color_filter_unittests.cc +++ b/display_list/effects/dl_color_filter_unittests.cc @@ -135,7 +135,7 @@ TEST(DisplayListColorFilter, SrgbToLinearEquals) { DlSrgbToLinearGammaColorFilter filter1; DlSrgbToLinearGammaColorFilter filter2; TestEquals(filter1, filter2); - TestEquals(filter1, *DlSrgbToLinearGammaColorFilter::instance); + TestEquals(filter1, *DlSrgbToLinearGammaColorFilter::kInstance); } TEST(DisplayListColorFilter, LinearToSrgbConstructor) { @@ -152,7 +152,7 @@ TEST(DisplayListColorFilter, LinearToSrgbEquals) { DlLinearToSrgbGammaColorFilter filter1; DlLinearToSrgbGammaColorFilter filter2; TestEquals(filter1, filter2); - TestEquals(filter1, *DlLinearToSrgbGammaColorFilter::instance); + TestEquals(filter1, *DlLinearToSrgbGammaColorFilter::kInstance); } } // namespace testing diff --git a/display_list/effects/dl_color_source.h b/display_list/effects/dl_color_source.h index 11a141da9c905..14eba1e23e2c7 100644 --- a/display_list/effects/dl_color_source.h +++ b/display_list/effects/dl_color_source.h @@ -178,7 +178,7 @@ class DlColorSource : public DlAttribute { class DlColorColorSource final : public DlColorSource { public: - DlColorColorSource(DlColor color) : color_(color) {} + explicit DlColorColorSource(DlColor color) : color_(color) {} bool isUIThreadSafe() const override { return true; } @@ -216,7 +216,7 @@ class DlMatrixColorSourceBase : public DlColorSource { } protected: - DlMatrixColorSourceBase(const SkMatrix* matrix) + explicit DlMatrixColorSourceBase(const SkMatrix* matrix) : matrix_(matrix ? *matrix : SkMatrix::I()) {} private: @@ -232,7 +232,7 @@ class DlImageColorSource final : public SkRefCnt, DlImageSampling sampling = DlImageSampling::kLinear, const SkMatrix* matrix = nullptr) : DlMatrixColorSourceBase(matrix), - image_(image), + image_(std::move(image)), horizontal_tile_mode_(horizontal_tile_mode), vertical_tile_mode_(vertical_tile_mode), sampling_(sampling) {} @@ -405,7 +405,8 @@ class DlLinearGradientColorSource final : public DlGradientColorSourceBase { store_color_stops(this + 1, colors, stops); } - DlLinearGradientColorSource(const DlLinearGradientColorSource* source) + explicit DlLinearGradientColorSource( + const DlLinearGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), source->tile_mode(), source->matrix_ptr()), @@ -468,7 +469,8 @@ class DlRadialGradientColorSource final : public DlGradientColorSourceBase { store_color_stops(this + 1, colors, stops); } - DlRadialGradientColorSource(const DlRadialGradientColorSource* source) + explicit DlRadialGradientColorSource( + const DlRadialGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), source->tile_mode(), source->matrix_ptr()), @@ -540,7 +542,8 @@ class DlConicalGradientColorSource final : public DlGradientColorSourceBase { store_color_stops(this + 1, colors, stops); } - DlConicalGradientColorSource(const DlConicalGradientColorSource* source) + explicit DlConicalGradientColorSource( + const DlConicalGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), source->tile_mode(), source->matrix_ptr()), @@ -610,7 +613,7 @@ class DlSweepGradientColorSource final : public DlGradientColorSourceBase { store_color_stops(this + 1, colors, stops); } - DlSweepGradientColorSource(const DlSweepGradientColorSource* source) + explicit DlSweepGradientColorSource(const DlSweepGradientColorSource* source) : DlGradientColorSourceBase(source->stop_count(), source->tile_mode(), source->matrix_ptr()), diff --git a/display_list/testing/dl_rendering_unittests.cc b/display_list/testing/dl_rendering_unittests.cc index 9f25788384caf..5e451ffc459fa 100644 --- a/display_list/testing/dl_rendering_unittests.cc +++ b/display_list/testing/dl_rendering_unittests.cc @@ -3488,8 +3488,8 @@ TEST_F(DisplayListCanvas, SaveLayerConsolidation) { DlBlendMode::kSrcATop), std::make_shared(commutable_color_matrix), std::make_shared(non_commutable_color_matrix), - DlSrgbToLinearGammaColorFilter::instance, - DlLinearToSrgbGammaColorFilter::instance, + DlSrgbToLinearGammaColorFilter::kInstance, + DlLinearToSrgbGammaColorFilter::kInstance, }; std::vector> image_filters = { std::make_shared(5.0f, 5.0f, DlTileMode::kDecal), diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index 0a53f0337afd9..eed9300e8307a 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -212,11 +212,11 @@ std::vector CreateAllAttributesOps() { }}, {0, 16, 0, 0, [](DlOpReceiver& r) { - r.setColorFilter(DlSrgbToLinearGammaColorFilter::instance.get()); + r.setColorFilter(DlSrgbToLinearGammaColorFilter::kInstance.get()); }}, {0, 16, 0, 0, [](DlOpReceiver& r) { - r.setColorFilter(DlLinearToSrgbGammaColorFilter::instance.get()); + r.setColorFilter(DlLinearToSrgbGammaColorFilter::kInstance.get()); }}, {0, 0, 0, 0, [](DlOpReceiver& r) { r.setColorFilter(nullptr); }}, }}, diff --git a/flow/layers/color_filter_layer_unittests.cc b/flow/layers/color_filter_layer_unittests.cc index 1ec1f35bd0bcc..efafe90864049 100644 --- a/flow/layers/color_filter_layer_unittests.cc +++ b/flow/layers/color_filter_layer_unittests.cc @@ -89,7 +89,7 @@ TEST_F(ColorFilterLayerTest, SimpleFilter) { const SkPath child_path = SkPath().addRect(child_bounds); const DlPaint child_paint = DlPaint(DlColor::kYellow()); - auto dl_color_filter = DlLinearToSrgbGammaColorFilter::instance; + auto dl_color_filter = DlLinearToSrgbGammaColorFilter::kInstance; auto mock_layer = std::make_shared(child_path, child_paint); auto layer = std::make_shared(dl_color_filter); layer->Add(mock_layer); @@ -129,7 +129,7 @@ TEST_F(ColorFilterLayerTest, MultipleChildren) { const DlPaint child_paint2 = DlPaint(DlColor::kCyan()); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); - auto dl_color_filter = DlSrgbToLinearGammaColorFilter::instance; + auto dl_color_filter = DlSrgbToLinearGammaColorFilter::kInstance; auto layer = std::make_shared(dl_color_filter); layer->Add(mock_layer1); layer->Add(mock_layer2); @@ -179,7 +179,7 @@ TEST_F(ColorFilterLayerTest, Nested) { const DlPaint child_paint2 = DlPaint(DlColor::kCyan()); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); - auto dl_color_filter = DlSrgbToLinearGammaColorFilter::instance; + auto dl_color_filter = DlSrgbToLinearGammaColorFilter::kInstance; auto layer1 = std::make_shared(dl_color_filter); auto layer2 = std::make_shared(dl_color_filter); @@ -239,7 +239,7 @@ TEST_F(ColorFilterLayerTest, Readback) { // ColorFilterLayer does not read from surface auto layer = std::make_shared( - DlLinearToSrgbGammaColorFilter::instance); + DlLinearToSrgbGammaColorFilter::kInstance); preroll_context()->surface_needs_readback = false; preroll_context()->state_stack.set_preroll_delegate(initial_transform); layer->Preroll(preroll_context()); @@ -255,7 +255,7 @@ TEST_F(ColorFilterLayerTest, Readback) { } TEST_F(ColorFilterLayerTest, CacheChild) { - auto layer_filter = DlSrgbToLinearGammaColorFilter::instance; + auto layer_filter = DlSrgbToLinearGammaColorFilter::kInstance; auto initial_transform = SkMatrix::Translate(50.0, 25.5); auto other_transform = SkMatrix::Scale(1.0, 2.0); const SkPath child_path = SkPath().addRect(SkRect::MakeWH(5.0f, 5.0f)); @@ -296,7 +296,7 @@ TEST_F(ColorFilterLayerTest, CacheChild) { } TEST_F(ColorFilterLayerTest, CacheChildren) { - auto layer_filter = DlSrgbToLinearGammaColorFilter::instance; + auto layer_filter = DlSrgbToLinearGammaColorFilter::kInstance; auto initial_transform = SkMatrix::Translate(50.0, 25.5); auto other_transform = SkMatrix::Scale(1.0, 2.0); const SkPath child_path1 = SkPath().addRect(SkRect::MakeWH(5.0f, 5.0f)); @@ -342,7 +342,7 @@ TEST_F(ColorFilterLayerTest, CacheChildren) { } TEST_F(ColorFilterLayerTest, CacheColorFilterLayerSelf) { - auto layer_filter = DlSrgbToLinearGammaColorFilter::instance; + auto layer_filter = DlSrgbToLinearGammaColorFilter::kInstance; auto initial_transform = SkMatrix::Translate(50.0, 25.5); auto other_transform = SkMatrix::Scale(1.0, 2.0); const SkPath child_path1 = SkPath().addRect(SkRect::MakeWH(5.0f, 5.0f)); diff --git a/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc index 7b692363c54b1..896989c34ef3d 100644 --- a/impeller/display_list/dl_unittests.cc +++ b/impeller/display_list/dl_unittests.cc @@ -1196,11 +1196,13 @@ TEST_P(DisplayListTest, CanDrawRectWithLinearToSrgbColorFilter) { flutter::DlPaint paint; paint.setColor(flutter::DlColor(0xFF2196F3).withAlpha(128)); flutter::DisplayListBuilder builder; - paint.setColorFilter(flutter::DlLinearToSrgbGammaColorFilter::instance.get()); + paint.setColorFilter( + flutter::DlLinearToSrgbGammaColorFilter::kInstance.get()); builder.DrawRect(SkRect::MakeXYWH(0, 0, 200, 200), paint); builder.Translate(0, 200); - paint.setColorFilter(flutter::DlSrgbToLinearGammaColorFilter::instance.get()); + paint.setColorFilter( + flutter::DlSrgbToLinearGammaColorFilter::kInstance.get()); builder.DrawRect(SkRect::MakeXYWH(0, 0, 200, 200), paint); ASSERT_TRUE(OpenPlaygroundHere(builder.Build())); diff --git a/lib/ui/painting/color_filter.cc b/lib/ui/painting/color_filter.cc index 06d421b377369..f0d780d27208d 100644 --- a/lib/ui/painting/color_filter.cc +++ b/lib/ui/painting/color_filter.cc @@ -43,11 +43,11 @@ void ColorFilter::initMatrix(const tonic::Float32List& color_matrix) { } void ColorFilter::initLinearToSrgbGamma() { - filter_ = DlLinearToSrgbGammaColorFilter::instance; + filter_ = DlLinearToSrgbGammaColorFilter::kInstance; } void ColorFilter::initSrgbToLinearGamma() { - filter_ = DlSrgbToLinearGammaColorFilter::instance; + filter_ = DlSrgbToLinearGammaColorFilter::kInstance; } ColorFilter::~ColorFilter() = default;