From 3b5ec940afc8e52effa176bc4544da684eba38c0 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 18 Feb 2022 09:19:09 -0800 Subject: [PATCH] DisplayList DlMaskFilter objects (flutter/engine#31535) --- DEPS | 2 +- .../ci/licenses_golden/licenses_flutter | 4 + engine/src/flutter/display_list/BUILD.gn | 3 + .../src/flutter/display_list/display_list.h | 5 +- .../display_list/display_list_builder.cc | 62 ++--- .../display_list/display_list_builder.h | 41 +--- .../display_list_canvas_unittests.cc | 24 +- .../display_list/display_list_color_filter.h | 6 - .../display_list_color_filter_unittests.cc | 5 +- .../display_list/display_list_comparable.h | 84 +++++++ .../display_list_complexity_helper.h | 3 +- .../display_list/display_list_dispatcher.h | 8 +- .../display_list/display_list_mask_filter.cc | 18 ++ .../display_list/display_list_mask_filter.h | 212 +++++++++++++++++ .../display_list_mask_filter_unittests.cc | 223 ++++++++++++++++++ .../flutter/display_list/display_list_ops.h | 98 ++++---- .../display_list/display_list_unittests.cc | 63 +---- .../display_list/display_list_utils.cc | 37 ++- .../flutter/display_list/display_list_utils.h | 12 +- engine/src/flutter/lib/ui/painting/paint.cc | 3 +- 20 files changed, 664 insertions(+), 249 deletions(-) create mode 100644 engine/src/flutter/display_list/display_list_comparable.h create mode 100644 engine/src/flutter/display_list/display_list_mask_filter.cc create mode 100644 engine/src/flutter/display_list/display_list_mask_filter.h create mode 100644 engine/src/flutter/display_list/display_list_mask_filter_unittests.cc diff --git a/DEPS b/DEPS index 7646d0860ce..2c8a11c6e06 100644 --- a/DEPS +++ b/DEPS @@ -110,7 +110,7 @@ deps = { 'src': 'https://github.com/flutter/buildroot.git' + '@' + '79643299bd052c53631b8b200bb582e8badb2708', 'src/flutter/impeller': - Var('github_git') + '/flutter/impeller' + '@' + '78bc2a026c554c444b2e67b36fd44cd548341a51', + Var('github_git') + '/flutter/impeller' + '@' + '2793516e27224b16e572138cb7cb18cc6b22adc7', # Fuchsia compatibility # diff --git a/engine/src/flutter/ci/licenses_golden/licenses_flutter b/engine/src/flutter/ci/licenses_golden/licenses_flutter index cdc62d2a370..ea21a5abf89 100755 --- a/engine/src/flutter/ci/licenses_golden/licenses_flutter +++ b/engine/src/flutter/ci/licenses_golden/licenses_flutter @@ -55,6 +55,7 @@ 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_complexity_gl.cc @@ -67,6 +68,9 @@ 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_test_utils.cc diff --git a/engine/src/flutter/display_list/BUILD.gn b/engine/src/flutter/display_list/BUILD.gn index 5f166c9ffd9..0b4c91f88fe 100644 --- a/engine/src/flutter/display_list/BUILD.gn +++ b/engine/src/flutter/display_list/BUILD.gn @@ -26,6 +26,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", @@ -48,6 +50,7 @@ source_set("unittests") { "display_list_canvas_unittests.cc", "display_list_color_filter_unittests.cc", "display_list_complexity_unittests.cc", + "display_list_mask_filter_unittests.cc", "display_list_test_utils.cc", "display_list_test_utils.h", "display_list_unittests.cc", diff --git a/engine/src/flutter/display_list/display_list.h b/engine/src/flutter/display_list/display_list.h index 3871c96fd2e..5052e614629 100644 --- a/engine/src/flutter/display_list/display_list.h +++ b/engine/src/flutter/display_list/display_list.h @@ -87,10 +87,7 @@ namespace flutter { \ V(ClearMaskFilter) \ V(SetMaskFilter) \ - V(SetMaskBlurFilterNormal) \ - V(SetMaskBlurFilterSolid) \ - V(SetMaskBlurFilterOuter) \ - V(SetMaskBlurFilterInner) \ + V(SetSkMaskFilter) \ \ V(Save) \ V(SaveLayer) \ diff --git a/engine/src/flutter/display_list/display_list_builder.cc b/engine/src/flutter/display_list/display_list_builder.cc index 086da0e3a12..2062444f34b 100644 --- a/engine/src/flutter/display_list/display_list_builder.cc +++ b/engine/src/flutter/display_list/display_list_builder.cc @@ -136,15 +136,9 @@ void DisplayListBuilder::onSetImageFilter(sk_sp filter) { } void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) { if (filter == nullptr) { - if (!current_color_filter_) { - return; - } current_color_filter_ = nullptr; Push(0, 0); } else { - if (current_color_filter_ && *current_color_filter_ == *filter) { - return; - } current_color_filter_ = filter->shared(); switch (filter->type()) { case DlColorFilter::kBlend: { @@ -172,8 +166,7 @@ void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) { break; } case DlColorFilter::kUnknown: { - const sk_sp sk_filter = filter->sk_filter(); - Push(0, 0, sk_filter); + Push(0, 0, filter->sk_filter()); break; } } @@ -185,32 +178,24 @@ void DisplayListBuilder::onSetPathEffect(sk_sp effect) { ? Push(0, 0, std::move(effect)) : Push(0, 0); } -void DisplayListBuilder::onSetMaskFilter(sk_sp filter) { - current_mask_sigma_ = kInvalidSigma; - (current_mask_filter_ = filter) // - ? Push(0, 0, std::move(filter)) - : Push(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(0, 0, sigma); - break; - case kSolid_SkBlurStyle: - Push(0, 0, sigma); - break; - case kOuter_SkBlurStyle: - Push(0, 0, sigma); - break; - case kInner_SkBlurStyle: - Push(0, 0, sigma); - break; +void DisplayListBuilder::onSetMaskFilter(const DlMaskFilter* filter) { + if (filter == nullptr) { + current_mask_filter_ = nullptr; + Push(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(blur_filter->size(), 0); + new (pod) DlBlurMaskFilter(blur_filter); + break; + } + case DlMaskFilter::kUnknown: + Push(0, 0, filter->sk_filter()); + break; + } } } @@ -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())); @@ -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()); } } diff --git a/engine/src/flutter/display_list/display_list_builder.h b/engine/src/flutter/display_list/display_list_builder.h index 8cd08ad49ba..a3c3f8ba559 100644 --- a/engine/src/flutter/display_list/display_list_builder.h +++ b/engine/src/flutter/display_list/display_list_builder.h @@ -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" @@ -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 effect) override { if (current_path_effect_ != effect) { onSetPathEffect(std::move(effect)); } } - void setMaskFilter(sk_sp 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); } } @@ -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 getShader() const { return current_shader_; } - sk_sp getColorFilter() const { - return current_color_filter_->sk_filter(); + std::shared_ptr getColorFilter() const { + return current_color_filter_; } bool isInvertColors() const { return current_invert_colors_; } std::optional getBlendMode() const { @@ -143,14 +135,9 @@ class DisplayListBuilder final : public virtual Dispatcher, : SkBlender::Mode(current_blend_mode_); } sk_sp getPathEffect() const { return current_path_effect_; } - sk_sp getMaskFilter() const { - return mask_sigma_valid(current_mask_sigma_) - ? SkMaskFilter::MakeBlur(current_mask_style_, - current_mask_sigma_) - : current_mask_filter_; + std::shared_ptr getMaskFilter() const { + return current_mask_filter_; } - // No utility getter for the utility setter: - // void setMaskBlurFilter (SkBlurStyle style, SkScalar sigma) sk_sp getImageFilter() const { return current_image_filter_; } void save() override; @@ -393,7 +380,7 @@ class DisplayListBuilder final : public virtual Dispatcher, void onSetImageFilter(sk_sp filter); void onSetColorFilter(const DlColorFilter* filter); void onSetPathEffect(sk_sp effect); - void onSetMaskFilter(sk_sp filter); + void onSetMaskFilter(const DlMaskFilter* filter); void onSetMaskBlurFilter(SkBlurStyle style, SkScalar sigma); // These values should match the defaults of the Dart Paint object. @@ -413,9 +400,7 @@ class DisplayListBuilder final : public virtual Dispatcher, std::shared_ptr current_color_filter_; sk_sp current_image_filter_; sk_sp current_path_effect_; - sk_sp current_mask_filter_; - SkBlurStyle current_mask_style_; - SkScalar current_mask_sigma_ = kInvalidSigma; + std::shared_ptr current_mask_filter_; }; } // namespace flutter diff --git a/engine/src/flutter/display_list/display_list_canvas_unittests.cc b/engine/src/flutter/display_list/display_list_canvas_unittests.cc index e0edc4b98c9..031e9b150aa 100644 --- a/engine/src/flutter/display_list/display_list_canvas_unittests.cc +++ b/engine/src/flutter/display_list/display_list_canvas_unittests.cc @@ -1237,8 +1237,7 @@ class CanvasCompareTester { } { - sk_sp 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 @@ -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"; } { diff --git a/engine/src/flutter/display_list/display_list_color_filter.h b/engine/src/flutter/display_list/display_list_color_filter.h index 0918bad6d84..5aea229bfa2 100644 --- a/engine/src/flutter/display_list/display_list_color_filter.h +++ b/engine/src/flutter/display_list/display_list_color_filter.h @@ -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 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. diff --git a/engine/src/flutter/display_list/display_list_color_filter_unittests.cc b/engine/src/flutter/display_list/display_list_color_filter_unittests.cc index b9de53264a5..14f6ae5b027 100644 --- a/engine/src/flutter/display_list/display_list_color_filter_unittests.cc +++ b/engine/src/flutter/display_list/display_list_color_filter_unittests.cc @@ -19,6 +19,7 @@ static const float matrix[20] = { TEST(DisplayListColorFilter, FromSkiaNullFilter) { std::shared_ptr filter = DlColorFilter::From(nullptr); ASSERT_EQ(filter, nullptr); + ASSERT_EQ(filter.get(), nullptr); } TEST(DisplayListColorFilter, FromSkiaBlendFilter) { @@ -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) { @@ -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); } diff --git a/engine/src/flutter/display_list/display_list_comparable.h b/engine/src/flutter/display_list/display_list_comparable.h new file mode 100644 index 00000000000..d88b2e5d21e --- /dev/null +++ b/engine/src/flutter/display_list/display_list_comparable.h @@ -0,0 +1,84 @@ +// 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_DISPLAY_LIST_DISPLAY_LIST_COMPARABLE_H_ +#define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_COMPARABLE_H_ + +#include "flutter/display_list/types.h" + +namespace flutter { + +// These templates implement deep pointer comparisons that compare not +// just the pointers to the objects, but also their contents (provided +// that the class implements the == operator override). +// Any combination of shared_ptr or T* are supported and null pointers +// are not equal to anything but another null pointer. + +template +bool Equals(const T* a, const T* b) { + if (a == b) { + return true; + } + if (!a || !b) { + return false; + } + return *a == *b; +} + +template +bool Equals(std::shared_ptr a, const T* b) { + if (!a) { + return !b; + } + if (!b) { + return false; + } + if (a.get() == b) { + return true; + } + return *a.get() == *b; +} + +template +bool Equals(const T* a, std::shared_ptr b) { + return Equals(b, a); +} + +template +bool Equals(std::shared_ptr a, std::shared_ptr b) { + if (!a) { + return !b; + } + if (!b) { + return false; + } + if (a.get() == b.get()) { + return true; + } + return *a.get() == *b.get(); +} + +template +bool NotEquals(const T* a, const T* b) { + return !Equals(a, b); +} + +template +bool NotEquals(std::shared_ptr a, const T* b) { + return !Equals(a, b); +} + +template +bool NotEquals(const T* a, std::shared_ptr b) { + return !Equals(b, a); +} + +template +bool NotEquals(std::shared_ptr a, std::shared_ptr b) { + return !Equals(a, b); +} + +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_COMPARABLE_H_ diff --git a/engine/src/flutter/display_list/display_list_complexity_helper.h b/engine/src/flutter/display_list/display_list_complexity_helper.h index f70f1af61a2..0196a391d00 100644 --- a/engine/src/flutter/display_list/display_list_complexity_helper.h +++ b/engine/src/flutter/display_list/display_list_complexity_helper.h @@ -115,8 +115,7 @@ class ComplexityCalculatorHelper void setImageFilter(sk_sp filter) override {} void setColorFilter(const DlColorFilter* filter) override {} void setPathEffect(sk_sp effect) override {} - void setMaskFilter(sk_sp filter) override {} - void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override {} + void setMaskFilter(const DlMaskFilter* filter) override {} void save() override {} // We accumulate the cost of restoring a saveLayer() in saveLayer() diff --git a/engine/src/flutter/display_list/display_list_dispatcher.h b/engine/src/flutter/display_list/display_list_dispatcher.h index 7d869408baf..c0f99ff090c 100644 --- a/engine/src/flutter/display_list/display_list_dispatcher.h +++ b/engine/src/flutter/display_list/display_list_dispatcher.h @@ -7,6 +7,7 @@ #include "flutter/display_list/display_list.h" #include "flutter/display_list/display_list_color_filter.h" +#include "flutter/display_list/display_list_mask_filter.h" namespace flutter { @@ -46,12 +47,7 @@ class Dispatcher { virtual void setBlendMode(SkBlendMode mode) = 0; virtual void setBlender(sk_sp blender) = 0; virtual void setPathEffect(sk_sp effect) = 0; - virtual void setMaskFilter(sk_sp filter) = 0; - // setMaskBlurFilter is a quick way to set the parameters for a - // mask blur filter without constructing an SkMaskFilter object. - // It is equivalent to setMaskFilter(SkMaskFilter::MakeBlur(style, sigma)). - // To reset the filter use setMaskFilter(nullptr). - virtual void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) = 0; + virtual void setMaskFilter(const DlMaskFilter* filter) = 0; virtual void setImageFilter(sk_sp filter) = 0; // All of the following methods are nearly 1:1 with their counterparts diff --git a/engine/src/flutter/display_list/display_list_mask_filter.cc b/engine/src/flutter/display_list/display_list_mask_filter.cc new file mode 100644 index 00000000000..fe762066e2f --- /dev/null +++ b/engine/src/flutter/display_list/display_list_mask_filter.cc @@ -0,0 +1,18 @@ +// 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/display_list/display_list_mask_filter.h" + +namespace flutter { + +std::shared_ptr DlMaskFilter::From(SkMaskFilter* sk_filter) { + if (sk_filter == nullptr) { + return nullptr; + } + // There are no inspection methods for SkMaskFilter so we cannot break + // the Skia filter down into a specific subclass (i.e. Blur). + return std::make_shared(sk_ref_sp(sk_filter)); +} + +} // namespace flutter diff --git a/engine/src/flutter/display_list/display_list_mask_filter.h b/engine/src/flutter/display_list/display_list_mask_filter.h new file mode 100644 index 00000000000..6443720df00 --- /dev/null +++ b/engine/src/flutter/display_list/display_list_mask_filter.h @@ -0,0 +1,212 @@ +// 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_DISPLAY_LIST_DISPLAY_LIST_MASK_FILTER_H_ +#define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_MASK_FILTER_H_ + +#include "flutter/display_list/types.h" +#include "flutter/fml/logging.h" + +namespace flutter { + +class DlBlurMaskFilter; + +// The DisplayList MaskFilter class. This class was designed to be: +// +// - Typed: +// Even though most references and pointers are passed around as the +// base class, a DlMaskFilter::Type can be queried using the |type| +// method to determine which type of MaskFilter is being used. +// +// - Inspectable: +// Any parameters required to full specify the filtering operations are +// provided on the specific base classes. +// +// - Safely Downcast: +// For the subclasses that have specific data to query, methods |asBlur| +// are provided to safely downcast the reference for inspection. +// +// - Skiafiable: +// The classes override an |sk_filter| method to easily obtain a Skia +// version of the filter on demand. +// +// - Immutable: +// Neither the base class or any of the subclasses specify any mutation +// methods. Instances are often passed around as const as a reminder, +// but the classes have no mutation methods anyway. +// +// - Flat and Embeddable: +// Bulk freed + bulk compared + zero memory fragmentation. +// +// All of these classes are designed to be stored in the DisplayList +// buffer allocated in-line with the rest of the data to avoid dangling +// pointers that require explicit freeing when the DisplayList goes +// away, or that fragment the memory needed to read the operations in +// the DisplayList. Furthermore, the data in the classes can be bulk +// compared using a |memcmp| when performing a |DisplayList::Equals|. +// +// - Passed by Pointer: +// The data shared via the |Dispatcher::setMaskFilter| call is stored +// in the buffer itself and so its lifetime is controlled by the +// DisplayList. That memory cannot be shared as by a |shared_ptr| +// because the memory may be freed outside the control of the shared +// pointer. Creating a shared version of the object would require a +// new instantiation which we'd like to avoid on every dispatch call, +// so a raw (const) pointer is shared instead with all of the +// responsibilities of non-ownership in the called method. +// +// But, for methods that need to keep a copy of the data... +// +// - Shared_Ptr-able: +// The classes support a method to return a |std::shared_ptr| version of +// themselves, safely instantiating a new copy of the object into a +// shared_ptr using |std::make_shared|. For those dispatcher objects +// that may want to hold on to the contents of the object (typically +// in a |current_mask_filter_| field), they can obtain a shared_ptr +// copy safely and easily using the |shared| method. + +class DlMaskFilter { + public: + // An enumerated type for the recognized MaskFilter operations. + // If a custom MaskFilter outside of the recognized types is needed + // then a |kUnknown| type that simply defers to an SkMaskFilter is + // provided as a fallback. + enum Type { kBlur, kUnknown }; + + // Return a shared_ptr holding a DlMaskFilter representing the indicated + // Skia SkMaskFilter pointer. + // + // Since there is no public SkBlurMaskFilter and since the SkMaskFilter + // class provides no |asABlur| style type inference methods, we cannot + // infer any specific data from the SkMaskFilter. As a result, the return + // value in this case will always be nullptr or DlUnknownMaskFilter. + static std::shared_ptr From(SkMaskFilter* sk_filter); + + // Return a shared_ptr holding a DlMaskFilter representing the indicated + // Skia SkMaskFilter pointer. + // + // Since there is no public SkBlurMaskFilter and since the SkMaskFilter + // class provides no |asABlur| style type inference methods, we cannot + // infer any specific data from the SkMaskFilter. As a result, the return + // value in this case will always be nullptr or DlUnknownMaskFilter. + static std::shared_ptr From(sk_sp sk_filter) { + return From(sk_filter.get()); + } + + // Return the recognized type of the MaskFilter operation. + virtual Type type() const = 0; + + // Return the size of the instantiated data (typically used to allocate) + // storage in the DisplayList buffer. + virtual size_t size() const = 0; + + // Return a shared version of |this| MaskFilter. 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. + virtual std::shared_ptr shared() const = 0; + + // Return an equivalent |SkMaskFilter| version of this object. + virtual sk_sp sk_filter() const = 0; + + // Return a DlBlurMaskFilter pointer to this object iff it is a Blur + // type of MaskFilter, otherwise return nullptr. + virtual const DlBlurMaskFilter* asBlur() const { return nullptr; } + + // Perform a content aware |==| comparison of the MaskFilter. + bool operator==(DlMaskFilter const& other) const { + return type() == other.type() && equals_(other); + } + // Perform a content aware |!=| comparison of the ColorFilter. + bool operator!=(DlMaskFilter const& other) const { return !(*this == other); } + + virtual ~DlMaskFilter() = default; + + protected: + // Virtual comparison method to support |==| and |!=|. + virtual bool equals_(DlMaskFilter const& other) const = 0; +}; + +// The Blur type of MaskFilter which specifies modifying the +// colors as if the color specified in the Blur filter is the +// source color and the color drawn by the rendering operation +// is the destination color. The mode parameter of the Blur +// filter is then used to combine those colors. +class DlBlurMaskFilter final : public DlMaskFilter { + public: + DlBlurMaskFilter(SkBlurStyle style, SkScalar sigma) + : style_(style), sigma_(sigma) {} + DlBlurMaskFilter(const DlBlurMaskFilter& filter) + : DlBlurMaskFilter(filter.style_, filter.sigma_) {} + DlBlurMaskFilter(const DlBlurMaskFilter* filter) + : DlBlurMaskFilter(filter->style_, filter->sigma_) {} + + Type type() const override { return kBlur; } + size_t size() const override { return sizeof(*this); } + + std::shared_ptr shared() const override { + return std::make_shared(this); + } + + sk_sp sk_filter() const override { + return SkMaskFilter::MakeBlur(style_, sigma_); + } + + const DlBlurMaskFilter* asBlur() const override { return this; } + + SkBlurStyle style() const { return style_; } + SkScalar sigma() const { return sigma_; } + + protected: + bool equals_(DlMaskFilter const& other) const override { + FML_DCHECK(other.type() == kBlur); + auto that = static_cast(other); + return style_ == that.style_ && sigma_ == that.sigma_; + } + + private: + SkBlurStyle style_; + SkScalar sigma_; +}; + +// A wrapper class for a Skia MaskFilter of unknown type. The above 4 types +// are the only types that can be constructed by Flutter using the +// ui.MaskFilter class so this class should be rarely used. The main use +// would come from the |DisplayListCanvasRecorder| recording Skia rendering +// calls that originated outside of the Flutter dart code. This would +// primarily happen in the Paragraph code that renders the text using the +// SkCanvas interface which we capture into DisplayList data structures. +class DlUnknownMaskFilter final : public DlMaskFilter { + public: + DlUnknownMaskFilter(sk_sp sk_filter) + : sk_filter_(std::move(sk_filter)) {} + DlUnknownMaskFilter(const DlUnknownMaskFilter& filter) + : DlUnknownMaskFilter(filter.sk_filter_) {} + DlUnknownMaskFilter(const DlUnknownMaskFilter* filter) + : DlUnknownMaskFilter(filter->sk_filter_) {} + + Type type() const override { return kUnknown; } + size_t size() const override { return sizeof(*this); } + + std::shared_ptr shared() const override { + return std::make_shared(this); + } + + sk_sp sk_filter() const override { return sk_filter_; } + + virtual ~DlUnknownMaskFilter() = default; + + protected: + bool equals_(const DlMaskFilter& other) const override { + FML_DCHECK(other.type() == kUnknown); + auto that = static_cast(other); + return sk_filter_ == that.sk_filter_; + } + + private: + sk_sp sk_filter_; +}; + +} // namespace flutter + +#endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_MASK_FILTER_H_ diff --git a/engine/src/flutter/display_list/display_list_mask_filter_unittests.cc b/engine/src/flutter/display_list/display_list_mask_filter_unittests.cc new file mode 100644 index 00000000000..39cb8bd9fa3 --- /dev/null +++ b/engine/src/flutter/display_list/display_list_mask_filter_unittests.cc @@ -0,0 +1,223 @@ +// 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/display_list/display_list_comparable.h" +#include "flutter/display_list/display_list_mask_filter.h" +#include "flutter/display_list/types.h" +#include "gtest/gtest.h" + +namespace flutter { +namespace testing { + +TEST(DisplayListMaskFilter, FromSkiaNullFilter) { + std::shared_ptr filter = DlMaskFilter::From(nullptr); + ASSERT_EQ(filter, nullptr); + ASSERT_EQ(filter.get(), nullptr); +} + +TEST(DisplayListMaskFilter, FromSkiaBlurFilter) { + sk_sp sk_filter = + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + std::shared_ptr filter = DlMaskFilter::From(sk_filter); + ASSERT_EQ(filter->type(), DlMaskFilter::kUnknown); + ASSERT_EQ(filter->asBlur(), nullptr); + ASSERT_EQ(filter->sk_filter(), sk_filter); +} + +TEST(DisplayListMaskFilter, BlurConstructor) { + DlBlurMaskFilter filter(SkBlurStyle::kNormal_SkBlurStyle, 5.0); +} + +TEST(DisplayListMaskFilter, BlurShared) { + DlBlurMaskFilter filter(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + ASSERT_NE(filter.shared().get(), &filter); + ASSERT_EQ(*filter.shared(), filter); +} + +TEST(DisplayListMaskFilter, BlurAsBlur) { + DlBlurMaskFilter filter(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + ASSERT_NE(filter.asBlur(), nullptr); + ASSERT_EQ(filter.asBlur(), &filter); +} + +TEST(DisplayListMaskFilter, BlurContents) { + DlBlurMaskFilter filter(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + ASSERT_EQ(filter.style(), SkBlurStyle::kNormal_SkBlurStyle); + ASSERT_EQ(filter.sigma(), 5.0); +} + +TEST(DisplayListMaskFilter, BlurEquals) { + DlBlurMaskFilter filter1(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + DlBlurMaskFilter filter2(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + ASSERT_TRUE(filter1 == filter2); + ASSERT_TRUE(filter2 == filter1); + ASSERT_FALSE(filter1 != filter2); + ASSERT_FALSE(filter2 != filter1); + ASSERT_EQ(filter1, filter2); +} + +TEST(DisplayListMaskFilter, BlurNotEquals) { + DlBlurMaskFilter filter1(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + DlBlurMaskFilter filter2(SkBlurStyle::kInner_SkBlurStyle, 5.0); + DlBlurMaskFilter filter3(SkBlurStyle::kNormal_SkBlurStyle, 6.0); + ASSERT_FALSE(filter1 == filter2); + ASSERT_FALSE(filter2 == filter1); + ASSERT_TRUE(filter1 != filter2); + ASSERT_TRUE(filter2 != filter1); + ASSERT_NE(filter1, filter2); + ASSERT_NE(filter2, filter3); + ASSERT_NE(filter3, filter1); +} + +TEST(DisplayListMaskFilter, UnknownConstructor) { + DlUnknownMaskFilter filter( + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0)); +} + +TEST(DisplayListMaskFilter, UnknownShared) { + DlUnknownMaskFilter filter( + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0)); + ASSERT_NE(filter.shared().get(), &filter); + ASSERT_EQ(*filter.shared(), filter); +} + +TEST(DisplayListMaskFilter, UnknownContents) { + sk_sp sk_filter = + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + DlUnknownMaskFilter filter(sk_filter); + ASSERT_EQ(filter.sk_filter(), sk_filter); + ASSERT_EQ(filter.sk_filter().get(), sk_filter.get()); +} + +TEST(DisplayListMaskFilter, UnknownEquals) { + sk_sp sk_filter = + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + DlUnknownMaskFilter filter1(sk_filter); + DlUnknownMaskFilter filter2(sk_filter); + ASSERT_TRUE(filter1 == filter2); + ASSERT_TRUE(filter2 == filter1); + ASSERT_FALSE(filter1 != filter2); + ASSERT_FALSE(filter2 != filter1); + ASSERT_EQ(filter1, filter2); +} + +TEST(DisplayListMaskFilter, UnknownNotEquals) { + // Even though the filter is the same, it is a different instance + // and we cannot currently tell them apart because the Skia + // MaskFilter objects do not implement == + DlUnknownMaskFilter filter1( + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0)); + DlUnknownMaskFilter filter2( + SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 5.0)); + ASSERT_TRUE(filter1 != filter2); + ASSERT_TRUE(filter2 != filter1); + ASSERT_FALSE(filter1 == filter2); + ASSERT_FALSE(filter2 == filter1); + ASSERT_NE(filter1, filter2); +} + +void testEquals(DlMaskFilter* a, DlMaskFilter* b) { + // a and b have the same nullness or values + ASSERT_TRUE(Equals(a, b)); + ASSERT_FALSE(NotEquals(a, b)); + ASSERT_TRUE(Equals(b, a)); + ASSERT_FALSE(NotEquals(b, a)); +} + +void testNotEquals(DlMaskFilter* a, DlMaskFilter* b) { + // a and b do not have the same nullness or values + ASSERT_FALSE(Equals(a, b)); + ASSERT_TRUE(NotEquals(a, b)); + ASSERT_FALSE(Equals(b, a)); + ASSERT_TRUE(NotEquals(b, a)); +} + +void testEquals(std::shared_ptr a, DlMaskFilter* b) { + // a and b have the same nullness or values + ASSERT_TRUE(Equals(a, b)); + ASSERT_FALSE(NotEquals(a, b)); + ASSERT_TRUE(Equals(b, a)); + ASSERT_FALSE(NotEquals(b, a)); +} + +void testNotEquals(std::shared_ptr a, DlMaskFilter* b) { + // a and b do not have the same nullness or values + ASSERT_FALSE(Equals(a, b)); + ASSERT_TRUE(NotEquals(a, b)); + ASSERT_FALSE(Equals(b, a)); + ASSERT_TRUE(NotEquals(b, a)); +} + +void testEquals(std::shared_ptr a, + std::shared_ptr b) { + // a and b have the same nullness or values + ASSERT_TRUE(Equals(a, b)); + ASSERT_FALSE(NotEquals(a, b)); + ASSERT_TRUE(Equals(b, a)); + ASSERT_FALSE(NotEquals(b, a)); +} + +void testNotEquals(std::shared_ptr a, + std::shared_ptr b) { + // a and b do not have the same nullness or values + ASSERT_FALSE(Equals(a, b)); + ASSERT_TRUE(NotEquals(a, b)); + ASSERT_FALSE(Equals(b, a)); + ASSERT_TRUE(NotEquals(b, a)); +} + +TEST(DisplayListMaskFilter, ComparableTemplates) { + DlBlurMaskFilter filter1a(SkBlurStyle::kNormal_SkBlurStyle, 3.0); + DlBlurMaskFilter filter1b(SkBlurStyle::kNormal_SkBlurStyle, 3.0); + DlBlurMaskFilter filter2(SkBlurStyle::kNormal_SkBlurStyle, 5.0); + std::shared_ptr shared_null; + + // null to null + testEquals(nullptr, nullptr); + testEquals(shared_null, nullptr); + testEquals(shared_null, shared_null); + + // ptr to null + testNotEquals(&filter1a, nullptr); + testNotEquals(&filter1b, nullptr); + testNotEquals(&filter2, nullptr); + + // shared_ptr to null and shared_null to ptr + testNotEquals(filter1a.shared(), nullptr); + testNotEquals(filter1b.shared(), nullptr); + testNotEquals(filter2.shared(), nullptr); + testNotEquals(shared_null, &filter1a); + testNotEquals(shared_null, &filter1b); + testNotEquals(shared_null, &filter2); + + // ptr to ptr + testEquals(&filter1a, &filter1a); + testEquals(&filter1a, &filter1b); + testEquals(&filter1b, &filter1b); + testEquals(&filter2, &filter2); + testNotEquals(&filter1a, &filter2); + + // shared_ptr to ptr + testEquals(filter1a.shared(), &filter1a); + testEquals(filter1a.shared(), &filter1b); + testEquals(filter1b.shared(), &filter1b); + testEquals(filter2.shared(), &filter2); + testNotEquals(filter1a.shared(), &filter2); + testNotEquals(filter1b.shared(), &filter2); + testNotEquals(filter2.shared(), &filter1a); + testNotEquals(filter2.shared(), &filter1b); + + // shared_ptr to shared_ptr + testEquals(filter1a.shared(), filter1a.shared()); + testEquals(filter1a.shared(), filter1b.shared()); + testEquals(filter1b.shared(), filter1b.shared()); + testEquals(filter2.shared(), filter2.shared()); + testNotEquals(filter1a.shared(), filter2.shared()); + testNotEquals(filter1b.shared(), filter2.shared()); + testNotEquals(filter2.shared(), filter1a.shared()); + testNotEquals(filter2.shared(), filter1b.shared()); +} + +} // namespace testing +} // namespace flutter diff --git a/engine/src/flutter/display_list/display_list_ops.h b/engine/src/flutter/display_list/display_list_ops.h index c99e43d53a0..e077790b7b7 100644 --- a/engine/src/flutter/display_list/display_list_ops.h +++ b/engine/src/flutter/display_list/display_list_ops.h @@ -177,65 +177,53 @@ struct SetBlendModeOp final : DLOp { DEFINE_SET_CLEAR_SKREF_OP(Blender, blender) DEFINE_SET_CLEAR_SKREF_OP(Shader, shader) DEFINE_SET_CLEAR_SKREF_OP(ImageFilter, filter) -DEFINE_SET_CLEAR_SKREF_OP(MaskFilter, filter) DEFINE_SET_CLEAR_SKREF_OP(PathEffect, effect) #undef DEFINE_SET_CLEAR_SKREF_OP -struct ClearColorFilterOp final : DLOp { - static const auto kType = DisplayListOpType::kClearColorFilter; - - ClearColorFilterOp() {} - - void dispatch(Dispatcher& dispatcher) const { - dispatcher.setColorFilter(nullptr); - } -}; - -struct SetColorFilterOp final : DLOp { - static const auto kType = DisplayListOpType::kSetColorFilter; - - SetColorFilterOp() {} - - void dispatch(Dispatcher& dispatcher) const { - const DlColorFilter* filter = - reinterpret_cast(this + 1); - dispatcher.setColorFilter(filter); - } -}; - -struct SetSkColorFilterOp final : DLOp { - static const auto kType = DisplayListOpType::kSetSkColorFilter; - - SetSkColorFilterOp(sk_sp filter) : filter(filter) {} - - sk_sp filter; - - void dispatch(Dispatcher& dispatcher) const { - DlUnknownColorFilter dl_filter(filter); - dispatcher.setColorFilter(&dl_filter); - } -}; - -// 4 byte header + 4 byte payload packs into minimum 8 bytes -// Note that the "blur style" is packed into the OpType to prevent -// needing an additional 8 bytes for a 4-value enum. -#define DEFINE_MASK_BLUR_FILTER_OP(name, style) \ - struct SetMaskBlurFilter##name##Op final : DLOp { \ - static const auto kType = DisplayListOpType::kSetMaskBlurFilter##name; \ - \ - explicit SetMaskBlurFilter##name##Op(SkScalar sigma) : sigma(sigma) {} \ - \ - SkScalar sigma; \ - \ - void dispatch(Dispatcher& dispatcher) const { \ - dispatcher.setMaskBlurFilter(style, sigma); \ - } \ +// Clear: 4 byte header + unused 4 byte payload uses 8 bytes +// (4 bytes unused) +// Set: 4 byte header + unused 4 byte struct padding + Dl +// instance copied to the memory following the record +// yields a size and efficiency that has somewhere between +// 4 and 8 bytes unused +// SetSk: 4 byte header + an sk_sp (ptr) uses 16 bytes due to the +// alignment of the ptr. +// (4 bytes unused) +#define DEFINE_SET_CLEAR_DLATTR_OP(name, field) \ + struct Clear##name##Op final : DLOp { \ + static const auto kType = DisplayListOpType::kClear##name; \ + \ + Clear##name##Op() {} \ + \ + void dispatch(Dispatcher& dispatcher) const { \ + dispatcher.set##name(nullptr); \ + } \ + }; \ + struct Set##name##Op final : DLOp { \ + static const auto kType = DisplayListOpType::kSet##name; \ + \ + Set##name##Op() {} \ + \ + void dispatch(Dispatcher& dispatcher) const { \ + const Dl##name* filter = reinterpret_cast(this + 1); \ + dispatcher.set##name(filter); \ + } \ + }; \ + struct SetSk##name##Op final : DLOp { \ + static const auto kType = DisplayListOpType::kSetSk##name; \ + \ + SetSk##name##Op(sk_sp field) : field(field) {} \ + \ + sk_sp field; \ + \ + void dispatch(Dispatcher& dispatcher) const { \ + DlUnknown##name dl_filter(field); \ + dispatcher.set##name(&dl_filter); \ + } \ }; -DEFINE_MASK_BLUR_FILTER_OP(Normal, kNormal_SkBlurStyle) -DEFINE_MASK_BLUR_FILTER_OP(Solid, kSolid_SkBlurStyle) -DEFINE_MASK_BLUR_FILTER_OP(Inner, kInner_SkBlurStyle) -DEFINE_MASK_BLUR_FILTER_OP(Outer, kOuter_SkBlurStyle) -#undef DEFINE_MASK_BLUR_FILTER_OP +DEFINE_SET_CLEAR_DLATTR_OP(ColorFilter, filter) +DEFINE_SET_CLEAR_DLATTR_OP(MaskFilter, filter) +#undef DEFINE_SET_CLEAR_DLATTR_OP // 4 byte header + no payload uses minimum 8 bytes (4 bytes unused) struct SaveOp final : DLOp { diff --git a/engine/src/flutter/display_list/display_list_unittests.cc b/engine/src/flutter/display_list/display_list_unittests.cc index aed1b51d41c..ee7fbab69f1 100644 --- a/engine/src/flutter/display_list/display_list_unittests.cc +++ b/engine/src/flutter/display_list/display_list_unittests.cc @@ -108,8 +108,11 @@ static const sk_sp TestPathEffect1 = SkDashPathEffect::Make(TestDashes1, 2, 0.0f); static const sk_sp TestPathEffect2 = SkDashPathEffect::Make(TestDashes2, 2, 0.0f); -static const sk_sp TestMaskFilter = - SkMaskFilter::MakeBlur(kNormal_SkBlurStyle, 5.0); +static const DlBlurMaskFilter TestMaskFilter1(kNormal_SkBlurStyle, 3.0); +static const DlBlurMaskFilter TestMaskFilter2(kNormal_SkBlurStyle, 5.0); +static const DlBlurMaskFilter TestMaskFilter3(kSolid_SkBlurStyle, 3.0); +static const DlBlurMaskFilter TestMaskFilter4(kInner_SkBlurStyle, 3.0); +static const DlBlurMaskFilter TestMaskFilter5(kOuter_SkBlurStyle, 3.0); constexpr SkRect TestBounds = SkRect::MakeLTRB(10, 10, 50, 60); static const SkRRect TestRRect = SkRRect::MakeRectXY(TestBounds, 5, 5); static const SkRRect TestRRectRect = SkRRect::MakeRect(TestBounds); @@ -364,12 +367,11 @@ std::vector allGroups = { } }, { "SetMaskFilter", { - {0, 16, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(TestMaskFilter);}}, - {0, 8, 0, 0, [](DisplayListBuilder& b) {b.setMaskBlurFilter(kNormal_SkBlurStyle, 3.0);}}, - {0, 8, 0, 0, [](DisplayListBuilder& b) {b.setMaskBlurFilter(kNormal_SkBlurStyle, 5.0);}}, - {0, 8, 0, 0, [](DisplayListBuilder& b) {b.setMaskBlurFilter(kSolid_SkBlurStyle, 3.0);}}, - {0, 8, 0, 0, [](DisplayListBuilder& b) {b.setMaskBlurFilter(kInner_SkBlurStyle, 3.0);}}, - {0, 8, 0, 0, [](DisplayListBuilder& b) {b.setMaskBlurFilter(kOuter_SkBlurStyle, 3.0);}}, + {0, 24, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(&TestMaskFilter1);}}, + {0, 24, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(&TestMaskFilter2);}}, + {0, 24, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(&TestMaskFilter3);}}, + {0, 24, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(&TestMaskFilter4);}}, + {0, 24, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(&TestMaskFilter5);}}, {0, 0, 0, 0, [](DisplayListBuilder& b) {b.setMaskFilter(nullptr);}}, } }, @@ -1264,27 +1266,6 @@ TEST(DisplayList, DisplayListImageFilterRefHandling) { ASSERT_TRUE(tester.ref_is_unique()); } -TEST(DisplayList, DisplayListMaskFilterRefHandling) { - class MaskFilterRefTester : public virtual AttributeRefTester { - public: - void setRefToPaint(SkPaint& paint) const override { - paint.setMaskFilter(mask_filter); - } - void setRefToDisplayList(DisplayListBuilder& builder) const override { - builder.setMaskFilter(mask_filter); - } - bool ref_is_unique() const override { return mask_filter->unique(); } - - private: - sk_sp mask_filter = - SkMaskFilter::MakeBlur(SkBlurStyle::kNormal_SkBlurStyle, 2.0); - }; - - MaskFilterRefTester tester; - tester.test(); - ASSERT_TRUE(tester.ref_is_unique()); -} - TEST(DisplayList, DisplayListBlenderRefHandling) { class BlenderRefTester : public virtual AttributeRefTester { public: @@ -1396,30 +1377,6 @@ TEST(DisplayList, DisplayListFullPerspectiveTransformHandling) { } } -TEST(DisplayList, SetMaskBlurSigmaZeroResetsMaskFilter) { - DisplayListBuilder builder; - builder.setMaskBlurFilter(SkBlurStyle::kNormal_SkBlurStyle, 2.0); - builder.drawRect({10, 10, 20, 20}); - builder.setMaskBlurFilter(SkBlurStyle::kNormal_SkBlurStyle, 0.0); - EXPECT_EQ(builder.getMaskFilter(), nullptr); - builder.drawRect({30, 30, 40, 40}); - sk_sp display_list = builder.Build(); - ASSERT_EQ(display_list->op_count(), 2u); - ASSERT_EQ(display_list->bytes(), sizeof(DisplayList) + 8u + 24u + 8u + 24u); -} - -TEST(DisplayList, SetMaskFilterNullResetsMaskFilter) { - DisplayListBuilder builder; - builder.setMaskBlurFilter(SkBlurStyle::kNormal_SkBlurStyle, 2.0); - builder.drawRect({10, 10, 20, 20}); - builder.setMaskFilter(nullptr); - EXPECT_EQ(builder.getMaskFilter(), nullptr); - builder.drawRect({30, 30, 40, 40}); - sk_sp display_list = builder.Build(); - ASSERT_EQ(display_list->op_count(), 2u); - ASSERT_EQ(display_list->bytes(), sizeof(DisplayList) + 8u + 24u + 8u + 24u); -} - TEST(DisplayList, SingleOpsMightSupportGroupOpacityWithOrWithoutBlendMode) { auto run_tests = [](std::string name, void build(DisplayListBuilder & builder), diff --git a/engine/src/flutter/display_list/display_list_utils.cc b/engine/src/flutter/display_list/display_list_utils.cc index c21b4a2f3e7..3a8cf8a264c 100644 --- a/engine/src/flutter/display_list/display_list_utils.cc +++ b/engine/src/flutter/display_list/display_list_utils.cc @@ -86,12 +86,8 @@ void SkPaintDispatchHelper::setColorFilter(const DlColorFilter* filter) { void SkPaintDispatchHelper::setPathEffect(sk_sp effect) { paint_.setPathEffect(effect); } -void SkPaintDispatchHelper::setMaskFilter(sk_sp filter) { - paint_.setMaskFilter(filter); -} -void SkPaintDispatchHelper::setMaskBlurFilter(SkBlurStyle style, - SkScalar sigma) { - paint_.setMaskFilter(SkMaskFilter::MakeBlur(style, sigma)); +void SkPaintDispatchHelper::setMaskFilter(const DlMaskFilter* filter) { + paint_.setMaskFilter(filter ? filter->sk_filter() : nullptr); } sk_sp SkPaintDispatchHelper::makeColorFilter() const { @@ -272,14 +268,8 @@ void DisplayListBoundsCalculator::setColorFilter(const DlColorFilter* filter) { void DisplayListBoundsCalculator::setPathEffect(sk_sp effect) { path_effect_ = std::move(effect); } -void DisplayListBoundsCalculator::setMaskFilter(sk_sp filter) { - mask_filter_ = std::move(filter); - mask_sigma_pad_ = 0.0f; -} -void DisplayListBoundsCalculator::setMaskBlurFilter(SkBlurStyle style, - SkScalar sigma) { - mask_sigma_pad_ = std::max(3.0f * sigma, 0.0f); - mask_filter_ = nullptr; +void DisplayListBoundsCalculator::setMaskFilter(const DlMaskFilter* filter) { + mask_filter_ = filter ? filter->shared() : nullptr; } void DisplayListBoundsCalculator::save() { SkMatrixDispatchHelper::save(); @@ -601,15 +591,18 @@ bool DisplayListBoundsCalculator::AdjustBoundsForPaint( if (flags.applies_mask_filter()) { if (mask_filter_) { - SkPaint p; - p.setMaskFilter(mask_filter_); - if (!p.canComputeFastBounds()) { - return false; + const DlBlurMaskFilter* blur_filter = mask_filter_->asBlur(); + if (blur_filter) { + SkScalar mask_sigma_pad = blur_filter->sigma() * 3.0; + bounds.outset(mask_sigma_pad, mask_sigma_pad); + } else { + SkPaint p; + p.setMaskFilter(mask_filter_->sk_filter()); + if (!p.canComputeFastBounds()) { + return false; + } + bounds = p.computeFastBounds(bounds, &bounds); } - bounds = p.computeFastBounds(bounds, &bounds); - } - if (mask_sigma_pad_ > 0.0f) { - bounds.outset(mask_sigma_pad_, mask_sigma_pad_); } } diff --git a/engine/src/flutter/display_list/display_list_utils.h b/engine/src/flutter/display_list/display_list_utils.h index db5ede9f718..3bf3b1dee73 100644 --- a/engine/src/flutter/display_list/display_list_utils.h +++ b/engine/src/flutter/display_list/display_list_utils.h @@ -58,8 +58,7 @@ class IgnoreAttributeDispatchHelper : public virtual Dispatcher { void setImageFilter(sk_sp filter) override {} void setColorFilter(const DlColorFilter* filter) override {} void setPathEffect(sk_sp effect) override {} - void setMaskFilter(sk_sp filter) override {} - void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override {} + void setMaskFilter(const DlMaskFilter* filter) override {} }; // A utility class that will ignore all Dispatcher methods relating @@ -184,8 +183,7 @@ class SkPaintDispatchHelper : public virtual Dispatcher { void setBlendMode(SkBlendMode mode) override; void setBlender(sk_sp blender) override; void setPathEffect(sk_sp effect) override; - void setMaskFilter(sk_sp filter) override; - void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override; + void setMaskFilter(const DlMaskFilter* filter) override; void setImageFilter(sk_sp filter) override; const SkPaint& paint() { return paint_; } @@ -402,8 +400,7 @@ class DisplayListBoundsCalculator final void setImageFilter(sk_sp filter) override; void setColorFilter(const DlColorFilter* filter) override; void setPathEffect(sk_sp effect) override; - void setMaskFilter(sk_sp filter) override; - void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override; + void setMaskFilter(const DlMaskFilter* filter) override; void save() override; void saveLayer(const SkRect* bounds, const SaveLayerOptions options) override; @@ -580,8 +577,7 @@ class DisplayListBoundsCalculator final bool cap_is_square_ = false; sk_sp image_filter_; sk_sp path_effect_; - sk_sp mask_filter_; - SkScalar mask_sigma_pad_ = 0.0; + std::shared_ptr mask_filter_; bool paint_nops_on_transparency(); diff --git a/engine/src/flutter/lib/ui/painting/paint.cc b/engine/src/flutter/lib/ui/painting/paint.cc index aedadc404b7..b35ff9cdb93 100644 --- a/engine/src/flutter/lib/ui/painting/paint.cc +++ b/engine/src/flutter/lib/ui/painting/paint.cc @@ -301,7 +301,8 @@ bool Paint::sync_to(DisplayListBuilder* builder, SkBlurStyle blur_style = static_cast(uint_data[kMaskFilterBlurStyleIndex]); double sigma = float_data[kMaskFilterSigmaIndex]; - builder->setMaskBlurFilter(blur_style, sigma); + DlBlurMaskFilter dl_filter(blur_style, sigma); + builder->setMaskFilter(&dl_filter); break; } }