Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Create DlLocalMatrixImageFilter #34878

Merged
auto-submit merged 8 commits into from
Aug 11, 2022
Merged

Create DlLocalMatrixImageFilter #34878

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
d48807e
Create DlLocalMatrixImageFilter
JsouLiang Jul 25, 2022
e19c17b
implement localMatrixImageFilter
JsouLiang Jul 26, 2022
8c91fb3
update test cases
JsouLiang Jul 27, 2022
de99e7d
test cases
JsouLiang Jul 28, 2022
c323d42
fix some comments
JsouLiang Jul 29, 2022
9f48c9c
update some comments
JsouLiang Aug 8, 2022
abd475b
fix some comments
JsouLiang Aug 9, 2022
97864c6
Add issue tracking
JsouLiang Aug 11, 2022
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions display_list/display_list_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -225,6 +225,7 @@ void DisplayListBuilder::onSetImageFilter(const DlImageFilter* filter) {
break;
}
case DlImageFilterType::kComposeFilter:
case DlImageFilterType::kLocalMatrixFilter:
case DlImageFilterType::kColorFilter: {
Push<SetSharedImageFilterOp>(0, 0, filter);
break;
Expand Down
25 changes: 25 additions & 0 deletions display_list/display_list_image_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,31 @@ std::shared_ptr<DlImageFilter> DlImageFilter::From(
return std::make_shared<DlUnknownImageFilter>(sk_ref_sp(sk_filter));
}

std::shared_ptr<DlImageFilter> DlImageFilter::makeWithLocalMatrix(
const SkMatrix& matrix) {
if (matrix.isIdentity()) {
return shared();
}
// Matrix
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<DlLocalMatrixImageFilter>(matrix, shared());
}

SkRect* DlComposeImageFilter::map_local_bounds(const SkRect& input_bounds,
SkRect& output_bounds) const {
SkRect cur_bounds = input_bounds;
Expand Down
111 changes: 111 additions & 0 deletions display_list/display_list_image_filter.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,19 +34,27 @@ enum class DlImageFilterType {
kMatrix,
kComposeFilter,
kColorFilter,
kLocalMatrixFilter,
kUnknown
};

class DlBlurImageFilter;
class DlDilateImageFilter;
class DlErodeImageFilter;
class DlMatrixImageFilter;
class DlLocalMatrixImageFilter;
class DlComposeImageFilter;
class DlColorFilterImageFilter;

class DlImageFilter
: public DlAttribute<DlImageFilter, SkImageFilter, DlImageFilterType> {
public:
enum class MatrixCapability {
kTranslate,
kScaleTranslate,
kComplex,
};

// Return a shared_ptr holding a DlImageFilter representing the indicated
// Skia SkImageFilter pointer.
//
Expand Down Expand Up @@ -82,6 +90,13 @@ class DlImageFilter
// type of ImageFilter, otherwise return nullptr.
virtual const DlMatrixImageFilter* asMatrix() const { return nullptr; }

virtual const DlLocalMatrixImageFilter* asLocalMatrix() const {
return nullptr;
}

virtual std::shared_ptr<DlImageFilter> 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; }
Expand Down Expand Up @@ -137,6 +152,10 @@ class DlImageFilter
const SkMatrix& ctm,
SkIRect& input_bounds) const = 0;

virtual MatrixCapability matrix_capability() const {
return MatrixCapability::kScaleTranslate;
Copy link
Contributor

@flar flar Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Where did you get information about the compatibility here?

Copy link
Contributor Author

@JsouLiang JsouLiang Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In Skia, the ImageFilter has a method onGetCTMCapability that will return the MatrixCapability.

}

protected:
static SkVector map_vectors_affine(const SkMatrix& ctm,
SkScalar x,
Expand Down Expand Up @@ -534,6 +553,10 @@ class DlComposeImageFilter final : public DlImageFilter {
inner_->skia_object());
}

MatrixCapability matrix_capability() const override {
return std::min(outer_->matrix_capability(), inner_->matrix_capability());
}

protected:
bool equals_(const DlImageFilter& other) const override {
FML_DCHECK(other.type() == DlImageFilterType::kComposeFilter);
Expand Down Expand Up @@ -606,6 +629,15 @@ class DlColorFilterImageFilter final : public DlImageFilter {
return SkImageFilters::ColorFilter(color_filter_->skia_object(), nullptr);
}

MatrixCapability matrix_capability() const override {
return MatrixCapability::kComplex;
}

std::shared_ptr<DlImageFilter> makeWithLocalMatrix(
const SkMatrix& matrix) override {
return shared();
}

protected:
bool equals_(const DlImageFilter& other) const override {
FML_DCHECK(other.type() == DlImageFilterType::kColorFilter);
Expand All @@ -617,6 +649,85 @@ class DlColorFilterImageFilter final : public DlImageFilter {
std::shared_ptr<DlColorFilter> color_filter_;
};

class DlLocalMatrixImageFilter final : public DlImageFilter {
public:
explicit DlLocalMatrixImageFilter(const SkMatrix& matrix,
std::shared_ptr<DlImageFilter> filter)
: matrix_(matrix), image_filter_(filter) {}
explicit DlLocalMatrixImageFilter(const DlLocalMatrixImageFilter* filter)
: DlLocalMatrixImageFilter(filter->matrix_, filter->image_filter_) {}
DlLocalMatrixImageFilter(const DlLocalMatrixImageFilter& filter)
: DlLocalMatrixImageFilter(&filter) {}
Copy link
Contributor

@flar flar Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is no protection against a null filter in the constructors so everywhere image_filter_ is used here has to protect against null.

std::shared_ptr<DlImageFilter> shared() const override {
return std::make_shared<DlLocalMatrixImageFilter>(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 {
if (!image_filter_) {
return false;
}
return image_filter_->modifies_transparent_black();
}

SkRect* map_local_bounds(const SkRect& input_bounds,
SkRect& output_bounds) const override {
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);
Copy link
Contributor

@flar flar Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The input bounds are relative to ctm, but not to matrix_. You need to return bounds relative to only ctm.

Copy link
Contributor

@flar flar Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actually, I'm wondering what Skia does with this. Have you run tests to see how the bounds methods behave with a local matrix?

Copy link
Contributor Author

@JsouLiang JsouLiang Jul 26, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I added a test case:

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<DlBlurImageFilter>(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);

the out_bouns is equal the rect when we use SkMatrix::Concat(ctm, matrix_)

Copy link
Contributor

@flar flar Jul 27, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is a very basic test of some of the cases that have the fewest quirks in them.

Try something like:

For each filter type:
  filter = instantiate that type;
  For each transform type local_matrix:
    transformed_filter = filter->makeWithLocal(local_matrix)
    For each transform type map_matrix:
      compare output of transformed_filter.map(..., map_matrix, ...);

Transform types should be:

  • translate
  • scale + translate
  • rotate + translate
  • perspective
  • any others?

}

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(
Copy link
Contributor

@flar flar Jul 25, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same as map_device_bounds - which coordinate system should the returned bounds be relative to?

output_bounds, SkMatrix::Concat(ctm, matrix_), input_bounds);
}

sk_sp<SkImageFilter> skia_object() const override {
if (!image_filter_) {
return nullptr;
}
return image_filter_->skia_object()->makeWithLocalMatrix(matrix_);
}

protected:
bool equals_(const DlImageFilter& other) const override {
FML_DCHECK(other.type() == DlImageFilterType::kMatrix);
auto that = static_cast<const DlLocalMatrixImageFilter*>(&other);
return (matrix_ == that->matrix_ &&
Equals(image_filter_, that->image_filter_));
}

private:
SkMatrix matrix_;
std::shared_ptr<DlImageFilter> 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
Expand Down
87 changes: 87 additions & 0 deletions display_list/display_list_image_filter_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -3,10 +3,14 @@
// found in the LICENSE file.

#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"

Expand Down Expand Up @@ -759,6 +763,89 @@ TEST(DisplayListImageFilter, UnknownContents) {
ASSERT_EQ(filter.skia_object().get(), sk_filter.get());
}

TEST(DisplayListImageFilter, LocalImageFilterBounds) {
auto filter_matrix = SkMatrix::MakeAll(2.0, 0.0, 10, //
0.5, 3.0, 15, //
0.0, 0.0, 1);
std::vector<sk_sp<SkImageFilter>> sk_filters{
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);
std::vector<std::shared_ptr<DlImageFilter>> dl_filters{
std::make_shared<DlBlurImageFilter>(5.0, 6.0, DlTileMode::kRepeat),
std::make_shared<DlColorFilterImageFilter>(dl_color_filter.shared()),
std::make_shared<DlDilateImageFilter>(5, 10),
std::make_shared<DlMatrixImageFilter>(filter_matrix,
DlImageSampling::kLinear),
std::make_shared<DlComposeImageFilter>(
std::make_shared<DlBlurImageFilter>(5.0, 6.0, DlTileMode::kRepeat),
std::make_shared<DlColorFilterImageFilter>(
dl_color_filter.shared()))};

auto persp = SkMatrix::I();
persp.setPerspY(0.001);
std::vector<SkMatrix> 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<SkMatrix> 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++) {
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);
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;
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);
}
{
// Test for: Know the outset bounds to get the inset bounds
// Skia have some bounds calculate error of DilateFilter and
// MatrixFilter
// 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;
}
auto outsetBounds = SkIRect::MakeLTRB(20, 20, 80, 80);
SkIRect sk_rect, 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);
}
}
}
}
}

TEST(DisplayListImageFilter, UnknownEquals) {
sk_sp<SkImageFilter> sk_filter =
SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr);
Expand Down
1 change: 1 addition & 0 deletions impeller/display_list/display_list_dispatcher.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ static std::optional<Paint::ImageFilterProc> 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:
Expand Down