From ffb28249ec432fba66db52b47770dc0300e1b398 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 1 Mar 2023 18:03:32 -0800 Subject: [PATCH] remove obsolete DlAttribute::From(SkObject) methods --- display_list/display_list.h | 5 - display_list/display_list_builder.cc | 42 +--- display_list/display_list_canvas_unittests.cc | 117 +++------ display_list/display_list_color_filter.cc | 28 --- display_list/display_list_color_filter.h | 70 +----- .../display_list_color_filter_unittests.cc | 101 -------- display_list/display_list_color_source.cc | 26 -- display_list/display_list_color_source.h | 53 ---- .../display_list_color_source_unittests.cc | 202 --------------- display_list/display_list_flags.cc | 29 +-- display_list/display_list_flags.h | 3 +- display_list/display_list_image_filter.cc | 21 -- display_list/display_list_image_filter.h | 110 +-------- .../display_list_image_filter_unittests.cc | 229 ++++++------------ display_list/display_list_mask_filter.cc | 9 +- display_list/display_list_mask_filter.h | 65 +---- .../display_list_mask_filter_unittests.cc | 55 ----- display_list/display_list_ops.h | 15 -- display_list/display_list_path_effect.cc | 32 --- display_list/display_list_path_effect.h | 51 +--- .../display_list_path_effect_unittests.cc | 57 ----- display_list/testing/dl_test_snippets.cc | 9 +- flow/layers/color_filter_layer_unittests.cc | 6 +- flow/layers/shader_mask_layer_unittests.cc | 41 ++-- flow/paint_utils.cc | 14 +- .../display_list/display_list_dispatcher.cc | 10 - testing/display_list_testing.cc | 24 +- 27 files changed, 206 insertions(+), 1218 deletions(-) diff --git a/display_list/display_list.h b/display_list/display_list.h index 0bb6b83736377..dde26766feedc 100644 --- a/display_list/display_list.h +++ b/display_list/display_list.h @@ -75,28 +75,23 @@ namespace flutter { V(SetColor) \ V(SetBlendMode) \ \ - V(SetSkPathEffect) \ V(SetPodPathEffect) \ V(ClearPathEffect) \ \ V(ClearColorFilter) \ V(SetPodColorFilter) \ - V(SetSkColorFilter) \ \ V(ClearColorSource) \ V(SetPodColorSource) \ - V(SetSkColorSource) \ V(SetImageColorSource) \ V(SetRuntimeEffectColorSource) \ \ V(ClearImageFilter) \ V(SetPodImageFilter) \ - V(SetSkImageFilter) \ V(SetSharedImageFilter) \ \ V(ClearMaskFilter) \ V(SetPodMaskFilter) \ - V(SetSkMaskFilter) \ \ V(Save) \ V(SaveLayer) \ diff --git a/display_list/display_list_builder.cc b/display_list/display_list_builder.cc index 934fe8e9ed87a..160f35e005eb8 100644 --- a/display_list/display_list_builder.cc +++ b/display_list/display_list_builder.cc @@ -212,9 +212,6 @@ void DisplayListBuilder::onSetColorSource(const DlColorSource* source) { break; } #endif // IMPELLER_ENABLE_3D - case DlColorSourceType::kUnknown: - Push(0, 0, source->skia_object()); - break; } } } @@ -259,10 +256,6 @@ void DisplayListBuilder::onSetImageFilter(const DlImageFilter* filter) { Push(0, 0, filter); break; } - case DlImageFilterType::kUnknown: { - Push(0, 0, filter->skia_object()); - break; - } } } } @@ -297,10 +290,6 @@ void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) { new (pod) DlLinearToSrgbGammaColorFilter(); break; } - case DlColorFilterType::kUnknown: { - Push(0, 0, filter->skia_object()); - break; - } } } UpdateCurrentOpacityCompatibility(); @@ -318,10 +307,6 @@ void DisplayListBuilder::onSetPathEffect(const DlPathEffect* effect) { new (pod) DlDashPathEffect(dash_effect); break; } - case DlPathEffectType::kUnknown: { - Push(0, 0, effect->skia_object()); - break; - } } } } @@ -339,9 +324,6 @@ void DisplayListBuilder::onSetMaskFilter(const DlMaskFilter* filter) { new (pod) DlBlurMaskFilter(blur_filter); break; } - case DlMaskFilterType::kUnknown: - Push(0, 0, filter->skia_object()); - break; } } } @@ -1183,9 +1165,11 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, } if (flags.is_geometric()) { + bool is_stroked = flags.is_stroked(current_.getDrawStyle()); + // Path effect occurs before stroking... DisplayListSpecialGeometryFlags special_flags = - flags.WithPathEffect(current_.getPathEffectPtr()); + flags.WithPathEffect(current_.getPathEffectPtr(), is_stroked); if (current_.getPathEffect()) { auto effect_bounds = current_.getPathEffect()->effect_bounds(bounds); if (!effect_bounds.has_value()) { @@ -1194,7 +1178,7 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, bounds = effect_bounds.value(); } - if (flags.is_stroked(current_.getDrawStyle())) { + if (is_stroked) { // Determine the max multiplier to the stroke width first. SkScalar pad = 1.0f; if (current_.getStrokeJoin() == DlStrokeJoin::kMiter && @@ -1212,18 +1196,14 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds, } if (flags.applies_mask_filter()) { - if (current_.getMaskFilter()) { - const DlBlurMaskFilter* blur_filter = current_.getMaskFilter()->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(current_.getMaskFilter()->skia_object()); - if (!p.canComputeFastBounds()) { - return false; + auto filter = current_.getMaskFilter(); + if (filter) { + switch (filter->type()) { + case DlMaskFilterType::kBlur: { + FML_DCHECK(filter->asBlur()); + SkScalar mask_sigma_pad = filter->asBlur()->sigma() * 3.0; + bounds.outset(mask_sigma_pad, mask_sigma_pad); } - bounds = p.computeFastBounds(bounds, &bounds); } } } diff --git a/display_list/display_list_canvas_unittests.cc b/display_list/display_list_canvas_unittests.cc index 513a2854ac0cb..a6aed32248c79 100644 --- a/display_list/display_list_canvas_unittests.cc +++ b/display_list/display_list_canvas_unittests.cc @@ -17,10 +17,6 @@ #include "flutter/testing/testing.h" #include "third_party/skia/include/core/SkPictureRecorder.h" #include "third_party/skia/include/core/SkSurface.h" -#include "third_party/skia/include/effects/SkDashPathEffect.h" -#include "third_party/skia/include/effects/SkDiscretePathEffect.h" -#include "third_party/skia/include/effects/SkGradientShader.h" -#include "third_party/skia/include/effects/SkImageFilters.h" namespace flutter { namespace testing { @@ -668,21 +664,22 @@ class TestParameters { return false; } + bool is_stroked = flags_.is_stroked(attr.getDrawStyle()); + if (flags_.is_stroked(ref_attr.getDrawStyle()) != is_stroked) { + return false; + } DisplayListSpecialGeometryFlags geo_flags = - flags_.WithPathEffect(attr.getPathEffect().get()); + flags_.WithPathEffect(attr.getPathEffect().get(), is_stroked); if (flags_.applies_path_effect() && // ref_attr.getPathEffect() != attr.getPathEffect()) { - if (attr.getPathEffect()->asDash() == nullptr) { - return false; - } - if (!ignores_dashes()) { - return false; + switch (attr.getPathEffect()->type()) { + case DlPathEffectType::kDash: { + if (is_stroked && !ignores_dashes()) { + return false; + } + } } } - bool is_stroked = flags_.is_stroked(ref_attr.getDrawStyle()); - if (flags_.is_stroked(attr.getDrawStyle()) != is_stroked) { - return false; - } if (!is_stroked) { return true; } @@ -766,7 +763,7 @@ class TestParameters { auto path_effect = paint.getPathEffect(); DisplayListSpecialGeometryFlags geo_flags = - flags_.WithPathEffect(path_effect.get()); + flags_.WithPathEffect(path_effect.get(), true); if (paint.getStrokeCap() == DlStrokeCap::kButt && !geo_flags.butt_cap_becomes_square()) { adjust = std::max(adjust, half_width); @@ -1117,10 +1114,18 @@ class CanvasCompareTester { .with_restore(sk_safe_restore, dl_safe_restore, true)); } } + { - sk_sp sk_filter = SkImageFilters::Arithmetic( - 0.1, 0.1, 0.1, 0.25, true, nullptr, nullptr); - DlUnknownImageFilter filter(sk_filter); + // clang-format off + constexpr float color_matrix[20] = { + 0.5, 0, 0, 0, 0.5, + 0, 0.5, 0, 0, 0.5, + 0, 0, 0.5, 0, 0.5, + 0, 0, 0, 1, 0, + }; + // clang-format on + DlMatrixColorFilter color_filter(color_matrix); + DlColorFilterImageFilter filter(color_filter); { RenderWith(testP, env, tolerance, CaseParameters( @@ -1463,67 +1468,6 @@ class CanvasCompareTester { } } - { - sk_sp effect = SkDiscretePathEffect::Make(3, 5); - { - // Discrete path effects need a stroke width for drawPointsAsPoints - // to do something realistic - // And a Discrete(3, 5) effect produces miters that are near - // maximal for a miter limit of 3.0. - BoundsTolerance discrete_tolerance = - tolerance - // register the discrete offset so adjusters can compensate - .addDiscreteOffset(5) - // the miters in the 3-5 discrete effect don't always fill - // their conservative bounds, so tolerate a couple of pixels - .addBoundsPadding(2, 2); - RenderWith(testP, env, discrete_tolerance, - CaseParameters( - "PathEffect == Discrete-3-5", - [=](SkCanvas*, SkPaint& p) { - p.setStrokeWidth(5.0); - p.setStrokeMiter(3.0); - p.setPathEffect(effect); - }, - [=](DlCanvas*, DlPaint& p) { - p.setStrokeWidth(5.0); - p.setStrokeMiter(3.0); - p.setPathEffect(DlPathEffect::From(effect)); - })); - } - EXPECT_TRUE(testP.is_draw_text_blob() || effect->unique()) - << "PathEffect == Discrete-3-5 Cleanup"; - effect = SkDiscretePathEffect::Make(2, 3); - { - // Discrete path effects need a stroke width for drawPointsAsPoints - // to do something realistic - // And a Discrete(2, 3) effect produces miters that are near - // maximal for a miter limit of 2.5. - BoundsTolerance discrete_tolerance = - tolerance - // register the discrete offset so adjusters can compensate - .addDiscreteOffset(3) - // the miters in the 3-5 discrete effect don't always fill - // their conservative bounds, so tolerate a couple of pixels - .addBoundsPadding(2, 2); - RenderWith(testP, env, discrete_tolerance, - CaseParameters( - "PathEffect == Discrete-2-3", - [=](SkCanvas*, SkPaint& p) { - p.setStrokeWidth(5.0); - p.setStrokeMiter(2.5); - p.setPathEffect(effect); - }, - [=](DlCanvas*, DlPaint& p) { - p.setStrokeWidth(5.0); - p.setStrokeMiter(2.5); - p.setPathEffect(DlPathEffect::From(effect)); - })); - } - EXPECT_TRUE(testP.is_draw_text_blob() || effect->unique()) - << "PathEffect == Discrete-2-3 Cleanup"; - } - { const DlBlurMaskFilter filter(kNormal_SkBlurStyle, 5.0); BoundsTolerance blur_5_tolerance = tolerance.addBoundsPadding(4, 4); @@ -1754,6 +1698,21 @@ class CanvasCompareTester { const SkScalar test_dashes_1[] = {29.0, 2.0}; const SkScalar test_dashes_2[] = {17.0, 1.5}; auto effect = DlDashPathEffect::Make(test_dashes_1, 2, 0.0f); + { + RenderWith(testP, stroke_base_env, tolerance, + CaseParameters( + "PathEffect without forced stroking == Dash-29-2", + [=](SkCanvas*, SkPaint& p) { + // Provide some non-trivial stroke size to get dashed + p.setStrokeWidth(5.0); + p.setPathEffect(effect->skia_object()); + }, + [=](DlCanvas*, DlPaint& p) { + // Provide some non-trivial stroke size to get dashed + p.setStrokeWidth(5.0); + p.setPathEffect(effect); + })); + } { RenderWith(testP, stroke_base_env, tolerance, CaseParameters( diff --git a/display_list/display_list_color_filter.cc b/display_list/display_list_color_filter.cc index 0204993d1b482..6f46b0256d564 100644 --- a/display_list/display_list_color_filter.cc +++ b/display_list/display_list_color_filter.cc @@ -8,34 +8,6 @@ namespace flutter { -std::shared_ptr DlColorFilter::From(SkColorFilter* sk_filter) { - if (sk_filter == nullptr) { - return nullptr; - } - if (sk_filter == DlSrgbToLinearGammaColorFilter::sk_filter_.get()) { - // Skia implements these filters as a singleton. - return DlSrgbToLinearGammaColorFilter::instance; - } - if (sk_filter == DlLinearToSrgbGammaColorFilter::sk_filter_.get()) { - // Skia implements these filters as a singleton. - return DlLinearToSrgbGammaColorFilter::instance; - } - { - SkColor color; - SkBlendMode mode; - if (sk_filter->asAColorMode(&color, &mode)) { - return std::make_shared(color, ToDl(mode)); - } - } - { - float matrix[20]; - if (sk_filter->asAColorMatrix(matrix)) { - return std::make_shared(matrix); - } - } - return std::make_shared(sk_ref_sp(sk_filter)); -} - const std::shared_ptr DlSrgbToLinearGammaColorFilter::instance = std::make_shared(); diff --git a/display_list/display_list_color_filter.h b/display_list/display_list_color_filter.h index 6fbef21821d26..240f3619320ed 100644 --- a/display_list/display_list_color_filter.h +++ b/display_list/display_list_color_filter.h @@ -20,37 +20,17 @@ class DlMatrixColorFilter; // facilities and adheres to the design goals of the |DlAttribute| base // class. -// An enumerated type for the recognized ColorFilter operations. -// If a custom ColorFilter outside of the recognized types is needed -// then a |kUnknown| type that simply defers to an SkColorFilter is -// provided as a fallback. +// An enumerated type for the supported ColorFilter operations. enum class DlColorFilterType { kBlend, kMatrix, kSrgbToLinearGamma, kLinearToSrgbGamma, - kUnknown }; class DlColorFilter : public DlAttribute { public: - // Return a shared_ptr holding a DlColorFilter representing the indicated - // Skia SkColorFilter pointer. - // - // This method can detect each of the 4 recognized types from an analogous - // SkColorFilter. - static std::shared_ptr From(SkColorFilter* sk_filter); - - // Return a shared_ptr holding a DlColorFilter representing the indicated - // Skia SkColorFilter pointer. - // - // This method can detect each of the 4 recognized types from an analogous - // SkColorFilter. - static std::shared_ptr From(sk_sp sk_filter) { - return From(sk_filter.get()); - } - // Return a boolean indicating whether the color filtering operation will // modify transparent black. This is typically used to determine if applying // the ColorFilter to a temporary saveLayer buffer will turn the surrounding @@ -70,9 +50,8 @@ class DlColorFilter // type of ColorFilter, otherwise return nullptr. virtual const DlMatrixColorFilter* asMatrix() const { return nullptr; } - // asSrgb<->Linear and asUnknown are not needed because they - // have no properties to query. Their type fully specifies their - // operation or can be accessed via the common skia_object() method. + // asSrgb<->Linear is not needed because it has no properties to query. + // Its type fully specifies its operation. }; // The Blend type of ColorFilter which specifies modifying the @@ -253,49 +232,6 @@ class DlLinearToSrgbGammaColorFilter final : public DlColorFilter { friend class DlColorFilter; }; -// A wrapper class for a Skia ColorFilter of unknown type. The above 4 types -// are the only types that can be constructed by Flutter using the -// ui.ColorFilter class so this class should be rarely used. -// In fact, now that the DisplayListCanvasRecorder is deleted and the -// Paragraph code talks directly to a DisplayListBuilder, there may be -// no more reasons to maintain this sub-class. -// See: https://github.com/flutter/flutter/issues/121389 -class DlUnknownColorFilter final : public DlColorFilter { - public: - DlUnknownColorFilter(sk_sp sk_filter) - : sk_filter_(std::move(sk_filter)) {} - DlUnknownColorFilter(const DlUnknownColorFilter& filter) - : DlUnknownColorFilter(filter.sk_filter_) {} - DlUnknownColorFilter(const DlUnknownColorFilter* filter) - : DlUnknownColorFilter(filter->sk_filter_) {} - - DlColorFilterType type() const override { - return DlColorFilterType::kUnknown; - } - size_t size() const override { return sizeof(*this); } - bool modifies_transparent_black() const override { - return sk_filter_->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT; - } - - std::shared_ptr shared() const override { - return std::make_shared(this); - } - - sk_sp skia_object() const override { return sk_filter_; } - - virtual ~DlUnknownColorFilter() = default; - - protected: - bool equals_(const DlColorFilter& other) const override { - FML_DCHECK(other.type() == DlColorFilterType::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_COLOR_FILTER_H_ diff --git a/display_list/display_list_color_filter_unittests.cc b/display_list/display_list_color_filter_unittests.cc index 6884c6068d052..2403a6db4c9b4 100644 --- a/display_list/display_list_color_filter_unittests.cc +++ b/display_list/display_list_color_filter_unittests.cc @@ -29,72 +29,6 @@ TEST(DisplayListColorFilter, BuilderSetGet) { ASSERT_EQ(builder.getColorFilter(), nullptr); } -TEST(DisplayListColorFilter, FromSkiaNullFilter) { - std::shared_ptr filter = DlColorFilter::From(nullptr); - ASSERT_EQ(filter, nullptr); - ASSERT_EQ(filter.get(), nullptr); -} - -TEST(DisplayListColorFilter, FromSkiaBlendFilter) { - sk_sp sk_filter = - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kDstATop); - std::shared_ptr filter = DlColorFilter::From(sk_filter); - DlBlendColorFilter dl_filter(DlColor::kRed(), DlBlendMode::kDstATop); - ASSERT_EQ(filter->type(), DlColorFilterType::kBlend); - ASSERT_EQ(*filter->asBlend(), dl_filter); - ASSERT_EQ(filter->asBlend()->color(), DlColor::kRed()); - ASSERT_EQ(filter->asBlend()->mode(), DlBlendMode::kDstATop); - - ASSERT_EQ(filter->asMatrix(), nullptr); -} - -TEST(DisplayListColorFilter, FromSkiaMatrixFilter) { - sk_sp sk_filter = SkColorFilters::Matrix(kMatrix); - std::shared_ptr filter = DlColorFilter::From(sk_filter); - DlMatrixColorFilter dl_filter(kMatrix); - ASSERT_EQ(filter->type(), DlColorFilterType::kMatrix); - ASSERT_EQ(*filter->asMatrix(), dl_filter); - const DlMatrixColorFilter* matrix_filter = filter->asMatrix(); - for (int i = 0; i < 20; i++) { - ASSERT_EQ((*matrix_filter)[i], kMatrix[i]); - } - - ASSERT_EQ(filter->asBlend(), nullptr); -} - -TEST(DisplayListColorFilter, FromSkiaSrgbToLinearFilter) { - sk_sp sk_filter = SkColorFilters::SRGBToLinearGamma(); - std::shared_ptr filter = DlColorFilter::From(sk_filter); - ASSERT_EQ(filter->type(), DlColorFilterType::kSrgbToLinearGamma); - - ASSERT_EQ(filter->asBlend(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); -} - -TEST(DisplayListColorFilter, FromSkiaLinearToSrgbFilter) { - sk_sp sk_filter = SkColorFilters::LinearToSRGBGamma(); - std::shared_ptr filter = DlColorFilter::From(sk_filter); - ASSERT_EQ(filter->type(), DlColorFilterType::kLinearToSrgbGamma); - - ASSERT_EQ(filter->asBlend(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); -} - -TEST(DisplayListColorFilter, FromSkiaUnrecognizedFilter) { - sk_sp sk_input_a = - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kOverlay); - sk_sp sk_input_b = - SkColorFilters::Blend(SK_ColorBLUE, SkBlendMode::kScreen); - sk_sp sk_filter = - SkColorFilters::Compose(sk_input_a, sk_input_b); - std::shared_ptr filter = DlColorFilter::From(sk_filter); - ASSERT_EQ(filter->type(), DlColorFilterType::kUnknown); - ASSERT_EQ(filter->skia_object(), sk_filter); - - ASSERT_EQ(filter->asBlend(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); -} - TEST(DisplayListColorFilter, BlendConstructor) { DlBlendColorFilter filter(DlColor::kRed(), DlBlendMode::kDstATop); } @@ -235,40 +169,5 @@ TEST(DisplayListColorFilter, LinearToSrgbEquals) { TestEquals(filter1, *DlLinearToSrgbGammaColorFilter::instance); } -TEST(DisplayListColorFilter, UnknownConstructor) { - DlUnknownColorFilter filter(SkColorFilters::LinearToSRGBGamma()); -} - -TEST(DisplayListColorFilter, UnknownShared) { - DlUnknownColorFilter filter(SkColorFilters::LinearToSRGBGamma()); - ASSERT_NE(filter.shared().get(), &filter); - ASSERT_EQ(*filter.shared(), filter); -} - -TEST(DisplayListColorFilter, UnknownContents) { - sk_sp sk_filter = SkColorFilters::LinearToSRGBGamma(); - DlUnknownColorFilter filter(sk_filter); - ASSERT_EQ(sk_filter, filter.skia_object()); - ASSERT_EQ(sk_filter.get(), filter.skia_object().get()); -} - -TEST(DisplayListColorFilter, UnknownEquals) { - sk_sp sk_filter = SkColorFilters::LinearToSRGBGamma(); - DlUnknownColorFilter filter1(sk_filter); - DlUnknownColorFilter filter2(sk_filter); - TestEquals(filter1, filter2); -} - -TEST(DisplayListColorFilter, UnknownNotEquals) { - // Even though the filter is the same, it is a different instance - // and we cannot currently tell them apart because the Skia - // ColorFilter objects do not implement == - DlUnknownColorFilter filter1( - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kDstATop)); - DlUnknownColorFilter filter2( - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kDstATop)); - TestNotEquals(filter1, filter2, "SkColorFilter instance differs"); -} - } // namespace testing } // namespace flutter diff --git a/display_list/display_list_color_source.cc b/display_list/display_list_color_source.cc index 0344de312161b..94c9fbb8b5975 100644 --- a/display_list/display_list_color_source.cc +++ b/display_list/display_list_color_source.cc @@ -10,32 +10,6 @@ namespace flutter { -std::shared_ptr DlColorSource::From(SkShader* sk_shader) { - if (sk_shader == nullptr) { - return nullptr; - } - { - SkMatrix local_matrix; - SkTileMode xy[2]; - SkImage* image = sk_shader->isAImage(&local_matrix, xy); - if (image) { - return std::make_shared( - DlImage::Make(image), ToDl(xy[0]), ToDl(xy[1]), - DlImageSampling::kLinear, &local_matrix); - } - } - // Skia provides |SkShader->asAGradient(&info)| method to access the - // parameters of a gradient, but the info object being filled has a number - // of parameters which are missing, including the local matrix in every - // gradient, and the sweep angles in the sweep gradients. - // - // Since we can't reproduce every Gradient, and customers rely on using - // gradients with matrices in text code, we have to just use an Unknown - // ColorSource to express all gradients. - // (see: https://github.com/flutter/flutter/issues/102947) - return std::make_shared(sk_ref_sp(sk_shader)); -} - static void DlGradientDeleter(void* p) { // Some of our target environments would prefer a sized delete, // but other target environments do not have that operator. diff --git a/display_list/display_list_color_source.h b/display_list/display_list_color_source.h index bf15441913b26..7705893ad4ed0 100644 --- a/display_list/display_list_color_source.h +++ b/display_list/display_list_color_source.h @@ -39,7 +39,6 @@ class DlRadialGradientColorSource; class DlConicalGradientColorSource; class DlSweepGradientColorSource; class DlRuntimeEffectColorSource; -class DlUnknownColorSource; // The DisplayList ColorSource class. This class implements all of the // facilities and adheres to the design goals of the |DlAttribute| base @@ -62,28 +61,11 @@ enum class DlColorSourceType { #ifdef IMPELLER_ENABLE_3D kScene, #endif // IMPELLER_ENABLE_3D - kUnknown }; class DlColorSource : public DlAttribute { public: - // Return a shared_ptr holding a DlColorSource representing the indicated - // Skia SkShader pointer. - // - // This method can detect each of the 4 recognized types from an analogous - // SkShader. - static std::shared_ptr From(SkShader* sk_filter); - - // Return a shared_ptr holding a DlColorFilter representing the indicated - // Skia SkShader pointer. - // - // This method can detect each of the 4 recognized types from an analogous - // SkShader. - static std::shared_ptr From(sk_sp sk_filter) { - return From(sk_filter.get()); - } - static std::shared_ptr MakeLinear( const SkPoint start_point, const SkPoint end_point, @@ -811,41 +793,6 @@ class DlSceneColorSource final : public DlColorSource { }; #endif // IMPELLER_ENABLE_3D -// Now that the DisplayListCanvasRecorder is deleted and the -// Paragraph code talks directly to a DisplayListBuilder, there may be -// no more reasons to maintain this sub-class. -// See: https://github.com/flutter/flutter/issues/121389 -class DlUnknownColorSource final : public DlColorSource { - public: - DlUnknownColorSource(sk_sp shader) - : sk_shader_(std::move(shader)) {} - - std::shared_ptr shared() const override { - return std::make_shared(sk_shader_); - } - - DlColorSourceType type() const override { - return DlColorSourceType::kUnknown; - } - size_t size() const override { return sizeof(*this); } - - bool is_opaque() const override { return sk_shader_->isOpaque(); } - - sk_sp skia_object() const override { return sk_shader_; } - - protected: - bool equals_(DlColorSource const& other) const override { - FML_DCHECK(other.type() == DlColorSourceType::kUnknown); - auto that = static_cast(&other); - return (sk_shader_ == that->sk_shader_); - } - - private: - sk_sp sk_shader_; - - FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlUnknownColorSource); -}; - } // namespace flutter #endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_COLOR_SOURCE_H_ diff --git a/display_list/display_list_color_source_unittests.cc b/display_list/display_list_color_source_unittests.cc index 8968ff503e1c0..a4c982c9c4d23 100644 --- a/display_list/display_list_color_source_unittests.cc +++ b/display_list/display_list_color_source_unittests.cc @@ -85,12 +85,6 @@ static constexpr SkPoint kTestPoints2[2] = { SkPoint::Make(100, 115), SkPoint::Make(107, 118), }; -static const sk_sp kShaderA = SkShaders::Color(SK_ColorRED); -static const sk_sp kShaderB = SkShaders::Color(SK_ColorBLUE); -static const sk_sp kTestUnknownShader = - SkShaders::Blend(SkBlendMode::kOverlay, kShaderA, kShaderB); -static const sk_sp kTestAlphaUnknownShader = - SkShaders::Blend(SkBlendMode::kDstOut, kShaderA, kShaderB); TEST(DisplayListColorSource, BuilderSetGet) { DlImageColorSource source(kTestImage1, DlTileMode::kClamp, DlTileMode::kClamp, @@ -105,155 +99,6 @@ TEST(DisplayListColorSource, BuilderSetGet) { ASSERT_EQ(builder.getColorSource(), nullptr); } -TEST(DisplayListColorSource, FromSkiaNullShader) { - std::shared_ptr source = DlColorSource::From(nullptr); - ASSERT_EQ(source, nullptr); - ASSERT_EQ(source.get(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaColorShader) { - // We cannot read back the matrix parameter from a Skia LinearGradient - // so we conservatively use an UnknownColorSource wrapper so as to not - // lose any data. Note that the Skia Color shader end is read back from - // the Skia asAGradient() method so while this type of color source - // does not really need the matrix, we represent all of the gradient - // sources using an unknown source. - // Note that this shader should never really happen in practice as it - // represents a degenerate gradient that collapsed to a single color. - sk_sp shader = SkShaders::Color(SK_ColorBLUE); - std::shared_ptr source = DlColorSource::From(shader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), shader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaImageShader) { - sk_sp shader = kTestImage1->skia_image()->makeShader( - ToSk(DlImageSampling::kLinear), &kTestMatrix1); - std::shared_ptr source = DlColorSource::From(shader); - DlImageColorSource dl_source(kTestImage1, DlTileMode::kClamp, - DlTileMode::kClamp, DlImageSampling::kLinear, - &kTestMatrix1); - ASSERT_EQ(source->type(), DlColorSourceType::kImage); - ASSERT_EQ(*source->asImage(), dl_source); - ASSERT_TRUE(source->asImage()->image()->Equals(kTestImage1)); - ASSERT_TRUE(kTestImage1->Equals(source->asImage()->image())); - ASSERT_EQ(source->asImage()->matrix(), kTestMatrix1); - ASSERT_EQ(source->asImage()->horizontal_tile_mode(), DlTileMode::kClamp); - ASSERT_EQ(source->asImage()->vertical_tile_mode(), DlTileMode::kClamp); - ASSERT_EQ(source->asImage()->sampling(), DlImageSampling::kLinear); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); - ASSERT_EQ(source->asRuntimeEffect(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaLinearGradient) { - // We cannot read back the matrix parameter from a Skia LinearGradient - // so we conservatively use an UnknownColorSource wrapper so as to not - // lose any data. - const SkColor* sk_colors = reinterpret_cast(kTestColors); - sk_sp shader = SkGradientShader::MakeLinear( - kTestPoints, sk_colors, kTestStops, kTestStopCount, SkTileMode::kClamp); - std::shared_ptr source = DlColorSource::From(shader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), shader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaRadialGradient) { - // We cannot read back the matrix parameter from a Skia RadialGradient - // so we conservatively use an UnknownColorSource wrapper so as to not - // lose any data. - const SkColor* sk_colors = reinterpret_cast(kTestColors); - sk_sp shader = - SkGradientShader::MakeRadial(kTestPoints[0], 10.0, sk_colors, kTestStops, - kTestStopCount, SkTileMode::kClamp); - std::shared_ptr source = DlColorSource::From(shader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), shader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); - ASSERT_EQ(source->asRuntimeEffect(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaConicalGradient) { - // We cannot read back the matrix parameter from a Skia ConicalGradient - // so we conservatively use an UnknownColorSource wrapper so as to not - // lose any data. - const SkColor* sk_colors = reinterpret_cast(kTestColors); - sk_sp shader = SkGradientShader::MakeTwoPointConical( - kTestPoints[0], 10.0, kTestPoints[1], 20.0, sk_colors, kTestStops, - kTestStopCount, SkTileMode::kClamp); - std::shared_ptr source = DlColorSource::From(shader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), shader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); - ASSERT_EQ(source->asRuntimeEffect(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaSweepGradient) { - // We cannot read back the matrix parameter, nor the sweep parameters from a - // Skia SweepGradient so we conservatively use an UnknownColorSource wrapper - // so as to not lose any data. - const SkColor* sk_colors = reinterpret_cast(kTestColors); - sk_sp shader = - SkGradientShader::MakeSweep(kTestPoints[0].fX, kTestPoints[0].fY, - sk_colors, kTestStops, kTestStopCount); - std::shared_ptr source = DlColorSource::From(shader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), shader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); - ASSERT_EQ(source->asRuntimeEffect(), nullptr); -} - -TEST(DisplayListColorSource, FromSkiaUnrecognizedShader) { - std::shared_ptr source = - DlColorSource::From(kTestUnknownShader); - ASSERT_EQ(source->type(), DlColorSourceType::kUnknown); - ASSERT_EQ(source->skia_object(), kTestUnknownShader); - - ASSERT_EQ(source->asColor(), nullptr); - ASSERT_EQ(source->asImage(), nullptr); - ASSERT_EQ(source->asLinearGradient(), nullptr); - ASSERT_EQ(source->asRadialGradient(), nullptr); - ASSERT_EQ(source->asConicalGradient(), nullptr); - ASSERT_EQ(source->asSweepGradient(), nullptr); - ASSERT_EQ(source->asRuntimeEffect(), nullptr); -} - TEST(DisplayListColorSource, ColorConstructor) { DlColorColorSource source(SK_ColorRED); } @@ -896,53 +741,6 @@ TEST(DisplayListColorSource, SweepGradientNotEquals) { } } -TEST(DisplayListColorSource, UnknownConstructor) { - DlUnknownColorSource source(kTestUnknownShader); -} - -TEST(DisplayListColorSource, UnknownShared) { - DlUnknownColorSource source(kTestUnknownShader); - ASSERT_NE(source.shared().get(), &source); - ASSERT_EQ(*source.shared(), source); -} - -TEST(DisplayListColorSource, UnknownAsNone) { - DlUnknownColorSource source(kTestUnknownShader); - ASSERT_EQ(source.asColor(), nullptr); - ASSERT_EQ(source.asImage(), nullptr); - ASSERT_EQ(source.asLinearGradient(), nullptr); - ASSERT_EQ(source.asRadialGradient(), nullptr); - ASSERT_EQ(source.asConicalGradient(), nullptr); - ASSERT_EQ(source.asSweepGradient(), nullptr); - ASSERT_EQ(source.asRuntimeEffect(), nullptr); -} - -TEST(DisplayListColorSource, UnknownContents) { - DlUnknownColorSource source(kTestUnknownShader); - ASSERT_EQ(source.skia_object(), kTestUnknownShader); - // Blend shaders always return false for is_opaque. - // See: https://bugs.chromium.org/p/skia/issues/detail?id=13046 - ASSERT_EQ(source.is_opaque(), false); -} - -TEST(DisplayListColorSource, AlphaUnknownContents) { - DlUnknownColorSource source(kTestAlphaUnknownShader); - ASSERT_EQ(source.skia_object(), kTestAlphaUnknownShader); - ASSERT_EQ(source.is_opaque(), false); -} - -TEST(DisplayListColorSource, UnknownEquals) { - DlUnknownColorSource source1(kTestUnknownShader); - DlUnknownColorSource source2(kTestUnknownShader); - TestEquals(source1, source2); -} - -TEST(DisplayListColorSource, UnknownNotEquals) { - DlUnknownColorSource source1(kTestUnknownShader); - DlUnknownColorSource source2(kTestAlphaUnknownShader); - TestNotEquals(source1, source2, "SkShader differs"); -} - TEST(DisplayListColorSource, RuntimeEffect) { std::shared_ptr source1 = DlColorSource::MakeRuntimeEffect( diff --git a/display_list/display_list_flags.cc b/display_list/display_list_flags.cc index 7d25b539a8116..741954a83cda2 100644 --- a/display_list/display_list_flags.cc +++ b/display_list/display_list_flags.cc @@ -8,21 +8,22 @@ namespace flutter { const DisplayListSpecialGeometryFlags DisplayListAttributeFlags::WithPathEffect( - const DlPathEffect* effect) const { + const DlPathEffect* effect, + bool is_stroked) const { if (is_geometric() && effect) { - if (effect->asDash()) { - // A dash effect has a very simple impact. It cannot introduce any - // miter joins that weren't already present in the original path - // and it does not grow the bounds of the path, but it can add - // end caps to areas that might not have had them before so all - // we need to do is to indicate the potential for diagonal - // end caps and move on. - return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_); - } else { - // An arbitrary path effect can introduce joins at an arbitrary - // angle and may change the geometry of the end caps - return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_ | - kMayHaveJoins_ | kMayHaveAcuteJoins_); + switch (effect->type()) { + case DlPathEffectType::kDash: { + // Dashing has no effect on filled geometry. + if (is_stroked) { + // A dash effect has a very simple impact. It cannot introduce any + // miter joins that weren't already present in the original path + // and it does not grow the bounds of the path, but it can add + // end caps to areas that might not have had them before so all + // we need to do is to indicate the potential for diagonal + // end caps and move on. + return special_flags_.with(kMayHaveCaps_ | kMayHaveDiagonalCaps_); + } + } } } return special_flags_; diff --git a/display_list/display_list_flags.h b/display_list/display_list_flags.h index 45f307372a212..15bdb661a33d1 100644 --- a/display_list/display_list_flags.h +++ b/display_list/display_list_flags.h @@ -168,7 +168,8 @@ class DisplayListSpecialGeometryFlags : DisplayListFlagsBase { class DisplayListAttributeFlags : DisplayListFlagsBase { public: const DisplayListSpecialGeometryFlags WithPathEffect( - const DlPathEffect* effect) const; + const DlPathEffect* effect, + bool is_stroked) const; constexpr bool ignores_paint() const { return has_any(kIgnoresPaint_); } diff --git a/display_list/display_list_image_filter.cc b/display_list/display_list_image_filter.cc index 4e43649a208e0..fbc97b87958dc 100644 --- a/display_list/display_list_image_filter.cc +++ b/display_list/display_list_image_filter.cc @@ -6,27 +6,6 @@ namespace flutter { -std::shared_ptr DlImageFilter::From( - const SkImageFilter* sk_filter) { - if (sk_filter == nullptr) { - return nullptr; - } - { - SkColorFilter* color_filter; - if (sk_filter->isColorFilterNode(&color_filter)) { - FML_DCHECK(color_filter != nullptr); - // If |isColorFilterNode| succeeds, the pointer it sets into color_filter - // will be ref'd already so we do not use sk_ref_sp() here as that would - // double-ref the color filter object. Instead we use a bare sk_sp - // constructor to adopt this reference into an sk_sp without - // reffing it and let the compiler manage the refs. - return std::make_shared( - DlColorFilter::From(sk_sp(color_filter))); - } - } - return std::make_shared(sk_ref_sp(sk_filter)); -} - std::shared_ptr DlImageFilter::makeWithLocalMatrix( const SkMatrix& matrix) const { if (matrix.isIdentity()) { diff --git a/display_list/display_list_image_filter.h b/display_list/display_list_image_filter.h index f8a084858ed42..5e29bdb6b677a 100644 --- a/display_list/display_list_image_filter.h +++ b/display_list/display_list_image_filter.h @@ -23,10 +23,7 @@ namespace flutter { // The objects here define operations that can take a location and one or // more input pixels and produce a color for that output pixel -// An enumerated type for the recognized ImageFilter operations. -// If a custom ImageFilter outside of the recognized types is needed -// then a |kUnknown| type that simply defers to an SkImageFilter is -// provided as a fallback. +// An enumerated type for the supported ImageFilter operations. enum class DlImageFilterType { kBlur, kDilate, @@ -35,7 +32,6 @@ enum class DlImageFilterType { kComposeFilter, kColorFilter, kLocalMatrixFilter, - kUnknown }; class DlBlurImageFilter; @@ -55,25 +51,6 @@ class DlImageFilter kComplex, }; - // Return a shared_ptr holding a DlImageFilter representing the indicated - // Skia SkImageFilter pointer. - // - // This method can only detect the ColorFilter type of ImageFilter from an - // analogous SkImageFilter as there are no "asA..." methods for the other - // types on SkImageFilter. - static std::shared_ptr From(const SkImageFilter* sk_filter); - - // Return a shared_ptr holding a DlImageFilter representing the indicated - // Skia SkImageFilter pointer. - // - // This method can only detect the ColorFilter type of ImageFilter from an - // analogous SkImageFilter as there are no "asA..." methods for the other - // types on SkImageFilter. - static std::shared_ptr From( - const sk_sp sk_filter) { - return From(sk_filter.get()); - } - // Return a DlBlurImageFilter pointer to this object iff it is a Blur // type of ImageFilter, otherwise return nullptr. virtual const DlBlurImageFilter* asBlur() const { return nullptr; } @@ -725,7 +702,7 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { protected: bool equals_(const DlImageFilter& other) const override { - FML_DCHECK(other.type() == DlImageFilterType::kMatrix); + FML_DCHECK(other.type() == DlImageFilterType::kLocalMatrixFilter); auto that = static_cast(&other); return (matrix_ == that->matrix_ && Equals(image_filter_, that->image_filter_)); @@ -736,89 +713,6 @@ class DlLocalMatrixImageFilter final : public DlImageFilter { 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. -// In fact, now that the DisplayListCanvasRecorder is deleted and the -// Paragraph code talks directly to a DisplayListBuilder, there may be -// no more reasons to maintain this sub-class. -// See: https://github.com/flutter/flutter/issues/121389 -class DlUnknownImageFilter final : public DlImageFilter { - public: - explicit DlUnknownImageFilter(sk_sp sk_filter) - : sk_filter_(std::move(sk_filter)) {} - explicit DlUnknownImageFilter(const SkImageFilter* sk_filter) - : sk_filter_(sk_ref_sp(sk_filter)) {} - explicit DlUnknownImageFilter(const DlUnknownImageFilter* filter) - : DlUnknownImageFilter(filter->sk_filter_) {} - explicit DlUnknownImageFilter(const DlUnknownImageFilter& filter) - : DlUnknownImageFilter(&filter) {} - - DlImageFilterType type() const override { - return DlImageFilterType::kUnknown; - } - size_t size() const override { return sizeof(*this); } - - std::shared_ptr shared() const override { - return std::make_shared(this); - } - - bool modifies_transparent_black() const override { - if (!sk_filter_) { - return false; - } - return !sk_filter_->canComputeFastBounds(); - } - - SkRect* map_local_bounds(const SkRect& input_bounds, - SkRect& output_bounds) const override { - if (!sk_filter_ || modifies_transparent_black()) { - output_bounds = input_bounds; - return nullptr; - } - output_bounds = sk_filter_->computeFastBounds(input_bounds); - return &output_bounds; - } - - SkIRect* map_device_bounds(const SkIRect& input_bounds, - const SkMatrix& ctm, - SkIRect& output_bounds) const override { - if (!sk_filter_ || modifies_transparent_black()) { - output_bounds = input_bounds; - return nullptr; - } - output_bounds = sk_filter_->filterBounds( - input_bounds, ctm, SkImageFilter::kForward_MapDirection); - return &output_bounds; - } - - SkIRect* get_input_device_bounds(const SkIRect& output_bounds, - const SkMatrix& ctm, - SkIRect& input_bounds) const override { - if (!sk_filter_ || modifies_transparent_black()) { - input_bounds = output_bounds; - return nullptr; - } - input_bounds = sk_filter_->filterBounds( - output_bounds, ctm, SkImageFilter::kReverse_MapDirection); - return &input_bounds; - } - - sk_sp skia_object() const override { return sk_filter_; } - - virtual ~DlUnknownImageFilter() = default; - - protected: - bool equals_(const DlImageFilter& other) const override { - FML_DCHECK(other.type() == DlImageFilterType::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_IMAGE_FILTER_H_ diff --git a/display_list/display_list_image_filter_unittests.cc b/display_list/display_list_image_filter_unittests.cc index 6170ab2bb6d0d..7e125dbd85ccb 100644 --- a/display_list/display_list_image_filter_unittests.cc +++ b/display_list/display_list_image_filter_unittests.cc @@ -32,120 +32,6 @@ TEST(DisplayListImageFilter, BuilderSetGet) { ASSERT_EQ(builder.getImageFilter(), nullptr); } -TEST(DisplayListImageFilter, FromSkiaNullFilter) { - std::shared_ptr filter = DlImageFilter::From(nullptr); - - ASSERT_EQ(filter, nullptr); - ASSERT_EQ(filter.get(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaBlurImageFilter) { - sk_sp sk_image_filter = - SkImageFilters::Blur(5.0, 5.0, SkTileMode::kRepeat, nullptr); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - - ASSERT_EQ(filter->type(), DlImageFilterType::kUnknown); - - // We cannot recapture the blur parameters from an SkBlurImageFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_EQ(filter->asColorFilter(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaDilateImageFilter) { - sk_sp sk_image_filter = - SkImageFilters::Dilate(5.0, 5.0, nullptr); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - - ASSERT_EQ(filter->type(), DlImageFilterType::kUnknown); - - // We cannot recapture the dilate parameters from an SkDilateImageFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_EQ(filter->asColorFilter(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaErodeImageFilter) { - sk_sp sk_image_filter = - SkImageFilters::Erode(5.0, 5.0, nullptr); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - - ASSERT_EQ(filter->type(), DlImageFilterType::kUnknown); - - // We cannot recapture the erode parameters from an SkErodeImageFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_EQ(filter->asColorFilter(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaMatrixImageFilter) { - sk_sp sk_image_filter = SkImageFilters::MatrixTransform( - SkMatrix::RotateDeg(45), ToSk(DlImageSampling::kLinear), nullptr); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - - ASSERT_EQ(filter->type(), DlImageFilterType::kUnknown); - - // We cannot recapture the blur parameters from an SkMatrixImageFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_EQ(filter->asColorFilter(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaComposeImageFilter) { - sk_sp sk_blur_filter = - SkImageFilters::Blur(5.0, 5.0, SkTileMode::kRepeat, nullptr); - sk_sp sk_matrix_filter = SkImageFilters::MatrixTransform( - SkMatrix::RotateDeg(45), ToSk(DlImageSampling::kLinear), nullptr); - sk_sp sk_image_filter = - SkImageFilters::Compose(sk_blur_filter, sk_matrix_filter); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - - ASSERT_EQ(filter->type(), DlImageFilterType::kUnknown); - - // We cannot recapture the blur parameters from an SkComposeImageFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_EQ(filter->asColorFilter(), nullptr); -} - -TEST(DisplayListImageFilter, FromSkiaColorFilterImageFilter) { - sk_sp sk_color_filter = - SkColorFilters::Blend(SK_ColorRED, SkBlendMode::kSrcIn); - sk_sp sk_image_filter = - SkImageFilters::ColorFilter(sk_color_filter, nullptr); - std::shared_ptr filter = DlImageFilter::From(sk_image_filter); - DlBlendColorFilter dl_color_filter(DlColor::kRed(), DlBlendMode::kSrcIn); - DlColorFilterImageFilter dl_image_filter(dl_color_filter.shared()); - - ASSERT_EQ(filter->type(), DlImageFilterType::kColorFilter); - - ASSERT_TRUE(*filter->asColorFilter() == dl_image_filter); - ASSERT_EQ(*filter.get(), dl_image_filter); - ASSERT_EQ(*filter->asColorFilter()->color_filter(), dl_color_filter); - - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->asDilate(), nullptr); - ASSERT_EQ(filter->asErode(), nullptr); - ASSERT_EQ(filter->asMatrix(), nullptr); - ASSERT_EQ(filter->asCompose(), nullptr); - ASSERT_NE(filter->asColorFilter(), nullptr); -} - // SkRect::contains treats the rect as a half-open interval which is // appropriate for so many operations. Unfortunately, we are using // it here to test containment of the corners of a transformed quad @@ -308,6 +194,15 @@ TEST(DisplayListImageFilter, BlurEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, BlurWithLocalMatrixEquals) { + DlBlurImageFilter filter1(5.0, 6.0, DlTileMode::kMirror); + DlBlurImageFilter filter2(5.0, 6.0, DlTileMode::kMirror); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, BlurNotEquals) { DlBlurImageFilter filter1(5.0, 6.0, DlTileMode::kMirror); DlBlurImageFilter filter2(7.0, 6.0, DlTileMode::kMirror); @@ -358,6 +253,15 @@ TEST(DisplayListImageFilter, DilateEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, DilateWithLocalMatrixEquals) { + DlDilateImageFilter filter1(5.0, 6.0); + DlDilateImageFilter filter2(5.0, 6.0); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, DilateNotEquals) { DlDilateImageFilter filter1(5.0, 6.0); DlDilateImageFilter filter2(7.0, 6.0); @@ -406,6 +310,15 @@ TEST(DisplayListImageFilter, ErodeEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, ErodeWithLocalMatrixEquals) { + DlErodeImageFilter filter1(5.0, 6.0); + DlErodeImageFilter filter2(5.0, 6.0); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, ErodeNotEquals) { DlErodeImageFilter filter1(5.0, 6.0); DlErodeImageFilter filter2(7.0, 6.0); @@ -469,6 +382,18 @@ TEST(DisplayListImageFilter, MatrixEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, MatrixWithLocalMatrixEquals) { + SkMatrix matrix = SkMatrix::MakeAll(2.0, 0.0, 10, // + 0.5, 3.0, 15, // + 0.0, 0.0, 1); + DlMatrixImageFilter filter1(matrix, DlImageSampling::kLinear); + DlMatrixImageFilter filter2(matrix, DlImageSampling::kLinear); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, MatrixNotEquals) { SkMatrix matrix1 = SkMatrix::MakeAll(2.0, 0.0, 10, // 0.5, 3.0, 15, // @@ -564,6 +489,26 @@ TEST(DisplayListImageFilter, ComposeEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, ComposeWithLocalMatrixEquals) { + DlMatrixImageFilter outer1(SkMatrix::MakeAll(2.0, 0.0, 10, // + 0.5, 3.0, 15, // + 0.0, 0.0, 1), + DlImageSampling::kLinear); + DlBlurImageFilter inner1(5.0, 6.0, DlTileMode::kMirror); + DlComposeImageFilter filter1(outer1, inner1); + + DlMatrixImageFilter outer2(SkMatrix::MakeAll(2.0, 0.0, 10, // + 0.5, 3.0, 15, // + 0.0, 0.0, 1), + DlImageSampling::kLinear); + DlBlurImageFilter inner2(5.0, 6.0, DlTileMode::kMirror); + DlComposeImageFilter filter2(outer1, inner1); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, ComposeNotEquals) { DlMatrixImageFilter outer1(SkMatrix::MakeAll(2.0, 0.0, 10, // 0.5, 3.0, 15, // @@ -714,6 +659,18 @@ TEST(DisplayListImageFilter, ColorFilterEquals) { TestEquals(filter1, filter2); } +TEST(DisplayListImageFilter, ColorFilterWithLocalMatrixEquals) { + DlBlendColorFilter dl_color_filter1(DlColor::kRed(), DlBlendMode::kLighten); + DlColorFilterImageFilter filter1(dl_color_filter1); + + DlBlendColorFilter dl_color_filter2(DlColor::kRed(), DlBlendMode::kLighten); + DlColorFilterImageFilter filter2(dl_color_filter2); + + SkMatrix local_matrix = SkMatrix::Translate(10, 10); + TestEquals(*filter1.makeWithLocalMatrix(local_matrix), + *filter2.makeWithLocalMatrix(local_matrix)); +} + TEST(DisplayListImageFilter, ColorFilterNotEquals) { DlBlendColorFilter dl_color_filter1(DlColor::kRed(), DlBlendMode::kLighten); DlColorFilterImageFilter filter1(dl_color_filter1); @@ -742,28 +699,6 @@ TEST(DisplayListImageFilter, ColorFilterModifiesTransparencyBounds) { TestInvalidBounds(filter, SkMatrix::I(), input_bounds); } -TEST(DisplayListImageFilter, UnknownConstructor) { - DlUnknownImageFilter filter( - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr)); -} - -TEST(DisplayListImageFilter, UnknownShared) { - DlUnknownImageFilter filter( - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr)); - - ASSERT_NE(filter.shared().get(), &filter); - ASSERT_EQ(*filter.shared(), filter); -} - -TEST(DisplayListImageFilter, UnknownContents) { - sk_sp sk_filter = - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); - DlUnknownImageFilter filter(sk_filter); - - ASSERT_EQ(filter.skia_object(), sk_filter); - 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, // @@ -860,27 +795,5 @@ TEST(DisplayListImageFilter, LocalImageSkiaNull) { ASSERT_EQ(dl_local_matrix_filter.skia_object(), nullptr); } -TEST(DisplayListImageFilter, UnknownEquals) { - sk_sp sk_filter = - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr); - - DlUnknownImageFilter filter1(sk_filter); - DlUnknownImageFilter filter2(sk_filter); - - TestEquals(filter1, filter2); -} - -TEST(DisplayListImageFilter, UnknownNotEquals) { - DlUnknownImageFilter filter1( - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr)); - DlUnknownImageFilter filter2( - SkImageFilters::Blur(5.0, 6.0, SkTileMode::kRepeat, nullptr)); - - // Even though the filter is the same, it is a different instance - // and we cannot currently tell them apart because the Skia - // ImageFilter objects do not implement == - TestNotEquals(filter1, filter2, "SkImageFilter instance differs"); -} - } // namespace testing } // namespace flutter diff --git a/display_list/display_list_mask_filter.cc b/display_list/display_list_mask_filter.cc index fe762066e2fbe..4095d2df1ff53 100644 --- a/display_list/display_list_mask_filter.cc +++ b/display_list/display_list_mask_filter.cc @@ -6,13 +6,6 @@ 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)); -} +// Just exists to ensure that the header can be cleanly imported. } // namespace flutter diff --git a/display_list/display_list_mask_filter.h b/display_list/display_list_mask_filter.h index 9210d45a2b318..7e8c67826ea99 100644 --- a/display_list/display_list_mask_filter.h +++ b/display_list/display_list_mask_filter.h @@ -17,35 +17,12 @@ class DlBlurMaskFilter; // facilities and adheres to the design goals of the |DlAttribute| base // class. -// 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 class DlMaskFilterType { kBlur, kUnknown }; +// An enumerated type for the supported MaskFilter operations. +enum class DlMaskFilterType { kBlur }; class DlMaskFilter : public DlAttribute { public: - // 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 method, 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 a DlBlurMaskFilter pointer to this object iff it is a Blur // type of MaskFilter, otherwise return nullptr. virtual const DlBlurMaskFilter* asBlur() const { return nullptr; } @@ -99,44 +76,6 @@ class DlBlurMaskFilter final : public DlMaskFilter { bool respect_ctm_; }; -// 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. -// In fact, now that the DisplayListCanvasRecorder is deleted and the -// Paragraph code talks directly to a DisplayListBuilder, there may be -// no more reasons to maintain this sub-class. -// See: https://github.com/flutter/flutter/issues/121389 -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_) {} - - DlMaskFilterType type() const override { return DlMaskFilterType::kUnknown; } - size_t size() const override { return sizeof(*this); } - - std::shared_ptr shared() const override { - return std::make_shared(this); - } - - sk_sp skia_object() const override { return sk_filter_; } - - virtual ~DlUnknownMaskFilter() = default; - - protected: - bool equals_(const DlMaskFilter& other) const override { - FML_DCHECK(other.type() == DlMaskFilterType::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/display_list/display_list_mask_filter_unittests.cc b/display_list/display_list_mask_filter_unittests.cc index a508dd25b7873..5ae2261c63204 100644 --- a/display_list/display_list_mask_filter_unittests.cc +++ b/display_list/display_list_mask_filter_unittests.cc @@ -24,22 +24,6 @@ TEST(DisplayListMaskFilter, BuilderSetGet) { ASSERT_EQ(builder.getMaskFilter(), nullptr); } -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(), DlMaskFilterType::kUnknown); - // We cannot recapture the blur parameters from an SkBlurMaskFilter - ASSERT_EQ(filter->asBlur(), nullptr); - ASSERT_EQ(filter->skia_object(), sk_filter); -} - TEST(DisplayListMaskFilter, BlurConstructor) { DlBlurMaskFilter filter(SkBlurStyle::kNormal_SkBlurStyle, 5.0); } @@ -76,45 +60,6 @@ TEST(DisplayListMaskFilter, BlurNotEquals) { TestNotEquals(filter1, filter3, "blur radius differs"); } -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.skia_object(), sk_filter); - ASSERT_EQ(filter.skia_object().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); - TestEquals(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)); - TestNotEquals(filter1, filter2, "SkMaskFilter instance differs"); -} - void testEquals(DlMaskFilter* a, DlMaskFilter* b) { // a and b have the same nullness or values ASSERT_TRUE(Equals(a, b)); diff --git a/display_list/display_list_ops.h b/display_list/display_list_ops.h index d5d21086cf1d8..fb949b5419b7c 100644 --- a/display_list/display_list_ops.h +++ b/display_list/display_list_ops.h @@ -200,9 +200,6 @@ struct SetBlendModeOp final : DLOp { // 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, sk_name, field) \ struct Clear##name##Op final : DLOp { \ static const auto kType = DisplayListOpType::kClear##name; \ @@ -222,18 +219,6 @@ struct SetBlendModeOp final : DLOp { const Dl##name* filter = reinterpret_cast(this + 1); \ ctx.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(DispatchContext& ctx) const { \ - DlUnknown##name dl_filter(field); \ - ctx.dispatcher.set##name(&dl_filter); \ - } \ }; DEFINE_SET_CLEAR_DLATTR_OP(ColorFilter, ColorFilter, filter) DEFINE_SET_CLEAR_DLATTR_OP(ImageFilter, ImageFilter, filter) diff --git a/display_list/display_list_path_effect.cc b/display_list/display_list_path_effect.cc index 72297464e4f15..2b203d564924f 100644 --- a/display_list/display_list_path_effect.cc +++ b/display_list/display_list_path_effect.cc @@ -22,26 +22,6 @@ static void DlPathEffectDeleter(void* p) { ::operator delete(p); } -std::shared_ptr DlPathEffect::From(SkPathEffect* sk_path_effect) { - if (sk_path_effect == nullptr) { - return nullptr; - } - - SkPathEffect::DashInfo info; - if (SkPathEffect::DashType::kDash_DashType == - sk_path_effect->asADash(&info)) { - auto dash_path_effect = - DlDashPathEffect::Make(nullptr, info.fCount, info.fPhase); - info.fIntervals = - reinterpret_cast(dash_path_effect.get()) - ->intervals_unsafe(); - sk_path_effect->asADash(&info); - return dash_path_effect; - } - // If not dash path effect, we will use UnknownPathEffect to wrap it. - return std::make_shared(sk_ref_sp(sk_path_effect)); -} - std::shared_ptr DlDashPathEffect::Make(const SkScalar* intervals, int count, SkScalar phase) { @@ -60,16 +40,4 @@ std::optional DlDashPathEffect::effect_bounds(SkRect& rect) const { return rect; } -std::optional DlUnknownPathEffect::effect_bounds(SkRect& rect) const { - if (!rect.isSorted()) { - return std::nullopt; - } - SkPaint p; - p.setPathEffect(sk_path_effect_); - if (!p.canComputeFastBounds()) { - return std::nullopt; - } - return p.computeFastBounds(rect, &rect); -} - } // namespace flutter diff --git a/display_list/display_list_path_effect.h b/display_list/display_list_path_effect.h index cf98d698f1c64..d03f1944dfc4d 100644 --- a/display_list/display_list_path_effect.h +++ b/display_list/display_list_path_effect.h @@ -20,26 +20,14 @@ class DlDashPathEffect; // facilities and adheres to the design goals of the |DlAttribute| base // class. -// An enumerated type for the recognized PathEffect operations. -// In current Flutter we only use the DashPathEffect. -// And another PathEffect outside of the recognized types is needed -// then a |kUnknown| type that simply defers to an SkPathEffect is -// provided as a fallback. +// An enumerated type for the supported PathEffect operations. enum class DlPathEffectType { kDash, - kUnknown, }; class DlPathEffect : public DlAttribute { public: - static std::shared_ptr From(SkPathEffect* sk_path_effect); - - static std::shared_ptr From( - sk_sp sk_path_effect) { - return From(sk_path_effect.get()); - } - virtual const DlDashPathEffect* asDash() const { return nullptr; } virtual std::optional effect_bounds(SkRect&) const = 0; @@ -136,43 +124,6 @@ class DlDashPathEffect final : public DlPathEffect { FML_DISALLOW_COPY_ASSIGN_AND_MOVE(DlDashPathEffect); }; -// Now that the DisplayListCanvasRecorder is deleted and the -// Paragraph code talks directly to a DisplayListBuilder, there may be -// no more reasons to maintain this sub-class. -// See: https://github.com/flutter/flutter/issues/121389 -class DlUnknownPathEffect final : public DlPathEffect { - public: - DlUnknownPathEffect(sk_sp effect) - : sk_path_effect_(std::move(effect)) {} - DlUnknownPathEffect(const DlUnknownPathEffect& effect) - : DlUnknownPathEffect(effect.sk_path_effect_) {} - DlUnknownPathEffect(const DlUnknownPathEffect* effect) - : DlUnknownPathEffect(effect->sk_path_effect_) {} - - DlPathEffectType type() const override { return DlPathEffectType::kUnknown; } - size_t size() const override { return sizeof(*this); } - - std::shared_ptr shared() const override { - return std::make_shared(this); - } - - sk_sp skia_object() const override { return sk_path_effect_; } - - virtual ~DlUnknownPathEffect() = default; - - std::optional effect_bounds(SkRect& rect) const override; - - protected: - bool equals_(const DlPathEffect& other) const override { - FML_DCHECK(other.type() == DlPathEffectType::kUnknown); - auto that = static_cast(&other); - return sk_path_effect_ == that->sk_path_effect_; - } - - private: - sk_sp sk_path_effect_; -}; - } // namespace flutter #endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_PATH_EFFECT_H_ diff --git a/display_list/display_list_path_effect_unittests.cc b/display_list/display_list_path_effect_unittests.cc index f9bdc0dc8c641..0067c37c3234b 100644 --- a/display_list/display_list_path_effect_unittests.cc +++ b/display_list/display_list_path_effect_unittests.cc @@ -26,24 +26,6 @@ TEST(DisplayListPathEffect, BuilderSetGet) { ASSERT_EQ(builder.getPathEffect(), nullptr); } -TEST(DisplayListPathEffect, FromSkiaNullPathEffect) { - std::shared_ptr path_effect = DlPathEffect::From(nullptr); - ASSERT_EQ(path_effect, nullptr); - ASSERT_EQ(path_effect.get(), nullptr); -} - -TEST(DisplayListPathEffect, FromSkiaPathEffect) { - const SkScalar TestDashes2[] = {1.0, 1.5}; - sk_sp sk_path_effect = - SkDashPathEffect::Make(TestDashes2, 2, 0.0); - std::shared_ptr dl_path_effect = - DlPathEffect::From(sk_path_effect); - - ASSERT_EQ(dl_path_effect->type(), DlPathEffectType::kDash); - ASSERT_TRUE( - Equals(dl_path_effect, DlDashPathEffect::Make(TestDashes2, 2, 0.0))); -} - TEST(DisplayListPathEffect, EffectShared) { const SkScalar TestDashes2[] = {1.0, 1.5}; auto effect = DlDashPathEffect::Make(TestDashes2, 2, 0.0); @@ -81,44 +63,5 @@ TEST(DisplayListPathEffect, CheckEffectProperties) { TestNotEquals(*effect1, *effect5, "Dash phase differs"); } -TEST(DisplayListPathEffect, UnknownConstructor) { - const SkScalar test_dashes[] = {4.0, 2.0}; - DlUnknownPathEffect path_effect(SkDashPathEffect::Make(test_dashes, 2, 0.0)); -} - -TEST(DisplayListPathEffect, UnknownShared) { - const SkScalar test_dashes[] = {4.0, 2.0}; - DlUnknownPathEffect path_effect(SkDashPathEffect::Make(test_dashes, 2, 0.0)); - ASSERT_NE(path_effect.shared().get(), &path_effect); - ASSERT_EQ(*path_effect.shared(), path_effect); -} - -TEST(DisplayListPathEffect, UnknownContents) { - const SkScalar test_dashes[] = {4.0, 2.0}; - sk_sp sk_effect = SkDashPathEffect::Make(test_dashes, 2, 0.0); - DlUnknownPathEffect effect(sk_effect); - ASSERT_EQ(effect.skia_object(), sk_effect); - ASSERT_EQ(effect.skia_object().get(), sk_effect.get()); -} - -TEST(DisplayListPathEffect, UnknownEquals) { - const SkScalar test_dashes[] = {4.0, 2.0}; - sk_sp sk_effect = SkDashPathEffect::Make(test_dashes, 2, 0.0); - DlUnknownPathEffect effect1(sk_effect); - DlUnknownPathEffect effect2(sk_effect); - TestEquals(effect1, effect1); -} - -TEST(DisplayListPathEffect, UnknownNotEquals) { - const SkScalar test_dashes[] = {4.0, 2.0}; - // Even though the effect is the same, it is a different instance - // and we cannot currently tell them apart because the Skia - // DashEffect::Make objects do not implement == - DlUnknownPathEffect path_effect1(SkDashPathEffect::Make(test_dashes, 2, 0.0)); - DlUnknownPathEffect path_effect2(SkDashPathEffect::Make(test_dashes, 2, 0.0)); - TestNotEquals(path_effect1, path_effect2, - "SkDashPathEffect instance differs"); -} - } // namespace testing } // namespace flutter diff --git a/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index bee9babd30144..741b0dbb5b10a 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/display_list/testing/dl_test_snippets.cc @@ -116,7 +116,7 @@ std::vector CreateAllAttributesOps() { {0, 0, 0, 0, [](DisplayListBuilder& b) { b.setColor(SK_ColorBLACK); }}, }}, - {"SetBlendModeOrBlender", + {"SetBlendMode", { {0, 8, 0, 0, [](DisplayListBuilder& b) { b.setBlendMode(DlBlendMode::kSrcIn); }}, @@ -227,6 +227,13 @@ std::vector CreateAllAttributesOps() { }}, {0, 0, 0, 0, [](DisplayListBuilder& b) { b.setImageFilter(nullptr); }}, + {0, 24, 0, 0, + [](DisplayListBuilder& b) { + b.setImageFilter( + kTestBlurImageFilter1 + .makeWithLocalMatrix(SkMatrix::Translate(2, 2)) + .get()); + }}, }}, {"SetColorFilter", { diff --git a/flow/layers/color_filter_layer_unittests.cc b/flow/layers/color_filter_layer_unittests.cc index c6d453e936960..e2678e428767a 100644 --- a/flow/layers/color_filter_layer_unittests.cc +++ b/flow/layers/color_filter_layer_unittests.cc @@ -28,8 +28,7 @@ using ColorFilterLayerTest = LayerTest; #ifndef NDEBUG TEST_F(ColorFilterLayerTest, PaintingEmptyLayerDies) { - auto layer = std::make_shared( - std::make_shared(sk_sp())); + auto layer = std::make_shared(nullptr); layer->Preroll(preroll_context()); EXPECT_EQ(layer->paint_bounds(), kEmptyRect); @@ -45,8 +44,7 @@ TEST_F(ColorFilterLayerTest, PaintBeforePrerollDies) { const SkPath child_path = SkPath().addRect(child_bounds); auto mock_layer = std::make_shared(child_path); - auto layer = std::make_shared( - std::make_shared(sk_sp())); + auto layer = std::make_shared(nullptr); layer->Add(mock_layer); EXPECT_EQ(layer->paint_bounds(), kEmptyRect); diff --git a/flow/layers/shader_mask_layer_unittests.cc b/flow/layers/shader_mask_layer_unittests.cc index b4d55ee2e7da7..886f0d00de198 100644 --- a/flow/layers/shader_mask_layer_unittests.cc +++ b/flow/layers/shader_mask_layer_unittests.cc @@ -21,6 +21,19 @@ namespace testing { using ShaderMaskLayerTest = LayerTest; +static std::shared_ptr MakeFilter(DlColor color) { + DlColor colors[] = { + color.withAlpha(0x7f), + color, + }; + float stops[] = { + 0, + 1, + }; + return DlColorSource::MakeLinear(SkPoint::Make(0, 0), SkPoint::Make(10, 10), + 2, colors, stops, DlTileMode::kRepeat); +} + #ifndef NDEBUG TEST_F(ShaderMaskLayerTest, PaintingEmptyLayerDies) { auto layer = @@ -98,9 +111,7 @@ TEST_F(ShaderMaskLayerTest, SimpleFilter) { const SkRect layer_bounds = SkRect::MakeLTRB(2.0f, 4.0f, 6.5f, 6.5f); const SkPath child_path = SkPath().addRect(child_bounds); const DlPaint child_paint = DlPaint(DlColor::kYellow()); - auto layer_filter = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter = DlColorSource::From(layer_filter); + auto dl_filter = MakeFilter(DlColor::kBlue()); auto mock_layer = std::make_shared(child_path, child_paint); auto layer = std::make_shared(dl_filter, layer_bounds, DlBlendMode::kSrc); @@ -144,9 +155,7 @@ TEST_F(ShaderMaskLayerTest, MultipleChildren) { SkPath().addRect(child_bounds.makeOffset(3.0f, 0.0f)); const DlPaint child_paint1 = DlPaint(DlColor::kYellow()); const DlPaint child_paint2 = DlPaint(DlColor::kCyan()); - auto layer_filter = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter = DlColorSource::From(layer_filter); + auto dl_filter = MakeFilter(DlColor::kBlue()); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); auto layer = std::make_shared(dl_filter, layer_bounds, @@ -201,12 +210,8 @@ TEST_F(ShaderMaskLayerTest, Nested) { SkPath().addRect(child_bounds.makeOffset(3.0f, 0.0f)); const DlPaint child_paint1 = DlPaint(DlColor::kYellow()); const DlPaint child_paint2 = DlPaint(DlColor::kCyan()); - auto layer_filter1 = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter1 = DlColorSource::From(layer_filter1); - auto layer_filter2 = - SkPerlinNoiseShader::MakeFractalNoise(2.0f, 2.0f, 2, 2.0f); - auto dl_filter2 = DlColorSource::From(layer_filter2); + auto dl_filter1 = MakeFilter(DlColor::kGreen()); + auto dl_filter2 = MakeFilter(DlColor::kMagenta()); auto mock_layer1 = std::make_shared(child_path1, child_paint1); auto mock_layer2 = std::make_shared(child_path2, child_paint2); auto layer1 = std::make_shared(dl_filter1, layer_bounds, @@ -275,9 +280,7 @@ TEST_F(ShaderMaskLayerTest, Nested) { TEST_F(ShaderMaskLayerTest, Readback) { const SkRect layer_bounds = SkRect::MakeLTRB(2.0f, 4.0f, 20.5f, 20.5f); - auto layer_filter = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter = DlColorSource::From(layer_filter); + auto dl_filter = MakeFilter(DlColor::kBlue()); auto layer = std::make_shared(dl_filter, layer_bounds, DlBlendMode::kSrc); @@ -296,9 +299,7 @@ TEST_F(ShaderMaskLayerTest, Readback) { } TEST_F(ShaderMaskLayerTest, LayerCached) { - auto layer_filter = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter = DlColorSource::From(layer_filter); + auto dl_filter = MakeFilter(DlColor::kBlue()); DlPaint paint; const SkRect layer_bounds = SkRect::MakeLTRB(2.0f, 4.0f, 20.5f, 20.5f); auto initial_transform = SkMatrix::Translate(50.0, 25.5); @@ -405,9 +406,7 @@ TEST_F(ShaderMaskLayerTest, SimpleFilterWithRasterCache) { const SkRect layer_bounds = SkRect::MakeLTRB(2.0f, 4.0f, 6.5f, 6.5f); const SkPath child_path = SkPath().addRect(child_bounds); const DlPaint child_paint = DlPaint(DlColor::kYellow()); - auto layer_filter = - SkPerlinNoiseShader::MakeFractalNoise(1.0f, 1.0f, 1, 1.0f); - auto dl_filter = DlColorSource::From(layer_filter); + auto dl_filter = MakeFilter(DlColor::kBlue()); auto mock_layer = std::make_shared(child_path, child_paint); auto layer = std::make_shared(dl_filter, layer_bounds, DlBlendMode::kSrc); diff --git a/flow/paint_utils.cc b/flow/paint_utils.cc index b594ccd8ce365..290c5157126fa 100644 --- a/flow/paint_utils.cc +++ b/flow/paint_utils.cc @@ -14,14 +14,18 @@ namespace flutter { namespace { -sk_sp CreateCheckerboardShader(SkColor c1, SkColor c2, int size) { +std::shared_ptr CreateCheckerboardShader(SkColor c1, + SkColor c2, + int size) { SkBitmap bm; bm.allocN32Pixels(2 * size, 2 * size); bm.eraseColor(c1); bm.eraseArea(SkIRect::MakeLTRB(0, 0, size, size), c2); bm.eraseArea(SkIRect::MakeLTRB(size, size, 2 * size, 2 * size), c2); - return bm.makeShader(SkTileMode::kRepeat, SkTileMode::kRepeat, - SkSamplingOptions()); + auto image = DlImage::Make(SkImage::MakeFromBitmap(bm)); + return std::make_shared( + image, DlTileMode::kRepeat, DlTileMode::kRepeat, + DlImageSampling::kNearestNeighbor); } } // anonymous namespace @@ -38,8 +42,8 @@ void DrawCheckerboard(DlCanvas* canvas, const SkRect& rect) { // NOLINTEND(clang-analyzer-security.insecureAPI.rand) DlPaint paint; - paint.setColorSource(DlColorSource::From( - CreateCheckerboardShader(checkerboard_color, 0x00000000, 12))); + paint.setColorSource( + CreateCheckerboardShader(checkerboard_color, 0x00000000, 12)); canvas->DrawPaint(paint); canvas->Restore(); diff --git a/impeller/display_list/display_list_dispatcher.cc b/impeller/display_list/display_list_dispatcher.cc index 1d5dba8e76dfb..afe8e8422722c 100644 --- a/impeller/display_list/display_list_dispatcher.cc +++ b/impeller/display_list/display_list_dispatcher.cc @@ -342,8 +342,6 @@ static std::optional ToColorSourceType( case flutter::DlColorSourceType::kScene: return Paint::ColorSourceType::kScene; #endif // IMPELLER_ENABLE_3D - case flutter::DlColorSourceType::kUnknown: - return std::nullopt; } } @@ -581,9 +579,6 @@ static std::optional ToColorFilterProc( return [](FilterInput::Ref input) { return ColorFilterContents::MakeLinearToSrgbFilter({std::move(input)}); }; - case flutter::DlColorFilterType::kUnknown: - FML_LOG(ERROR) << "Requested DlColorFilterType::kUnknown"; - UNIMPLEMENTED; } return std::nullopt; } @@ -641,9 +636,6 @@ void DisplayListDispatcher::setMaskFilter(const flutter::DlMaskFilter* filter) { }; break; } - case flutter::DlMaskFilterType::kUnknown: - UNIMPLEMENTED; - break; } } @@ -771,8 +763,6 @@ static std::optional ToImageFilterProc( }; break; } - case flutter::DlImageFilterType::kUnknown: - return std::nullopt; } } diff --git a/testing/display_list_testing.cc b/testing/display_list_testing.cc index d17a40b00dcb8..2caadcb7245ad 100644 --- a/testing/display_list_testing.cc +++ b/testing/display_list_testing.cc @@ -569,11 +569,15 @@ void DisplayListStreamDispatcher::out(const DlImageFilter& filter) { os_ << "DlComposeImageFilter(" << std::endl; indent(); startl() << "outer: "; + indent(7); out(compose->outer().get()); os_ << "," << std::endl; + outdent(7); startl() << "inner: "; + indent(7); out(compose->inner().get()); - os_ << "," << std::endl; + os_ << std::endl; + outdent(7); outdent(); startl() << ")"; break; @@ -586,9 +590,19 @@ void DisplayListStreamDispatcher::out(const DlImageFilter& filter) { os_ << ")"; break; } - default: - os_ << "DlUnknownImageFilter(" << filter.skia_object().get() << ")"; + case DlImageFilterType::kLocalMatrixFilter: { + const DlLocalMatrixImageFilter* local_matrix = filter.asLocalMatrix(); + FML_DCHECK(local_matrix); + os_ << "DlLocalMatrixImageFilter(" << local_matrix->matrix(); + os_ << "," << std::endl; + indent(25); + startl() << "filter: "; + out(local_matrix->image_filter().get()); + os_ << std::endl; + outdent(25); + startl() << ")"; break; + } } } void DisplayListStreamDispatcher::out(const DlImageFilter* filter) { @@ -596,12 +610,16 @@ void DisplayListStreamDispatcher::out(const DlImageFilter* filter) { os_ << "no ImageFilter"; } else { os_ << "&"; + indent(1); out(*filter); + outdent(1); } } void DisplayListStreamDispatcher::setImageFilter(const DlImageFilter* filter) { startl() << "setImageFilter("; + indent(15); out(filter); + outdent(15); os_ << ");" << std::endl; } void DisplayListStreamDispatcher::save() {