Skip to content

Commit b7ce4df

Browse files
committed
Revert "Revert layer state stack (flutter#37090)"
This reverts commit b5c5256.
1 parent f4da34f commit b7ce4df

File tree

85 files changed

+3557
-1799
lines changed

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

85 files changed

+3557
-1799
lines changed

ci/licenses_golden/licenses_flutter

+3
Original file line numberDiff line numberDiff line change
@@ -777,6 +777,9 @@ FILE: ../../../flutter/flow/layers/layer.cc
777777
FILE: ../../../flutter/flow/layers/layer.h
778778
FILE: ../../../flutter/flow/layers/layer_raster_cache_item.cc
779779
FILE: ../../../flutter/flow/layers/layer_raster_cache_item.h
780+
FILE: ../../../flutter/flow/layers/layer_state_stack.cc
781+
FILE: ../../../flutter/flow/layers/layer_state_stack.h
782+
FILE: ../../../flutter/flow/layers/layer_state_stack_unittests.cc
780783
FILE: ../../../flutter/flow/layers/layer_tree.cc
781784
FILE: ../../../flutter/flow/layers/layer_tree.h
782785
FILE: ../../../flutter/flow/layers/layer_tree_unittests.cc

display_list/display_list_builder.cc

+15
Original file line numberDiff line numberDiff line change
@@ -709,6 +709,21 @@ SkRect DisplayListBuilder::getLocalClipBounds() {
709709
return kMaxCullRect_;
710710
}
711711

