Skip to content

Commit 95e9a15

Browse files
flarflar
authored
Cache Impeller paths in the DisplayList to amortize conversion (flutter#50076)
DisplayList by default contains Skia paths stored as `SkPath` objects. Impeller must convert these to its internal `impeller::Path` format on every dispatch of the DisplayList on every frame. This change allows Impeller to store a cached copy of its version of the path into the DisplayList and reuse that instance if it ever encounters the same DisplayList again (likely to happen as most paths come from the Framework which does a lot of work to re-use ui.Picture objects - i.e. DisplayLists). In order to facilitate this change, `impeller::Path` was modified to have fast-copy-constructors that share the data and the PathBuilder will copy the mutable data into an immutable version in `TakePath()`.
1 parent 9592be4 commit 95e9a15

29 files changed

Lines changed: 878 additions & 458 deletions

‎display_list/display_list_unittests.cc‎

Lines changed: 93 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1554,7 +1554,7 @@ TEST_F(DisplayListTest, FlutterSvgIssue661BoundsWereEmpty) {
15541554
// This is the more practical result. The bounds are "almost" 0,0,100x100
15551555
EXPECT_EQ(display_list->bounds().roundOut(), SkIRect::MakeWH(100, 100));
15561556
EXPECT_EQ(display_list->op_count(), 19u);
1557-
EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 352u);
1557+
EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 384u);
15581558
}
15591559

15601560
TEST_F(DisplayListTest, TranslateAffectsCurrentTransform) {
@@ -3239,5 +3239,97 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
32393239
});
32403240
}
32413241

3242+
TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) {
3243+
class Tester : virtual public DlOpReceiver,
3244+
public IgnoreClipDispatchHelper,
3245+
public IgnoreDrawDispatchHelper,
3246+
public IgnoreAttributeDispatchHelper,
3247+
public IgnoreTransformDispatchHelper {
3248+
public:
3249+
explicit Tester(bool prefer_impeller_paths)
3250+
: prefer_impeller_paths_(prefer_impeller_paths) {}
3251+
3252+
bool PrefersImpellerPaths() const override {
3253+
return prefer_impeller_paths_;
3254+
}
3255+
3256+
void drawPath(const SkPath& path) override { skia_draw_path_calls_++; }
3257+
3258+
void drawPath(const CacheablePath& cache) override {
3259+
impeller_draw_path_calls_++;
3260+
}
3261+
3262+
void clipPath(const SkPath& path, ClipOp op, bool is_aa) override {
3263+
skia_clip_path_calls_++;
3264+
}
3265+
3266+
void clipPath(const CacheablePath& cache, ClipOp op, bool is_aa) override {
3267+
impeller_clip_path_calls_++;
3268+
}
3269+
3270+
virtual void drawShadow(const SkPath& sk_path,
3271+
const DlColor color,
3272+
const SkScalar elevation,
3273+
bool transparent_occluder,
3274+
SkScalar dpr) override {
3275+
skia_draw_shadow_calls_++;
3276+
}
3277+
3278+
virtual void drawShadow(const CacheablePath& cache,
3279+
const DlColor color,
3280+
const SkScalar elevation,
3281+
bool transparent_occluder,
3282+
SkScalar dpr) override {
3283+
impeller_draw_shadow_calls_++;
3284+
}
3285+
3286+
int skia_draw_path_calls() const { return skia_draw_path_calls_; }
3287+
int skia_clip_path_calls() const { return skia_draw_path_calls_; }
3288+
int skia_draw_shadow_calls() const { return skia_draw_path_calls_; }
3289+
int impeller_draw_path_calls() const { return impeller_draw_path_calls_; }
3290+
int impeller_clip_path_calls() const { return impeller_draw_path_calls_; }
3291+
int impeller_draw_shadow_calls() const { return impeller_draw_path_calls_; }
3292+
3293+
private:
3294+
const bool prefer_impeller_paths_;
3295+
int skia_draw_path_calls_ = 0;
3296+
int skia_clip_path_calls_ = 0;
3297+
int skia_draw_shadow_calls_ = 0;
3298+
int impeller_draw_path_calls_ = 0;
3299+
int impeller_clip_path_calls_ = 0;
3300+
int impeller_draw_shadow_calls_ = 0;
3301+
};
3302+
3303+
DisplayListBuilder builder;
3304+
builder.DrawPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)), DlPaint());
3305+
builder.ClipPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)),
3306+
ClipOp::kIntersect, true);
3307+
builder.DrawShadow(SkPath::Rect(SkRect::MakeLTRB(20, 20, 80, 80)),
3308+
DlColor::kBlue(), 1.0f, true, 1.0f);
3309+
auto display_list = builder.Build();
3310+
3311+
{
3312+
Tester skia_tester(false);
3313+
display_list->Dispatch(skia_tester);
3314+
EXPECT_EQ(skia_tester.skia_draw_path_calls(), 1);
3315+
EXPECT_EQ(skia_tester.skia_clip_path_calls(), 1);
3316+
EXPECT_EQ(skia_tester.skia_draw_shadow_calls(), 1);
3317+
EXPECT_EQ(skia_tester.impeller_draw_path_calls(), 0);
3318+
EXPECT_EQ(skia_tester.impeller_clip_path_calls(), 0);
3319+
EXPECT_EQ(skia_tester.impeller_draw_shadow_calls(), 0);
3320+
}
3321+
3322+
{
3323+
Tester impeller_tester(true);
3324+
display_list->Dispatch(impeller_tester);
3325+
EXPECT_EQ(impeller_tester.skia_draw_path_calls(), 0);
3326+
EXPECT_EQ(impeller_tester.skia_clip_path_calls(), 0);
3327+
EXPECT_EQ(impeller_tester.skia_draw_shadow_calls(), 0);
3328+
EXPECT_EQ(impeller_tester.impeller_draw_path_calls(), 1);
3329+
EXPECT_EQ(impeller_tester.impeller_clip_path_calls(), 1);
3330+
EXPECT_EQ(impeller_tester.impeller_draw_shadow_calls(), 1);
3331+
}
3332+
}
3333+
32423334
} // namespace testing
32433335
} // namespace flutter

