Skip to content

Commit

Permalink
[DisplayList] delete obsolete PathEffect sources (#53441)
Browse files Browse the repository at this point in the history
As of #53411 there are no more (non-test) references to path effects anywhere in the engine.

Deleting the dead code.
  • Loading branch information
flar authored Jun 18, 2024
1 parent 2d5c28e commit 74f42ca
Show file tree
Hide file tree
Showing 35 changed files with 18 additions and 435 deletions.
1 change: 0 additions & 1 deletion ci/licenses_golden/excluded_files
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@
../../../flutter/display_list/effects/dl_color_source_unittests.cc
../../../flutter/display_list/effects/dl_image_filter_unittests.cc
../../../flutter/display_list/effects/dl_mask_filter_unittests.cc
../../../flutter/display_list/effects/dl_path_effect_unittests.cc
../../../flutter/display_list/geometry/dl_geometry_types_unittests.cc
../../../flutter/display_list/geometry/dl_region_unittests.cc
../../../flutter/display_list/geometry/dl_rtree_unittests.cc
Expand Down
4 changes: 0 additions & 4 deletions ci/licenses_golden/licenses_flutter
Original file line number Diff line number Diff line change
Expand Up @@ -41545,8 +41545,6 @@ ORIGIN: ../../../flutter/display_list/effects/dl_image_filter.cc + ../../../flut
ORIGIN: ../../../flutter/display_list/effects/dl_image_filter.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_mask_filter.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_mask_filter.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_path_effect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE
ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE
Expand Down Expand Up @@ -44411,8 +44409,6 @@ FILE: ../../../flutter/display_list/effects/dl_image_filter.cc
FILE: ../../../flutter/display_list/effects/dl_image_filter.h
FILE: ../../../flutter/display_list/effects/dl_mask_filter.cc
FILE: ../../../flutter/display_list/effects/dl_mask_filter.h
FILE: ../../../flutter/display_list/effects/dl_path_effect.cc
FILE: ../../../flutter/display_list/effects/dl_path_effect.h
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
Expand Down
3 changes: 0 additions & 3 deletions display_list/BUILD.gn
Original file line number Diff line number Diff line change
Expand Up @@ -56,8 +56,6 @@ source_set("display_list") {
"effects/dl_image_filter.h",
"effects/dl_mask_filter.cc",
"effects/dl_mask_filter.h",
"effects/dl_path_effect.cc",
"effects/dl_path_effect.h",
"effects/dl_runtime_effect.cc",
"effects/dl_runtime_effect.h",
"geometry/dl_geometry_types.h",
Expand Down Expand Up @@ -119,7 +117,6 @@ if (enable_unittests) {
"effects/dl_color_source_unittests.cc",
"effects/dl_image_filter_unittests.cc",
"effects/dl_mask_filter_unittests.cc",
"effects/dl_path_effect_unittests.cc",
"geometry/dl_geometry_types_unittests.cc",
"geometry/dl_region_unittests.cc",
"geometry/dl_rtree_unittests.cc",
Expand Down
1 change: 0 additions & 1 deletion display_list/benchmarking/dl_complexity_helper.h
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,6 @@ class ComplexityCalculatorHelper
void setColorSource(const DlColorSource* source) override {}
void setImageFilter(const DlImageFilter* filter) override {}
void setColorFilter(const DlColorFilter* filter) override {}
void setPathEffect(const DlPathEffect* effect) override {}
void setMaskFilter(const DlMaskFilter* filter) override {}

void save() override {}
Expand Down
3 changes: 0 additions & 3 deletions display_list/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -69,9 +69,6 @@ namespace flutter {
V(SetColor) \
V(SetBlendMode) \
\
V(SetPodPathEffect) \
V(ClearPathEffect) \
\
V(ClearColorFilter) \
V(SetPodColorFilter) \
\
Expand Down
5 changes: 0 additions & 5 deletions display_list/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,6 @@ class DisplayListTestBase : public BaseT {
EXPECT_EQ(builder_paint.getColorFilter(), defaults.getColorFilter());
EXPECT_EQ(builder_paint.getImageFilter(), defaults.getImageFilter());
EXPECT_EQ(builder_paint.getMaskFilter(), defaults.getMaskFilter());
EXPECT_EQ(builder_paint.getPathEffect(), defaults.getPathEffect());
EXPECT_EQ(builder_paint, defaults);
EXPECT_TRUE(builder_paint.isDefault());

Expand Down Expand Up @@ -445,10 +444,6 @@ TEST_F(DisplayListTest, BuildRestoresAttributes) {
receiver.setMaskFilter(&kTestMaskFilter1);
builder.Build();
check_defaults(builder, cull_rect);

receiver.setPathEffect(kTestPathEffect1.get());
builder.Build();
check_defaults(builder, cull_rect);
}

TEST_F(DisplayListTest, BuilderBoundsTransformComparedToSkia) {
Expand Down
28 changes: 1 addition & 27 deletions display_list/dl_builder.cc
Original file line number Diff line number Diff line change
Expand Up @@ -338,22 +338,6 @@ void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) {
}
UpdateCurrentOpacityCompatibility();
}
void DisplayListBuilder::onSetPathEffect(const DlPathEffect* effect) {
if (effect == nullptr) {
current_.setPathEffect(nullptr);
Push<ClearPathEffectOp>(0);
} else {
current_.setPathEffect(effect->shared());
switch (effect->type()) {
case DlPathEffectType::kDash: {
const DlDashPathEffect* dash_effect = effect->asDash();
void* pod = Push<SetPodPathEffectOp>(dash_effect->size());
new (pod) DlDashPathEffect(dash_effect);
break;
}
}
}
}
void DisplayListBuilder::onSetMaskFilter(const DlMaskFilter* filter) {
if (filter == nullptr) {
current_.setMaskFilter(nullptr);
Expand Down Expand Up @@ -405,9 +389,6 @@ void DisplayListBuilder::SetAttributesFromPaint(
if (flags.applies_image_filter()) {
setImageFilter(paint.getImageFilter().get());
}
if (flags.applies_path_effect()) {
setPathEffect(paint.getPathEffect().get());
}
if (flags.applies_mask_filter()) {
setMaskFilter(paint.getMaskFilter().get());
}
Expand Down Expand Up @@ -1726,14 +1707,7 @@ bool DisplayListBuilder::AdjustBoundsForPaint(SkRect& bounds,

// Path effect occurs before stroking...
DisplayListSpecialGeometryFlags special_flags =
flags.WithPathEffect(current_.getPathEffectPtr(), is_stroked);
if (current_.getPathEffect()) {
auto effect_bounds = current_.getPathEffect()->effect_bounds(bounds);
if (!effect_bounds.has_value()) {
return false;
}
bounds = effect_bounds.value();
}
flags.GeometryFlags(is_stroked);

if (is_stroked) {
// Determine the max multiplier to the stroke width first.
Expand Down
8 changes: 0 additions & 8 deletions display_list/dl_builder.h
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,6 @@
#include "flutter/display_list/dl_op_receiver.h"
#include "flutter/display_list/dl_paint.h"
#include "flutter/display_list/dl_sampling_options.h"
#include "flutter/display_list/effects/dl_path_effect.h"
#include "flutter/display_list/geometry/dl_geometry_types.h"
#include "flutter/display_list/image/dl_image.h"
#include "flutter/display_list/utils/dl_accumulation_rect.h"
Expand Down Expand Up @@ -346,12 +345,6 @@ class DisplayListBuilder final : public virtual DlCanvas,
}
}
// |DlOpReceiver|
void setPathEffect(const DlPathEffect* effect) override {
if (NotEquals(current_.getPathEffect(), effect)) {
onSetPathEffect(effect);
}
}
// |DlOpReceiver|
void setMaskFilter(const DlMaskFilter* filter) override {
if (NotEquals(current_.getMaskFilter(), filter)) {
onSetMaskFilter(filter);
Expand Down Expand Up @@ -771,7 +764,6 @@ class DisplayListBuilder final : public virtual DlCanvas,
void onSetColorSource(const DlColorSource* source);
void onSetImageFilter(const DlImageFilter* filter);
void onSetColorFilter(const DlColorFilter* filter);
void onSetPathEffect(const DlPathEffect* effect);
void onSetMaskFilter(const DlMaskFilter* filter);

static DisplayListAttributeFlags FlagsForPointMode(PointMode mode);
Expand Down
23 changes: 1 addition & 22 deletions display_list/dl_op_flags.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,30 +3,9 @@
// found in the LICENSE file.

#include "flutter/display_list/dl_op_flags.h"
#include "flutter/display_list/effects/dl_path_effect.h"

namespace flutter {

const DisplayListSpecialGeometryFlags DisplayListAttributeFlags::WithPathEffect(
const DlPathEffect* effect,
bool is_stroked) const {
if (is_geometric() && effect) {
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_;
}
// Just exists to ensure that the header can be cleanly imported.

} // namespace flutter
23 changes: 8 additions & 15 deletions display_list/dl_op_flags.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@

namespace flutter {

class DlPathEffect;
/// The base class for the classes that maintain a list of
/// attributes that might be important for a number of operations
/// including which rendering attributes need to be set before
Expand Down Expand Up @@ -86,9 +85,8 @@ class DisplayListFlags {
static constexpr int kUsesBlend = 1 << 13;
static constexpr int kUsesShader = 1 << 14;
static constexpr int kUsesColorFilter = 1 << 15;
static constexpr int kUsesPathEffect = 1 << 16;
static constexpr int kUsesMaskFilter = 1 << 17;
static constexpr int kUsesImageFilter = 1 << 18;
static constexpr int kUsesMaskFilter = 1 << 16;
static constexpr int kUsesImageFilter = 1 << 17;

// Some ops have an optional paint argument. If the version
// stored in the DisplayList ignores the paint, but there
Expand All @@ -102,7 +100,7 @@ class DisplayListFlags {

static constexpr int kAnyAttributeMask = //
kUsesAntiAlias | kUsesAlpha | kUsesColor | kUsesBlend | kUsesShader |
kUsesColorFilter | kUsesPathEffect | kUsesMaskFilter | kUsesImageFilter;
kUsesColorFilter | kUsesMaskFilter | kUsesImageFilter;
};

class DisplayListFlagsBase : protected DisplayListFlags {
Expand Down Expand Up @@ -164,9 +162,9 @@ class DisplayListSpecialGeometryFlags : DisplayListFlagsBase {

class DisplayListAttributeFlags : DisplayListFlagsBase {
public:
const DisplayListSpecialGeometryFlags WithPathEffect(
const DlPathEffect* effect,
bool is_stroked) const;
const DisplayListSpecialGeometryFlags GeometryFlags(bool is_stroked) const {
return special_flags_;
}

constexpr bool ignores_paint() const { return has_any(kIgnoresPaint); }

Expand Down Expand Up @@ -198,9 +196,6 @@ class DisplayListAttributeFlags : DisplayListFlagsBase {
}
/// The primitive honors the DlBlendMode
constexpr bool applies_blend() const { return has_any(kUsesBlend); }
constexpr bool applies_path_effect() const {
return has_any(kUsesPathEffect);
}
/// The primitive honors the DlMaskFilter whether set using the
/// filter object or using the convenience method |setMaskBlurFilter|
constexpr bool applies_mask_filter() const {
Expand Down Expand Up @@ -264,14 +259,12 @@ class DisplayListOpFlags : DisplayListFlags {
// Flags common to all primitives that stroke or fill
static constexpr int kBaseStrokeOrFillFlags = (kIsDrawnGeometry | //
kUsesAntiAlias | //
kUsesMaskFilter | //
kUsesPathEffect);
kUsesMaskFilter);

// Flags common to primitives that stroke geometry
static constexpr int kBaseStrokeFlags = (kIsStrokedGeometry | //
kUsesAntiAlias | //
kUsesMaskFilter | //
kUsesPathEffect);
kUsesMaskFilter);

// Flags common to primitives that render an image with paint attributes
static constexpr int kBaseImageFlags = (kIsNonGeometric | //
Expand Down
2 changes: 0 additions & 2 deletions display_list/dl_op_receiver.h
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@
#include "flutter/display_list/effects/dl_color_source.h"
#include "flutter/display_list/effects/dl_image_filter.h"
#include "flutter/display_list/effects/dl_mask_filter.h"
#include "flutter/display_list/effects/dl_path_effect.h"
#include "flutter/display_list/image/dl_image.h"

#include "flutter/impeller/geometry/path.h"
Expand Down Expand Up @@ -157,7 +156,6 @@ class DlOpReceiver {
// filter so that the color inversion happens after the ColorFilter.
virtual void setInvertColors(bool invert) = 0;
virtual void setBlendMode(DlBlendMode mode) = 0;
virtual void setPathEffect(const DlPathEffect* effect) = 0;
virtual void setMaskFilter(const DlMaskFilter* filter) = 0;
virtual void setImageFilter(const DlImageFilter* filter) = 0;

Expand Down
1 change: 0 additions & 1 deletion display_list/dl_op_records.h
Original file line number Diff line number Diff line change
Expand Up @@ -231,7 +231,6 @@ DEFINE_SET_CLEAR_DLATTR_OP(ColorFilter, ColorFilter, filter)
DEFINE_SET_CLEAR_DLATTR_OP(ImageFilter, ImageFilter, filter)
DEFINE_SET_CLEAR_DLATTR_OP(MaskFilter, MaskFilter, filter)
DEFINE_SET_CLEAR_DLATTR_OP(ColorSource, Shader, source)
DEFINE_SET_CLEAR_DLATTR_OP(PathEffect, PathEffect, effect)
#undef DEFINE_SET_CLEAR_DLATTR_OP

// 4 byte header + 80 bytes for the embedded DlImageColorSource
Expand Down
3 changes: 1 addition & 2 deletions display_list/dl_paint.cc
Original file line number Diff line number Diff line change
Expand Up @@ -30,8 +30,7 @@ bool DlPaint::operator==(DlPaint const& other) const {
Equals(color_source_, other.color_source_) && //
Equals(color_filter_, other.color_filter_) && //
Equals(image_filter_, other.image_filter_) && //
Equals(mask_filter_, other.mask_filter_) && //
Equals(path_effect_, other.path_effect_);
Equals(mask_filter_, other.mask_filter_);
}

const DlPaint DlPaint::kDefault;
Expand Down
15 changes: 0 additions & 15 deletions display_list/dl_paint.h
Original file line number Diff line number Diff line change
Expand Up @@ -13,7 +13,6 @@
#include "flutter/display_list/effects/dl_color_source.h"
#include "flutter/display_list/effects/dl_image_filter.h"
#include "flutter/display_list/effects/dl_mask_filter.h"
#include "flutter/display_list/effects/dl_path_effect.h"

namespace flutter {

Expand Down Expand Up @@ -177,19 +176,6 @@ class DlPaint {
return *this;
}

std::shared_ptr<const DlPathEffect> getPathEffect() const {
return path_effect_;
}
const DlPathEffect* getPathEffectPtr() const { return path_effect_.get(); }
DlPaint& setPathEffect(const std::shared_ptr<DlPathEffect>& pathEffect) {
path_effect_ = pathEffect;
return *this;
}
DlPaint& setPathEffect(const DlPathEffect* effect) {
path_effect_ = effect ? effect->shared() : nullptr;
return *this;
}

bool isDefault() const { return *this == kDefault; }

bool operator==(DlPaint const& other) const;
Expand Down Expand Up @@ -228,7 +214,6 @@ class DlPaint {
std::shared_ptr<const DlColorFilter> color_filter_;
std::shared_ptr<const DlImageFilter> image_filter_;
std::shared_ptr<const DlMaskFilter> mask_filter_;
std::shared_ptr<const DlPathEffect> path_effect_;
};

} // namespace flutter
Expand Down
16 changes: 1 addition & 15 deletions display_list/dl_paint_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -26,7 +26,6 @@ TEST(DisplayListPaint, ConstructorDefaults) {
EXPECT_EQ(paint.getColorFilter(), nullptr);
EXPECT_EQ(paint.getImageFilter(), nullptr);
EXPECT_EQ(paint.getMaskFilter(), nullptr);
EXPECT_EQ(paint.getPathEffect(), nullptr);
EXPECT_TRUE(paint.isDefault());
EXPECT_EQ(paint, DlPaint::kDefault);

Expand Down Expand Up @@ -67,44 +66,33 @@ TEST(DisplayListPaint, ConstructorDefaults) {

DlBlurMaskFilter mask_filter(DlBlurStyle::kInner, 3.14);
EXPECT_NE(paint, DlPaint().setMaskFilter(mask_filter.shared()));

SkScalar intervals[] = {1.0f, 2.0f};
auto path_effect = DlDashPathEffect::Make(intervals, 2, 0.0f);
EXPECT_NE(paint, DlPaint().setPathEffect(path_effect.get()));
}

TEST(DisplayListPaint, NullPointerSetGet) {
DlColorSource* null_color_source = nullptr;
DlColorFilter* null_color_filter = nullptr;
DlImageFilter* null_image_filter = nullptr;
DlMaskFilter* null_mask_filter = nullptr;
DlPathEffect* null_path_effect = nullptr;
DlPaint paint;
EXPECT_EQ(paint.setColorSource(null_color_source).getColorSource(), nullptr);
EXPECT_EQ(paint.setColorFilter(null_color_filter).getColorFilter(), nullptr);
EXPECT_EQ(paint.setImageFilter(null_image_filter).getImageFilter(), nullptr);
EXPECT_EQ(paint.setMaskFilter(null_mask_filter).getMaskFilter(), nullptr);
EXPECT_EQ(paint.setPathEffect(null_path_effect).getPathEffect(), nullptr);
}

TEST(DisplayListPaint, NullSharedPointerSetGet) {
std::shared_ptr<DlColorSource> null_color_source;
std::shared_ptr<DlColorFilter> null_color_filter;
std::shared_ptr<DlImageFilter> null_image_filter;
std::shared_ptr<DlMaskFilter> null_mask_filter;
std::shared_ptr<DlPathEffect> null_path_effect;
DlPaint paint;
EXPECT_EQ(paint.setColorSource(null_color_source).getColorSource(), nullptr);
EXPECT_EQ(paint.setColorFilter(null_color_filter).getColorFilter(), nullptr);
EXPECT_EQ(paint.setImageFilter(null_image_filter).getImageFilter(), nullptr);
EXPECT_EQ(paint.setMaskFilter(null_mask_filter).getMaskFilter(), nullptr);
EXPECT_EQ(paint.setPathEffect(null_path_effect).getPathEffect(), nullptr);
}

TEST(DisplayListPaint, ChainingConstructor) {
SkScalar intervals[] = {1.0f, 2.0f};
auto path_effect = DlDashPathEffect::Make(intervals, 2, 0.0f);

DlPaint paint =
DlPaint() //
.setAntiAlias(true) //
Expand All @@ -123,8 +111,7 @@ TEST(DisplayListPaint, ChainingConstructor) {
.shared())
.setImageFilter(
DlBlurImageFilter(1.3, 4.7, DlTileMode::kClamp).shared())
.setMaskFilter(DlBlurMaskFilter(DlBlurStyle::kInner, 3.14).shared())
.setPathEffect(path_effect);
.setMaskFilter(DlBlurMaskFilter(DlBlurStyle::kInner, 3.14).shared());
EXPECT_TRUE(paint.isAntiAlias());
EXPECT_TRUE(paint.isInvertColors());
EXPECT_EQ(paint.getColor(), DlColor::kGreen().withAlpha(0x7F));
Expand All @@ -142,7 +129,6 @@ TEST(DisplayListPaint, ChainingConstructor) {
DlBlurImageFilter(1.3, 4.7, DlTileMode::kClamp));
EXPECT_EQ(*paint.getMaskFilter(),
DlBlurMaskFilter(DlBlurStyle::kInner, 3.14));
EXPECT_EQ(*paint.getPathEffect(), *path_effect);

EXPECT_NE(paint, DlPaint());
}
Expand Down
Loading

0 comments on commit 74f42ca

Please sign in to comment.