712+
bool DisplayListBuilder::quickReject(const SkRect& bounds) const {
713+
if (bounds.isEmpty()) {
714+
return true;
715+
}
716+
SkMatrix matrix = getTransform();
717+
// We don't need the inverse, but this method tells us if the matrix
718+
// is singular in which case we can reject all rendering.
719+
if (!matrix.invert(nullptr)) {
720+
return true;
721+
}
722+
SkRect dev_bounds;
723+
matrix.mapRect(bounds).roundOut(&dev_bounds);
724+
return !current_layer_->clip_bounds.intersects(dev_bounds);
725+
}
726+
712727
void DisplayListBuilder::drawPaint() {
713728
Push<DrawPaintOp>(0, 1);
714729
CheckLayerOpacityCompatibility();

display_list/display_list_builder.h

+6-1
Original file line numberDiff line numberDiff line change
@@ -206,7 +206,7 @@ class DisplayListBuilder final : public virtual Dispatcher,
206206
/// Returns the 3x3 partial perspective transform representing all transform
207207
/// operations executed so far in this DisplayList within the enclosing
208208
/// save stack.
209-
SkMatrix getTransform() { return current_layer_->matrix.asM33(); }
209+
SkMatrix getTransform() const { return current_layer_->matrix.asM33(); }
210210

211211
void clipRect(const SkRect& rect, SkClipOp clip_op, bool is_aa) override;
212212
void clipRRect(const SkRRect& rrect, SkClipOp clip_op, bool is_aa) override;
@@ -221,6 +221,11 @@ class DisplayListBuilder final : public virtual Dispatcher,
221221
/// recorded rendering operations are interpreted.
222222
SkRect getLocalClipBounds();
223223

224+
/// Return true iff the supplied bounds are easily shown to be outside
225+
/// of the current clip bounds. This method may conservatively return
226+
/// false if it cannot make the determination.
227+
bool quickReject(const SkRect& bounds) const;
228+
224229
void drawPaint() override;
225230
void drawPaint(const DlPaint& paint);
226231
void drawColor(DlColor color, DlBlendMode mode) override;

display_list/display_list_canvas_unittests.cc

+174
Original file line numberDiff line numberDiff line change
@@ -2040,6 +2040,9 @@ class CanvasCompareTester {
20402040
const uint32_t* test_row = test_pixels->addr32(0, y);
20412041
for (int x = 0; x < test_pixels->width(); x++) {
20422042
if (ref_row[x] != test_row[x]) {
2043+
if (should_match) {
2044+
FML_LOG(ERROR) << std::hex << ref_row[x] << " != " << test_row[x];
2045+
}
20432046
pixels_different++;
20442047
}
20452048
}
@@ -3238,5 +3241,176 @@ TEST_F(DisplayListCanvas, DrawShadowDpr) {
32383241
CanvasCompareTester::DefaultTolerance.addBoundsPadding(3, 3));
32393242
}
32403243

3244+
TEST_F(DisplayListCanvas, SaveLayerConsolidation) {
3245+
float commutable_color_matrix[]{
3246+
// clang-format off
3247+
0, 1, 0, 0, 0,
3248+
0, 0, 1, 0, 0,
3249+
1, 0, 0, 0, 0,
3250+
0, 0, 0, 1, 0,
3251+
// clang-format on
3252+
};
3253+
float non_commutable_color_matrix[]{
3254+
// clang-format off
3255+
0, 1, 0, .1, 0,
3256+
0, 0, 1, .1, 0,
3257+
1, 0, 0, .1, 0,
3258+
0, 0, 0, .7, 0,
3259+
// clang-format on
3260+
};
3261+
SkMatrix contract_matrix;
3262+
contract_matrix.setScale(0.9f, 0.9f, kRenderCenterX, kRenderCenterY);
3263+
3264+
std::vector<SkScalar> opacities = {
3265+
0,
3266+
0.5f,
3267+
SK_Scalar1,
3268+
};
3269+
std::vector<std::shared_ptr<DlColorFilter>> color_filters = {
3270+
std::make_shared<DlBlendColorFilter>(DlColor::kCyan(),
3271+
DlBlendMode::kSrcATop),
3272+
std::make_shared<DlMatrixColorFilter>(commutable_color_matrix),
3273+
std::make_shared<DlMatrixColorFilter>(non_commutable_color_matrix),
3274+
DlSrgbToLinearGammaColorFilter::instance,
3275+
DlLinearToSrgbGammaColorFilter::instance,
3276+
};
3277+
std::vector<std::shared_ptr<DlImageFilter>> image_filters = {
3278+
std::make_shared<DlBlurImageFilter>(5.0f, 5.0f, DlTileMode::kDecal),
3279+
std::make_shared<DlDilateImageFilter>(5.0f, 5.0f),
3280+
std::make_shared<DlErodeImageFilter>(5.0f, 5.0f),
3281+
std::make_shared<DlMatrixImageFilter>(contract_matrix,
3282+
DlImageSampling::kLinear),
3283+
};
3284+
RenderEnvironment env = RenderEnvironment::MakeN32();
3285+
3286+
auto render_content = [](DisplayListBuilder& builder) {
3287+
builder.drawRect(
3288+
SkRect{kRenderLeft, kRenderTop, kRenderCenterX, kRenderCenterY},
3289+
DlPaint().setColor(DlColor::kYellow()));
3290+
builder.drawRect(
3291+
SkRect{kRenderCenterX, kRenderTop, kRenderRight, kRenderCenterY},
3292+
DlPaint().setColor(DlColor::kRed()));
3293+
builder.drawRect(
3294+
SkRect{kRenderLeft, kRenderCenterY, kRenderCenterX, kRenderBottom},
3295+
DlPaint().setColor(DlColor::kBlue()));
3296+
builder.drawRect(
3297+
SkRect{kRenderCenterX, kRenderCenterY, kRenderRight, kRenderBottom},
3298+
DlPaint().setColor(DlColor::kRed().modulateOpacity(0.5f)));
3299+
};
3300+
3301+
// clang-format off
3302+
auto test_attributes = [&env, render_content]
3303+
(DlPaint& paint1, DlPaint& paint2, const DlPaint& paint_both,
3304+
bool same, bool rev_same, const std::string& desc1,
3305+
const std::string& desc2) {
3306+
// clang-format on
3307+
DisplayListBuilder nested_builder;
3308+
nested_builder.saveLayer(&kTestBounds, &paint1);
3309+
nested_builder.saveLayer(&kTestBounds, &paint2);
3310+
render_content(nested_builder);
3311+
auto nested_surface = env.MakeSurface();
3312+
nested_builder.Build()->RenderTo(nested_surface->canvas());
3313+
3314+
DisplayListBuilder reverse_builder;
3315+
reverse_builder.saveLayer(&kTestBounds, &paint2);
3316+
reverse_builder.saveLayer(&kTestBounds, &paint1);
3317+
render_content(reverse_builder);
3318+
auto reverse_surface = env.MakeSurface();
3319+
reverse_builder.Build()->RenderTo(reverse_surface->canvas());
3320+
3321+
DisplayListBuilder combined_builder;
3322+
combined_builder.saveLayer(&kTestBounds, &paint_both);
3323+
render_content(combined_builder);
3324+
auto combined_surface = env.MakeSurface();
3325+
combined_builder.Build()->RenderTo(combined_surface->canvas());
3326+
3327+
// Set this boolean to true to test if combinations that are marked
3328+
// as incompatible actually are compatible despite our predictions.
3329+
// Some of the combinations that we treat as incompatible actually
3330+
// are compatible with swapping the order of the operations, but
3331+
// it would take a bit of new infrastructure to really identify
3332+
// those combinations. The only hard constraint to test here is
3333+
// when we claim that they are compatible and they aren't.
3334+
const bool always = false;
3335+
3336+
if (always || same) {
3337+
CanvasCompareTester::quickCompareToReference(
3338+
nested_surface->pixmap(), combined_surface->pixmap(), same,
3339+
"nested " + desc1 + " then " + desc2);
3340+
}
3341+
if (always || rev_same) {
3342+
CanvasCompareTester::quickCompareToReference(
3343+
reverse_surface->pixmap(), combined_surface->pixmap(), rev_same,
3344+
"nested " + desc2 + " then " + desc1);
3345+
}
3346+
};
3347+
3348+
// CF then Opacity should always work.
3349+
// The reverse sometimes works.
3350+
for (size_t cfi = 0; cfi < color_filters.size(); cfi++) {
3351+
auto color_filter = color_filters[cfi];
3352+
std::string cf_desc = "color filter #" + std::to_string(cfi + 1);
3353+
DlPaint nested_paint1 = DlPaint().setColorFilter(color_filter);
3354+
3355+
for (size_t oi = 0; oi < opacities.size(); oi++) {
3356+
SkScalar opacity = opacities[oi];
3357+
std::string op_desc = "opacity " + std::to_string(opacity);
3358+
DlPaint nested_paint2 = DlPaint().setOpacity(opacity);
3359+
3360+
DlPaint combined_paint = nested_paint1;
3361+
combined_paint.setOpacity(opacity);
3362+
3363+
bool op_then_cf_works = opacity <= 0.0 || opacity >= 1.0 ||
3364+
color_filter->can_commute_with_opacity();
3365+
3366+
test_attributes(nested_paint1, nested_paint2, combined_paint, true,
3367+
op_then_cf_works, cf_desc, op_desc);
3368+
}
3369+
}
3370+
3371+
// Opacity then IF should always work.
3372+
// The reverse can also work for some values of opacity.
3373+
// The reverse should also theoretically work for some IFs, but we
3374+
// get some rounding errors that are more than just trivial.
3375+
for (size_t oi = 0; oi < opacities.size(); oi++) {
3376+
SkScalar opacity = opacities[oi];
3377+
std::string op_desc = "opacity " + std::to_string(opacity);
3378+
DlPaint nested_paint1 = DlPaint().setOpacity(opacity);
3379+
3380+
for (size_t ifi = 0; ifi < image_filters.size(); ifi++) {
3381+
auto image_filter = image_filters[ifi];
3382+
std::string if_desc = "image filter #" + std::to_string(ifi + 1);
3383+
DlPaint nested_paint2 = DlPaint().setImageFilter(image_filter);
3384+
3385+
DlPaint combined_paint = nested_paint1;
3386+
combined_paint.setImageFilter(image_filter);
3387+
3388+
bool if_then_op_works = opacity <= 0.0 || opacity >= 1.0;
3389+
test_attributes(nested_paint1, nested_paint2, combined_paint, true,
3390+
if_then_op_works, op_desc, if_desc);
3391+
}
3392+
}
3393+
3394+
// CF then IF should always work.
3395+
// The reverse might work, but we lack the infrastructure to check it.
3396+
for (size_t cfi = 0; cfi < color_filters.size(); cfi++) {
3397+
auto color_filter = color_filters[cfi];
3398+
std::string cf_desc = "color filter #" + std::to_string(cfi + 1);
3399+
DlPaint nested_paint1 = DlPaint().setColorFilter(color_filter);
3400+
3401+
for (size_t ifi = 0; ifi < image_filters.size(); ifi++) {
3402+
auto image_filter = image_filters[ifi];
3403+
std::string if_desc = "image filter #" + std::to_string(ifi + 1);
3404+
DlPaint nested_paint2 = DlPaint().setImageFilter(image_filter);
3405+
3406+
DlPaint combined_paint = nested_paint1;
3407+
combined_paint.setImageFilter(image_filter);
3408+
3409+
test_attributes(nested_paint1, nested_paint2, combined_paint, true, false,
3410+
cf_desc, if_desc);
3411+
}
3412+
}
3413+
}
3414+
32413415
} // namespace testing
32423416
} // namespace flutter

