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

Commit d9f466f

Browse files
authored
[DisplayList] DlPath object provides auto-conversion from Skia to Impeller (#55055)
Switch from using the clumsy manual CacheablePath object to a more automatic DlPath object for holding paths in DisplayLists and dispatching them to either Skia or Impeller with auto-conversion. For now DlPath is just a wrapper around SkPath with an auto-generating Impeller Path object which is very similar in design from what was done with the CacheablePath object except that it manages the caching of the Impeller path internally without extra burden on Impeller or Skia. There is also no need to communicate with the Dispatch method as to which type of path you prefer, they're all "auto-converting" DlPath objects now. For now, ui.Path still generates an SkPath and so we wrap it when we record it into a DisplayList, just like the former CacheablePath mechanism. It will be a simple conversion to create the DlPath wrapper in ui.Path, though, so as to maintain the cached Impeller paths across frames even if the DisplayList itself is not preserved. Eventually DlPath will take on more of a role of hiding the construction and internal representation of the paths so that we could be using SkPath, impeller::Path, or some other internal storage. For now, SkPath will likely remain primary storage for a while so that we can deal with PathOps.
1 parent b18adaf commit d9f466f

31 files changed

+731
-439
lines changed

ci/licenses_golden/excluded_files

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -41,6 +41,7 @@
4141
../../../flutter/display_list/effects/dl_image_filter_unittests.cc
4242
../../../flutter/display_list/effects/dl_mask_filter_unittests.cc
4343
../../../flutter/display_list/geometry/dl_geometry_types_unittests.cc
44+
../../../flutter/display_list/geometry/dl_path_unittests.cc
4445
../../../flutter/display_list/geometry/dl_region_unittests.cc
4546
../../../flutter/display_list/geometry/dl_rtree_unittests.cc
4647
../../../flutter/display_list/skia/dl_sk_conversions_unittests.cc

ci/licenses_golden/licenses_flutter

Lines changed: 4 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42345,6 +42345,8 @@ ORIGIN: ../../../flutter/display_list/effects/dl_mask_filter.h + ../../../flutte
4234542345
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.cc + ../../../flutter/LICENSE
4234642346
ORIGIN: ../../../flutter/display_list/effects/dl_runtime_effect.h + ../../../flutter/LICENSE
4234742347
ORIGIN: ../../../flutter/display_list/geometry/dl_geometry_types.h + ../../../flutter/LICENSE
42348+
ORIGIN: ../../../flutter/display_list/geometry/dl_path.cc + ../../../flutter/LICENSE
42349+
ORIGIN: ../../../flutter/display_list/geometry/dl_path.h + ../../../flutter/LICENSE
4234842350
ORIGIN: ../../../flutter/display_list/geometry/dl_region.cc + ../../../flutter/LICENSE
4234942351
ORIGIN: ../../../flutter/display_list/geometry/dl_region.h + ../../../flutter/LICENSE
4235042352
ORIGIN: ../../../flutter/display_list/geometry/dl_rtree.cc + ../../../flutter/LICENSE
@@ -45240,6 +45242,8 @@ FILE: ../../../flutter/display_list/effects/dl_mask_filter.h
4524045242
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.cc
4524145243
FILE: ../../../flutter/display_list/effects/dl_runtime_effect.h
4524245244
FILE: ../../../flutter/display_list/geometry/dl_geometry_types.h
45245+
FILE: ../../../flutter/display_list/geometry/dl_path.cc
45246+
FILE: ../../../flutter/display_list/geometry/dl_path.h
4524345247
FILE: ../../../flutter/display_list/geometry/dl_region.cc
4524445248
FILE: ../../../flutter/display_list/geometry/dl_region.h
4524545249
FILE: ../../../flutter/display_list/geometry/dl_rtree.cc

display_list/BUILD.gn

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -60,6 +60,8 @@ source_set("display_list") {
6060
"effects/dl_runtime_effect.cc",
6161
"effects/dl_runtime_effect.h",
6262
"geometry/dl_geometry_types.h",
63+
"geometry/dl_path.cc",
64+
"geometry/dl_path.h",
6365
"geometry/dl_region.cc",
6466
"geometry/dl_region.h",
6567
"geometry/dl_rtree.cc",
@@ -119,6 +121,7 @@ if (enable_unittests) {
119121
"effects/dl_image_filter_unittests.cc",
120122
"effects/dl_mask_filter_unittests.cc",
121123
"geometry/dl_geometry_types_unittests.cc",
124+
"geometry/dl_path_unittests.cc",
122125
"geometry/dl_region_unittests.cc",
123126
"geometry/dl_rtree_unittests.cc",
124127
"skia/dl_sk_conversions_unittests.cc",

display_list/benchmarking/dl_complexity_gl.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -332,7 +332,7 @@ void DisplayListGLComplexityCalculator::GLHelper::drawDRRect(
332332
AccumulateComplexity(complexity);
333333
}
334334

335-
void DisplayListGLComplexityCalculator::GLHelper::drawPath(const SkPath& path) {
335+
void DisplayListGLComplexityCalculator::GLHelper::drawPath(const DlPath& path) {
336336
if (IsComplex()) {
337337
return;
338338
}
@@ -655,7 +655,7 @@ void DisplayListGLComplexityCalculator::GLHelper::drawTextFrame(
655655
DlScalar y) {}
656656

657657
void DisplayListGLComplexityCalculator::GLHelper::drawShadow(
658-
const SkPath& path,
658+
const DlPath& path,
659659
const DlColor color,
660660
const DlScalar elevation,
661661
bool transparent_occluder,

display_list/benchmarking/dl_complexity_gl.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class DisplayListGLComplexityCalculator
4949
void drawCircle(const DlPoint& center, DlScalar radius) override;
5050
void drawRRect(const SkRRect& rrect) override;
5151
void drawDRRect(const SkRRect& outer, const SkRRect& inner) override;
52-
void drawPath(const SkPath& path) override;
52+
void drawPath(const DlPath& path) override;
5353
void drawArc(const DlRect& oval_bounds,
5454
DlScalar start_degrees,
5555
DlScalar sweep_degrees,
@@ -76,7 +76,7 @@ class DisplayListGLComplexityCalculator
7676
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
7777
DlScalar x,
7878
DlScalar y) override;
79-
void drawShadow(const SkPath& path,
79+
void drawShadow(const DlPath& path,
8080
const DlColor color,
8181
const DlScalar elevation,
8282
bool transparent_occluder,

display_list/benchmarking/dl_complexity_helper.h

Lines changed: 2 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -214,11 +214,12 @@ class ComplexityCalculatorHelper
214214
inline unsigned int Ceiling() { return ceiling_; }
215215
inline unsigned int CurrentComplexityScore() { return complexity_score_; }
216216

217-
unsigned int CalculatePathComplexity(const SkPath& path,
217+
unsigned int CalculatePathComplexity(const DlPath& dl_path,
218218
unsigned int line_verb_cost,
219219
unsigned int quad_verb_cost,
220220
unsigned int conic_verb_cost,
221221
unsigned int cubic_verb_cost) {
222+
const SkPath& path = dl_path.GetSkPath();
222223
int verb_count = path.countVerbs();
223224
std::vector<uint8_t> verbs(verb_count);
224225
path.getVerbs(verbs.data(), verbs.size());

display_list/benchmarking/dl_complexity_metal.cc

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -329,7 +329,7 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawDRRect(
329329
}
330330

331331
void DisplayListMetalComplexityCalculator::MetalHelper::drawPath(
332-
const SkPath& path) {
332+
const DlPath& path) {
333333
if (IsComplex()) {
334334
return;
335335
}
@@ -599,7 +599,7 @@ void DisplayListMetalComplexityCalculator::MetalHelper::drawTextFrame(
599599
DlScalar y) {}
600600

601601
void DisplayListMetalComplexityCalculator::MetalHelper::drawShadow(
602-
const SkPath& path,
602+
const DlPath& path,
603603
const DlColor color,
604604
const DlScalar elevation,
605605
bool transparent_occluder,

display_list/benchmarking/dl_complexity_metal.h

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class DisplayListMetalComplexityCalculator
4949
void drawCircle(const DlPoint& center, DlScalar radius) override;
5050
void drawRRect(const SkRRect& rrect) override;
5151
void drawDRRect(const SkRRect& outer, const SkRRect& inner) override;
52-
void drawPath(const SkPath& path) override;
52+
void drawPath(const DlPath& path) override;
5353
void drawArc(const DlRect& oval_bounds,
5454
DlScalar start_degrees,
5555
DlScalar sweep_degrees,
@@ -76,7 +76,7 @@ class DisplayListMetalComplexityCalculator
7676
void drawTextFrame(const std::shared_ptr<impeller::TextFrame>& text_frame,
7777
DlScalar x,
7878
DlScalar y) override;
79-
void drawShadow(const SkPath& path,
79+
void drawShadow(const DlPath& path,
8080
const DlColor color,
8181
const DlScalar elevation,
8282
bool transparent_occluder,

display_list/display_list_unittests.cc

Lines changed: 11 additions & 97 deletions
Original file line numberDiff line numberDiff line change
@@ -3618,98 +3618,6 @@ TEST_F(DisplayListTest, NopOperationsOmittedFromRecords) {
36183618
});
36193619
}
36203620

3621-
TEST_F(DisplayListTest, ImpellerPathPreferenceIsHonored) {
3622-
class Tester : public virtual DlOpReceiver,
3623-
public IgnoreClipDispatchHelper,
3624-
public IgnoreDrawDispatchHelper,
3625-
public IgnoreAttributeDispatchHelper,
3626-
public IgnoreTransformDispatchHelper {
3627-
public:
3628-
explicit Tester(bool prefer_impeller_paths)
3629-
: prefer_impeller_paths_(prefer_impeller_paths) {}
3630-
3631-
bool PrefersImpellerPaths() const override {
3632-
return prefer_impeller_paths_;
3633-
}
3634-
3635-
void drawPath(const SkPath& path) override { skia_draw_path_calls_++; }
3636-
3637-
void drawPath(const CacheablePath& cache) override {
3638-
impeller_draw_path_calls_++;
3639-
}
3640-
3641-
void clipPath(const SkPath& path, ClipOp op, bool is_aa) override {
3642-
skia_clip_path_calls_++;
3643-
}
3644-
3645-
void clipPath(const CacheablePath& cache, ClipOp op, bool is_aa) override {
3646-
impeller_clip_path_calls_++;
3647-
}
3648-
3649-
virtual void drawShadow(const SkPath& sk_path,
3650-
const DlColor color,
3651-
const SkScalar elevation,
3652-
bool transparent_occluder,
3653-
SkScalar dpr) override {
3654-
skia_draw_shadow_calls_++;
3655-
}
3656-
3657-
virtual void drawShadow(const CacheablePath& cache,
3658-
const DlColor color,
3659-
const SkScalar elevation,
3660-
bool transparent_occluder,
3661-
SkScalar dpr) override {
3662-
impeller_draw_shadow_calls_++;
3663-
}
3664-
3665-
int skia_draw_path_calls() const { return skia_draw_path_calls_; }
3666-
int skia_clip_path_calls() const { return skia_draw_path_calls_; }
3667-
int skia_draw_shadow_calls() const { return skia_draw_path_calls_; }
3668-
int impeller_draw_path_calls() const { return impeller_draw_path_calls_; }
3669-
int impeller_clip_path_calls() const { return impeller_draw_path_calls_; }
3670-
int impeller_draw_shadow_calls() const { return impeller_draw_path_calls_; }
3671-
3672-
private:
3673-
const bool prefer_impeller_paths_;
3674-
int skia_draw_path_calls_ = 0;
3675-
int skia_clip_path_calls_ = 0;
3676-
int skia_draw_shadow_calls_ = 0;
3677-
int impeller_draw_path_calls_ = 0;
3678-
int impeller_clip_path_calls_ = 0;
3679-
int impeller_draw_shadow_calls_ = 0;
3680-
};
3681-
3682-
DisplayListBuilder builder;
3683-
builder.DrawPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)), DlPaint());
3684-
builder.ClipPath(SkPath::Rect(SkRect::MakeLTRB(0, 0, 100, 100)),
3685-
ClipOp::kIntersect, true);
3686-
builder.DrawShadow(SkPath::Rect(SkRect::MakeLTRB(20, 20, 80, 80)),
3687-
DlColor::kBlue(), 1.0f, true, 1.0f);
3688-
auto display_list = builder.Build();
3689-
3690-
{
3691-
Tester skia_tester(false);
3692-
display_list->Dispatch(skia_tester);
3693-
EXPECT_EQ(skia_tester.skia_draw_path_calls(), 1);
3694-
EXPECT_EQ(skia_tester.skia_clip_path_calls(), 1);
3695-
EXPECT_EQ(skia_tester.skia_draw_shadow_calls(), 1);
3696-
EXPECT_EQ(skia_tester.impeller_draw_path_calls(), 0);
3697-
EXPECT_EQ(skia_tester.impeller_clip_path_calls(), 0);
3698-
EXPECT_EQ(skia_tester.impeller_draw_shadow_calls(), 0);
3699-
}
3700-
3701-
{
3702-
Tester impeller_tester(true);
3703-
display_list->Dispatch(impeller_tester);
3704-
EXPECT_EQ(impeller_tester.skia_draw_path_calls(), 0);
3705-
EXPECT_EQ(impeller_tester.skia_clip_path_calls(), 0);
3706-
EXPECT_EQ(impeller_tester.skia_draw_shadow_calls(), 0);
3707-
EXPECT_EQ(impeller_tester.impeller_draw_path_calls(), 1);
3708-
EXPECT_EQ(impeller_tester.impeller_clip_path_calls(), 1);
3709-
EXPECT_EQ(impeller_tester.impeller_draw_shadow_calls(), 1);
3710-
}
3711-
}
3712-
37133621
class SaveLayerBoundsExpector : public virtual DlOpReceiver,
37143622
public IgnoreAttributeDispatchHelper,
37153623
public IgnoreClipDispatchHelper,
@@ -4616,7 +4524,7 @@ TEST_F(DisplayListTest, DrawDisplayListForwardsBackdropFlag) {
46164524
#define CLIP_EXPECTOR(name) ClipExpector name(__FILE__, __LINE__)
46174525

46184526
struct ClipExpectation {
4619-
std::variant<DlRect, SkRRect, SkPath> shape;
4527+
std::variant<DlRect, SkRRect, DlPath> shape;
46204528
bool is_oval;
46214529
ClipOp clip_op;
46224530
bool is_aa;
@@ -4628,7 +4536,7 @@ struct ClipExpectation {
46284536
case 1:
46294537
return "SkRRect";
46304538
case 2:
4631-
return "SkPath";
4539+
return "DlPath";
46324540
default:
46334541
return "Unknown";
46344542
}
@@ -4648,7 +4556,7 @@ ::std::ostream& operator<<(::std::ostream& os, const ClipExpectation& expect) {
46484556
os << std::get<SkRRect>(expect.shape);
46494557
break;
46504558
case 2:
4651-
os << std::get<SkPath>(expect.shape);
4559+
os << std::get<DlPath>(expect.shape).GetSkPath();
46524560
break;
46534561
case 3:
46544562
os << "Unknown";
@@ -4713,7 +4621,7 @@ class ClipExpector : public virtual DlOpReceiver,
47134621
return *this;
47144622
}
47154623

4716-
ClipExpector& addExpectation(const SkPath& path,
4624+
ClipExpector& addExpectation(const DlPath& path,
47174625
ClipOp clip_op = ClipOp::kIntersect,
47184626
bool is_aa = false) {
47194627
clip_expectations_.push_back({
@@ -4725,6 +4633,12 @@ class ClipExpector : public virtual DlOpReceiver,
47254633
return *this;
47264634
}
47274635

4636+
ClipExpector& addExpectation(const SkPath& path,
4637+
ClipOp clip_op = ClipOp::kIntersect,
4638+
bool is_aa = false) {
4639+
return addExpectation(DlPath(path), clip_op, is_aa);
4640+
}
4641+
47284642
void clipRect(const DlRect& rect,
47294643
DlCanvas::ClipOp clip_op,
47304644
bool is_aa) override {
@@ -4740,7 +4654,7 @@ class ClipExpector : public virtual DlOpReceiver,
47404654
bool is_aa) override {
47414655
check(rrect, clip_op, is_aa);
47424656
}
4743-
void clipPath(const SkPath& path,
4657+
void clipPath(const DlPath& path,
47444658
DlCanvas::ClipOp clip_op,
47454659
bool is_aa) override {
47464660
check(path, clip_op, is_aa);

display_list/dl_builder.cc

Lines changed: 20 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -1044,30 +1044,30 @@ void DisplayListBuilder::ClipRRect(const SkRRect& rrect,
10441044
break;
10451045
}
10461046
}
1047-
void DisplayListBuilder::ClipPath(const SkPath& path,
1047+
void DisplayListBuilder::ClipPath(const DlPath& path,
10481048
ClipOp clip_op,
10491049
bool is_aa) {
10501050
if (current_info().is_nop) {
10511051
return;
10521052
}
1053-
if (!path.isInverseFillType()) {
1053+
if (!path.IsInverseFillType()) {
10541054
SkRect rect;
1055-
if (path.isRect(&rect)) {
1055+
if (path.IsSkRect(&rect)) {
10561056
ClipRect(rect, clip_op, is_aa);
10571057
return;
10581058
}
1059-
if (path.isOval(&rect)) {
1059+
if (path.IsSkOval(&rect)) {
10601060
ClipOval(rect, clip_op, is_aa);
10611061
return;
10621062
}
10631063
SkRRect rrect;
1064-
if (path.isRRect(&rrect)) {
1064+
if (path.IsSkRRect(&rrect)) {
10651065
ClipRRect(rrect, clip_op, is_aa);
10661066
return;
10671067
}
10681068
}
1069-
global_state().clipPath(path, clip_op, is_aa);
1070-
layer_local_state().clipPath(path, clip_op, is_aa);
1069+
global_state().clipPath(path.GetSkPath(), clip_op, is_aa);
1070+
layer_local_state().clipPath(path.GetSkPath(), clip_op, is_aa);
10711071
if (global_state().is_cull_rect_empty() ||
10721072
layer_local_state().is_cull_rect_empty()) {
10731073
current_info().is_nop = true;
@@ -1234,13 +1234,14 @@ void DisplayListBuilder::DrawDRRect(const SkRRect& outer,
12341234
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawDRRectFlags);
12351235
drawDRRect(outer, inner);
12361236
}
1237-
void DisplayListBuilder::drawPath(const SkPath& path) {
1237+
void DisplayListBuilder::drawPath(const DlPath& path) {
12381238
DisplayListAttributeFlags flags = kDrawPathFlags;
12391239
OpResult result = PaintResult(current_, flags);
12401240
if (result != OpResult::kNoEffect) {
1241-
bool is_visible = path.isInverseFillType()
1242-
? AccumulateUnbounded()
1243-
: AccumulateOpBounds(path.getBounds(), flags);
1241+
bool is_visible =
1242+
path.IsInverseFillType()
1243+
? AccumulateUnbounded()
1244+
: AccumulateOpBounds(ToSkRect(path.GetBounds()), flags);
12441245
if (is_visible) {
12451246
Push<DrawPathOp>(0, path);
12461247
CheckLayerOpacityHairlineCompatibility();
@@ -1249,6 +1250,11 @@ void DisplayListBuilder::drawPath(const SkPath& path) {
12491250
}
12501251
}
12511252
void DisplayListBuilder::DrawPath(const SkPath& path, const DlPaint& paint) {
1253+
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawPathFlags);
1254+
DlPath dl_path(path);
1255+
drawPath(dl_path);
1256+
}
1257+
void DisplayListBuilder::DrawPath(const DlPath& path, const DlPaint& paint) {
12521258
SetAttributesFromPaint(paint, DisplayListOpFlags::kDrawPathFlags);
12531259
drawPath(path);
12541260
}
@@ -1731,15 +1737,15 @@ void DisplayListBuilder::DrawTextFrame(
17311737
drawTextFrame(text_frame, x, y);
17321738
}
17331739

1734-
void DisplayListBuilder::DrawShadow(const SkPath& path,
1740+
void DisplayListBuilder::DrawShadow(const DlPath& path,
17351741
const DlColor color,
17361742
const DlScalar elevation,
17371743
bool transparent_occluder,
17381744
DlScalar dpr) {
17391745
OpResult result = PaintResult(DlPaint(color));
17401746
if (result != OpResult::kNoEffect) {
1741-
SkRect shadow_bounds =
1742-
DlCanvas::ComputeShadowBounds(path, elevation, dpr, GetTransform());
1747+
SkRect shadow_bounds = DlCanvas::ComputeShadowBounds(
1748+
path.GetSkPath(), elevation, dpr, GetTransform());
17431749
if (AccumulateOpBounds(shadow_bounds, kDrawShadowFlags)) {
17441750
transparent_occluder //
17451751
? Push<DrawShadowTransparentOccluderOp>(0, path, color, elevation,

0 commit comments

Comments
 (0)