Skip to content

Commit eb57d70

Browse files
flarflar
authored
Reland "add non-rendering operation culling to DisplayListBuilder" (flutter#41463) (flutter#43698)
Fixes flutter#129862 This reverts commit fc9fc93. These changes were exposing an underlying bug in the DisplayListMatrixClipTracker that has now been fixed independently (flutter/engine#43664). There are no changes to the commit being relanded here, it has been tested on the Wondrous app which demonstrated the regression before the NOP culling feature was reverted and it now works fine due to the fix of the underlying cause.
1 parent efa624a commit eb57d70

20 files changed

Lines changed: 1267 additions & 311 deletions

‎display_list/benchmarking/dl_complexity_unittests.cc‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -423,7 +423,7 @@ TEST(DisplayListComplexity, DrawAtlas) {
423423
std::vector<SkRSXform> xforms;
424424
for (int i = 0; i < 10; i++) {
425425
rects.push_back(SkRect::MakeXYWH(0, 0, 10, 10));
426-
xforms.push_back(SkRSXform::Make(0, 0, 0, 0));
426+
xforms.push_back(SkRSXform::Make(1, 0, 0, 0));
427427
}
428428

429429
DisplayListBuilder builder;

‎display_list/display_list.cc‎

Lines changed: 4 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,8 @@ DisplayList::DisplayList()
2222
unique_id_(0),
2323
bounds_({0, 0, 0, 0}),
2424
can_apply_group_opacity_(true),
25-
is_ui_thread_safe_(true) {}
25+
is_ui_thread_safe_(true),
26+
modifies_transparent_black_(false) {}
2627

2728
DisplayList::DisplayList(DisplayListStorage&& storage,
2829
size_t byte_count,
@@ -32,6 +33,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage,
3233
const SkRect& bounds,
3334
bool can_apply_group_opacity,
3435
bool is_ui_thread_safe,
36+
bool modifies_transparent_black,
3537
sk_sp<const DlRTree> rtree)
3638
: storage_(std::move(storage)),
3739
byte_count_(byte_count),
@@ -42,6 +44,7 @@ DisplayList::DisplayList(DisplayListStorage&& storage,
4244
bounds_(bounds),
4345
can_apply_group_opacity_(can_apply_group_opacity),
4446
is_ui_thread_safe_(is_ui_thread_safe),
47+
modifies_transparent_black_(modifies_transparent_black),
4548
rtree_(std::move(rtree)) {}
4649

4750
DisplayList::~DisplayList() {

‎display_list/display_list.h‎

Lines changed: 16 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -265,6 +265,19 @@ class DisplayList : public SkRefCnt {
265265
bool can_apply_group_opacity() const { return can_apply_group_opacity_; }
266266
bool isUIThreadSafe() const { return is_ui_thread_safe_; }
267267

268+
/// @brief Indicates if there are any rendering operations in this
269+
/// DisplayList that will modify a surface of transparent black
270+
/// pixels.
271+
///
272+
/// This condition can be used to determine whether to create a cleared
273+
/// surface, render a DisplayList into it, and then composite the
274+
/// result into a scene. It is not uncommon for code in the engine to
275+
/// come across such degenerate DisplayList objects when slicing up a
276+
/// frame between platform views.
277+
bool modifies_transparent_black() const {
278+
return modifies_transparent_black_;
279+
}
280+
268281
private:
269282
DisplayList(DisplayListStorage&& ptr,
270283
size_t byte_count,
@@ -274,6 +287,7 @@ class DisplayList : public SkRefCnt {
274287
const SkRect& bounds,
275288
bool can_apply_group_opacity,
276289
bool is_ui_thread_safe,
290+
bool modifies_transparent_black,
277291
sk_sp<const DlRTree> rtree);
278292

279293
static uint32_t next_unique_id();
@@ -292,6 +306,8 @@ class DisplayList : public SkRefCnt {
292306

293307
const bool can_apply_group_opacity_;
294308
const bool is_ui_thread_safe_;
309+
const bool modifies_transparent_black_;
310+
295311
const sk_sp<const DlRTree> rtree_;
296312

297313
void Dispatch(DlOpReceiver& ctx,

‎display_list/display_list_unittests.cc‎

Lines changed: 160 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -24,6 +24,7 @@
2424
#include "third_party/skia/include/core/SkBBHFactory.h"
2525
#include "third_party/skia/include/core/SkColorFilter.h"
2626
#include "third_party/skia/include/core/SkPictureRecorder.h"
27+
#include "third_party/skia/include/core/SkRSXform.h"
2728
#include "third_party/skia/include/core/SkSurface.h"
2829

2930
namespace flutter {
@@ -3018,5 +3019,164 @@ TEST_F(DisplayListTest, DrawUnorderedRoundRectPathCCW) {
30183019
check_inverted_bounds(renderer, "DrawRoundRectPath Counter-Clockwise");
30193020
}
30203021

3022+
TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
3023+
auto run_tests = [](const std::string& name,
3024+
void init(DisplayListBuilder & builder, DlPaint & paint),
3025+
uint32_t expected_op_count = 0u) {
3026+
auto run_one_test =
3027+
[init](const std::string& name,
3028+
void build(DisplayListBuilder & builder, DlPaint & paint),
3029+
uint32_t expected_op_count = 0u) {
3030+
DisplayListBuilder builder;
3031+
DlPaint paint;
3032+
init(builder, paint);
3033+
build(builder, paint);
3034+
auto list = builder.Build();
3035+
if (list->op_count() != expected_op_count) {
3036+
FML_LOG(ERROR) << *list;
3037+
}
3038+
ASSERT_EQ(list->op_count(), expected_op_count) << name;
3039+
ASSERT_TRUE(list->bounds().isEmpty()) << name;
3040+
};
3041+
run_one_test(
3042+
name + " DrawColor",
3043+
[](DisplayListBuilder& builder, DlPaint& paint) {
3044+
builder.DrawColor(paint.getColor(), paint.getBlendMode());
3045+
},
3046+
expected_op_count);
3047+
run_one_test(
3048+
name + " DrawPaint",
3049+
[](DisplayListBuilder& builder, DlPaint& paint) {
3050+
builder.DrawPaint(paint);
3051+
},
3052+
expected_op_count);
3053+
run_one_test(
3054+
name + " DrawRect",
3055+
[](DisplayListBuilder& builder, DlPaint& paint) {
3056+
builder.DrawRect({10, 10, 20, 20}, paint);
3057+
},
3058+
expected_op_count);
3059+
run_one_test(
3060+
name + " Other Draw Ops",
3061+
[](DisplayListBuilder& builder, DlPaint& paint) {
3062+
builder.DrawLine({10, 10}, {20, 20}, paint);
3063+
builder.DrawOval({10, 10, 20, 20}, paint);
3064+
builder.DrawCircle({50, 50}, 20, paint);
3065+
builder.DrawRRect(SkRRect::MakeRectXY({10, 10, 20, 20}, 5, 5), paint);
3066+
builder.DrawDRRect(SkRRect::MakeRectXY({5, 5, 100, 100}, 5, 5),
3067+
SkRRect::MakeRectXY({10, 10, 20, 20}, 5, 5),
3068+
paint);
3069+
builder.DrawPath(kTestPath1, paint);
3070+
builder.DrawArc({10, 10, 20, 20}, 45, 90, true, paint);
3071+
SkPoint pts[] = {{10, 10}, {20, 20}};
3072+
builder.DrawPoints(PointMode::kLines, 2, pts, paint);
3073+
builder.DrawVertices(TestVertices1, DlBlendMode::kSrcOver, paint);
3074+
builder.DrawImage(TestImage1, {10, 10}, DlImageSampling::kLinear,
3075+
&paint);
3076+
builder.DrawImageRect(TestImage1, SkRect{0.0f, 0.0f, 10.0f, 10.0f},
3077+
SkRect{10.0f, 10.0f, 25.0f, 25.0f},
3078+
DlImageSampling::kLinear, &paint);
3079+
builder.DrawImageNine(TestImage1, {10, 10, 20, 20},
3080+
{10, 10, 100, 100}, DlFilterMode::kLinear,
3081+
&paint);
3082+
SkRSXform xforms[] = {{1, 0, 10, 10}, {0, 1, 10, 10}};
3083+
SkRect rects[] = {{10, 10, 20, 20}, {10, 20, 30, 20}};
3084+
builder.DrawAtlas(TestImage1, xforms, rects, nullptr, 2,
3085+
DlBlendMode::kSrcOver, DlImageSampling::kLinear,
3086+
nullptr, &paint);
3087+
builder.DrawTextBlob(TestBlob1, 10, 10, paint);
3088+
3089+
// Dst mode eliminates most rendering ops except for
3090+
// the following two, so we'll prune those manually...
3091+
if (paint.getBlendMode() != DlBlendMode::kDst) {
3092+
builder.DrawDisplayList(TestDisplayList1, paint.getOpacity());
3093+
builder.DrawShadow(kTestPath1, paint.getColor(), 1, true, 1);
3094+
}
3095+
},
3096+
expected_op_count);
3097+
run_one_test(
3098+
name + " SaveLayer",
3099+
[](DisplayListBuilder& builder, DlPaint& paint) {
3100+
builder.SaveLayer(nullptr, &paint, nullptr);
3101+
builder.DrawRect({10, 10, 20, 20}, DlPaint());
3102+
builder.Restore();
3103+
},
3104+
expected_op_count);
3105+
run_one_test(
3106+
name + " inside Save",
3107+
[](DisplayListBuilder& builder, DlPaint& paint) {
3108+
builder.Save();
3109+
builder.DrawRect({10, 10, 20, 20}, paint);
3110+
builder.Restore();
3111+
},
3112+
expected_op_count);
3113+
};
3114+
run_tests("transparent color", //
3115+
[](DisplayListBuilder& builder, DlPaint& paint) {
3116+
paint.setColor(DlColor::kTransparent());
3117+
});
3118+
run_tests("0 alpha", //
3119+
[](DisplayListBuilder& builder, DlPaint& paint) {
3120+
// The transparent test above already tested transparent
3121+
// black (all 0s), we set White color here so we can test
3122+
// the case of all 1s with a 0 alpha
3123+
paint.setColor(DlColor::kWhite());
3124+
paint.setAlpha(0);
3125+
});
3126+
run_tests("BlendMode::kDst", //
3127+
[](DisplayListBuilder& builder, DlPaint& paint) {
3128+
paint.setBlendMode(DlBlendMode::kDst);
3129+
});
3130+
run_tests("Empty rect clip", //
3131+
[](DisplayListBuilder& builder, DlPaint& paint) {
3132+
builder.ClipRect(SkRect::MakeEmpty(), ClipOp::kIntersect, false);
3133+
});
3134+
run_tests("Empty rrect clip", //
3135+
[](DisplayListBuilder& builder, DlPaint& paint) {
3136+
builder.ClipRRect(SkRRect::MakeEmpty(), ClipOp::kIntersect,
3137+
false);
3138+
});
3139+
run_tests("Empty path clip", //
3140+
[](DisplayListBuilder& builder, DlPaint& paint) {
3141+
builder.ClipPath(SkPath(), ClipOp::kIntersect, false);
3142+
});
3143+
run_tests("Transparent SaveLayer", //
3144+
[](DisplayListBuilder& builder, DlPaint& paint) {
3145+
DlPaint save_paint;
3146+
save_paint.setColor(DlColor::kTransparent());
3147+
builder.SaveLayer(nullptr, &save_paint);
3148+
});
3149+
run_tests("0 alpha SaveLayer", //
3150+
[](DisplayListBuilder& builder, DlPaint& paint) {
3151+
DlPaint save_paint;
3152+
// The transparent test above already tested transparent
3153+
// black (all 0s), we set White color here so we can test
3154+
// the case of all 1s with a 0 alpha
3155+
save_paint.setColor(DlColor::kWhite());
3156+
save_paint.setAlpha(0);
3157+
builder.SaveLayer(nullptr, &save_paint);
3158+
});
3159+
run_tests("Dst blended SaveLayer", //
3160+
[](DisplayListBuilder& builder, DlPaint& paint) {
3161+
DlPaint save_paint;
3162+
save_paint.setBlendMode(DlBlendMode::kDst);
3163+
builder.SaveLayer(nullptr, &save_paint);
3164+
});
3165+
run_tests(
3166+
"Nop inside SaveLayer",
3167+
[](DisplayListBuilder& builder, DlPaint& paint) {
3168+
builder.SaveLayer(nullptr, nullptr);
3169+
paint.setBlendMode(DlBlendMode::kDst);
3170+
},
3171+
2u);
3172+
run_tests("DrawImage inside Culled SaveLayer", //
3173+
[](DisplayListBuilder& builder, DlPaint& paint) {
3174+
DlPaint save_paint;
3175+
save_paint.setColor(DlColor::kTransparent());
3176+
builder.SaveLayer(nullptr, &save_paint);
3177+
builder.DrawImage(TestImage1, {10, 10}, DlImageSampling::kLinear);
3178+
});
3179+
}
3180+
30213181
} // namespace testing
30223182
} // namespace flutter

0 commit comments

Comments
 (0)