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

add support for SkPathEffect to DisplayList #27155

Merged
merged 2 commits into from
Jul 4, 2021
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
14 changes: 10 additions & 4 deletions flow/display_list.cc
Original file line number Diff line number Diff line change
Expand Up @@ -205,6 +205,7 @@ DEFINE_SET_CLEAR_SKREF_OP(Shader, shader)
DEFINE_SET_CLEAR_SKREF_OP(ImageFilter, filter)
DEFINE_SET_CLEAR_SKREF_OP(ColorFilter, filter)
DEFINE_SET_CLEAR_SKREF_OP(MaskFilter, filter)
DEFINE_SET_CLEAR_SKREF_OP(PathEffect, effect)
#undef DEFINE_SET_CLEAR_SKREF_OP

// 4 byte header + 4 byte payload packs into minimum 8 bytes
Expand Down Expand Up @@ -1074,22 +1075,27 @@ void DisplayListBuilder::setBlendMode(SkBlendMode mode) {
void DisplayListBuilder::setFilterQuality(SkFilterQuality quality) {
Push<SetFilterQualityOp>(0, quality);
}
void DisplayListBuilder::setShader(sk_sp<SkShader> shader) {
void DisplayListBuilder::setShader(const sk_sp<SkShader> shader) {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I don't think this use of const does anything useful. If anything, it makes the std::move(shader) resort to a copy instead of a move. Did you perhaps mean sk_sp<const SkShader> shader?

shader //
? Push<SetShaderOp>(0, std::move(shader))
: Push<ClearShaderOp>(0);
}
void DisplayListBuilder::setImageFilter(sk_sp<SkImageFilter> filter) {
void DisplayListBuilder::setImageFilter(const sk_sp<SkImageFilter> filter) {
filter //
? Push<SetImageFilterOp>(0, std::move(filter))
: Push<ClearImageFilterOp>(0);
}
void DisplayListBuilder::setColorFilter(sk_sp<SkColorFilter> filter) {
void DisplayListBuilder::setColorFilter(const sk_sp<SkColorFilter> filter) {
filter //
? Push<SetColorFilterOp>(0, std::move(filter))
: Push<ClearColorFilterOp>(0);
}
void DisplayListBuilder::setMaskFilter(sk_sp<SkMaskFilter> filter) {
void DisplayListBuilder::setPathEffect(const sk_sp<SkPathEffect> effect) {
effect //
? Push<SetPathEffectOp>(0, std::move(effect))
: Push<ClearPathEffectOp>(0);
}
void DisplayListBuilder::setMaskFilter(const sk_sp<SkMaskFilter> filter) {
Push<SetMaskFilterOp>(0, std::move(filter));
}
void DisplayListBuilder::setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) {
Expand Down
13 changes: 9 additions & 4 deletions flow/display_list.h
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "third_party/skia/include/core/SkColorFilter.h"
#include "third_party/skia/include/core/SkImage.h"
#include "third_party/skia/include/core/SkImageFilter.h"
#include "third_party/skia/include/core/SkPathEffect.h"
#include "third_party/skia/include/core/SkPicture.h"
#include "third_party/skia/include/core/SkShader.h"
#include "third_party/skia/include/core/SkVertices.h"
Expand Down Expand Up @@ -84,6 +85,8 @@ namespace flutter {
V(ClearColorFilter) \
V(SetImageFilter) \
V(ClearImageFilter) \
V(SetPathEffect) \
V(ClearPathEffect) \
\
V(ClearMaskFilter) \
V(SetMaskFilter) \
Expand Down Expand Up @@ -233,6 +236,7 @@ class Dispatcher {
virtual void setShader(const sk_sp<SkShader> shader) = 0;
virtual void setImageFilter(const sk_sp<SkImageFilter> filter) = 0;
virtual void setColorFilter(const sk_sp<SkColorFilter> filter) = 0;
virtual void setPathEffect(const sk_sp<SkPathEffect> effect) = 0;
virtual void setMaskFilter(const sk_sp<SkMaskFilter> filter) = 0;
virtual void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) = 0;

Expand Down Expand Up @@ -341,10 +345,11 @@ class DisplayListBuilder final : public virtual Dispatcher, public SkRefCnt {
void setColor(SkColor color) override;
void setBlendMode(SkBlendMode mode) override;
void setFilterQuality(SkFilterQuality quality) override;
void setShader(sk_sp<SkShader> shader) override;
void setImageFilter(sk_sp<SkImageFilter> filter) override;
void setColorFilter(sk_sp<SkColorFilter> filter) override;
void setMaskFilter(sk_sp<SkMaskFilter> filter) override;
void setShader(const sk_sp<SkShader> shader) override;
void setImageFilter(const sk_sp<SkImageFilter> filter) override;
void setColorFilter(const sk_sp<SkColorFilter> filter) override;
void setPathEffect(const sk_sp<SkPathEffect> effect) override;
void setMaskFilter(const sk_sp<SkMaskFilter> filter) override;
void setMaskBlurFilter(SkBlurStyle style, SkScalar sigma) override;

void save() override;
Expand Down
5 changes: 5 additions & 0 deletions flow/display_list_canvas.cc
Original file line number Diff line number Diff line change
Expand Up @@ -471,6 +471,11 @@ void DisplayListCanvasRecorder::RecordPaintAttributes(const SkPaint* paint,
builder_->setImageFilter(current_image_filter_ =
sk_ref_sp(paint->getImageFilter()));
}
if ((dataNeeded & kPathEffectNeeded_) != 0 &&
current_path_effect_.get() != paint->getPathEffect()) {
builder_->setPathEffect(current_path_effect_ =
sk_ref_sp(paint->getPathEffect()));
}
if ((dataNeeded & kMaskFilterNeeded_) != 0 &&
current_mask_filter_.get() != paint->getMaskFilter()) {
builder_->setMaskFilter(current_mask_filter_ =
Expand Down
14 changes: 8 additions & 6 deletions flow/display_list_canvas.h
Original file line number Diff line number Diff line change
Expand Up @@ -265,8 +265,9 @@ class DisplayListCanvasRecorder
static constexpr int kShaderNeeded_ = 1 << 7;
static constexpr int kColorFilterNeeded_ = 1 << 8;
static constexpr int kImageFilterNeeded_ = 1 << 9;
static constexpr int kMaskFilterNeeded_ = 1 << 10;
static constexpr int kDitherNeeded_ = 1 << 11;
static constexpr int kPathEffectNeeded_ = 1 << 10;
static constexpr int kMaskFilterNeeded_ = 1 << 11;
static constexpr int kDitherNeeded_ = 1 << 12;
// clang-format on

// Combinations of the above mask bits that are common to typical "draw"
Expand All @@ -278,10 +279,10 @@ class DisplayListCanvasRecorder
kBlendNeeded_ | kInvertColorsNeeded_ |
kColorFilterNeeded_ | kShaderNeeded_ |
kDitherNeeded_ | kImageFilterNeeded_;
static constexpr int kDrawMask_ =
kPaintMask_ | kPaintStyleNeeded_ | kMaskFilterNeeded_;
static constexpr int kStrokeMask_ =
kPaintMask_ | kStrokeStyleNeeded_ | kMaskFilterNeeded_;
static constexpr int kDrawMask_ = kPaintMask_ | kPaintStyleNeeded_ |
kMaskFilterNeeded_ | kPathEffectNeeded_;
static constexpr int kStrokeMask_ = kPaintMask_ | kStrokeStyleNeeded_ |
kMaskFilterNeeded_ | kPathEffectNeeded_;
static constexpr int kImageMask_ =
kColorNeeded_ | kBlendNeeded_ | kInvertColorsNeeded_ |
kColorFilterNeeded_ | kDitherNeeded_ | kImageFilterNeeded_ |
Expand All @@ -304,6 +305,7 @@ class DisplayListCanvasRecorder
sk_sp<SkShader> current_shader_;
sk_sp<SkColorFilter> current_color_filter_;
sk_sp<SkImageFilter> current_image_filter_;
sk_sp<SkPathEffect> current_path_effect_;
sk_sp<SkMaskFilter> current_mask_filter_;
};

Expand Down
94 changes: 91 additions & 3 deletions flow/display_list_canvas_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/core/SkTextBlob.h"
#include "third_party/skia/include/core/SkVertices.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"

Expand Down Expand Up @@ -173,6 +175,14 @@ class CanvasCompareTester {
cv_renderer, dl_renderer, "ImageFilter == Decal Blur 5");
}
ASSERT_TRUE(filter->unique()) << "ImageFilter Cleanup";
filter =
SkImageFilters::Blur(5.0, 5.0, SkTileMode::kClamp, nullptr, nullptr);
{
RenderWith([=](SkCanvas*, SkPaint& p) { p.setImageFilter(filter); },
[=](DisplayListBuilder& b) { b.setImageFilter(filter); },
cv_renderer, dl_renderer, "ImageFilter == Clamp Blur 5");
}
ASSERT_TRUE(filter->unique()) << "ImageFilter Cleanup";
}

{
Expand Down Expand Up @@ -222,6 +232,41 @@ class CanvasCompareTester {
ASSERT_TRUE(filter->unique()) << "ColorFilter Cleanup";
}

{
// Discrete path effects need a stroke width for drawPointsAsPoints
// to do something realistic
sk_sp<SkPathEffect> effect = SkDiscretePathEffect::Make(3, 5);
{
// Discrete path effects need a stroke width for drawPointsAsPoints
// to do something realistic
RenderWith(
[=](SkCanvas*, SkPaint& p) {
p.setStrokeWidth(5.0);
p.setPathEffect(effect);
},
[=](DisplayListBuilder& b) {
b.setStrokeWidth(5.0);
b.setPathEffect(effect);
},
cv_renderer, dl_renderer, "PathEffect == Discrete-3-5");
}
ASSERT_TRUE(effect->unique()) << "PathEffect Cleanup";
effect = SkDiscretePathEffect::Make(2, 3);
{
RenderWith(
[=](SkCanvas*, SkPaint& p) {
p.setStrokeWidth(5.0);
p.setPathEffect(effect);
},
[=](DisplayListBuilder& b) {
b.setStrokeWidth(5.0);
b.setPathEffect(effect);
},
cv_renderer, dl_renderer, "PathEffect == Discrete-2-3");
}
ASSERT_TRUE(effect->unique()) << "PathEffect Cleanup";
}

{
sk_sp<SkMaskFilter> filter =
SkMaskFilter::MakeBlur(kNormal_SkBlurStyle, 5.0);
Expand Down Expand Up @@ -389,6 +434,43 @@ class CanvasCompareTester {
b.setJoins(SkPaint::kMiter_Join);
},
cv_renderer, dl_renderer, "Stroke Width 5, Miter 0");

{
const SkScalar TestDashes1[] = {4.0, 2.0};
const SkScalar TestDashes2[] = {1.0, 1.5};
sk_sp<SkPathEffect> effect = SkDashPathEffect::Make(TestDashes1, 2, 0.0f);
{
RenderWith(
[=](SkCanvas*, SkPaint& p) {
p.setStyle(SkPaint::kStroke_Style);
p.setStrokeWidth(5.0);
p.setPathEffect(effect);
},
[=](DisplayListBuilder& b) {
b.setDrawStyle(SkPaint::kStroke_Style);
b.setStrokeWidth(5.0);
b.setPathEffect(effect);
},
cv_renderer, dl_renderer, "PathEffect == Dash-4-2");
}
ASSERT_TRUE(effect->unique()) << "PathEffect Cleanup";
effect = SkDashPathEffect::Make(TestDashes2, 2, 0.0f);
{
RenderWith(
[=](SkCanvas*, SkPaint& p) {
p.setStyle(SkPaint::kStroke_Style);
p.setStrokeWidth(5.0);
p.setPathEffect(effect);
},
[=](DisplayListBuilder& b) {
b.setDrawStyle(SkPaint::kStroke_Style);
b.setStrokeWidth(5.0);
b.setPathEffect(effect);
},
cv_renderer, dl_renderer, "PathEffect == Dash-1-1.5");
}
ASSERT_TRUE(effect->unique()) << "PathEffect Cleanup";
}
}

static void RenderWithTransforms(CvRenderer& cv_renderer,
Expand Down Expand Up @@ -862,7 +944,9 @@ TEST(DisplayListCanvas, DrawPointsAsPoints) {
const int count = sizeof(points) / sizeof(points[0]);
CanvasCompareTester::RenderAll(
[=](SkCanvas* canvas, SkPaint& paint) { //
canvas->drawPoints(SkCanvas::kPoints_PointMode, count, points, paint);
SkPaint p = paint;
p.setStyle(SkPaint::kStroke_Style);
canvas->drawPoints(SkCanvas::kPoints_PointMode, count, points, p);
},
[=](DisplayListBuilder& builder) { //
builder.drawPoints(SkCanvas::kPoints_PointMode, count, points);
Expand Down Expand Up @@ -901,7 +985,9 @@ TEST(DisplayListCanvas, DrawPointsAsLines) {
ASSERT_TRUE((count & 1) == 0);
CanvasCompareTester::RenderAll(
[=](SkCanvas* canvas, SkPaint& paint) { //
canvas->drawPoints(SkCanvas::kLines_PointMode, count, points, paint);
SkPaint p = paint;
p.setStyle(SkPaint::kStroke_Style);
canvas->drawPoints(SkCanvas::kLines_PointMode, count, points, p);
},
[=](DisplayListBuilder& builder) { //
builder.drawPoints(SkCanvas::kLines_PointMode, count, points);
Expand All @@ -918,7 +1004,9 @@ TEST(DisplayListCanvas, DrawPointsAsPolygon) {
};
CanvasCompareTester::RenderAll(
[=](SkCanvas* canvas, SkPaint& paint) { //
canvas->drawPoints(SkCanvas::kPolygon_PointMode, 4, points, paint);
SkPaint p = paint;
p.setStyle(SkPaint::kStroke_Style);
canvas->drawPoints(SkCanvas::kPolygon_PointMode, 4, points, p);
},
[=](DisplayListBuilder& builder) { //
builder.drawPoints(SkCanvas::kPolygon_PointMode, 4, points);
Expand Down
45 changes: 37 additions & 8 deletions flow/display_list_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
#include "third_party/skia/include/core/SkSurface.h"
#include "third_party/skia/include/core/SkTextBlob.h"
#include "third_party/skia/include/core/SkVertices.h"
#include "third_party/skia/include/effects/SkDashPathEffect.h"
#include "third_party/skia/include/effects/SkGradientShader.h"
#include "third_party/skia/include/effects/SkImageFilters.h"

Expand All @@ -38,12 +39,24 @@ constexpr float stops[] = {
0.5,
1.0,
};

// clang-format off
constexpr float rotate_color_matrix[20] = {
0, 1, 0, 0, 0, //
0, 0, 1, 0, 0, //
1, 0, 0, 0, 0, //
0, 0, 0, 1, 0, //
0, 1, 0, 0, 0,
0, 0, 1, 0, 0,
1, 0, 0, 0, 0,
0, 0, 0, 1, 0,
};
constexpr float invert_color_matrix[20] = {
-1.0, 0, 0, 1.0, 0,
0, -1.0, 0, 1.0, 0,
0, 0, -1.0, 1.0, 0,
1.0, 1.0, 1.0, 1.0, 0,
};
// clang-format on

const SkScalar TestDashes1[] = {4.0, 2.0};
const SkScalar TestDashes2[] = {1.0, 1.5};

constexpr SkPoint TestPoints[] = {
{10, 10},
Expand Down Expand Up @@ -80,10 +93,18 @@ static const sk_sp<SkShader> TestShader3 =
SkTileMode::kDecal,
0,
nullptr);
static const sk_sp<SkImageFilter> TestImageFilter =
static const sk_sp<SkImageFilter> TestImageFilter1 =
SkImageFilters::Blur(5.0, 5.0, SkTileMode::kDecal, nullptr, nullptr);
static const sk_sp<SkColorFilter> TestColorFilter =
static const sk_sp<SkImageFilter> TestImageFilter2 =
SkImageFilters::Blur(5.0, 5.0, SkTileMode::kClamp, nullptr, nullptr);
static const sk_sp<SkColorFilter> TestColorFilter1 =
SkColorFilters::Matrix(rotate_color_matrix);
static const sk_sp<SkColorFilter> TestColorFilter2 =
SkColorFilters::Matrix(invert_color_matrix);
static const sk_sp<SkPathEffect> TestPathEffect1 =
SkDashPathEffect::Make(TestDashes1, 2, 0.0f);
static const sk_sp<SkPathEffect> TestPathEffect2 =
SkDashPathEffect::Make(TestDashes2, 2, 0.0f);
static const sk_sp<SkMaskFilter> TestMaskFilter =
SkMaskFilter::MakeBlur(kNormal_SkBlurStyle, 5.0);
constexpr SkRect TestBounds = SkRect::MakeLTRB(10, 10, 50, 60);
Expand Down Expand Up @@ -290,12 +311,20 @@ std::vector<DisplayListInvocationGroup> allGroups = {
},
{ "SetImageFilter", {
{1, 8, 0, 0, [](DisplayListBuilder& b) {b.setImageFilter(nullptr);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setImageFilter(TestImageFilter);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setImageFilter(TestImageFilter1);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setImageFilter(TestImageFilter2);}},
}
},
{ "SetColorFilter", {
{1, 8, 0, 0, [](DisplayListBuilder& b) {b.setColorFilter(nullptr);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setColorFilter(TestColorFilter);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setColorFilter(TestColorFilter1);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setColorFilter(TestColorFilter2);}},
}
},
{ "SetPathEffect", {
{1, 8, 0, 0, [](DisplayListBuilder& b) {b.setPathEffect(nullptr);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setPathEffect(TestPathEffect1);}},
{1, 16, 0, 0, [](DisplayListBuilder& b) {b.setPathEffect(TestPathEffect2);}},
}
},
{ "SetMaskFilter", {
Expand Down
11 changes: 7 additions & 4 deletions flow/display_list_utils.cc
Original file line number Diff line number Diff line change
Expand Up @@ -60,17 +60,20 @@ void SkPaintDispatchHelper::setBlendMode(SkBlendMode mode) {
void SkPaintDispatchHelper::setFilterQuality(SkFilterQuality quality) {
paint_.setFilterQuality(quality);
}
void SkPaintDispatchHelper::setShader(sk_sp<SkShader> shader) {
void SkPaintDispatchHelper::setShader(const sk_sp<SkShader> shader) {
paint_.setShader(shader);
}
void SkPaintDispatchHelper::setImageFilter(sk_sp<SkImageFilter> filter) {
void SkPaintDispatchHelper::setImageFilter(const sk_sp<SkImageFilter> filter) {
paint_.setImageFilter(filter);
}
void SkPaintDispatchHelper::setColorFilter(sk_sp<SkColorFilter> filter) {
void SkPaintDispatchHelper::setColorFilter(const sk_sp<SkColorFilter> filter) {
color_filter_ = filter;
paint_.setColorFilter(makeColorFilter());
}
void SkPaintDispatchHelper::setMaskFilter(sk_sp<SkMaskFilter> filter) {
void SkPaintDispatchHelper::setPathEffect(const sk_sp<SkPathEffect> effect) {
paint_.setPathEffect(effect);
}
void SkPaintDispatchHelper::setMaskFilter(const sk_sp<SkMaskFilter> filter) {
paint_.setMaskFilter(filter);
}
void SkPaintDispatchHelper::setMaskBlurFilter(SkBlurStyle style,
Expand Down
Loading