Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

Cache Impeller paths in the DisplayList to amortize conversion #50076

Merged
auto-submit merged 4 commits into from
Jan 27, 2024
Merged

Cache Impeller paths in the DisplayList to amortize conversion #50076

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
e5a6b20
Cache Impeller paths in the DisplayList to amortize conversion
flar Jan 26, 2024
6eabe45
clang-tidy
flar Jan 26, 2024
7dfe022
simpler data sharing, Path is truly immutable, unit test
flar Jan 27, 2024
0fe6e45
update obsolete comments
flar Jan 27, 2024
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
94 changes: 93 additions & 1 deletion display_list/display_list_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1554,7 +1554,7 @@ TEST_F(DisplayListTest, FlutterSvgIssue661BoundsWereEmpty) {
// This is the more practical result. The bounds are "almost" 0,0,100x100
EXPECT_EQ(display_list->bounds().roundOut(), SkIRect::MakeWH(100, 100));
EXPECT_EQ(display_list->op_count(), 19u);
EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 352u);
EXPECT_EQ(display_list->bytes(), sizeof(DisplayList) + 384u);
}

TEST_F(DisplayListTest, TranslateAffectsCurrentTransform) {
Expand Down Expand Up @@ -3239,5 +3239,97 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
});
}

TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) {
class Tester : virtual public DlOpReceiver,
public IgnoreClipDispatchHelper,
public IgnoreDrawDispatchHelper,
public IgnoreAttributeDispatchHelper,
public IgnoreTransformDispatchHelper {
public:
explicit Tester(bool prefer_impeller_paths)
: prefer_impeller_paths_(prefer_impeller_paths) {}

bool PrefersImpellerPaths() const override {
return prefer_impeller_paths_;
}

void drawPath(const SkPath& path) override { skia_draw_path_calls_++; }

void drawPath(const CacheablePath& cache) override {
impeller_draw_path_calls_++;
}

void clipPath(const SkPath& path, ClipOp op, bool is_aa) override {
skia_clip_path_calls_++;
}

void clipPath(const CacheablePath& cache, ClipOp op, bool is_aa) override {
impeller_clip_path_calls_++;
}

virtual void drawShadow(const SkPath& sk_path,
const DlColor color,
const SkScalar elevation,
bool transparent_occluder,
SkScalar dpr) override {
skia_draw_shadow_calls_++;
}

virtual void drawShadow(const CacheablePath& cache,
const DlColor color,
const SkScalar elevation,
bool transparent_occluder,
SkScalar dpr) override {
impeller_draw_shadow_calls_++;
}

int skia_draw_path_calls() const { return skia_draw_path_calls_; }
int skia_clip_path_calls() const { return skia_draw_path_calls_; }
int skia_draw_shadow_calls() const { return skia_draw_path_calls_; }
int impeller_draw_path_calls() const { return impeller_draw_path_calls_; }
int impeller_clip_path_calls() const { return impeller_draw_path_calls_; }
int impeller_draw_shadow_calls() const { return impeller_draw_path_calls_; }

private:
const bool prefer_impeller_paths_;
int skia_draw_path_calls_ = 0;
int skia_clip_path_calls_ = 0;
int skia_draw_shadow_calls_ = 0;
int impeller_draw_path_calls_ = 0;
int impeller_clip_path_calls_ = 0;
int impeller_draw_shadow_calls_ = 0;
};

DisplayListBuilder builder;
builder.DrawPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)), DlPaint());
builder.ClipPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)),
ClipOp::kIntersect, true);
builder.DrawShadow(SkPath::Rect(SkRect::MakeLTRB(20, 20, 80, 80)),
DlColor::kBlue(), 1.0f, true, 1.0f);
auto display_list = builder.Build();

{
Tester skia_tester(false);
display_list->Dispatch(skia_tester);
EXPECT_EQ(skia_tester.skia_draw_path_calls(), 1);
EXPECT_EQ(skia_tester.skia_clip_path_calls(), 1);
EXPECT_EQ(skia_tester.skia_draw_shadow_calls(), 1);
EXPECT_EQ(skia_tester.impeller_draw_path_calls(), 0);
EXPECT_EQ(skia_tester.impeller_clip_path_calls(), 0);
EXPECT_EQ(skia_tester.impeller_draw_shadow_calls(), 0);
}