display_list/display_list_color.h

+3
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ struct DlColor {
1414
constexpr DlColor() : argb(0xFF000000) {}
1515
constexpr DlColor(uint32_t argb) : argb(argb) {}
1616

17+
static constexpr uint8_t toAlpha(SkScalar opacity) { return toC(opacity); }
18+
static constexpr SkScalar toOpacity(uint8_t alpha) { return toF(alpha); }
19+
1720
// clang-format off
1821
static constexpr DlColor kTransparent() {return 0x00000000;};
1922
static constexpr DlColor kBlack() {return 0xFF000000;};

display_list/display_list_color_filter.h

+13
Original file line numberDiff line numberDiff line change
@@ -57,6 +57,11 @@ class DlColorFilter
5757
// pixels non-transparent and therefore expand the bounds.
5858
virtual bool modifies_transparent_black() const = 0;
5959

60+
// Return a boolean indicating whether the color filtering operation can
61+
// be applied either before or after modulating the pixels with an opacity
62+
// value without changing the operation.
63+
virtual bool can_commute_with_opacity() const { return false; }
64+
6065
// Return a DlBlendColorFilter pointer to this object iff it is a Blend
6166
// type of ColorFilter, otherwise return nullptr.
6267
virtual const DlBlendColorFilter* asBlend() const { return nullptr; }
@@ -150,6 +155,12 @@ class DlMatrixColorFilter final : public DlColorFilter {
150155
sk_filter->filterColor(SK_ColorTRANSPARENT) != SK_ColorTRANSPARENT;
151156
}
152157

158+
bool can_commute_with_opacity() const override {
159+
return matrix_[3] == 0 && matrix_[8] == 0 && matrix_[13] == 0 &&
160+
matrix_[15] == 0 && matrix_[16] == 0 && matrix_[17] == 0 &&
161+
(matrix_[18] >= 0.0 && matrix_[18] <= 1.0) && matrix_[19] == 0;
162+
}
163+
153164
std::shared_ptr<DlColorFilter> shared() const override {
154165
return std::make_shared<DlMatrixColorFilter>(this);
155166
}
@@ -193,6 +204,7 @@ class DlSrgbToLinearGammaColorFilter final : public DlColorFilter {
193204
}
194205
size_t size() const override { return sizeof(*this); }
195206
bool modifies_transparent_black() const override { return false; }
207+
bool can_commute_with_opacity() const override { return true; }
196208

197209
std::shared_ptr<DlColorFilter> shared() const override { return instance; }
198210
sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }
@@ -225,6 +237,7 @@ class DlLinearToSrgbGammaColorFilter final : public DlColorFilter {
225237
}
226238
size_t size() const override { return sizeof(*this); }
227239
bool modifies_transparent_black() const override { return false; }
240+
bool can_commute_with_opacity() const override { return true; }
228241

