From d48807e021ba53a762aecead3852ca7cf54ca776 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Mon, 25 Jul 2022 16:38:10 +0800 Subject: [PATCH 1/8] Create DlLocalMatrixImageFilter --- display_list/display_list_image_filter.h | 71 ++++++++++++++++++++++++ 1 file changed, 71 insertions(+) diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 5cc789e337795..8edbaca6e8327 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -12,6 +12,7 @@ #include "flutter/display_list/display_list_tile_mode.h" #include "flutter/display_list/types.h" #include "flutter/fml/logging.h" +#include "src/core/SkLocalMatrixImageFilter.h" #include "third_party/skia/include/effects/SkImageFilters.h" namespace flutter { @@ -34,6 +35,7 @@ enum class DlImageFilterType { kMatrix, kComposeFilter, kColorFilter, + kLocalMatrixFilter, kUnknown }; @@ -41,6 +43,7 @@ class DlBlurImageFilter; class DlDilateImageFilter; class DlErodeImageFilter; class DlMatrixImageFilter; +class DlLocalMatrixImageFilter; class DlComposeImageFilter; class DlColorFilterImageFilter; @@ -82,6 +85,10 @@ class DlImageFilter // type of ImageFilter, otherwise return nullptr. virtual const DlMatrixImageFilter* asMatrix() const { return nullptr; } + virtual const DlLocalMatrixImageFilter* asLocalMatrix() const { + return nullptr; + } + // Return a DlComposeImageFilter pointer to this object iff it is a Compose // type of ImageFilter, otherwise return nullptr. virtual const DlComposeImageFilter* asCompose() const { return nullptr; } @@ -617,6 +624,70 @@ class DlColorFilterImageFilter final : public DlImageFilter { std::shared_ptr color_filter_; }; +class DlLocalMatrixImageFilter final : public DlImageFilter { + public: + explicit DlLocalMatrixImageFilter(const SkMatrix& matrix, + std::shared_ptr filter) + : matrix_(matrix), image_filter_(filter) {} + explicit DlLocalMatrixImageFilter(const DlLocalMatrixImageFilter* filter) + : DlLocalMatrixImageFilter(filter->matrix_, filter->image_filter_) {} + DlLocalMatrixImageFilter(const DlLocalMatrixImageFilter& filter) + : DlLocalMatrixImageFilter(&filter) {} + + std::shared_ptr shared() const override { + return std::make_shared(this); + } + + DlImageFilterType type() const override { + return DlImageFilterType::kLocalMatrixFilter; + } + size_t size() const override { return sizeof(*this); } + + const SkMatrix& matrix() const { return matrix_; } + + const DlLocalMatrixImageFilter* asLocalMatrix() const override { + return this; + } + + bool modifies_transparent_black() const override { return false; } + + SkRect* map_local_bounds(const SkRect& input_bounds, + SkRect& output_bounds) const override { + output_bounds = matrix_.mapRect(input_bounds); + return &output_bounds; + } + + SkIRect* map_device_bounds(const SkIRect& input_bounds, + const SkMatrix& ctm, + SkIRect& output_bounds) const override { + return image_filter_->map_device_bounds( + input_bounds, SkMatrix::Concat(ctm, matrix_), output_bounds); + } + + SkIRect* get_input_device_bounds(const SkIRect& output_bounds, + const SkMatrix& ctm, + SkIRect& input_bounds) const override { + return image_filter_->get_input_device_bounds( + output_bounds, SkMatrix::Concat(ctm, matrix_), input_bounds); + } + + sk_sp skia_object() const override { + return SkLocalMatrixImageFilter::Make(matrix_, + image_filter_->skia_object()); + } + + protected: + bool equals_(const DlImageFilter& other) const override { + FML_DCHECK(other.type() == DlImageFilterType::kMatrix); + auto that = static_cast(&other); + return (matrix_ == that->matrix_ && image_filter_ == that->image_filter_); + } + + private: + SkMatrix matrix_; + std::shared_ptr image_filter_; +}; + // A wrapper class for a Skia ImageFilter of unknown type. The above 4 types // are the only types that can be constructed by Flutter using the // ui.ImageFilter class so this class should be rarely used. The main use From e19c17bb81abf14cbf52d525ddd777e596d8acc8 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Tue, 26 Jul 2022 13:52:53 +0800 Subject: [PATCH 2/8] implement localMatrixImageFilter --- display_list/display_list_builder.cc | 8 ++++ display_list/display_list_image_filter.h | 37 ++++++++++++++++--- .../display_list_image_filter_unittests.cc | 26 +++++++++++++ .../display_list/display_list_dispatcher.cc | 1 + 4 files changed, 66 insertions(+), 6 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 970e019ff56bf..fd54b20e72dea 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -224,6 +224,14 @@ void DisplayListBuilder::onSetImageFilter(const DlImageFilter* filter) { new (pod) DlMatrixImageFilter(matrix_filter); break; } + case DlImageFilterType::kLocalMatrixFilter: { + const DlLocalMatrixImageFilter* local_matrix_filter = + filter->asLocalMatrix(); + FML_DCHECK(local_matrix_filter); + void* pod = Push(local_matrix_filter->size(), 0); + new (pod) DlLocalMatrixImageFilter(local_matrix_filter); + break; + } case DlImageFilterType::kComposeFilter: case DlImageFilterType::kColorFilter: { Push(0, 0, filter); diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 8edbaca6e8327..8fb2eab88f00c 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ +#include #include "flutter/display_list/display_list_attributes.h" #include "flutter/display_list/display_list_color_filter.h" #include "flutter/display_list/display_list_comparable.h" @@ -12,7 +13,6 @@ #include "flutter/display_list/display_list_tile_mode.h" #include "flutter/display_list/types.h" #include "flutter/fml/logging.h" -#include "src/core/SkLocalMatrixImageFilter.h" #include "third_party/skia/include/effects/SkImageFilters.h" namespace flutter { @@ -50,6 +50,12 @@ class DlColorFilterImageFilter; class DlImageFilter : public DlAttribute { public: + enum class MatrixCapability { + kTranslate, + kScaleTranslate, + kComplex, + }; + // Return a shared_ptr holding a DlImageFilter representing the indicated // Skia SkImageFilter pointer. // @@ -89,6 +95,8 @@ class DlImageFilter return nullptr; } + std::shared_ptr makeWithLocalMatrix(const SkMatrix& matrix); + // Return a DlComposeImageFilter pointer to this object iff it is a Compose // type of ImageFilter, otherwise return nullptr. virtual const DlComposeImageFilter* asCompose() const { return nullptr; } @@ -144,6 +152,10 @@ class DlImageFilter const SkMatrix& ctm, SkIRect& input_bounds) const = 0; + virtual MatrixCapability get_matrix_capability() const { + return MatrixCapability::kScaleTranslate; + } + protected: static SkVector map_vectors_affine(const SkMatrix& ctm, SkScalar x, @@ -541,6 +553,10 @@ class DlComposeImageFilter final : public DlImageFilter { inner_->skia_object()); } + MatrixCapability get_matrix_capability() const override { + return MatrixCapability::kComplex; + } + protected: bool equals_(const DlImageFilter& other) const override { FML_DCHECK(other.type() == DlImageFilterType::kComposeFilter); @@ -613,6 +629,10 @@ class DlColorFilterImageFilter final : public DlImageFilter { return SkImageFilters::ColorFilter(color_filter_->skia_object(), nullptr); } + MatrixCapability get_matrix_capability() const override { + return MatrixCapability::kComplex; + } + protected: bool equals_(const DlImageFilter& other) const override { FML_DCHECK(other.type() == DlImageFilterType::kColorFilter); @@ -633,7 +653,6 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { : DlLocalMatrixImageFilter(filter->matrix_, filter->image_filter_) {} DlLocalMatrixImageFilter(const DlLocalMatrixImageFilter& filter) : DlLocalMatrixImageFilter(&filter) {} - std::shared_ptr shared() const override { return std::make_shared(this); } @@ -653,13 +672,16 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { - output_bounds = matrix_.mapRect(input_bounds); - return &output_bounds; + if (!image_filter_) + return nullptr; + return image_filter_->map_local_bounds(input_bounds, output_bounds); } SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { + if (!image_filter_) + return nullptr; return image_filter_->map_device_bounds( input_bounds, SkMatrix::Concat(ctm, matrix_), output_bounds); } @@ -667,13 +689,16 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { SkIRect* get_input_device_bounds(const SkIRect& output_bounds, const SkMatrix& ctm, SkIRect& input_bounds) const override { + if (!image_filter_) + return nullptr; return image_filter_->get_input_device_bounds( output_bounds, SkMatrix::Concat(ctm, matrix_), input_bounds); } sk_sp skia_object() const override { - return SkLocalMatrixImageFilter::Make(matrix_, - image_filter_->skia_object()); + if (!image_filter_) + return nullptr; + return image_filter_->skia_object()->makeWithLocalMatrix(matrix_); } protected: diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 346a2f7ff56e3..1cfc54dc3cb56 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -2,13 +2,22 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include "flutter/display_list/display_list_attributes_testing.h" +#include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_builder.h" +#include "flutter/display_list/display_list_color.h" +#include "flutter/display_list/display_list_color_filter.h" #include "flutter/display_list/display_list_comparable.h" #include "flutter/display_list/display_list_image_filter.h" #include "flutter/display_list/display_list_sampling_options.h" +#include "flutter/display_list/display_list_tile_mode.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" +#include "include/core/SkColorFilter.h" +#include "include/core/SkMatrix.h" +#include "include/core/SkRect.h" +#include "include/core/SkSamplingOptions.h" namespace flutter { namespace testing { @@ -759,6 +768,23 @@ TEST(DisplayListImageFilter, UnknownContents) { ASSERT_EQ(filter.skia_object().get(), sk_filter.get()); } +TEST(DisplayListImageFilter, LocalImageFilterBounds) { + auto blur_filter = + SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); + auto local_filter = blur_filter->makeWithLocalMatrix(SkMatrix::Scale(2, 2)); + auto inputBounds = SkIRect::MakeLTRB(20, 20, 80, 80); + auto rect = local_filter->filterBounds( + inputBounds, SkMatrix::I(), + SkImageFilter::MapDirection::kForward_MapDirection); + auto dl_color_filter = + std::make_shared(5.0, 6.0, DlTileMode::kRepeat); + auto local_matrix_filter = + DlLocalMatrixImageFilter(SkMatrix::Scale(2, 2), dl_color_filter); + SkIRect out_bounds; + local_matrix_filter.map_device_bounds(inputBounds, SkMatrix::I(), out_bounds); + ASSERT_EQ(out_bounds, rect); +} + TEST(DisplayListImageFilter, UnknownEquals) { sk_sp sk_filter = SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc index a1d80528d6f09..145a2db905c46 100644 --- a/impeller/display_list/display_list_dispatcher.cc +++ b/impeller/display_list/display_list_dispatcher.cc @@ -412,6 +412,7 @@ static std::optional ToImageFilterProc( case flutter::DlImageFilterType::kDilate: case flutter::DlImageFilterType::kErode: case flutter::DlImageFilterType::kMatrix: + case flutter::DlImageFilterType::kLocalMatrixFilter: case flutter::DlImageFilterType::kComposeFilter: case flutter::DlImageFilterType::kColorFilter: case flutter::DlImageFilterType::kUnknown: From 8c91fb3b42a84fcb8bd3b93aecebbe13437e4358 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Wed, 27 Jul 2022 20:20:04 +0800 Subject: [PATCH 3/8] update test cases --- display_list/display_list_builder.cc | 9 +-- display_list/display_list_image_filter.h | 12 ++-- .../display_list_image_filter_unittests.cc | 68 +++++++++++++++---- 3 files changed, 65 insertions(+), 24 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index fd54b20e72dea..7dfe4a6054b57 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -224,19 +224,12 @@ void DisplayListBuilder::onSetImageFilter(const DlImageFilter* filter) { new (pod) DlMatrixImageFilter(matrix_filter); break; } - case DlImageFilterType::kLocalMatrixFilter: { - const DlLocalMatrixImageFilter* local_matrix_filter = - filter->asLocalMatrix(); - FML_DCHECK(local_matrix_filter); - void* pod = Push(local_matrix_filter->size(), 0); - new (pod) DlLocalMatrixImageFilter(local_matrix_filter); - break; - } case DlImageFilterType::kComposeFilter: case DlImageFilterType::kColorFilter: { Push(0, 0, filter); break; } + case DlImageFilterType::kLocalMatrixFilter: case DlImageFilterType::kUnknown: { Push(0, 0, filter->skia_object()); break; diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 8fb2eab88f00c..530ac121c0d7a 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -672,16 +672,18 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { - if (!image_filter_) + if (!image_filter_) { return nullptr; + } return image_filter_->map_local_bounds(input_bounds, output_bounds); } SkIRect* map_device_bounds(const SkIRect& input_bounds, const SkMatrix& ctm, SkIRect& output_bounds) const override { - if (!image_filter_) + if (!image_filter_) { return nullptr; + } return image_filter_->map_device_bounds( input_bounds, SkMatrix::Concat(ctm, matrix_), output_bounds); } @@ -689,15 +691,17 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { SkIRect* get_input_device_bounds(const SkIRect& output_bounds, const SkMatrix& ctm, SkIRect& input_bounds) const override { - if (!image_filter_) + if (!image_filter_) { return nullptr; + } return image_filter_->get_input_device_bounds( output_bounds, SkMatrix::Concat(ctm, matrix_), input_bounds); } sk_sp skia_object() const override { - if (!image_filter_) + if (!image_filter_) { return nullptr; + } return image_filter_->skia_object()->makeWithLocalMatrix(matrix_); } diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 1cfc54dc3cb56..d43f32240ae37 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -15,9 +15,12 @@ #include "flutter/display_list/types.h" #include "gtest/gtest.h" #include "include/core/SkColorFilter.h" +#include "include/core/SkImageFilter.h" #include "include/core/SkMatrix.h" #include "include/core/SkRect.h" +#include "include/core/SkRefCnt.h" #include "include/core/SkSamplingOptions.h" +#include "include/core/SkTileMode.h" namespace flutter { namespace testing { @@ -769,20 +772,61 @@ TEST(DisplayListImageFilter, UnknownContents) { } TEST(DisplayListImageFilter, LocalImageFilterBounds) { - auto blur_filter = - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); - auto local_filter = blur_filter->makeWithLocalMatrix(SkMatrix::Scale(2, 2)); auto inputBounds = SkIRect::MakeLTRB(20, 20, 80, 80); - auto rect = local_filter->filterBounds( - inputBounds, SkMatrix::I(), - SkImageFilter::MapDirection::kForward_MapDirection); - auto dl_color_filter = + auto filter_matrix = SkMatrix::MakeAll(2.0, 0.0, 10, // + 0.5, 3.0, 15, // + 0.0, 0.0, 1); + + auto sk_filter1 = + SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); + auto sk_filter2 = SkImageFilters::ColorFilter( + SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcOver), nullptr); + auto sk_filter3 = SkImageFilters::Dilate(5.0, 10.0, nullptr); + auto sk_filter4 = SkImageFilters::MatrixTransform( + filter_matrix, ToSk(DlImageSampling::kLinear), nullptr); + // auto sk_filter5 = SkImageFilters::Compose(sk_filter1, sk_filter2); + std::vector> sk_filters{ + sk_filter1, sk_filter2, sk_filter3, sk_filter4 /*, sk_filter5*/}; + + DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcOver); + auto dl_filter1 = std::make_shared(5.0, 6.0, DlTileMode::kRepeat); - auto local_matrix_filter = - DlLocalMatrixImageFilter(SkMatrix::Scale(2, 2), dl_color_filter); - SkIRect out_bounds; - local_matrix_filter.map_device_bounds(inputBounds, SkMatrix::I(), out_bounds); - ASSERT_EQ(out_bounds, rect); + auto dl_filter2 = + std::make_shared(dl_color_filter.shared()); + auto dl_filter3 = std::make_shared(5, 10); + auto dl_filter4 = std::make_shared( + filter_matrix, DlImageSampling::kLinear); + // auto dl_filter5 = + // std::make_shared(dl_filter1->shared(), + // dl_filter2->shared()); + std::vector> dl_filters{ + dl_filter1, dl_filter2, dl_filter3, dl_filter4, /* dl_filter5*/}; + + auto translate = SkMatrix::Translate(10.0, 10.0); + auto scale_translate = SkMatrix::Scale(2.0, 2.0).preTranslate(10.0, 10.0); + auto rotate_translate = SkMatrix::RotateDeg(45).preTranslate(5.0, 5.0); + auto persp = SkMatrix::I(); + persp.setPerspY(10); + std::vector matrixs = {translate, scale_translate, rotate_translate, + persp}; + + for (unsigned i = 0; i < sk_filters.size(); i++) { + for (unsigned j = 0; j < matrixs.size(); j++) { + auto& m = matrixs[j]; + auto sk_local_filter = sk_filters[i]->makeWithLocalMatrix(m); + auto dl_local_filter = dl_filters[i]->makeWithLocalMatrix(m); + SkIRect sk_rect, dl_rect; + if (sk_local_filter) { + sk_rect = sk_local_filter->filterBounds( + inputBounds, SkMatrix::I(), + SkImageFilter::MapDirection::kForward_MapDirection); + } + if (dl_local_filter) { + dl_local_filter->map_device_bounds(inputBounds, SkMatrix::I(), dl_rect); + } + ASSERT_EQ(sk_rect, dl_rect); + } + } } TEST(DisplayListImageFilter, UnknownEquals) { From de99e7d49ec148983de95e51e7c43b1b3768ff5f Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Thu, 28 Jul 2022 16:43:18 +0800 Subject: [PATCH 4/8] test cases --- display_list/display_list_image_filter.cc | 17 ++++++++++++ display_list/display_list_image_filter.h | 27 ++++++++++++++++--- .../display_list_image_filter_unittests.cc | 11 ++++---- 3 files changed, 46 insertions(+), 9 deletions(-) diff --git a/display_list/display_list_image_filter.cc b/display_list/display_list_image_filter.cc index f9eb71d35cca1..1b6cdfed5dc77 100644 --- a/display_list/display_list_image_filter.cc +++ b/display_list/display_list_image_filter.cc @@ -27,6 +27,23 @@ std::shared_ptr DlImageFilter::From( return std::make_shared(sk_ref_sp(sk_filter)); } +std::shared_ptr DlImageFilter::makeWithLocalMatrix( + const SkMatrix& matrix) { + if (matrix.isIdentity()) { + return shared(); + } + // Matrix + MatrixCapability inputCapability = this->get_matrix_capability(); + if ((inputCapability == MatrixCapability::kTranslate && + !matrix.isTranslate()) || + (inputCapability == MatrixCapability::kScaleTranslate && + !matrix.isScaleTranslate())) { + // Nothing we can do at this point + return nullptr; + } + return std::make_shared(matrix, shared()); +} + SkRect* DlComposeImageFilter::map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const { SkRect cur_bounds = input_bounds; diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 530ac121c0d7a..4a03603bd27db 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -6,6 +6,7 @@ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ #include +#include #include "flutter/display_list/display_list_attributes.h" #include "flutter/display_list/display_list_color_filter.h" #include "flutter/display_list/display_list_comparable.h" @@ -152,10 +153,26 @@ class DlImageFilter const SkMatrix& ctm, SkIRect& input_bounds) const = 0; - virtual MatrixCapability get_matrix_capability() const { + MatrixCapability get_matrix_capability() const { + MatrixCapability result = this->matrix_capability(); + // CropRects need to apply in the source coordinate system, but are not + // aware of complex CTMs when performing clipping. For a simple fix, any + // filter with a crop rect set cannot support more than scale+translate CTMs + // until that's updated. if (this->cropRectIsSet()) { + // result = std::min(result, MatrixCapability::kScaleTranslate); + // } + for (auto* filter : filters()) { + result = std::min(result, filter->matrix_capability()); + } + return result; + } + + virtual MatrixCapability matrix_capability() const { return MatrixCapability::kScaleTranslate; } + virtual std::vector filters() const { return {this}; } + protected: static SkVector map_vectors_affine(const SkMatrix& ctm, SkScalar x, @@ -553,10 +570,14 @@ class DlComposeImageFilter final : public DlImageFilter { inner_->skia_object()); } - MatrixCapability get_matrix_capability() const override { + MatrixCapability matrix_capability() const override { return MatrixCapability::kComplex; } + virtual std::vector filters() const override { + return {outer_.get(), inner_.get()}; + } + protected: bool equals_(const DlImageFilter& other) const override { FML_DCHECK(other.type() == DlImageFilterType::kComposeFilter); @@ -629,7 +650,7 @@ class DlColorFilterImageFilter final : public DlImageFilter { return SkImageFilters::ColorFilter(color_filter_->skia_object(), nullptr); } - MatrixCapability get_matrix_capability() const override { + MatrixCapability matrix_capability() const override { return MatrixCapability::kComplex; } diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index d43f32240ae37..706c58e20d6cb 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -784,9 +784,9 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { auto sk_filter3 = SkImageFilters::Dilate(5.0, 10.0, nullptr); auto sk_filter4 = SkImageFilters::MatrixTransform( filter_matrix, ToSk(DlImageSampling::kLinear), nullptr); - // auto sk_filter5 = SkImageFilters::Compose(sk_filter1, sk_filter2); + auto sk_filter5 = SkImageFilters::Compose(sk_filter1, sk_filter2); std::vector> sk_filters{ - sk_filter1, sk_filter2, sk_filter3, sk_filter4 /*, sk_filter5*/}; + sk_filter1, sk_filter2, sk_filter3, sk_filter4, sk_filter5}; DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcOver); auto dl_filter1 = @@ -796,11 +796,10 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { auto dl_filter3 = std::make_shared(5, 10); auto dl_filter4 = std::make_shared( filter_matrix, DlImageSampling::kLinear); - // auto dl_filter5 = - // std::make_shared(dl_filter1->shared(), - // dl_filter2->shared()); + auto dl_filter5 = std::make_shared( + dl_filter1->shared(), dl_filter2->shared()); std::vector> dl_filters{ - dl_filter1, dl_filter2, dl_filter3, dl_filter4, /* dl_filter5*/}; + dl_filter1, dl_filter2, dl_filter3, dl_filter4, dl_filter5}; auto translate = SkMatrix::Translate(10.0, 10.0); auto scale_translate = SkMatrix::Scale(2.0, 2.0).preTranslate(10.0, 10.0); From c323d42811e5b62a67ac76d5f87cfbde020c5fc9 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Fri, 29 Jul 2022 15:44:52 +0800 Subject: [PATCH 5/8] fix some comments --- display_list/display_list_builder.cc | 2 +- display_list/display_list_image_filter.h | 16 +++++----------- .../display_list_image_filter_unittests.cc | 13 ++++++------- display_list/display_list_ops.h | 1 + 4 files changed, 13 insertions(+), 19 deletions(-) diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 7dfe4a6054b57..a4741ad24c470 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -225,11 +225,11 @@ void DisplayListBuilder::onSetImageFilter(const DlImageFilter* filter) { break; } case DlImageFilterType::kComposeFilter: + case DlImageFilterType::kLocalMatrixFilter: case DlImageFilterType::kColorFilter: { Push(0, 0, filter); break; } - case DlImageFilterType::kLocalMatrixFilter: case DlImageFilterType::kUnknown: { Push(0, 0, filter->skia_object()); break; diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 4a03603bd27db..987ae37a6c11e 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -5,6 +5,7 @@ #ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ +#include #include #include #include "flutter/display_list/display_list_attributes.h" @@ -154,17 +155,14 @@ class DlImageFilter SkIRect& input_bounds) const = 0; MatrixCapability get_matrix_capability() const { - MatrixCapability result = this->matrix_capability(); // CropRects need to apply in the source coordinate system, but are not // aware of complex CTMs when performing clipping. For a simple fix, any // filter with a crop rect set cannot support more than scale+translate CTMs - // until that's updated. if (this->cropRectIsSet()) { + // until that's updated. + // if (this->cropRectIsSet()) { // result = std::min(result, MatrixCapability::kScaleTranslate); // } - for (auto* filter : filters()) { - result = std::min(result, filter->matrix_capability()); - } - return result; + return this->matrix_capability(); } virtual MatrixCapability matrix_capability() const { @@ -571,11 +569,7 @@ class DlComposeImageFilter final : public DlImageFilter { } MatrixCapability matrix_capability() const override { - return MatrixCapability::kComplex; - } - - virtual std::vector filters() const override { - return {outer_.get(), inner_.get()}; + return std::min(outer_->matrix_capability(), inner_->matrix_capability()); } protected: diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 706c58e20d6cb..c3463e8b11c54 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -806,22 +806,21 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { auto rotate_translate = SkMatrix::RotateDeg(45).preTranslate(5.0, 5.0); auto persp = SkMatrix::I(); persp.setPerspY(10); - std::vector matrixs = {translate, scale_translate, rotate_translate, - persp}; + std::vector matrices = {translate, scale_translate, + rotate_translate, persp}; for (unsigned i = 0; i < sk_filters.size(); i++) { - for (unsigned j = 0; j < matrixs.size(); j++) { - auto& m = matrixs[j]; + for (unsigned j = 0; j < matrices.size(); j++) { + auto& m = matrices[j]; auto sk_local_filter = sk_filters[i]->makeWithLocalMatrix(m); auto dl_local_filter = dl_filters[i]->makeWithLocalMatrix(m); SkIRect sk_rect, dl_rect; if (sk_local_filter) { sk_rect = sk_local_filter->filterBounds( - inputBounds, SkMatrix::I(), - SkImageFilter::MapDirection::kForward_MapDirection); + inputBounds, m, SkImageFilter::MapDirection::kForward_MapDirection); } if (dl_local_filter) { - dl_local_filter->map_device_bounds(inputBounds, SkMatrix::I(), dl_rect); + dl_local_filter->map_device_bounds(inputBounds, m, dl_rect); } ASSERT_EQ(sk_rect, dl_rect); } diff --git a/display_list/display_list_ops.h b/display_list/display_list_ops.h index 052e7c9343bc9..046258ad3297b 100644 --- a/display_list/display_list_ops.h +++ b/display_list/display_list_ops.h @@ -11,6 +11,7 @@ #include "flutter/display_list/display_list_sampling_options.h" #include "flutter/display_list/types.h" #include "flutter/fml/macros.h" +#include "include/core/SkMatrix.h" namespace flutter { From 9f48c9c3b16fe0cf44630318100c2a5cb47edd16 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Mon, 8 Aug 2022 14:24:20 +0800 Subject: [PATCH 6/8] update some comments --- display_list/display_list_image_filter.cc | 22 ++-- display_list/display_list_image_filter.h | 19 ++-- .../display_list_image_filter_unittests.cc | 104 +++++++++++------- 3 files changed, 89 insertions(+), 56 deletions(-) diff --git a/display_list/display_list_image_filter.cc b/display_list/display_list_image_filter.cc index 1b6cdfed5dc77..cae1321ffda34 100644 --- a/display_list/display_list_image_filter.cc +++ b/display_list/display_list_image_filter.cc @@ -33,13 +33,21 @@ std::shared_ptr DlImageFilter::makeWithLocalMatrix( return shared(); } // Matrix - MatrixCapability inputCapability = this->get_matrix_capability(); - if ((inputCapability == MatrixCapability::kTranslate && - !matrix.isTranslate()) || - (inputCapability == MatrixCapability::kScaleTranslate && - !matrix.isScaleTranslate())) { - // Nothing we can do at this point - return nullptr; + switch (this->matrix_capability()) { + case MatrixCapability::kTranslate: { + if (!matrix.isTranslate()) { + // Nothing we can do at this point + return nullptr; + } + } + case MatrixCapability::kScaleTranslate: { + if (!matrix.isScaleTranslate()) { + // Nothing we can do at this point + return nullptr; + } + } + default: + break; } return std::make_shared(matrix, shared()); } diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 987ae37a6c11e..6561f93b77231 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -97,7 +97,8 @@ class DlImageFilter return nullptr; } - std::shared_ptr makeWithLocalMatrix(const SkMatrix& matrix); + virtual std::shared_ptr makeWithLocalMatrix( + const SkMatrix& matrix); // Return a DlComposeImageFilter pointer to this object iff it is a Compose // type of ImageFilter, otherwise return nullptr. @@ -154,17 +155,6 @@ class DlImageFilter const SkMatrix& ctm, SkIRect& input_bounds) const = 0; - MatrixCapability get_matrix_capability() const { - // CropRects need to apply in the source coordinate system, but are not - // aware of complex CTMs when performing clipping. For a simple fix, any - // filter with a crop rect set cannot support more than scale+translate CTMs - // until that's updated. - // if (this->cropRectIsSet()) { - // result = std::min(result, MatrixCapability::kScaleTranslate); - // } - return this->matrix_capability(); - } - virtual MatrixCapability matrix_capability() const { return MatrixCapability::kScaleTranslate; } @@ -648,6 +638,11 @@ class DlColorFilterImageFilter final : public DlImageFilter { return MatrixCapability::kComplex; } + std::shared_ptr makeWithLocalMatrix( + const SkMatrix& matrix) override { + return shared(); + } + protected: bool equals_(const DlImageFilter& other) const override { FML_DCHECK(other.type() == DlImageFilterType::kColorFilter); diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index c3463e8b11c54..13a410870d1b5 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include +#include #include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_builder.h" @@ -772,57 +773,86 @@ TEST(DisplayListImageFilter, UnknownContents) { } TEST(DisplayListImageFilter, LocalImageFilterBounds) { - auto inputBounds = SkIRect::MakeLTRB(20, 20, 80, 80); auto filter_matrix = SkMatrix::MakeAll(2.0, 0.0, 10, // 0.5, 3.0, 15, // 0.0, 0.0, 1); - - auto sk_filter1 = - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); - auto sk_filter2 = SkImageFilters::ColorFilter( - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcOver), nullptr); - auto sk_filter3 = SkImageFilters::Dilate(5.0, 10.0, nullptr); - auto sk_filter4 = SkImageFilters::MatrixTransform( - filter_matrix, ToSk(DlImageSampling::kLinear), nullptr); - auto sk_filter5 = SkImageFilters::Compose(sk_filter1, sk_filter2); std::vector> sk_filters{ - sk_filter1, sk_filter2, sk_filter3, sk_filter4, sk_filter5}; + SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr), + SkImageFilters::ColorFilter( + SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcOver), nullptr), + SkImageFilters::Dilate(5.0, 10.0, nullptr), + SkImageFilters::MatrixTransform(filter_matrix, + ToSk(DlImageSampling::kLinear), nullptr), + SkImageFilters::Compose( + SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr), + SkImageFilters::ColorFilter( + SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcOver), + nullptr))}; DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcOver); - auto dl_filter1 = - std::make_shared(5.0, 6.0, DlTileMode::kRepeat); - auto dl_filter2 = - std::make_shared(dl_color_filter.shared()); - auto dl_filter3 = std::make_shared(5, 10); - auto dl_filter4 = std::make_shared( - filter_matrix, DlImageSampling::kLinear); - auto dl_filter5 = std::make_shared( - dl_filter1->shared(), dl_filter2->shared()); std::vector> dl_filters{ - dl_filter1, dl_filter2, dl_filter3, dl_filter4, dl_filter5}; + std::make_shared(5.0, 6.0, DlTileMode::kRepeat), + std::make_shared(dl_color_filter.shared()), + std::make_shared(5, 10), + std::make_shared(filter_matrix, + DlImageSampling::kLinear), + std::make_shared( + std::make_shared(5.0, 6.0, DlTileMode::kRepeat), + std::make_shared( + dl_color_filter.shared()))}; - auto translate = SkMatrix::Translate(10.0, 10.0); - auto scale_translate = SkMatrix::Scale(2.0, 2.0).preTranslate(10.0, 10.0); - auto rotate_translate = SkMatrix::RotateDeg(45).preTranslate(5.0, 5.0); auto persp = SkMatrix::I(); persp.setPerspY(10); - std::vector matrices = {translate, scale_translate, - rotate_translate, persp}; + std::vector matrices = { + SkMatrix::Translate(10.0, 10.0), + SkMatrix::Scale(2.0, 2.0).preTranslate(10.0, 10.0), + SkMatrix::RotateDeg(45).preTranslate(5.0, 5.0), persp}; + std::vector bounds_matrices{SkMatrix::Translate(5.0, 10.0), + SkMatrix::Scale(2.0, 2.0)}; for (unsigned i = 0; i < sk_filters.size(); i++) { for (unsigned j = 0; j < matrices.size(); j++) { - auto& m = matrices[j]; - auto sk_local_filter = sk_filters[i]->makeWithLocalMatrix(m); - auto dl_local_filter = dl_filters[i]->makeWithLocalMatrix(m); - SkIRect sk_rect, dl_rect; - if (sk_local_filter) { - sk_rect = sk_local_filter->filterBounds( - inputBounds, m, SkImageFilter::MapDirection::kForward_MapDirection); - } - if (dl_local_filter) { - dl_local_filter->map_device_bounds(inputBounds, m, dl_rect); + for (unsigned k = 0; k < bounds_matrices.size(); k++) { + auto& m = matrices[j]; + auto& bounds_matrix = bounds_matrices[k]; + auto sk_local_filter = sk_filters[i]->makeWithLocalMatrix(m); + auto dl_local_filter = dl_filters[i]->makeWithLocalMatrix(m); + { + auto inputBounds = SkIRect::MakeLTRB(20, 20, 80, 80); + SkIRect sk_rect, dl_rect; + if (sk_local_filter) { + sk_rect = sk_local_filter->filterBounds( + inputBounds, bounds_matrix, + SkImageFilter::MapDirection::kForward_MapDirection); + } + if (dl_local_filter) { + dl_local_filter->map_device_bounds(inputBounds, bounds_matrix, + dl_rect); + } + ASSERT_EQ(sk_rect, dl_rect); + } + { + // Test for: Know the outset bounds to get the inset bounds + // Skia have some bounds calculate error of DilateFilter and + // MatrixFilter + // https://fiddle.skia.org/c/a0dc934dd7f900fb958797edd0e86ee3 + if (i == 2 || i == 3) { + continue; + } + auto outsetBounds = SkIRect::MakeLTRB(20, 20, 80, 80); + SkIRect sk_rect, dl_rect; + if (sk_local_filter) { + sk_rect = sk_local_filter->filterBounds( + outsetBounds, bounds_matrix, + SkImageFilter::MapDirection::kReverse_MapDirection); + } + if (dl_local_filter) { + dl_local_filter->get_input_device_bounds(outsetBounds, + bounds_matrix, dl_rect); + } + ASSERT_EQ(sk_rect, dl_rect); + } } - ASSERT_EQ(sk_rect, dl_rect); } } } From abd475b3c2e85adb2216da361f5cb17b018fd3eb Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Tue, 9 Aug 2022 11:48:50 +0800 Subject: [PATCH 7/8] fix some comments --- display_list/display_list_image_filter.h | 15 ++++--- .../display_list_image_filter_unittests.cc | 44 +++++++------------ display_list/display_list_ops.h | 1 - 3 files changed, 24 insertions(+), 36 deletions(-) diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index 6561f93b77231..4a0063b8b48cb 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -5,9 +5,6 @@ #ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ #define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_IMAGE_FILTER_H_ -#include -#include -#include #include "flutter/display_list/display_list_attributes.h" #include "flutter/display_list/display_list_color_filter.h" #include "flutter/display_list/display_list_comparable.h" @@ -159,8 +156,6 @@ class DlImageFilter return MatrixCapability::kScaleTranslate; } - virtual std::vector filters() const { return {this}; } - protected: static SkVector map_vectors_affine(const SkMatrix& ctm, SkScalar x, @@ -678,7 +673,12 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { return this; } - bool modifies_transparent_black() const override { return false; } + bool modifies_transparent_black() const override { + if (!image_filter_) { + return false; + } + return image_filter_->modifies_transparent_black(); + } SkRect* map_local_bounds(const SkRect& input_bounds, SkRect& output_bounds) const override { @@ -719,7 +719,8 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { bool equals_(const DlImageFilter& other) const override { FML_DCHECK(other.type() == DlImageFilterType::kMatrix); auto that = static_cast(&other); - return (matrix_ == that->matrix_ && image_filter_ == that->image_filter_); + return (matrix_ == that->matrix_ && + Equals(image_filter_, that->image_filter_)); } private: diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 13a410870d1b5..7aaa6082347cc 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -2,8 +2,6 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. -#include -#include #include "flutter/display_list/display_list_attributes_testing.h" #include "flutter/display_list/display_list_blend_mode.h" #include "flutter/display_list/display_list_builder.h" @@ -15,13 +13,6 @@ #include "flutter/display_list/display_list_tile_mode.h" #include "flutter/display_list/types.h" #include "gtest/gtest.h" -#include "include/core/SkColorFilter.h" -#include "include/core/SkImageFilter.h" -#include "include/core/SkMatrix.h" -#include "include/core/SkRect.h" -#include "include/core/SkRefCnt.h" -#include "include/core/SkSamplingOptions.h" -#include "include/core/SkTileMode.h" namespace flutter { namespace testing { @@ -802,7 +793,7 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { dl_color_filter.shared()))}; auto persp = SkMatrix::I(); - persp.setPerspY(10); + persp.setPerspY(0.001); std::vector matrices = { SkMatrix::Translate(10.0, 10.0), SkMatrix::Scale(2.0, 2.0).preTranslate(10.0, 10.0), @@ -817,18 +808,19 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { auto& bounds_matrix = bounds_matrices[k]; auto sk_local_filter = sk_filters[i]->makeWithLocalMatrix(m); auto dl_local_filter = dl_filters[i]->makeWithLocalMatrix(m); + if (!sk_local_filter || !dl_local_filter) { + ASSERT_TRUE(!sk_local_filter); + ASSERT_TRUE(!dl_local_filter); + continue; + } { auto inputBounds = SkIRect::MakeLTRB(20, 20, 80, 80); SkIRect sk_rect, dl_rect; - if (sk_local_filter) { - sk_rect = sk_local_filter->filterBounds( - inputBounds, bounds_matrix, - SkImageFilter::MapDirection::kForward_MapDirection); - } - if (dl_local_filter) { - dl_local_filter->map_device_bounds(inputBounds, bounds_matrix, - dl_rect); - } + sk_rect = sk_local_filter->filterBounds( + inputBounds, bounds_matrix, + SkImageFilter::MapDirection::kForward_MapDirection); + dl_local_filter->map_device_bounds(inputBounds, bounds_matrix, + dl_rect); ASSERT_EQ(sk_rect, dl_rect); } { @@ -841,15 +833,11 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { } auto outsetBounds = SkIRect::MakeLTRB(20, 20, 80, 80); SkIRect sk_rect, dl_rect; - if (sk_local_filter) { - sk_rect = sk_local_filter->filterBounds( - outsetBounds, bounds_matrix, - SkImageFilter::MapDirection::kReverse_MapDirection); - } - if (dl_local_filter) { - dl_local_filter->get_input_device_bounds(outsetBounds, - bounds_matrix, dl_rect); - } + sk_rect = sk_local_filter->filterBounds( + outsetBounds, bounds_matrix, + SkImageFilter::MapDirection::kReverse_MapDirection); + dl_local_filter->get_input_device_bounds(outsetBounds, bounds_matrix, + dl_rect); ASSERT_EQ(sk_rect, dl_rect); } } diff --git a/display_list/display_list_ops.h b/display_list/display_list_ops.h index 046258ad3297b..052e7c9343bc9 100644 --- a/display_list/display_list_ops.h +++ b/display_list/display_list_ops.h @@ -11,7 +11,6 @@ #include "flutter/display_list/display_list_sampling_options.h" #include "flutter/display_list/types.h" #include "flutter/fml/macros.h" -#include "include/core/SkMatrix.h" namespace flutter { From 97864c6b67e4280509bec3ad93b84d93d54efdd4 Mon Sep 17 00:00:00 2001 From: JsouLiang Date: Thu, 11 Aug 2022 14:56:25 +0800 Subject: [PATCH 8/8] Add issue tracking --- display_list/display_list_image_filter_unittests.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 7aaa6082347cc..f3f2507cf3b82 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -827,7 +827,8 @@ TEST(DisplayListImageFilter, LocalImageFilterBounds) { // Test for: Know the outset bounds to get the inset bounds // Skia have some bounds calculate error of DilateFilter and // MatrixFilter - // https://fiddle.skia.org/c/a0dc934dd7f900fb958797edd0e86ee3 + // Skia issue: https://bugs.chromium.org/p/skia/issues/detail?id=13444 + // flutter issue: https://github.com/flutter/flutter/issues/108693 if (i == 2 || i == 3) { continue; }