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.

Sign up for GitHub

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 ColorFilter objects #31491

Merged
merged 8 commits into from
Feb 17, 2022
Merged

Displaylist ColorFilter objects #31491

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
8 commits
Select commit Hold shift + click to select a range
41f7b15
move ColorFilter attributes into an inspectable DL specific object
flar Feb 10, 2022
99d159a
move de-Skia-ifying code to a DlColorFilter static factory, + formatting
flar Feb 14, 2022
e28062f
switch to virtual methods to clean up the code a bit at the cost of 8…
flar Feb 16, 2022
9969ac4
formatter updates
flar Feb 16, 2022
1f15d6c
update impeller to version supporting DlColorFilter
flar Feb 16, 2022
28d8cbc
fix ordering of licenses
flar Feb 16, 2022
2a01090
add test to be clear that objects from shared() are copies
flar Feb 17, 2022
1aa7466
doc comments
flar Feb 17, 2022
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
2 changes: 1 addition & 1 deletion DEPS
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ deps = {
'src': 'https://github.com/flutter/buildroot.git' + '@' + '79643299bd052c53631b8b200bb582e8badb2708',

'src/flutter/impeller':
Var('github_git') + '/flutter/impeller' + '@' + 'c55014e747541b9a2cca15e6b2cb1b1ef9123da3',
Var('github_git') + '/flutter/impeller' + '@' + '78bc2a026c554c444b2e67b36fd44cd548341a51',

# Fuchsia compatibility
#
Expand Down
3 changes: 3 additions & 0 deletions ci/licenses_golden/licenses_flutter
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,9 @@ FILE: ../../../flutter/display_list/display_list_canvas_dispatcher.h
FILE: ../../../flutter/display_list/display_list_canvas_recorder.cc
FILE: ../../../flutter/display_list/display_list_canvas_recorder.h
FILE: ../../../flutter/display_list/display_list_canvas_unittests.cc
FILE: ../../../flutter/display_list/display_list_color_filter.cc
FILE: ../../../flutter/display_list/display_list_color_filter.h
FILE: ../../../flutter/display_list/display_list_color_filter_unittests.cc
FILE: ../../../flutter/display_list/display_list_complexity.cc
FILE: ../../../flutter/display_list/display_list_complexity.h
FILE: ../../../flutter/display_list/display_list_dispatcher.cc
Expand Down
3 changes: 3 additions & 0 deletions display_list/BUILD.gn
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ source_set("display_list") {
"display_list_canvas_dispatcher.h",
"display_list_canvas_recorder.cc",
"display_list_canvas_recorder.h",
"display_list_color_filter.cc",
"display_list_color_filter.h",
"display_list_complexity.cc",
"display_list_complexity.h",
"display_list_dispatcher.cc",
Expand All @@ -40,6 +42,7 @@ source_set("unittests") {

sources = [
"display_list_canvas_unittests.cc",
"display_list_color_filter_unittests.cc",
"display_list_unittests.cc",
]

Expand Down
12 changes: 7 additions & 5 deletions display_list/display_list.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2,8 +2,8 @@
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#ifndef FLUTTER_FLOW_DISPLAY_LIST_H_
#define FLUTTER_FLOW_DISPLAY_LIST_H_
#ifndef FLUTTER_DISPLAY_LIST_DISPLAY_LIST_H_
#define FLUTTER_DISPLAY_LIST_DISPLAY_LIST_H_

#include <optional>

Expand Down Expand Up @@ -76,13 +76,15 @@ namespace flutter {
V(ClearBlender) \
V(SetShader) \
V(ClearShader) \
V(SetColorFilter) \
V(ClearColorFilter) \
V(SetImageFilter) \
V(ClearImageFilter) \
V(SetPathEffect) \
V(ClearPathEffect) \
\
V(ClearColorFilter) \
V(SetColorFilter) \
V(SetSkColorFilter) \
\
V(ClearMaskFilter) \
V(SetMaskFilter) \
V(SetMaskBlurFilterNormal) \
Expand Down Expand Up @@ -278,4 +280,4 @@ class DisplayList : public SkRefCnt {

} // namespace flutter

#endif // FLUTTER_FLOW_DISPLAY_LIST_H_
#endif // FLUTTER_DISPLAY_LIST_DISPLAY_LIST_H_
55 changes: 50 additions & 5 deletions display_list/display_list_builder.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -134,10 +134,50 @@ void DisplayListBuilder::onSetImageFilter(sk_sp<SkImageFilter> filter) {
? Push<SetImageFilterOp>(0, 0, std::move(filter))
: Push<ClearImageFilterOp>(0, 0);
}
void DisplayListBuilder::onSetColorFilter(sk_sp<SkColorFilter> filter) {
(current_color_filter_ = filter) //
? Push<SetColorFilterOp>(0, 0, std::move(filter))
: Push<ClearColorFilterOp>(0, 0);
void DisplayListBuilder::onSetColorFilter(const DlColorFilter* filter) {
if (filter == nullptr) {
if (!current_color_filter_) {
return;
}
current_color_filter_ = nullptr;
Push<ClearColorFilterOp>(0, 0);
} else {
if (current_color_filter_ && *current_color_filter_ == *filter) {
return;
}
current_color_filter_ = filter->shared();
switch (filter->type()) {
case DlColorFilter::kBlend: {
const DlBlendColorFilter* blend_filter = filter->asBlend();
FML_DCHECK(blend_filter);
void* pod = Push<SetColorFilterOp>(blend_filter->size(), 0);
new (pod) DlBlendColorFilter(blend_filter);
break;
}
case DlColorFilter::kMatrix: {
const DlMatrixColorFilter* matrix_filter = filter->asMatrix();
FML_DCHECK(matrix_filter);
void* pod = Push<SetColorFilterOp>(matrix_filter->size(), 0);
new (pod) DlMatrixColorFilter(matrix_filter);
break;
}
case DlColorFilter::kSrgbToLinearGamma: {
void* pod = Push<SetColorFilterOp>(filter->size(), 0);
new (pod) DlSrgbToLinearGammaColorFilter();
break;
}
case DlColorFilter::kLinearToSrgbGamma: {
void* pod = Push<SetColorFilterOp>(filter->size(), 0);
new (pod) DlLinearToSrgbGammaColorFilter();
break;
}
case DlColorFilter::kUnknown: {
const sk_sp<SkColorFilter> sk_filter = filter->sk_filter();
Push<SetSkColorFilterOp>(0, 0, sk_filter);
break;
}
}
}
UpdateCurrentOpacityCompatibility();
}
void DisplayListBuilder::onSetPathEffect(sk_sp<SkPathEffect> effect) {
Expand Down Expand Up @@ -211,7 +251,12 @@ void DisplayListBuilder::setAttributesFromPaint(
// we must clear it because it is a second potential color filter
// that is composed with the paint's color filter.
setInvertColors(false);
setColorFilter(sk_ref_sp(paint.getColorFilter()));
SkColorFilter* color_filter = paint.getColorFilter();
if (color_filter) {
setColorFilter(DlColorFilter::From(color_filter).get());
} else {
setColorFilter(nullptr);
}
}
if (flags.applies_image_filter()) {
setImageFilter(sk_ref_sp(paint.getImageFilter()));
Expand Down
15 changes: 8 additions & 7 deletions display_list/display_list_builder.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -93,10 +93,9 @@ class DisplayListBuilder final : public virtual Dispatcher,
onSetImageFilter(std::move(filter));
}
}
void setColorFilter(sk_sp<SkColorFilter> filter) override {
if (current_color_filter_ != filter) {
onSetColorFilter(std::move(filter));
}
void setColorFilter(const DlColorFilter* filter) override {
// onSetColorFilter will deal with whether the filter is new
onSetColorFilter(filter);
}
void setPathEffect(sk_sp<SkPathEffect> effect) override {
if (current_path_effect_ != effect) {
Expand Down Expand Up @@ -128,7 +127,9 @@ class DisplayListBuilder final : public virtual Dispatcher,
SkPaint::Cap getStrokeCap() const { return current_stroke_cap_; }
SkPaint::Join getStrokeJoin() const { return current_stroke_join_; }
sk_sp<SkShader> getShader() const { return current_shader_; }
sk_sp<SkColorFilter> getColorFilter() const { return current_color_filter_; }
sk_sp<SkColorFilter> getColorFilter() const {
return current_color_filter_->sk_filter();
}
bool isInvertColors() const { return current_invert_colors_; }
std::optional<SkBlendMode> getBlendMode() const {
if (current_blender_) {
Expand Down Expand Up @@ -390,7 +391,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
void onSetBlender(sk_sp<SkBlender> blender);
void onSetShader(sk_sp<SkShader> shader);
void onSetImageFilter(sk_sp<SkImageFilter> filter);
void onSetColorFilter(sk_sp<SkColorFilter> filter);
void onSetColorFilter(const DlColorFilter* filter);
Copy link
Contributor

Choose a reason for hiding this comment

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

Is there any reason to not use a shared_ptr here? Even, for example, a shared_ptr?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

If we do that, then when the dispatcher wants to dispatch the op from the record in the DL, it has to reinstantiate the object.

Alternately, I suppose I could back off to storing the shared_ptr in the buffer, but then equals and destructor have to visit each node and manually compare or manually destruct. One of my goals was to embed the info directly in the DL buffer so that we could mass compare and mass free the buffers.

void onSetPathEffect(sk_sp<SkPathEffect> effect);
void onSetMaskFilter(sk_sp<SkMaskFilter> filter);
void onSetMaskBlurFilter(SkBlurStyle style, SkScalar sigma);
Expand All @@ -409,7 +410,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
SkBlendMode current_blend_mode_ = SkBlendMode::kSrcOver;
sk_sp<SkBlender> current_blender_;
sk_sp<SkShader> current_shader_;
sk_sp<SkColorFilter> current_color_filter_;
std::shared_ptr<const DlColorFilter> current_color_filter_;
sk_sp<SkImageFilter> current_image_filter_;
sk_sp<SkPathEffect> current_path_effect_;
sk_sp<SkMaskFilter> current_mask_filter_;
Expand Down
27 changes: 10 additions & 17 deletions display_list/display_list_canvas_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -836,48 +836,43 @@ class CanvasCompareTester {
0, 0, 0, 0.5, 0,
};
// clang-format on
sk_sp<SkColorFilter> filter =
SkColorFilters::Matrix(rotate_alpha_color_matrix);
DlMatrixColorFilter filter(rotate_alpha_color_matrix);
{
RenderWith(testP, env, tolerance,
CaseParameters(
"saveLayer ColorFilter, no bounds",
[=](SkCanvas* cv, SkPaint& p) {
SkPaint save_p;
save_p.setColorFilter(filter);
save_p.setColorFilter(filter.sk_filter());
cv->saveLayer(nullptr, &save_p);
p.setStrokeWidth(5.0);
},
[=](DisplayListBuilder& b) {
b.setColorFilter(filter);
b.setColorFilter(&filter);
b.saveLayer(nullptr, true);
b.setColorFilter(nullptr);
b.setStrokeWidth(5.0);
})
.with_restore(cv_safe_restore, dl_safe_restore, true));
}
EXPECT_TRUE(filter->unique())
<< "saveLayer ColorFilter, no bounds Cleanup";
{
RenderWith(testP, env, tolerance,
CaseParameters(
"saveLayer ColorFilter and bounds",
[=](SkCanvas* cv, SkPaint& p) {
SkPaint save_p;
save_p.setColorFilter(filter);
save_p.setColorFilter(filter.sk_filter());
cv->saveLayer(RenderBounds, &save_p);
p.setStrokeWidth(5.0);
},
[=](DisplayListBuilder& b) {
b.setColorFilter(filter);
b.setColorFilter(&filter);
b.saveLayer(&RenderBounds, true);
b.setColorFilter(nullptr);
b.setStrokeWidth(5.0);
})
.with_restore(cv_safe_restore, dl_safe_restore, true));
}
EXPECT_TRUE(filter->unique())
<< "saveLayer ColorFilter and bounds Cleanup";
}
{
sk_sp<SkImageFilter> filter = SkImageFilters::Arithmetic(
Expand Down Expand Up @@ -1146,40 +1141,38 @@ class CanvasCompareTester {
1.0, 1.0, 1.0, 1.0, 0,
};
// clang-format on
sk_sp<SkColorFilter> filter = SkColorFilters::Matrix(rotate_color_matrix);
DlMatrixColorFilter filter(rotate_color_matrix);
{
SkColor bg = SK_ColorWHITE;
RenderWith(testP, env, tolerance,
CaseParameters(
"ColorFilter == RotateRGB",
[=](SkCanvas*, SkPaint& p) {
p.setColor(SK_ColorYELLOW);
p.setColorFilter(filter);
p.setColorFilter(filter.sk_filter());
},
[=](DisplayListBuilder& b) {
b.setColor(SK_ColorYELLOW);
b.setColorFilter(filter);
b.setColorFilter(&filter);
})
.with_bg(bg));
}
EXPECT_TRUE(filter->unique()) << "ColorFilter == RotateRGB Cleanup";
filter = SkColorFilters::Matrix(invert_color_matrix);
filter = DlMatrixColorFilter(invert_color_matrix);
{
SkColor bg = SK_ColorWHITE;
RenderWith(testP, env, tolerance,
CaseParameters(
"ColorFilter == Invert",
[=](SkCanvas*, SkPaint& p) {
p.setColor(SK_ColorYELLOW);
p.setColorFilter(filter);
p.setColorFilter(filter.sk_filter());
},
[=](DisplayListBuilder& b) {
b.setColor(SK_ColorYELLOW);
b.setInvertColors(true);
})
.with_bg(bg));
}
EXPECT_TRUE(filter->unique()) << "ColorFilter == Invert Cleanup";
}

{
Expand Down
49 changes: 49 additions & 0 deletions display_list/display_list_color_filter.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -0,0 +1,49 @@
// Copyright 2013 The Flutter Authors. All rights reserved.
// Use of this source code is governed by a BSD-style license that can be
// found in the LICENSE file.

#include "flutter/display_list/display_list_color_filter.h"

namespace flutter {

std::shared_ptr<DlColorFilter> 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<DlBlendColorFilter>(color, mode);
}
}
{
float matrix[20];
if (sk_filter->asAColorMatrix(matrix)) {
return std::make_shared<DlMatrixColorFilter>(matrix);
}
}
return std::make_shared<DlUnknownColorFilter>(sk_ref_sp(sk_filter));
}

const std::shared_ptr<DlSrgbToLinearGammaColorFilter>
DlSrgbToLinearGammaColorFilter::instance =
std::make_shared<DlSrgbToLinearGammaColorFilter>();
const sk_sp<SkColorFilter> DlSrgbToLinearGammaColorFilter::sk_filter_ =
SkColorFilters::SRGBToLinearGamma();

const std::shared_ptr<DlLinearToSrgbGammaColorFilter>
DlLinearToSrgbGammaColorFilter::instance =
std::make_shared<DlLinearToSrgbGammaColorFilter>();
const sk_sp<SkColorFilter> DlLinearToSrgbGammaColorFilter::sk_filter_ =
SkColorFilters::LinearToSRGBGamma();

} // namespace flutter
Loading