229242
std::shared_ptr<DlColorFilter> shared() const override { return instance; }
230243
sk_sp<SkColorFilter> skia_object() const override { return sk_filter_; }

display_list/display_list_paint.h

+6-2
Original file line numberDiff line numberDiff line change
@@ -104,6 +104,10 @@ class DlPaint {
104104
color_.argb = alpha << 24 | (color_.argb & 0x00FFFFFF);
105105
return *this;
106106
}
107+
DlPaint& setOpacity(SkScalar opacity) {
108+
setAlpha(SkScalarRoundToInt(opacity * 0xff));
109+
return *this;
110+
}
107111

108112
DlBlendMode getBlendMode() const {
109113
return static_cast<DlBlendMode>(blendMode_);
@@ -166,7 +170,7 @@ class DlPaint {
166170
return colorFilter_;
167171
}
168172
const DlColorFilter* getColorFilterPtr() const { return colorFilter_.get(); }
169-
DlPaint& setColorFilter(std::shared_ptr<const DlColorFilter> filter) {
173+
DlPaint& setColorFilter(const std::shared_ptr<const DlColorFilter> filter) {
170174
colorFilter_ = filter ? filter->shared() : nullptr;
171175
return *this;
172176
}
@@ -179,7 +183,7 @@ class DlPaint {
179183
return imageFilter_;
180184
}
181185
const DlImageFilter* getImageFilterPtr() const { return imageFilter_.get(); }
182-
DlPaint& setImageFilter(std::shared_ptr<const DlImageFilter> filter) {
186+
DlPaint& setImageFilter(const std::shared_ptr<const DlImageFilter> filter) {
183187
imageFilter_ = filter;
184188
return *this;
185189
}

flow/BUILD.gn

+3-1
Original file line numberDiff line numberDiff line change
@@ -45,6 +45,8 @@ source_set("flow") {
4545
"layers/layer.h",
4646
"layers/layer_raster_cache_item.cc",
4747
"layers/layer_raster_cache_item.h",
48+
"layers/layer_state_stack.cc",
49+
"layers/layer_state_stack.h",
4850
"layers/layer_tree.cc",
4951
"layers/layer_tree.h",
5052
"layers/offscreen_surface.cc",
@@ -156,6 +158,7 @@ if (enable_unittests) {
156158
"layers/container_layer_unittests.cc",
157159
"layers/display_list_layer_unittests.cc",
158160
"layers/image_filter_layer_unittests.cc",
161+
"layers/layer_state_stack_unittests.cc",
159162
"layers/layer_tree_unittests.cc",
160163
"layers/offscreen_surface_unittests.cc",
161164
"layers/opacity_layer_unittests.cc",
@@ -170,7 +173,6 @@ if (enable_unittests) {
170173
"rtree_unittests.cc",
171174
"skia_gpu_object_unittests.cc",
172175
"surface_frame_unittests.cc",
173-
"testing/auto_save_layer_unittests.cc",
174176
"testing/mock_layer_unittests.cc",
175177
"testing/mock_texture_unittests.cc",
176178
"texture_unittests.cc",

flow/embedded_view_params_unittests.cc

-1
Original file line numberDiff line numberDiff line change
@@ -56,7 +56,6 @@ TEST(EmbeddedViewParams, GetBoundingRectAfterMutationsWithRotation90) {
5656
EmbeddedViewParams params(matrix, SkSize::Make(1, 1), stack);
5757
const SkRect& rect = params.finalBoundingRect();
5858

59-
FML_DLOG(ERROR) << rect.x();
6059
ASSERT_TRUE(SkScalarNearlyEqual(rect.x(), -1));
6160
ASSERT_TRUE(SkScalarNearlyEqual(rect.y(), 0));
6261
ASSERT_TRUE(SkScalarNearlyEqual(rect.width(), 1));

flow/embedded_views.cc

+6
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,12 @@ void MutatorsStack::Pop() {
111111
vector_.pop_back();
112112
};
113113

114+
void MutatorsStack::PopTo(size_t stack_count) {
115+
while (vector_.size() > stack_count) {
116+
Pop();
117+
}
118+
}
119+
114120
const std::vector<std::shared_ptr<Mutator>>::const_reverse_iterator
115121
MutatorsStack::Top() const {
116122
return vector_.rend();

flow/embedded_views.h

+3
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,8 @@ class MutatorsStack {
192192
// and destroys it.
193193
void Pop();
194194

195+
void PopTo(size_t stack_count);
196+
195197
// Returns a reverse iterator pointing to the top of the stack, which is the
196198
// mutator that is furtherest from the leaf node.
197199
const std::vector<std::shared_ptr<Mutator>>::const_reverse_iterator Top()
@@ -210,6 +212,7 @@ class MutatorsStack {
210212
const std::vector<std::shared_ptr<Mutator>>::const_iterator End() const;
211213

212214
bool is_empty() const { return vector_.empty(); }
215+
size_t stack_count() const { return vector_.size(); }
213216

214217
bool operator==(const MutatorsStack& other) const {
215218
if (vector_.size() != other.vector_.size()) {

0 commit comments

Comments
 (0)