{
Tester impeller_tester(true);
display_list->Dispatch(impeller_tester);
EXPECT_EQ(impeller_tester.skia_draw_path_calls(), 0);
EXPECT_EQ(impeller_tester.skia_clip_path_calls(), 0);
EXPECT_EQ(impeller_tester.skia_draw_shadow_calls(), 0);
EXPECT_EQ(impeller_tester.impeller_draw_path_calls(), 1);
EXPECT_EQ(impeller_tester.impeller_clip_path_calls(), 1);
EXPECT_EQ(impeller_tester.impeller_draw_shadow_calls(), 1);
}
}

} // namespace testing
} // namespace flutter
42 changes: 42 additions & 0 deletions display_list/dl_op_receiver.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@
#include "flutter/display_list/effects/dl_path_effect.h"
#include "flutter/display_list/image/dl_image.h"

#include "flutter/impeller/geometry/path.h"

namespace flutter {

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

// ---------------------------------------------------------------------
// The CacheablePath forms of the drawPath, clipPath, and drawShadow
// methods are only called if the DlOpReceiver indicates that it prefers
// impeller paths by returning true from |PrefersImpellerPaths|.
// Note that we pass in both the SkPath and (a place to cache the)
// impeller::Path forms of the path since the SkPath version can contain
// information about the type of path that lets the receiver optimize
// the operation (and potentially avoid the need to cache it).
// It is up to the receiver to convert the path to Impeller form and
// cache it to avoid needing to do a lot of Impeller-specific processing
// inside the DisplayList code.

virtual bool PrefersImpellerPaths() const { return false; }

struct CacheablePath {
explicit CacheablePath(const SkPath& path) : sk_path(path) {}

const SkPath sk_path;
mutable impeller::Path cached_impeller_path;

bool operator==(const CacheablePath& other) const {
return sk_path == other.sk_path;
}
};

virtual void clipPath(const CacheablePath& cache,
ClipOp clip_op,
bool is_aa) {
FML_UNREACHABLE();
}
virtual void drawPath(const CacheablePath& cache) { FML_UNREACHABLE(); }
virtual void drawShadow(const CacheablePath& cache,
const DlColor color,
const SkScalar elevation,
bool transparent_occluder,
SkScalar dpr) {
FML_UNREACHABLE();
}
// ---------------------------------------------------------------------

// The following methods are nearly 1:1 with the methods on DlPaint and
// carry the same meanings. Each method sets a persistent value for the
// attribute for the rest of the display list or until it is reset by
Expand Down
127 changes: 75 additions & 52 deletions display_list/dl_op_records.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,8 @@
#include "flutter/display_list/effects/dl_color_source.h"
#include "flutter/fml/macros.h"

#include "impeller/typographer/text_frame.h"
#include "flutter/impeller/geometry/path.h"
#include "flutter/impeller/typographer/text_frame.h"
#include "third_party/skia/include/core/SkRSXform.h"

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

#define DEFINE_CLIP_PATH_OP(clipop) \
struct Clip##clipop##PathOp final : TransformClipOpBase { \
static const auto kType = DisplayListOpType::kClip##clipop##Path; \
\
Clip##clipop##PathOp(const SkPath& path, bool is_aa) \
: is_aa(is_aa), path(path) {} \
\
const bool is_aa; \
const SkPath path; \
\
void dispatch(DispatchContext& ctx) const { \
if (op_needed(ctx)) { \
ctx.receiver.clipPath(path, DlCanvas::ClipOp::k##clipop, is_aa); \
} \
} \
\
DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \
return is_aa == other->is_aa && path == other->path \
? DisplayListCompare::kEqual \
: DisplayListCompare::kNotEqual; \
} \
#define DEFINE_CLIP_PATH_OP(clipop) \
struct Clip##clipop##PathOp final : TransformClipOpBase { \
static const auto kType = DisplayListOpType::kClip##clipop##Path; \
\
Clip##clipop##PathOp(const SkPath& path, bool is_aa) \
: is_aa(is_aa), cached_path(path) {} \
\
const bool is_aa; \
const DlOpReceiver::CacheablePath cached_path; \
\
void dispatch(DispatchContext& ctx) const { \
if (op_needed(ctx)) { \
if (ctx.receiver.PrefersImpellerPaths()) { \
ctx.receiver.clipPath(cached_path, DlCanvas::ClipOp::k##clipop, \
is_aa); \
} else { \
ctx.receiver.clipPath(cached_path.sk_path, \
DlCanvas::ClipOp::k##clipop, is_aa); \
} \
} \
} \
\
DisplayListCompare equals(const Clip##clipop##PathOp* other) const { \
return is_aa == other->is_aa && cached_path == other->cached_path \
? DisplayListCompare::kEqual \
: DisplayListCompare::kNotEqual; \
} \
};
DEFINE_CLIP_PATH_OP(Intersect)
DEFINE_CLIP_PATH_OP(Difference)
Expand Down Expand Up @@ -685,24 +692,28 @@ DEFINE_DRAW_1ARG_OP(Oval, SkRect, oval)
DEFINE_DRAW_1ARG_OP(RRect, SkRRect, rrect)
#undef DEFINE_DRAW_1ARG_OP

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

explicit DrawPathOp(const SkPath& path) : path(path) {}
explicit DrawPathOp(const SkPath& path) : cached_path(path) {}

const SkPath path;
const DlOpReceiver::CacheablePath cached_path;

void dispatch(DispatchContext& ctx) const {
if (op_needed(ctx)) {
ctx.receiver.drawPath(path);
if (ctx.receiver.PrefersImpellerPaths()) {
ctx.receiver.drawPath(cached_path);
} else {
ctx.receiver.drawPath(cached_path.sk_path);
}
}
}

DisplayListCompare equals(const DrawPathOp* other) const {
return path == other->path ? DisplayListCompare::kEqual
: DisplayListCompare::kNotEqual;
return cached_path == other->cached_path ? DisplayListCompare::kEqual
: DisplayListCompare::kNotEqual;
}
};

