Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[DisplayList] delete obsolete PathEffect sources #53441

Merged
merged 1 commit into from
Jun 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 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