Skip to content

Commit

Permalink
create DlMaskFilter objects to track mask filters in DisplayList
Browse files Browse the repository at this point in the history
  • Loading branch information
flar committed Feb 18, 2022
1 parent 70c1173 commit d60dad2
Show file tree
Hide file tree
Showing 18 changed files with 662 additions and 246 deletions.
4 changes: 4 additions & 0 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -55,12 +55,16 @@ FILE: ../../../flutter/display_list/display_list_canvas_unittests.cc
FILE: ../../../flutter/display_list/display_list_color_filter.cc
FILE: ../../../flutter/display_list/display_list_color_filter.h
FILE: ../../../flutter/display_list/display_list_color_filter_unittests.cc
FILE: ../../../flutter/display_list/display_list_comparable.h
FILE: ../../../flutter/display_list/display_list_complexity.cc
FILE: ../../../flutter/display_list/display_list_complexity.h
FILE: ../../../flutter/display_list/display_list_dispatcher.cc
FILE: ../../../flutter/display_list/display_list_dispatcher.h
FILE: ../../../flutter/display_list/display_list_flags.cc
FILE: ../../../flutter/display_list/display_list_flags.h
FILE: ../../../flutter/display_list/display_list_mask_filter.cc
FILE: ../../../flutter/display_list/display_list_mask_filter.h
FILE: ../../../flutter/display_list/display_list_mask_filter_unittests.cc
FILE: ../../../flutter/display_list/display_list_ops.cc
FILE: ../../../flutter/display_list/display_list_ops.h
FILE: ../../../flutter/display_list/display_list_unittests.cc
Expand Down
3 changes: 3 additions & 0 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,8 @@ source_set("display_list") {
"display_list_dispatcher.h",
"display_list_flags.cc",
"display_list_flags.h",
"display_list_mask_filter.cc",
"display_list_mask_filter.h",
"display_list_ops.cc",
"display_list_ops.h",
"display_list_utils.cc",
Expand All @@ -43,6 +45,7 @@ source_set("unittests") {
sources = [
"display_list_canvas_unittests.cc",
"display_list_color_filter_unittests.cc",
"display_list_mask_filter_unittests.cc",
"display_list_unittests.cc",
]

Expand Down
5 changes: 1 addition & 4 deletions display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -87,10 +87,7 @@ namespace flutter {
\
V(ClearMaskFilter) \
V(SetMaskFilter) \
V(SetMaskBlurFilterNormal) \
V(SetMaskBlurFilterSolid) \
V(SetMaskBlurFilterOuter) \
V(SetMaskBlurFilterInner) \
V(SetSkMaskFilter) \
\
V(Save) \
V(SaveLayer) \
Expand Down
62 changes: 22 additions & 40 deletions display_list/display_list_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -136,15 +136,9 @@ void DisplayListBuilder::onSetImageFilter(sk_sp<SkImageFilter> filter) {
}
void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) {
if (filter == nullptr) {
if (!current_color_filter_) {
return;
}
current_color_filter_ = nullptr;
Push<ClearColorFilterOp>(0, 0);
} else {
if (current_color_filter_ && *current_color_filter_ == *filter) {
return;
}
current_color_filter_ = filter->shared();
switch (filter->type()) {
case DlColorFilter::kBlend: {
Expand Down Expand Up @@ -172,8 +166,7 @@ void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) {
break;
}
case DlColorFilter::kUnknown: {
const sk_sp<SkColorFilter> sk_filter = filter->sk_filter();
Push<SetSkColorFilterOp>(0, 0, sk_filter);
Push<SetSkColorFilterOp>(0, 0, filter->sk_filter());
break;
}
}
Expand All @@ -185,32 +178,24 @@ void DisplayListBuilder::onSetPathEffect(sk_sp<SkPathEffect> effect) {
? Push<SetPathEffectOp>(0, 0, std::move(effect))
: Push<ClearPathEffectOp>(0, 0);
}
void DisplayListBuilder::onSetMaskFilter(sk_sp<SkMaskFilter> filter) {
current_mask_sigma_ = kInvalidSigma;
(current_mask_filter_ = filter) //
? Push<SetMaskFilterOp>(0, 0, std::move(filter))
: Push<ClearMaskFilterOp>(0, 0);
}
void DisplayListBuilder::onSetMaskBlurFilter(SkBlurStyle style,
SkScalar sigma) {
// Valid sigma is checked by setMaskBlurFilter
FML_DCHECK(mask_sigma_valid(sigma));
current_mask_filter_ = nullptr;
current_mask_style_ = style;
current_mask_sigma_ = sigma;
switch (style) {
case kNormal_SkBlurStyle:
Push<SetMaskBlurFilterNormalOp>(0, 0, sigma);
break;
case kSolid_SkBlurStyle:
Push<SetMaskBlurFilterSolidOp>(0, 0, sigma);
break;
case kOuter_SkBlurStyle:
Push<SetMaskBlurFilterOuterOp>(0, 0, sigma);
break;
case kInner_SkBlurStyle:
Push<SetMaskBlurFilterInnerOp>(0, 0, sigma);
break;
void DisplayListBuilder::onSetMaskFilter(const DlMaskFilter* filter) {
if (filter == nullptr) {
current_mask_filter_ = nullptr;
Push<ClearMaskFilterOp>(0, 0);
} else {
current_mask_filter_ = filter->shared();
switch (filter->type()) {
case DlMaskFilter::kBlur: {
const DlBlurMaskFilter* blur_filter = filter->asBlur();
FML_DCHECK(blur_filter);
void* pod = Push<SetMaskFilterOp>(blur_filter->size(), 0);
new (pod) DlBlurMaskFilter(blur_filter);
break;
}
case DlMaskFilter::kUnknown:
Push<SetSkMaskFilterOp>(0, 0, filter->sk_filter());
break;
}
}
}

Expand Down Expand Up @@ -252,11 +237,7 @@ void DisplayListBuilder::setAttributesFromPaint(
// that is composed with the paint's color filter.
setInvertColors(false);
SkColorFilter* color_filter = paint.getColorFilter();
if (color_filter) {
setColorFilter(DlColorFilter::From(color_filter).get());
} else {
setColorFilter(nullptr);
}
setColorFilter(DlColorFilter::From(color_filter).get());
}
if (flags.applies_image_filter()) {
setImageFilter(sk_ref_sp(paint.getImageFilter()));
Expand All @@ -265,7 +246,8 @@ void DisplayListBuilder::setAttributesFromPaint(
setPathEffect(sk_ref_sp(paint.getPathEffect()));
}
if (flags.applies_mask_filter()) {
setMaskFilter(sk_ref_sp(paint.getMaskFilter()));
SkMaskFilter* mask_filter = paint.getMaskFilter();
setMaskFilter(DlMaskFilter::From(mask_filter).get());
}
}

Expand Down
41 changes: 13 additions & 28 deletions display_list/display_list_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
#define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_BUILDER_H_

#include "flutter/display_list/display_list.h"
#include "flutter/display_list/display_list_comparable.h"
#include "flutter/display_list/display_list_dispatcher.h"
#include "flutter/display_list/display_list_flags.h"
#include "flutter/display_list/types.h"
Expand Down Expand Up @@ -94,27 +95,18 @@ class DisplayListBuilder final : public virtual Dispatcher,
}
}
void setColorFilter(const DlColorFilter* filter) override {
// onSetColorFilter will deal with whether the filter is new
onSetColorFilter(filter);
if (NotEquals(current_color_filter_, filter)) {
onSetColorFilter(filter);
}
}
void setPathEffect(sk_sp<SkPathEffect> effect) override {
if (current_path_effect_ != effect) {
onSetPathEffect(std::move(effect));
}
}
void setMaskFilter(sk_sp<SkMaskFilter> filter) override {
if (mask_sigma_valid(current_mask_sigma_) ||
current_mask_filter_ != filter) {
onSetMaskFilter(std::move(filter));
}
}
void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override {
if (!mask_sigma_valid(sigma)) {
// SkMastFilter::MakeBlur(invalid sigma) returns a nullptr, so we
// reset the mask filter here rather than recording the invalid values.
setMaskFilter(nullptr);
} else if (current_mask_style_ != style || current_mask_sigma_ != sigma) {
onSetMaskBlurFilter(style, sigma);
void setMaskFilter(const DlMaskFilter* filter) override {
if (NotEquals(current_mask_filter_, filter)) {
onSetMaskFilter(filter);
}
}

Expand All @@ -127,8 +119,8 @@ class DisplayListBuilder final : public virtual Dispatcher,
SkPaint::Cap getStrokeCap() const { return current_stroke_cap_; }
SkPaint::Join getStrokeJoin() const { return current_stroke_join_; }
sk_sp<SkShader> getShader() const { return current_shader_; }
sk_sp<SkColorFilter> getColorFilter() const {
return current_color_filter_->sk_filter();
std::shared_ptr<const DlColorFilter> getColorFilter() const {
return current_color_filter_;
}
bool isInvertColors() const { return current_invert_colors_; }
std::optional<SkBlendMode> getBlendMode() const {
Expand All @@ -143,14 +135,9 @@ class DisplayListBuilder final : public virtual Dispatcher,
: SkBlender::Mode(current_blend_mode_);
}
sk_sp<SkPathEffect> getPathEffect() const { return current_path_effect_; }
sk_sp<SkMaskFilter> getMaskFilter() const {
return mask_sigma_valid(current_mask_sigma_)
? SkMaskFilter::MakeBlur(current_mask_style_,
current_mask_sigma_)
: current_mask_filter_;
std::shared_ptr<const DlMaskFilter> getMaskFilter() const {
return current_mask_filter_;
}
// No utility getter for the utility setter:
// void setMaskBlurFilter (SkBlurStyle style, SkScalar sigma)
sk_sp<SkImageFilter> getImageFilter() const { return current_image_filter_; }

void save() override;
Expand Down Expand Up @@ -393,7 +380,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
void onSetImageFilter(sk_sp<SkImageFilter> filter);
void onSetColorFilter(const DlColorFilter* filter);
void onSetPathEffect(sk_sp<SkPathEffect> effect);
void onSetMaskFilter(sk_sp<SkMaskFilter> filter);
void onSetMaskFilter(const DlMaskFilter* filter);
void onSetMaskBlurFilter(SkBlurStyle style, SkScalar sigma);

// These values should match the defaults of the Dart Paint object.
Expand All @@ -413,9 +400,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
std::shared_ptr<const DlColorFilter> current_color_filter_;
sk_sp<SkImageFilter> current_image_filter_;
sk_sp<SkPathEffect> current_path_effect_;
sk_sp<SkMaskFilter> current_mask_filter_;
SkBlurStyle current_mask_style_;
SkScalar current_mask_sigma_ = kInvalidSigma;
std::shared_ptr<const DlMaskFilter> current_mask_filter_;
};

} // namespace flutter
Expand Down
24 changes: 3 additions & 21 deletions display_list/display_list_canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -1237,8 +1237,7 @@ class CanvasCompareTester {
}

{
sk_sp<SkMaskFilter> filter =
SkMaskFilter::MakeBlur(kNormal_SkBlurStyle, 5.0);
const DlBlurMaskFilter filter(kNormal_SkBlurStyle, 5.0);
BoundsTolerance blur5Tolerance = tolerance.addBoundsPadding(4, 4);
{
// Stroked primitives need some non-trivial stroke size to be blurred
Expand All @@ -1247,30 +1246,13 @@ class CanvasCompareTester {
"MaskFilter == Blur 5",
[=](SkCanvas*, SkPaint& p) {
p.setStrokeWidth(5.0);
p.setMaskFilter(filter);
p.setMaskFilter(filter.sk_filter());
},
[=](DisplayListBuilder& b) {
b.setStrokeWidth(5.0);
b.setMaskFilter(filter);
b.setMaskFilter(&filter);
}));
}
EXPECT_TRUE(testP.is_draw_text_blob() || filter->unique())
<< "MaskFilter == Blur 5 Cleanup";
{
RenderWith(testP, env, blur5Tolerance,
CaseParameters(
"MaskFilter == Blur(Normal, 5.0)",
[=](SkCanvas*, SkPaint& p) {
p.setStrokeWidth(5.0);
p.setMaskFilter(filter);
},
[=](DisplayListBuilder& b) {
b.setStrokeWidth(5.0);
b.setMaskBlurFilter(kNormal_SkBlurStyle, 5.0);
}));
}
EXPECT_TRUE(testP.is_draw_text_blob() || filter->unique())
<< "MaskFilter == Blur(Normal, 5.0) Cleanup";
}

{
Expand Down
6 changes: 0 additions & 6 deletions display_list/display_list_color_filter.h
Original file line number Diff line number Diff line change
Expand Up @@ -111,12 +111,6 @@ class DlColorFilter {
// pixels non-transparent and therefore expand the bounds.
virtual bool modifies_transparent_black() const = 0;

// Return a shared version of a DlColorFilter pointer, or nullptr if the
// pointer is null.
static std::shared_ptr<DlColorFilter> Shared(const DlColorFilter* filter) {
return filter == nullptr ? nullptr : filter->shared();
}

// Return a shared version of |this| ColorFilter. The |shared_ptr| returned
// will reference a copy of this object so that the lifetime of the shared
// version is not tied to the storage of this particular instance.
Expand Down
5 changes: 3 additions & 2 deletions display_list/display_list_color_filter_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ static const float matrix[20] = {
TEST(DisplayListColorFilter, FromSkiaNullFilter) {
std::shared_ptr<DlColorFilter> filter = DlColorFilter::From(nullptr);
ASSERT_EQ(filter, nullptr);
ASSERT_EQ(filter.get(), nullptr);
}

TEST(DisplayListColorFilter, FromSkiaBlendFilter) {
Expand Down Expand Up @@ -75,6 +76,7 @@ TEST(DisplayListColorFilter, FromSkiaUnrecognizedFilter) {
ASSERT_EQ(filter->type(), DlColorFilter::kUnknown);
ASSERT_EQ(filter->asBlend(), nullptr);
ASSERT_EQ(filter->asMatrix(), nullptr);
ASSERT_EQ(filter->sk_filter(), sk_filter);
}

TEST(DisplayListColorFilter, BlendConstructor) {
Expand All @@ -88,8 +90,7 @@ TEST(DisplayListColorFilter, BlendShared) {
}

TEST(DisplayListColorFilter, BlendAsBlend) {
DlBlendColorFilter filter =
DlBlendColorFilter(SK_ColorRED, SkBlendMode::kDstATop);
DlBlendColorFilter filter(SK_ColorRED, SkBlendMode::kDstATop);
ASSERT_NE(filter.asBlend(), nullptr);
ASSERT_EQ(filter.asBlend(), &filter);
}
Expand Down
Loading

0 comments on commit d60dad2

Please sign in to comment.