Expand Down Expand Up @@ -1104,28 +1115,40 @@ struct DrawTextFrameOp final : DrawOpBase {
}
};

// 4 byte header + 28 byte payload packs evenly into 32 bytes
#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \
struct Draw##name##Op final : DrawOpBase { \
static const auto kType = DisplayListOpType::kDraw##name; \
\
Draw##name##Op(const SkPath& path, \
DlColor color, \
SkScalar elevation, \
SkScalar dpr) \
: color(color), elevation(elevation), dpr(dpr), path(path) {} \
\
const DlColor color; \
const SkScalar elevation; \
const SkScalar dpr; \
const SkPath path; \
\
void dispatch(DispatchContext& ctx) const { \
if (op_needed(ctx)) { \
ctx.receiver.drawShadow(path, color, elevation, transparent_occluder, \
dpr); \
} \
} \
// 4 byte header + 140 byte payload packs evenly into 140 bytes
#define DEFINE_DRAW_SHADOW_OP(name, transparent_occluder) \
struct Draw##name##Op final : DrawOpBase { \
static const auto kType = DisplayListOpType::kDraw##name; \
\
Draw##name##Op(const SkPath& path, \
DlColor color, \
SkScalar elevation, \
SkScalar dpr) \
: color(color), elevation(elevation), dpr(dpr), cached_path(path) {} \
\
const DlColor color; \
const SkScalar elevation; \
const SkScalar dpr; \
const DlOpReceiver::CacheablePath cached_path; \
\
void dispatch(DispatchContext& ctx) const { \
if (op_needed(ctx)) { \
if (ctx.receiver.PrefersImpellerPaths()) { \
ctx.receiver.drawShadow(cached_path, color, elevation, \
transparent_occluder, dpr); \
} else { \
ctx.receiver.drawShadow(cached_path.sk_path, color, elevation, \
transparent_occluder, dpr); \
} \
} \
} \
\
DisplayListCompare equals(const Draw##name##Op* other) const { \
return color == other->color && elevation == other->elevation && \
dpr == other->dpr && cached_path == other->cached_path \
? DisplayListCompare::kEqual \
: DisplayListCompare::kNotEqual; \
} \
};
DEFINE_DRAW_SHADOW_OP(Shadow, false)
DEFINE_DRAW_SHADOW_OP(ShadowTransparentOccluder, true)
Expand Down
Loading