‎display_list/dl_op_receiver.h‎

Lines changed: 42 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,8 @@
1818
#include "flutter/display_list/effects/dl_path_effect.h"
1919
#include "flutter/display_list/image/dl_image.h"
2020

21+
#include "flutter/impeller/geometry/path.h"
22+
2123
namespace flutter {
2224

2325
class DisplayList;
@@ -49,6 +51,46 @@ class DlOpReceiver {
4951
// MaxDrawPointsCount * sizeof(SkPoint) must be less than 1 << 32
5052
static constexpr int kMaxDrawPointsCount = ((1 << 29) - 1);
5153

54+
// ---------------------------------------------------------------------
55+
// The CacheablePath forms of the drawPath, clipPath, and drawShadow
56+
// methods are only called if the DlOpReceiver indicates that it prefers
57+
// impeller paths by returning true from |PrefersImpellerPaths|.
58+
// Note that we pass in both the SkPath and (a place to cache the)
59+
// impeller::Path forms of the path since the SkPath version can contain
60+
// information about the type of path that lets the receiver optimize
61+
// the operation (and potentially avoid the need to cache it).
62+
// It is up to the receiver to convert the path to Impeller form and
63+
// cache it to avoid needing to do a lot of Impeller-specific processing
64+
// inside the DisplayList code.
65+
66+
virtual bool PrefersImpellerPaths() const { return false; }
67+
68+
struct CacheablePath {
69+
explicit CacheablePath(const SkPath& path) : sk_path(path) {}
70+
71+
const SkPath sk_path;
72+
mutable impeller::Path cached_impeller_path;
73+
74+
bool operator==(const CacheablePath& other) const {
75+
return sk_path == other.sk_path;
76+
}
77+
};
78+
79+
virtual void clipPath(const CacheablePath& cache,
80+
ClipOp clip_op,
81+
bool is_aa) {
82+
FML_UNREACHABLE();
83+
}
84+
virtual void drawPath(const CacheablePath& cache) { FML_UNREACHABLE(); }
85+
virtual void drawShadow(const CacheablePath& cache,
86+
const DlColor color,
87+
const SkScalar elevation,
88+
bool transparent_occluder,
89+
SkScalar dpr) {
90+
FML_UNREACHABLE();
91+
}
92+
// ---------------------------------------------------------------------
93+
5294
// The following methods are nearly 1:1 with the methods on DlPaint and
5395
// carry the same meanings. Each method sets a persistent value for the
5496
// attribute for the rest of the display list or until it is reset by

‎display_list/dl_op_records.h‎

Lines changed: 75 additions & 52 deletions
Original file line numberDiff line numberDiff line change
@@ -12,7 +12,8 @@
1212
#include "flutter/display_list/effects/dl_color_source.h"
1313
#include "flutter/fml/macros.h"
1414

15-
#include "impeller/typographer/text_frame.h"
15+
#include "flutter/impeller/geometry/path.h"
16+
#include "flutter/impeller/typographer/text_frame.h"
1617
#include "third_party/skia/include/core/SkRSXform.h"
1718

1819
namespace flutter {
@@ -571,7 +572,7 @@ struct TransformResetOp final : TransformClipOpBase {
571572
// SkRect is 16 more bytes, which packs efficiently into 24 bytes total
572573
// SkRRect is 52 more bytes, which rounds up to 56 bytes (4 bytes unused)
573574
// which packs into 64 bytes total
574-
// SkPath is 16 more bytes, which packs efficiently into 24 bytes total
575+
// CacheablePath is 128 more bytes, which packs efficiently into 136 bytes total
575576
//
576577
// We could pack the clip_op and the bool both into the free 4 bytes after
577578
// the header, but the Windows compiler keeps wanting to expand that
@@ -600,27 +601,33 @@ DEFINE_CLIP_SHAPE_OP(Rect, Difference)
600601
DEFINE_CLIP_SHAPE_OP(RRect, Difference)
601602
#undef DEFINE_CLIP_SHAPE_OP
602603

603-
#define DEFINE_CLIP_PATH_OP(clipop) \
604-
struct Clip##clipop##PathOp final : TransformClipOpBase { \
605-
static const auto kType = DisplayListOpType::kClip##clipop##Path; \
606-
\
607-
Clip##clipop##PathOp(const SkPath& path, bool is_aa) \
608-
: is_aa(is_aa), path(path) {} \
609-
\
610-
const bool is_aa; \
611-
const SkPath path; \
612-
\
613-
void dispatch(DispatchContext& ctx) const { \
614-
if (op_needed(ctx)) { \
615-
ctx.receiver.clipPath(path, DlCanvas::ClipOp::k##clipop, is_aa); \
616-
} \
617-
} \
618-
\
619-
DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \
620-
return is_aa == other->is_aa && path == other->path \
621-
? DisplayListCompare::kEqual \
622-
: DisplayListCompare::kNotEqual; \
623-
} \
604+
#define DEFINE_CLIP_PATH_OP(clipop) \
605+
struct Clip##clipop##PathOp final : TransformClipOpBase { \
606+
static const auto kType = DisplayListOpType::kClip##clipop##Path; \
607+
\
608+
Clip##clipop##PathOp(const SkPath& path, bool is_aa) \
609+
: is_aa(is_aa), cached_path(path) {} \
610+
\
611+
const bool is_aa; \
612+
const DlOpReceiver::CacheablePath cached_path; \
613+
\
614+
void dispatch(DispatchContext& ctx) const { \
615+
if (op_needed(ctx)) { \
616+
if (ctx.receiver.PrefersImpellerPaths()) { \
617+
ctx.receiver.clipPath(cached_path, DlCanvas::ClipOp::k##clipop, \
618+
is_aa); \
619+
} else { \
620+
ctx.receiver.clipPath(cached_path.sk_path, \
621+
DlCanvas::ClipOp::k##clipop, is_aa); \
622+
} \
623+
} \
624+
} \
625+
\
626+
DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \
627+
return is_aa == other->is_aa && cached_path == other->cached_path \
628+
? DisplayListCompare::kEqual \
629+
: DisplayListCompare::kNotEqual; \
630+
} \
624631
};
625632
DEFINE_CLIP_PATH_OP(Intersect)
626633
DEFINE_CLIP_PATH_OP(Difference)
@@ -685,24 +692,28 @@ DEFINE_DRAW_1ARG_OP(Oval, SkRect, oval)
685692
DEFINE_DRAW_1ARG_OP(RRect, SkRRect, rrect)
686693
#undef DEFINE_DRAW_1ARG_OP
687694

688-
// 4 byte header + 16 byte payload uses 20 bytes but is rounded up to 24 bytes
689-
// (4 bytes unused)
695+
// 4 byte header + 128 byte payload uses 132 bytes but is rounded
696+
// up to 136 bytes (4 bytes unused)
690697
struct DrawPathOp final : DrawOpBase {
691698
static const auto kType = DisplayListOpType::kDrawPath;
692699

693-
explicit DrawPathOp(const SkPath& path) : path(path) {}
700+
explicit DrawPathOp(const SkPath& path) : cached_path(path) {}
694701

695-
const SkPath path;
702+
const DlOpReceiver::CacheablePath cached_path;
696703

697704
void dispatch(DispatchContext& ctx) const {
698705
if (op_needed(ctx)) {
699-
ctx.receiver.drawPath(path);
706+
if (ctx.receiver.PrefersImpellerPaths()) {
707+
ctx.receiver.drawPath(cached_path);
708+
} else {
709+
ctx.receiver.drawPath(cached_path.sk_path);
710+
}
700711
}
701712
}
702713

703714
DisplayListCompare equals(const DrawPathOp* other) const {
704-
return path == other->path ? DisplayListCompare::kEqual
705-
: DisplayListCompare::kNotEqual;
715+
return cached_path == other->cached_path ? DisplayListCompare::kEqual
716+
: DisplayListCompare::kNotEqual;
706717
}
707718
};
708719

@@ -1104,28 +1115,40 @@ struct DrawTextFrameOp final : DrawOpBase {
11041115
}
11051116
};
11061117

1107-
// 4 byte header + 28 byte payload packs evenly into 32 bytes
1108-
#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \
1109-
struct Draw##name##Op final : DrawOpBase { \
1110-
static const auto kType = DisplayListOpType::kDraw##name; \
1111-
\
1112-
Draw##name##Op(const SkPath& path, \
1113-
DlColor color, \
1114-
SkScalar elevation, \
1115-
SkScalar dpr) \
1116-
: color(color), elevation(elevation), dpr(dpr), path(path) {} \
1117-
\
1118-
const DlColor color; \
1119-
const SkScalar elevation; \
1120-
const SkScalar dpr; \
1121-
const SkPath path; \
1122-
\
1123-
void dispatch(DispatchContext& ctx) const { \
1124-
if (op_needed(ctx)) { \
1125-
ctx.receiver.drawShadow(path, color, elevation, transparent_occluder, \
1126-
dpr); \
1127-
} \
1128-
} \
1118+
// 4 byte header + 140 byte payload packs evenly into 140 bytes
1119+
#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \
1120+
struct Draw##name##Op final : DrawOpBase { \
1121+
static const auto kType = DisplayListOpType::kDraw##name; \
1122+
\
1123+
Draw##name##Op(const SkPath& path, \
1124+
DlColor color, \
1125+
SkScalar elevation, \
1126+
SkScalar dpr) \
1127+
: color(color), elevation(elevation), dpr(dpr), cached_path(path) {} \
1128+
\
1129+
const DlColor color; \
1130+
const SkScalar elevation; \
1131+
const SkScalar dpr; \
1132+
const DlOpReceiver::CacheablePath cached_path; \
1133+
\
1134+
void dispatch(DispatchContext& ctx) const { \
1135+
if (op_needed(ctx)) { \
1136+
if (ctx.receiver.PrefersImpellerPaths()) { \
1137+
ctx.receiver.drawShadow(cached_path, color, elevation, \
1138+
transparent_occluder, dpr); \
1139+
} else { \
1140+
ctx.receiver.drawShadow(cached_path.sk_path, color, elevation, \
1141+
transparent_occluder, dpr); \
1142+
} \
1143+
} \
1144+
} \
1145+
\
1146+
DisplayListCompare equals(const Draw##name##Op* other) const { \
1147+
return color == other->color && elevation == other->elevation && \
1148+
dpr == other->dpr && cached_path == other->cached_path \
1149+
? DisplayListCompare::kEqual \
1150+
: DisplayListCompare::kNotEqual; \
1151+
} \
11291152
};
11301153
DEFINE_DRAW_SHADOW_OP(Shadow, false)
11311154
DEFINE_DRAW_SHADOW_OP(ShadowTransparentOccluder, true)

0 commit comments

Comments
 (0)