From 0b9de40ced59bd2f6d710cab48021bce71c3aa1b Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Fri, 26 Jan 2024 21:06:18 -0800 Subject: [PATCH] Cache Impeller paths in the DisplayList to amortize conversion (flutter/engine#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()`. --- .../display_list/display_list_unittests.cc | 94 +++++- .../src/flutter/display_list/dl_op_receiver.h | 42 +++ .../src/flutter/display_list/dl_op_records.h | 127 ++++---- .../display_list/testing/dl_test_snippets.cc | 34 +-- .../impeller/aiks/aiks_gradient_unittests.cc | 2 +- .../impeller/aiks/aiks_path_unittests.cc | 8 +- .../flutter/impeller/aiks/aiks_unittests.cc | 4 +- engine/src/flutter/impeller/aiks/canvas.cc | 20 +- engine/src/flutter/impeller/aiks/canvas.h | 4 +- .../flutter/impeller/aiks/canvas_unittests.cc | 10 +- .../impeller/display_list/dl_dispatcher.cc | 50 +++- .../impeller/display_list/dl_dispatcher.h | 22 +- .../impeller/display_list/dl_unittests.cc | 5 +- .../entity/contents/solid_color_contents.cc | 4 +- .../entity/contents/solid_color_contents.h | 3 +- .../impeller/entity/entity_unittests.cc | 24 +- .../entity/geometry/fill_path_geometry.cc | 5 +- .../entity/geometry/fill_path_geometry.h | 2 +- .../impeller/entity/geometry/geometry.cc | 10 +- .../impeller/entity/geometry/geometry.h | 4 +- .../entity/geometry/geometry_unittests.cc | 4 +- .../entity/geometry/stroke_path_geometry.cc | 4 +- .../entity/geometry/stroke_path_geometry.h | 2 +- .../impeller/geometry/geometry_benchmarks.cc | 4 +- engine/src/flutter/impeller/geometry/path.cc | 282 +++++------------- engine/src/flutter/impeller/geometry/path.h | 76 ++--- .../flutter/impeller/geometry/path_builder.cc | 259 +++++++++++----- .../flutter/impeller/geometry/path_builder.h | 28 +- .../impeller/geometry/path_unittests.cc | 203 ++++++++++++- 29 files changed, 878 insertions(+), 458 deletions(-) diff --git a/engine/src/flutter/display_list/display_list_unittests.cc b/engine/src/flutter/display_list/display_list_unittests.cc index 729897cb353..8845ce3ac60 100644 --- a/engine/src/flutter/display_list/display_list_unittests.cc +++ b/engine/src/flutter/display_list/display_list_unittests.cc @@ -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) { @@ -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 diff --git a/engine/src/flutter/display_list/dl_op_receiver.h b/engine/src/flutter/display_list/dl_op_receiver.h index d15cbba4d87..dff97c3c15b 100644 --- a/engine/src/flutter/display_list/dl_op_receiver.h +++ b/engine/src/flutter/display_list/dl_op_receiver.h @@ -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; @@ -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 diff --git a/engine/src/flutter/display_list/dl_op_records.h b/engine/src/flutter/display_list/dl_op_records.h index fa9aeaafa00..259102c1425 100644 --- a/engine/src/flutter/display_list/dl_op_records.h +++ b/engine/src/flutter/display_list/dl_op_records.h @@ -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 { @@ -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 @@ -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) @@ -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; } }; @@ -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) diff --git a/engine/src/flutter/display_list/testing/dl_test_snippets.cc b/engine/src/flutter/display_list/testing/dl_test_snippets.cc index c831005482c..c26d95ab91e 100644 --- a/engine/src/flutter/display_list/testing/dl_test_snippets.cc +++ b/engine/src/flutter/display_list/testing/dl_test_snippets.cc @@ -466,27 +466,27 @@ std::vector CreateAllClipOps() { }}, {"ClipPath", { - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath1, DlCanvas::ClipOp::kIntersect, true); }}, - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath2, DlCanvas::ClipOp::kIntersect, true); }}, - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath3, DlCanvas::ClipOp::kIntersect, true); }}, - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath1, DlCanvas::ClipOp::kIntersect, false); }}, - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath1, DlCanvas::ClipOp::kDifference, true); }}, - {1, 24, 1, 24, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.clipPath(kTestPath1, DlCanvas::ClipOp::kDifference, false); }}, @@ -617,11 +617,11 @@ std::vector CreateAllRenderingOps() { }}, {"DrawPath", { - {1, 24, 1, 24, [](DlOpReceiver& r) { r.drawPath(kTestPath1); }}, - {1, 24, 1, 24, [](DlOpReceiver& r) { r.drawPath(kTestPath2); }}, - {1, 24, 1, 24, [](DlOpReceiver& r) { r.drawPath(kTestPath3); }}, - {1, 24, 1, 24, [](DlOpReceiver& r) { r.drawPath(kTestPathRect); }}, - {1, 24, 1, 24, [](DlOpReceiver& r) { r.drawPath(kTestPathOval); }}, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.drawPath(kTestPath1); }}, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.drawPath(kTestPath2); }}, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.drawPath(kTestPath3); }}, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.drawPath(kTestPathRect); }}, + {1, 40, 1, 40, [](DlOpReceiver& r) { r.drawPath(kTestPathOval); }}, }}, {"DrawArc", { @@ -929,27 +929,27 @@ std::vector CreateAllRenderingOps() { { // cv shadows are turned into an opaque ShadowRec which is not // exposed - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, false, 1.0); }}, - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath2, DlColor(SK_ColorGREEN), 1.0, false, 1.0); }}, - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorBLUE), 1.0, false, 1.0); }}, - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 2.0, false, 1.0); }}, - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, true, 1.0); }}, - {1, 32, -1, 32, + {1, 48, -1, 48, [](DlOpReceiver& r) { r.drawShadow(kTestPath1, DlColor(SK_ColorGREEN), 1.0, false, 2.5); }}, diff --git a/engine/src/flutter/impeller/aiks/aiks_gradient_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_gradient_unittests.cc index bfecaac904e..70fe173daaf 100644 --- a/engine/src/flutter/impeller/aiks/aiks_gradient_unittests.cc +++ b/engine/src/flutter/impeller/aiks/aiks_gradient_unittests.cc @@ -713,7 +713,7 @@ TEST_P(AiksTest, GradientStrokesRenderCorrectly) { paint.stroke_join = join; for (auto cap : {Cap::kButt, Cap::kSquare, Cap::kRound}) { paint.stroke_cap = cap; - canvas.DrawPath(path.Clone(), paint); + canvas.DrawPath(path, paint); canvas.Translate({80, 0}); } canvas.Translate({-240, 60}); diff --git a/engine/src/flutter/impeller/aiks/aiks_path_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_path_unittests.cc index 47b835b1c45..6f194d0623e 100644 --- a/engine/src/flutter/impeller/aiks/aiks_path_unittests.cc +++ b/engine/src/flutter/impeller/aiks/aiks_path_unittests.cc @@ -37,8 +37,8 @@ TEST_P(AiksTest, RotateColorFilteredPath) { ColorFilter::MakeBlend(BlendMode::kSourceIn, Color::AliceBlue()), }; - canvas.DrawPath(std::move(arrow_stem), paint); - canvas.DrawPath(std::move(arrow_head), paint); + canvas.DrawPath(arrow_stem, paint); + canvas.DrawPath(arrow_head, paint); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } @@ -125,7 +125,7 @@ TEST_P(AiksTest, CanRenderDifferencePaths) { canvas.DrawImage( std::make_shared(CreateTextureForFixture("boston.jpg")), {10, 10}, Paint{}); - canvas.DrawPath(std::move(path), paint); + canvas.DrawPath(path, paint); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } @@ -229,7 +229,7 @@ TEST_P(AiksTest, SolidStrokesRenderCorrectly) { paint.stroke_join = join; for (auto cap : {Cap::kButt, Cap::kSquare, Cap::kRound}) { paint.stroke_cap = cap; - canvas.DrawPath(path.Clone(), paint); + canvas.DrawPath(path, paint); canvas.Translate({80, 0}); } canvas.Translate({-240, 60}); diff --git a/engine/src/flutter/impeller/aiks/aiks_unittests.cc b/engine/src/flutter/impeller/aiks/aiks_unittests.cc index 9e36f2e3ed9..b8ace127462 100644 --- a/engine/src/flutter/impeller/aiks/aiks_unittests.cc +++ b/engine/src/flutter/impeller/aiks/aiks_unittests.cc @@ -672,7 +672,7 @@ TEST_P(AiksTest, CanRenderRoundedRectWithNonUniformRadii) { .AddRoundedRect(Rect::MakeXYWH(100, 100, 500, 500), radii) .TakePath(); - canvas.DrawPath(std::move(path), paint); + canvas.DrawPath(path, paint); ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } @@ -2392,7 +2392,7 @@ TEST_P(AiksTest, ImageFilteredSaveLayerWithUnboundedContents) { .TakePath(); Paint paint = p; paint.style = Paint::Style::kStroke; - canvas.DrawPath(std::move(path), paint); + canvas.DrawPath(path, paint); }; // Registration marks for the edge of the SaveLayer DrawLine(Point(75, 100), Point(225, 100), {.color = Color::White()}); diff --git a/engine/src/flutter/impeller/aiks/canvas.cc b/engine/src/flutter/impeller/aiks/canvas.cc index cb40eb802ab..6c72e097db7 100644 --- a/engine/src/flutter/impeller/aiks/canvas.cc +++ b/engine/src/flutter/impeller/aiks/canvas.cc @@ -76,16 +76,16 @@ static std::shared_ptr CreateContentsForGeometryWithFilters( static std::shared_ptr CreatePathContentsWithFilters( const Paint& paint, - Path path = {}) { + const Path& path) { std::shared_ptr geometry; switch (paint.style) { case Paint::Style::kFill: - geometry = Geometry::MakeFillPath(std::move(path)); + geometry = Geometry::MakeFillPath(path); break; case Paint::Style::kStroke: - geometry = Geometry::MakeStrokePath(std::move(path), paint.stroke_width, - paint.stroke_miter, paint.stroke_cap, - paint.stroke_join); + geometry = + Geometry::MakeStrokePath(path, paint.stroke_width, paint.stroke_miter, + paint.stroke_cap, paint.stroke_join); break; } @@ -283,12 +283,12 @@ void Canvas::RestoreToCount(size_t count) { } } -void Canvas::DrawPath(Path path, const Paint& paint) { +void Canvas::DrawPath(const Path& path, const Paint& paint) { Entity entity; entity.SetTransform(GetCurrentTransform()); entity.SetClipDepth(GetClipDepth()); entity.SetBlendMode(paint.blend_mode); - entity.SetContents(CreatePathContentsWithFilters(paint, std::move(path))); + entity.SetContents(CreatePathContentsWithFilters(paint, path)); GetCurrentPass().AddEntity(std::move(entity)); } @@ -425,7 +425,7 @@ void Canvas::DrawRRect(const Rect& rect, .AddRoundedRect(rect, corner_radii) .SetBounds(rect) .TakePath(); - DrawPath(std::move(path), paint); + DrawPath(path, paint); } void Canvas::DrawCircle(const Point& center, @@ -452,9 +452,9 @@ void Canvas::DrawCircle(const Point& center, GetCurrentPass().AddEntity(std::move(entity)); } -void Canvas::ClipPath(Path path, Entity::ClipOperation clip_op) { +void Canvas::ClipPath(const Path& path, Entity::ClipOperation clip_op) { auto bounds = path.GetBoundingBox(); - ClipGeometry(Geometry::MakeFillPath(std::move(path)), clip_op); + ClipGeometry(Geometry::MakeFillPath(path), clip_op); if (clip_op == Entity::ClipOperation::kIntersect) { if (bounds.has_value()) { IntersectCulling(bounds.value()); diff --git a/engine/src/flutter/impeller/aiks/canvas.h b/engine/src/flutter/impeller/aiks/canvas.h index 2067ebe7937..6285a2cf130 100644 --- a/engine/src/flutter/impeller/aiks/canvas.h +++ b/engine/src/flutter/impeller/aiks/canvas.h @@ -106,7 +106,7 @@ class Canvas { void Rotate(Radians radians); - void DrawPath(Path path, const Paint& paint); + void DrawPath(const Path& path, const Paint& paint); void DrawPaint(const Paint& paint); @@ -141,7 +141,7 @@ class Canvas { SourceRectConstraint src_rect_constraint = SourceRectConstraint::kFast); void ClipPath( - Path path, + const Path& path, Entity::ClipOperation clip_op = Entity::ClipOperation::kIntersect); void ClipRect( diff --git a/engine/src/flutter/impeller/aiks/canvas_unittests.cc b/engine/src/flutter/impeller/aiks/canvas_unittests.cc index 8dfe051ed6b..a2deabcb900 100644 --- a/engine/src/flutter/impeller/aiks/canvas_unittests.cc +++ b/engine/src/flutter/impeller/aiks/canvas_unittests.cc @@ -268,7 +268,7 @@ TEST(AiksCanvasTest, PathClipIntersectAgainstEmptyCullRect) { Rect rect_clip = Rect::MakeXYWH(5, 5, 10, 10); Canvas canvas; - canvas.ClipPath(std::move(path), Entity::ClipOperation::kIntersect); + canvas.ClipPath(path, Entity::ClipOperation::kIntersect); ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value()); ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), rect_clip); @@ -283,7 +283,7 @@ TEST(AiksCanvasTest, PathClipDiffAgainstEmptyCullRect) { Path path = builder.TakePath(); Canvas canvas; - canvas.ClipPath(std::move(path), Entity::ClipOperation::kDifference); + canvas.ClipPath(path, Entity::ClipOperation::kDifference); ASSERT_FALSE(canvas.GetCurrentLocalCullingBounds().has_value()); } @@ -299,7 +299,7 @@ TEST(AiksCanvasTest, PathClipIntersectAgainstCullRect) { Rect result_cull = Rect::MakeXYWH(5, 5, 5, 5); Canvas canvas(initial_cull); - canvas.ClipPath(std::move(path), Entity::ClipOperation::kIntersect); + canvas.ClipPath(path, Entity::ClipOperation::kIntersect); ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value()); ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull); @@ -316,7 +316,7 @@ TEST(AiksCanvasTest, PathClipDiffAgainstNonCoveredCullRect) { Rect result_cull = Rect::MakeXYWH(0, 0, 10, 10); Canvas canvas(initial_cull); - canvas.ClipPath(std::move(path), Entity::ClipOperation::kDifference); + canvas.ClipPath(path, Entity::ClipOperation::kDifference); ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value()); ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull); @@ -331,7 +331,7 @@ TEST(AiksCanvasTest, PathClipDiffAgainstFullyCoveredCullRect) { Rect result_cull = Rect::MakeXYWH(5, 5, 10, 10); Canvas canvas(initial_cull); - canvas.ClipPath(std::move(path), Entity::ClipOperation::kDifference); + canvas.ClipPath(path, Entity::ClipOperation::kDifference); ASSERT_TRUE(canvas.GetCurrentLocalCullingBounds().has_value()); ASSERT_EQ(canvas.GetCurrentLocalCullingBounds().value(), result_cull); diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc index dca79aba9ee..d999c5d9eaa 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.cc +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.cc @@ -742,21 +742,35 @@ void DlDispatcher::clipRRect(const SkRRect& rrect, ClipOp sk_op, bool is_aa) { // |flutter::DlOpReceiver| void DlDispatcher::clipPath(const SkPath& path, ClipOp sk_op, bool is_aa) { + UNIMPLEMENTED; +} + +const Path& DlDispatcher::GetOrCachePath(const CacheablePath& cache) { + if (cache.cached_impeller_path.IsEmpty() && !cache.sk_path.isEmpty()) { + cache.cached_impeller_path = skia_conversions::ToPath(cache.sk_path); + } + return cache.cached_impeller_path; +} + +// |flutter::DlOpReceiver| +void DlDispatcher::clipPath(const CacheablePath& cache, + ClipOp sk_op, + bool is_aa) { auto clip_op = ToClipOperation(sk_op); SkRect rect; - if (path.isRect(&rect)) { + if (cache.sk_path.isRect(&rect)) { canvas_.ClipRect(skia_conversions::ToRect(rect), clip_op); - } else if (path.isOval(&rect)) { + } else if (cache.sk_path.isOval(&rect)) { canvas_.ClipOval(skia_conversions::ToRect(rect), clip_op); } else { SkRRect rrect; - if (path.isRRect(&rrect) && rrect.isSimple()) { + if (cache.sk_path.isRRect(&rrect) && rrect.isSimple()) { canvas_.ClipRRect(skia_conversions::ToRect(rrect.rect()), skia_conversions::ToSize(rrect.getSimpleRadii()), clip_op); } else { - canvas_.ClipPath(skia_conversions::ToPath(path), clip_op); + canvas_.ClipPath(GetOrCachePath(cache), clip_op); } } } @@ -816,35 +830,40 @@ void DlDispatcher::drawDRRect(const SkRRect& outer, const SkRRect& inner) { // |flutter::DlOpReceiver| void DlDispatcher::drawPath(const SkPath& path) { - SimplifyOrDrawPath(canvas_, path, paint_); + UNIMPLEMENTED; +} + +// |flutter::DlOpReceiver| +void DlDispatcher::drawPath(const CacheablePath& cache) { + SimplifyOrDrawPath(canvas_, cache, paint_); } void DlDispatcher::SimplifyOrDrawPath(CanvasType& canvas, - const SkPath& path, + const CacheablePath& cache, const Paint& paint) { SkRect rect; // We can't "optimize" a path into a rectangle if it's open. bool closed; - if (path.isRect(&rect, &closed) && closed) { + if (cache.sk_path.isRect(&rect, &closed) && closed) { canvas.DrawRect(skia_conversions::ToRect(rect), paint); return; } SkRRect rrect; - if (path.isRRect(&rrect) && rrect.isSimple()) { + if (cache.sk_path.isRRect(&rrect) && rrect.isSimple()) { canvas.DrawRRect(skia_conversions::ToRect(rrect.rect()), skia_conversions::ToSize(rrect.getSimpleRadii()), paint); return; } SkRect oval; - if (path.isOval(&oval)) { + if (cache.sk_path.isOval(&oval)) { canvas.DrawOval(skia_conversions::ToRect(oval), paint); return; } - canvas.DrawPath(skia_conversions::ToPath(path), paint); + canvas.DrawPath(GetOrCachePath(cache), paint); } // |flutter::DlOpReceiver| @@ -1062,6 +1081,15 @@ void DlDispatcher::drawShadow(const SkPath& path, const SkScalar elevation, bool transparent_occluder, SkScalar dpr) { + UNIMPLEMENTED; +} + +// |flutter::DlOpReceiver| +void DlDispatcher::drawShadow(const CacheablePath& cache, + const flutter::DlColor color, + const SkScalar elevation, + bool transparent_occluder, + SkScalar dpr) { Color spot_color = skia_conversions::ToColor(color); spot_color.alpha *= 0.25; @@ -1110,7 +1138,7 @@ void DlDispatcher::drawShadow(const SkPath& path, canvas_.PreConcat( Matrix::MakeTranslation(Vector2(0, -occluder_z * light_position.y))); - SimplifyOrDrawPath(canvas_, path, paint); + SimplifyOrDrawPath(canvas_, cache, paint); canvas_.Restore(); } diff --git a/engine/src/flutter/impeller/display_list/dl_dispatcher.h b/engine/src/flutter/impeller/display_list/dl_dispatcher.h index a0a1fcfb8d3..4ef1da1c29a 100644 --- a/engine/src/flutter/impeller/display_list/dl_dispatcher.h +++ b/engine/src/flutter/impeller/display_list/dl_dispatcher.h @@ -23,6 +23,9 @@ class DlDispatcher final : public flutter::DlOpReceiver { Picture EndRecordingAsPicture(); + // |flutter::DlOpReceiver| + bool PrefersImpellerPaths() const override { return true; } + // |flutter::DlOpReceiver| void setAntiAlias(bool aa) override; @@ -126,6 +129,11 @@ class DlDispatcher final : public flutter::DlOpReceiver { // |flutter::DlOpReceiver| void clipPath(const SkPath& path, ClipOp clip_op, bool is_aa) override; + // |flutter::DlOpReceiver| + void clipPath(const CacheablePath& cache, + ClipOp clip_op, + bool is_aa) override; + // |flutter::DlOpReceiver| void drawColor(flutter::DlColor color, flutter::DlBlendMode mode) override; @@ -153,6 +161,9 @@ class DlDispatcher final : public flutter::DlOpReceiver { // |flutter::DlOpReceiver| void drawPath(const SkPath& path) override; + // |flutter::DlOpReceiver| + void drawPath(const CacheablePath& cache) override; + // |flutter::DlOpReceiver| void drawArc(const SkRect& oval_bounds, SkScalar start_degrees, @@ -221,13 +232,22 @@ class DlDispatcher final : public flutter::DlOpReceiver { bool transparent_occluder, SkScalar dpr) override; + // |flutter::DlOpReceiver| + void drawShadow(const CacheablePath& cache, + const flutter::DlColor color, + const SkScalar elevation, + bool transparent_occluder, + SkScalar dpr) override; + private: Paint paint_; CanvasType canvas_; Matrix initial_matrix_; + static const Path& GetOrCachePath(const CacheablePath& cache); + static void SimplifyOrDrawPath(CanvasType& canvas, - const SkPath& path, + const CacheablePath& cache, const Paint& paint); DlDispatcher(const DlDispatcher&) = delete; diff --git a/engine/src/flutter/impeller/display_list/dl_unittests.cc b/engine/src/flutter/impeller/display_list/dl_unittests.cc index 258141e2400..112c8563bf3 100644 --- a/engine/src/flutter/impeller/display_list/dl_unittests.cc +++ b/engine/src/flutter/impeller/display_list/dl_unittests.cc @@ -961,8 +961,9 @@ TEST_P(DisplayListTest, TransparentShadowProducesCorrectColor) { DlDispatcher dispatcher; dispatcher.save(); dispatcher.scale(1.618, 1.618); - dispatcher.drawShadow(SkPath{}.addRect(SkRect::MakeXYWH(0, 0, 200, 100)), - flutter::DlColor::kTransparent(), 15, false, 1); + SkPath path = SkPath{}.addRect(SkRect::MakeXYWH(0, 0, 200, 100)); + flutter::DlOpReceiver::CacheablePath cache(path); + dispatcher.drawShadow(cache, flutter::DlColor::kTransparent(), 15, false, 1); dispatcher.restore(); auto picture = dispatcher.EndRecordingAsPicture(); diff --git a/engine/src/flutter/impeller/entity/contents/solid_color_contents.cc b/engine/src/flutter/impeller/entity/contents/solid_color_contents.cc index d7c3f985a81..c2545fc0a01 100644 --- a/engine/src/flutter/impeller/entity/contents/solid_color_contents.cc +++ b/engine/src/flutter/impeller/entity/contents/solid_color_contents.cc @@ -86,10 +86,10 @@ bool SolidColorContents::Render(const ContentContext& renderer, return true; } -std::unique_ptr SolidColorContents::Make(Path path, +std::unique_ptr SolidColorContents::Make(const Path& path, Color color) { auto contents = std::make_unique(); - contents->SetGeometry(Geometry::MakeFillPath(std::move(path))); + contents->SetGeometry(Geometry::MakeFillPath(path)); contents->SetColor(color); return contents; } diff --git a/engine/src/flutter/impeller/entity/contents/solid_color_contents.h b/engine/src/flutter/impeller/entity/contents/solid_color_contents.h index 6d0b54359a2..59358ac68f3 100644 --- a/engine/src/flutter/impeller/entity/contents/solid_color_contents.h +++ b/engine/src/flutter/impeller/entity/contents/solid_color_contents.h @@ -24,7 +24,8 @@ class SolidColorContents final : public ColorSourceContents { ~SolidColorContents() override; - static std::unique_ptr Make(Path path, Color color); + static std::unique_ptr Make(const Path& path, + Color color); void SetColor(Color color); diff --git a/engine/src/flutter/impeller/entity/entity_unittests.cc b/engine/src/flutter/impeller/entity/entity_unittests.cc index f3d40b009cf..17d83085de4 100644 --- a/engine/src/flutter/impeller/entity/entity_unittests.cc +++ b/engine/src/flutter/impeller/entity/entity_unittests.cc @@ -238,7 +238,7 @@ TEST_P(EntityTest, CanDrawRRect) { .SetConvexity(Convexity::kConvex) .AddRoundedRect(Rect::MakeXYWH(100, 100, 100, 100), 10.0) .TakePath(); - contents->SetGeometry(Geometry::MakeFillPath(std::move(path))); + contents->SetGeometry(Geometry::MakeFillPath(path)); contents->SetColor(Color::Red()); Entity entity; @@ -269,7 +269,7 @@ TEST_P(EntityTest, ThreeStrokesInOnePath) { Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); auto contents = std::make_unique(); - contents->SetGeometry(Geometry::MakeStrokePath(std::move(path), 5.0)); + contents->SetGeometry(Geometry::MakeStrokePath(path, 5.0)); contents->SetColor(Color::Red()); entity.SetContents(std::move(contents)); ASSERT_TRUE(OpenPlaygroundHere(std::move(entity))); @@ -289,7 +289,7 @@ TEST_P(EntityTest, StrokeWithTextureContents) { Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); auto contents = std::make_unique(); - contents->SetGeometry(Geometry::MakeStrokePath(std::move(path), 100.0)); + contents->SetGeometry(Geometry::MakeStrokePath(path, 100.0)); contents->SetTexture(bridge); contents->SetTileModes(Entity::TileMode::kClamp, Entity::TileMode::kClamp); entity.SetContents(std::move(contents)); @@ -329,7 +329,7 @@ TEST_P(EntityTest, TriangleInsideASquare) { Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); auto contents = std::make_unique(); - contents->SetGeometry(Geometry::MakeStrokePath(std::move(path), 20.0)); + contents->SetGeometry(Geometry::MakeStrokePath(path, 20.0)); contents->SetColor(Color::Red()); entity.SetContents(std::move(contents)); @@ -364,8 +364,8 @@ TEST_P(EntityTest, StrokeCapAndJoinTest) { auto render_path = [width = width, &context, &pass, &world_matrix]( const Path& path, Cap cap, Join join) { auto contents = std::make_unique(); - contents->SetGeometry(Geometry::MakeStrokePath(path.Clone(), width, - miter_limit, cap, join)); + contents->SetGeometry( + Geometry::MakeStrokePath(path, width, miter_limit, cap, join)); contents->SetColor(Color::Red()); Entity entity; @@ -478,7 +478,7 @@ TEST_P(EntityTest, CubicCurveTest) { .TakePath(); Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); - entity.SetContents(SolidColorContents::Make(std::move(path), Color::Red())); + entity.SetContents(SolidColorContents::Make(path, Color::Red())); ASSERT_TRUE(OpenPlaygroundHere(std::move(entity))); } @@ -522,7 +522,7 @@ TEST_P(EntityTest, CanDrawCorrectlyWithRotatedTransform) { Entity entity; entity.SetTransform(result_transform); - entity.SetContents(SolidColorContents::Make(std::move(path), Color::Red())); + entity.SetContents(SolidColorContents::Make(path, Color::Red())); return entity.Render(context, pass); }; ASSERT_TRUE(OpenPlaygroundHere(callback)); @@ -752,7 +752,7 @@ TEST_P(EntityTest, CubicCurveAndOverlapTest) { .TakePath(); Entity entity; entity.SetTransform(Matrix::MakeScale(GetContentScale())); - entity.SetContents(SolidColorContents::Make(std::move(path), Color::Red())); + entity.SetContents(SolidColorContents::Make(path, Color::Red())); ASSERT_TRUE(OpenPlaygroundHere(std::move(entity))); } @@ -952,7 +952,7 @@ TEST_P(EntityTest, BezierCircleScaled) { .TakePath(); entity.SetTransform( Matrix::MakeScale({scale, scale, 1.0}).Translate({-90, -20, 0})); - entity.SetContents(SolidColorContents::Make(std::move(path), Color::Red())); + entity.SetContents(SolidColorContents::Make(path, Color::Red())); return entity.Render(context, pass); }; ASSERT_TRUE(OpenPlaygroundHere(callback)); @@ -2450,8 +2450,8 @@ TEST_P(EntityTest, CoverageForStrokePathWithNegativeValuesInTransform) { .LineTo({120, 190}) .LineTo({190, 120}) .TakePath(); - auto geometry = Geometry::MakeStrokePath(std::move(arrow_head), 15.0, 4.0, - Cap::kRound, Join::kRound); + auto geometry = Geometry::MakeStrokePath(arrow_head, 15.0, 4.0, Cap::kRound, + Join::kRound); auto transform = Matrix::MakeTranslation({300, 300}) * Matrix::MakeRotationZ(Radians(kPiOver2)); diff --git a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc index a906a661636..8069ef9aa87 100644 --- a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc +++ b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.cc @@ -7,8 +7,9 @@ namespace impeller { -FillPathGeometry::FillPathGeometry(Path path, std::optional inner_rect) - : path_(std::move(path)), inner_rect_(inner_rect) {} +FillPathGeometry::FillPathGeometry(const Path& path, + std::optional inner_rect) + : path_(path), inner_rect_(inner_rect) {} GeometryResult FillPathGeometry::GetPositionBuffer( const ContentContext& renderer, diff --git a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.h b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.h index a1210846b31..c93a04b90cc 100644 --- a/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.h +++ b/engine/src/flutter/impeller/entity/geometry/fill_path_geometry.h @@ -15,7 +15,7 @@ namespace impeller { /// @brief A geometry that is created from a filled path object. class FillPathGeometry final : public Geometry { public: - explicit FillPathGeometry(Path path, + explicit FillPathGeometry(const Path& path, std::optional inner_rect = std::nullopt); ~FillPathGeometry() = default; diff --git a/engine/src/flutter/impeller/entity/geometry/geometry.cc b/engine/src/flutter/impeller/entity/geometry/geometry.cc index 0690cc5abb6..65b2b43c583 100644 --- a/engine/src/flutter/impeller/entity/geometry/geometry.cc +++ b/engine/src/flutter/impeller/entity/geometry/geometry.cc @@ -171,9 +171,9 @@ GeometryResult Geometry::GetPositionUVBuffer(Rect texture_coverage, } std::shared_ptr Geometry::MakeFillPath( - Path path, + const Path& path, std::optional inner_rect) { - return std::make_shared(std::move(path), inner_rect); + return std::make_shared(path, inner_rect); } std::shared_ptr Geometry::MakePointField(std::vector points, @@ -182,7 +182,7 @@ std::shared_ptr Geometry::MakePointField(std::vector points, return std::make_shared(std::move(points), radius, round); } -std::shared_ptr Geometry::MakeStrokePath(Path path, +std::shared_ptr Geometry::MakeStrokePath(const Path& path, Scalar stroke_width, Scalar miter_limit, Cap stroke_cap, @@ -191,8 +191,8 @@ std::shared_ptr Geometry::MakeStrokePath(Path path, if (miter_limit < 0) { miter_limit = 4.0; } - return std::make_shared( - std::move(path), stroke_width, miter_limit, stroke_cap, stroke_join); + return std::make_shared(path, stroke_width, miter_limit, + stroke_cap, stroke_join); } std::shared_ptr Geometry::MakeCover() { diff --git a/engine/src/flutter/impeller/entity/geometry/geometry.h b/engine/src/flutter/impeller/entity/geometry/geometry.h index 7c639e6b35c..ef574fd7278 100644 --- a/engine/src/flutter/impeller/entity/geometry/geometry.h +++ b/engine/src/flutter/impeller/entity/geometry/geometry.h @@ -68,11 +68,11 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect, class Geometry { public: static std::shared_ptr MakeFillPath( - Path path, + const Path& path, std::optional inner_rect = std::nullopt); static std::shared_ptr MakeStrokePath( - Path path, + const Path& path, Scalar stroke_width = 0.0, Scalar miter_limit = 4.0, Cap stroke_cap = Cap::kButt, diff --git a/engine/src/flutter/impeller/entity/geometry/geometry_unittests.cc b/engine/src/flutter/impeller/entity/geometry/geometry_unittests.cc index 170a01ff98e..c2c5c747671 100644 --- a/engine/src/flutter/impeller/entity/geometry/geometry_unittests.cc +++ b/engine/src/flutter/impeller/entity/geometry/geometry_unittests.cc @@ -20,7 +20,7 @@ TEST(EntityGeometryTest, RectGeometryCoversArea) { TEST(EntityGeometryTest, FillPathGeometryCoversArea) { auto path = PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath(); auto geometry = Geometry::MakeFillPath( - std::move(path), /* inner rect */ Rect::MakeLTRB(0, 0, 100, 100)); + path, /* inner rect */ Rect::MakeLTRB(0, 0, 100, 100)); ASSERT_TRUE(geometry->CoversArea({}, Rect::MakeLTRB(0, 0, 100, 100))); ASSERT_FALSE(geometry->CoversArea({}, Rect::MakeLTRB(-1, 0, 100, 100))); ASSERT_TRUE(geometry->CoversArea({}, Rect::MakeLTRB(1, 1, 100, 100))); @@ -29,7 +29,7 @@ TEST(EntityGeometryTest, FillPathGeometryCoversArea) { TEST(EntityGeometryTest, FillPathGeometryCoversAreaNoInnerRect) { auto path = PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 100, 100)).TakePath(); - auto geometry = Geometry::MakeFillPath(std::move(path)); + auto geometry = Geometry::MakeFillPath(path); ASSERT_FALSE(geometry->CoversArea({}, Rect::MakeLTRB(0, 0, 100, 100))); ASSERT_FALSE(geometry->CoversArea({}, Rect::MakeLTRB(-1, 0, 100, 100))); ASSERT_FALSE(geometry->CoversArea({}, Rect::MakeLTRB(1, 1, 100, 100))); diff --git a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc index 12378452615..bd06de87ffd 100644 --- a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc +++ b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.cc @@ -8,12 +8,12 @@ namespace impeller { -StrokePathGeometry::StrokePathGeometry(Path path, +StrokePathGeometry::StrokePathGeometry(const Path& path, Scalar stroke_width, Scalar miter_limit, Cap stroke_cap, Join stroke_join) - : path_(std::move(path)), + : path_(path), stroke_width_(stroke_width), miter_limit_(miter_limit), stroke_cap_(stroke_cap), diff --git a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.h b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.h index ef08a98ed5f..7b0d0443261 100644 --- a/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.h +++ b/engine/src/flutter/impeller/entity/geometry/stroke_path_geometry.h @@ -12,7 +12,7 @@ namespace impeller { /// @brief A geometry that is created from a stroked path object. class StrokePathGeometry final : public Geometry { public: - StrokePathGeometry(Path path, + StrokePathGeometry(const Path& path, Scalar stroke_width, Scalar miter_limit, Cap stroke_cap, diff --git a/engine/src/flutter/impeller/geometry/geometry_benchmarks.cc b/engine/src/flutter/impeller/geometry/geometry_benchmarks.cc index 52064eca8b4..c4801e09fa6 100644 --- a/engine/src/flutter/impeller/geometry/geometry_benchmarks.cc +++ b/engine/src/flutter/impeller/geometry/geometry_benchmarks.cc @@ -25,7 +25,7 @@ static Tessellator tess; template static void BM_Polyline(benchmark::State& state, Args&&... args) { auto args_tuple = std::make_tuple(std::move(args)...); - auto path = std::get(args_tuple).Clone(); + auto path = std::get(args_tuple); bool tessellate = std::get(args_tuple); size_t point_count = 0u; @@ -67,7 +67,7 @@ static void BM_Polyline(benchmark::State& state, Args&&... args) { template static void BM_Convex(benchmark::State& state, Args&&... args) { auto args_tuple = std::make_tuple(std::move(args)...); - auto path = std::get(args_tuple).Clone(); + auto path = std::get(args_tuple); size_t point_count = 0u; size_t single_point_count = 0u; diff --git a/engine/src/flutter/impeller/geometry/path.cc b/engine/src/flutter/impeller/geometry/path.cc index b68e9b28456..1e175decec8 100644 --- a/engine/src/flutter/impeller/geometry/path.cc +++ b/engine/src/flutter/impeller/geometry/path.cc @@ -13,9 +13,13 @@ namespace impeller { -Path::Path() { - AddContourComponent({}); -}; +Path::Path() : data_(new Data()) {} + +Path::Path(const Data& data) : data_(new Data(data)) { + FML_DCHECK(data_->points.size() == data_->points.capacity()); + FML_DCHECK(data_->components.size() == data_->components.capacity()); + FML_DCHECK(data_->contours.size() == data_->contours.capacity()); +} Path::~Path() = default; @@ -33,14 +37,14 @@ std::tuple Path::Polyline::GetContourPointBounds( size_t Path::GetComponentCount(std::optional type) const { if (!type.has_value()) { - return components_.size(); + return data_->components.size(); } auto type_value = type.value(); if (type_value == ComponentType::kContour) { - return contours_.size(); + return data_->contours.size(); } size_t count = 0u; - for (const auto& component : components_) { + for (const auto& component : data_->components) { if (component.type == type_value) { count++; } @@ -48,82 +52,16 @@ size_t Path::GetComponentCount(std::optional type) const { return count; } -void Path::SetFillType(FillType fill) { - fill_ = fill; -} - FillType Path::GetFillType() const { - return fill_; + return data_->fill; } bool Path::IsConvex() const { - return convexity_ == Convexity::kConvex; + return data_->convexity == Convexity::kConvex; } -void Path::SetConvexity(Convexity value) { - convexity_ = value; -} - -void Path::Shift(Point shift) { - for (auto i = 0u; i < points_.size(); i++) { - points_[i] += shift; - } - for (auto& contour : contours_) { - contour.destination += shift; - } -} - -Path Path::Clone() const { - Path new_path = *this; - return new_path; -} - -Path& Path::AddLinearComponent(const Point& p1, const Point& p2) { - auto index = points_.size(); - points_.emplace_back(p1); - points_.emplace_back(p2); - components_.emplace_back(ComponentType::kLinear, index); - return *this; -} - -Path& Path::AddQuadraticComponent(const Point& p1, - const Point& cp, - const Point& p2) { - auto index = points_.size(); - points_.emplace_back(p1); - points_.emplace_back(cp); - points_.emplace_back(p2); - components_.emplace_back(ComponentType::kQuadratic, index); - return *this; -} - -Path& Path::AddCubicComponent(const Point& p1, - const Point& cp1, - const Point& cp2, - const Point& p2) { - auto index = points_.size(); - points_.emplace_back(p1); - points_.emplace_back(cp1); - points_.emplace_back(cp2); - points_.emplace_back(p2); - components_.emplace_back(ComponentType::kCubic, index); - return *this; -} - -Path& Path::AddContourComponent(const Point& destination, bool is_closed) { - if (components_.size() > 0 && - components_.back().type == ComponentType::kContour) { - // Never insert contiguous contours. - contours_.back() = ContourComponent(destination, is_closed); - } else { - contours_.emplace_back(ContourComponent(destination, is_closed)); - components_.emplace_back(ComponentType::kContour, contours_.size() - 1); - } - return *this; -} - -void Path::SetContourClosed(bool is_closed) { - contours_.back().is_closed = is_closed; +bool Path::IsEmpty() const { + return data_->points.empty(); } void Path::EnumerateComponents( @@ -131,36 +69,37 @@ void Path::EnumerateComponents( const Applier& quad_applier, const Applier& cubic_applier, const Applier& contour_applier) const { + auto& points = data_->points; size_t currentIndex = 0; - for (const auto& component : components_) { + for (const auto& component : data_->components) { switch (component.type) { case ComponentType::kLinear: if (linear_applier) { linear_applier(currentIndex, - LinearPathComponent(points_[component.index], - points_[component.index + 1])); + LinearPathComponent(points[component.index], + points[component.index + 1])); } break; case ComponentType::kQuadratic: if (quad_applier) { quad_applier(currentIndex, - QuadraticPathComponent(points_[component.index], - points_[component.index + 1], - points_[component.index + 2])); + QuadraticPathComponent(points[component.index], + points[component.index + 1], + points[component.index + 2])); } break; case ComponentType::kCubic: if (cubic_applier) { cubic_applier(currentIndex, - CubicPathComponent(points_[component.index], - points_[component.index + 1], - points_[component.index + 2], - points_[component.index + 3])); + CubicPathComponent(points[component.index], + points[component.index + 1], + points[component.index + 2], + points[component.index + 3])); } break; case ComponentType::kContour: if (contour_applier) { - contour_applier(currentIndex, contours_[component.index]); + contour_applier(currentIndex, data_->contours[component.index]); } break; } @@ -170,64 +109,74 @@ void Path::EnumerateComponents( bool Path::GetLinearComponentAtIndex(size_t index, LinearPathComponent& linear) const { - if (index >= components_.size()) { + auto& components = data_->components; + + if (index >= components.size()) { return false; } - if (components_[index].type != ComponentType::kLinear) { + if (components[index].type != ComponentType::kLinear) { return false; } - auto point_index = components_[index].index; - linear = LinearPathComponent(points_[point_index], points_[point_index + 1]); + auto& points = data_->points; + auto point_index = components[index].index; + linear = LinearPathComponent(points[point_index], points[point_index + 1]); return true; } bool Path::GetQuadraticComponentAtIndex( size_t index, QuadraticPathComponent& quadratic) const { - if (index >= components_.size()) { + auto& components = data_->components; + + if (index >= components.size()) { return false; } - if (components_[index].type != ComponentType::kQuadratic) { + if (components[index].type != ComponentType::kQuadratic) { return false; } - auto point_index = components_[index].index; + auto& points = data_->points; + auto point_index = components[index].index; quadratic = QuadraticPathComponent( - points_[point_index], points_[point_index + 1], points_[point_index + 2]); + points[point_index], points[point_index + 1], points[point_index + 2]); return true; } bool Path::GetCubicComponentAtIndex(size_t index, CubicPathComponent& cubic) const { - if (index >= components_.size()) { + auto& components = data_->components; + + if (index >= components.size()) { return false; } - if (components_[index].type != ComponentType::kCubic) { + if (components[index].type != ComponentType::kCubic) { return false; } - auto point_index = components_[index].index; - cubic = - CubicPathComponent(points_[point_index], points_[point_index + 1], - points_[point_index + 2], points_[point_index + 3]); + auto& points = data_->points; + auto point_index = components[index].index; + cubic = CubicPathComponent(points[point_index], points[point_index + 1], + points[point_index + 2], points[point_index + 3]); return true; } bool Path::GetContourComponentAtIndex(size_t index, ContourComponent& move) const { - if (index >= components_.size()) { + auto& components = data_->components; + + if (index >= components.size()) { return false; } - if (components_[index].type != ComponentType::kContour) { + if (components[index].type != ComponentType::kContour) { return false; } - move = contours_[components_[index].index]; + move = data_->contours[components[index].index]; return true; } @@ -256,21 +205,25 @@ Path::Polyline Path::CreatePolyline( Path::Polyline::ReclaimPointBufferCallback reclaim) const { Polyline polyline(std::move(point_buffer), std::move(reclaim)); - auto get_path_component = [this](size_t component_i) -> PathComponentVariant { - if (component_i >= components_.size()) { + auto& path_components = data_->components; + auto& path_points = data_->points; + + auto get_path_component = [&path_components, &path_points]( + size_t component_i) -> PathComponentVariant { + if (component_i >= path_components.size()) { return std::monostate{}; } - const auto& component = components_[component_i]; + const auto& component = path_components[component_i]; switch (component.type) { case ComponentType::kLinear: return reinterpret_cast( - &points_[component.index]); + &path_points[component.index]); case ComponentType::kQuadratic: return reinterpret_cast( - &points_[component.index]); + &path_points[component.index]); case ComponentType::kCubic: return reinterpret_cast( - &points_[component.index]); + &path_points[component.index]); case ComponentType::kContour: return std::monostate{}; } @@ -293,10 +246,10 @@ Path::Polyline Path::CreatePolyline( return Vector2(0, -1); }; - std::vector components; + std::vector poly_components; std::optional previous_path_component_index; auto end_contour = [&polyline, &previous_path_component_index, - &get_path_component, &components]() { + &get_path_component, &poly_components]() { // Whenever a contour has ended, extract the exact end direction from // the last component. if (polyline.contours.empty()) { @@ -309,8 +262,8 @@ Path::Polyline Path::CreatePolyline( auto& contour = polyline.contours.back(); contour.end_direction = Vector2(0, 1); - contour.components = components; - components.clear(); + contour.components = poly_components; + poly_components.clear(); size_t previous_index = previous_path_component_index.value(); while (!std::holds_alternative( @@ -330,40 +283,42 @@ Path::Polyline Path::CreatePolyline( } }; - for (size_t component_i = 0; component_i < components_.size(); + for (size_t component_i = 0; component_i < path_components.size(); component_i++) { - const auto& component = components_[component_i]; - switch (component.type) { + const auto& path_component = path_components[component_i]; + switch (path_component.type) { case ComponentType::kLinear: - components.push_back({ + poly_components.push_back({ .component_start_index = polyline.points->size() - 1, .is_curve = false, }); - reinterpret_cast(&points_[component.index]) + reinterpret_cast( + &path_points[path_component.index]) ->AppendPolylinePoints(*polyline.points); previous_path_component_index = component_i; break; case ComponentType::kQuadratic: - components.push_back({ + poly_components.push_back({ .component_start_index = polyline.points->size() - 1, .is_curve = true, }); reinterpret_cast( - &points_[component.index]) + &path_points[path_component.index]) ->AppendPolylinePoints(scale, *polyline.points); previous_path_component_index = component_i; break; case ComponentType::kCubic: - components.push_back({ + poly_components.push_back({ .component_start_index = polyline.points->size() - 1, .is_curve = true, }); - reinterpret_cast(&points_[component.index]) + reinterpret_cast( + &path_points[path_component.index]) ->AppendPolylinePoints(scale, *polyline.points); previous_path_component_index = component_i; break; case ComponentType::kContour: - if (component_i == components_.size() - 1) { + if (component_i == path_components.size() - 1) { // If the last component is a contour, that means it's an empty // contour, so skip it. continue; @@ -371,11 +326,11 @@ Path::Polyline Path::CreatePolyline( end_contour(); Vector2 start_direction = compute_contour_start_direction(component_i); - const auto& contour = contours_[component.index]; + const auto& contour = data_->contours[path_component.index]; polyline.contours.push_back({.start_index = polyline.points->size(), .is_closed = contour.is_closed, .start_direction = start_direction, - .components = components}); + .components = poly_components}); polyline.points->push_back(contour.destination); break; @@ -386,19 +341,7 @@ Path::Polyline Path::CreatePolyline( } std::optional Path::GetBoundingBox() const { - return computed_bounds_; -} - -void Path::ComputeBounds() { - auto min_max = GetMinMaxCoveragePoints(); - if (!min_max.has_value()) { - computed_bounds_ = std::nullopt; - return; - } - auto min = min_max->first; - auto max = min_max->second; - const auto difference = max - min; - computed_bounds_ = Rect::MakeXYWH(min.x, min.y, difference.x, difference.y); + return data_->bounds; } std::optional Path::GetTransformedBoundingBox( @@ -410,65 +353,4 @@ std::optional Path::GetTransformedBoundingBox( return bounds->TransformBounds(transform); } -std::optional> Path::GetMinMaxCoveragePoints() const { - if (points_.empty()) { - return std::nullopt; - } - - std::optional min, max; - - auto clamp = [&min, &max](const Point& point) { - if (min.has_value()) { - min = min->Min(point); - } else { - min = point; - } - - if (max.has_value()) { - max = max->Max(point); - } else { - max = point; - } - }; - - for (const auto& component : components_) { - switch (component.type) { - case ComponentType::kLinear: { - auto* linear = reinterpret_cast( - &points_[component.index]); - clamp(linear->p1); - clamp(linear->p2); - break; - } - case ComponentType::kQuadratic: - for (const auto& extrema : - reinterpret_cast( - &points_[component.index]) - ->Extrema()) { - clamp(extrema); - } - break; - case ComponentType::kCubic: - for (const auto& extrema : reinterpret_cast( - &points_[component.index]) - ->Extrema()) { - clamp(extrema); - } - break; - case ComponentType::kContour: - break; - } - } - - if (!min.has_value() || !max.has_value()) { - return std::nullopt; - } - - return std::make_pair(min.value(), max.value()); -} - -void Path::SetBounds(Rect rect) { - computed_bounds_ = rect; -} - } // namespace impeller diff --git a/engine/src/flutter/impeller/geometry/path.h b/engine/src/flutter/impeller/geometry/path.h index f4f5800ab12..ccca96aa363 100644 --- a/engine/src/flutter/impeller/geometry/path.h +++ b/engine/src/flutter/impeller/geometry/path.h @@ -133,17 +133,14 @@ class Path { ~Path(); - Path(Path&& other) = default; - - /// @brief Deeply clone this path and all data associated with it. - Path Clone() const; - size_t GetComponentCount(std::optional type = {}) const; FillType GetFillType() const; bool IsConvex() const; + bool IsEmpty() const; + template using Applier = std::function; void EnumerateComponents( @@ -179,42 +176,9 @@ class Path { std::optional GetTransformedBoundingBox(const Matrix& transform) const; - std::optional> GetMinMaxCoveragePoints() const; - private: friend class PathBuilder; - Path(const Path& other) = default; - - void SetConvexity(Convexity value); - - void SetFillType(FillType fill); - - void SetBounds(Rect rect); - - Path& AddLinearComponent(const Point& p1, const Point& p2); - - Path& AddQuadraticComponent(const Point& p1, - const Point& cp, - const Point& p2); - - Path& AddCubicComponent(const Point& p1, - const Point& cp1, - const Point& cp2, - const Point& p2); - - Path& AddContourComponent(const Point& destination, bool is_closed = false); - - /// @brief Called by `PathBuilder` to compute the bounds for certain paths. - /// - /// `PathBuilder` may set the bounds directly, in case they come from a source - /// with already computed bounds, such as an SkPath. - void ComputeBounds(); - - void SetContourClosed(bool is_closed); - - void Shift(Point shift); - struct ComponentIndexPair { ComponentType type = ComponentType::kLinear; size_t index = 0; @@ -225,15 +189,39 @@ class Path { : type(a_type), index(a_index) {} }; - FillType fill_ = FillType::kNonZero; - Convexity convexity_ = Convexity::kUnknown; - std::vector components_; - std::vector points_; - std::vector contours_; + // All of the data for the path is stored in this structure which is + // held by a shared_ptr. Since they all share the structure, the + // copy constructor for Path is very cheap and we don't need to deal + // with shared pointers for Path fields and method arguments. + // + // PathBuilder also uses this structure to accumulate the path data + // but the Path constructor used in |TakePath()| will clone the + // structure to prevent sharing and future modifications within the + // builder from affecting the existing taken paths. + struct Data { + Data() = default; + Data(const Data& other) = default; - std::optional computed_bounds_; + ~Data() = default; + + FillType fill = FillType::kNonZero; + Convexity convexity = Convexity::kUnknown; + std::vector components; + std::vector points; + std::vector contours; + + std::optional bounds; + + bool locked = false; + }; + + explicit Path(const Data& data); + + std::shared_ptr data_; }; +static_assert(sizeof(Path) == sizeof(std::shared_ptr)); + } // namespace impeller #endif // FLUTTER_IMPELLER_GEOMETRY_PATH_H_ diff --git a/engine/src/flutter/impeller/geometry/path_builder.cc b/engine/src/flutter/impeller/geometry/path_builder.cc index 599e8f2e75d..c70a04958fc 100644 --- a/engine/src/flutter/impeller/geometry/path_builder.cc +++ b/engine/src/flutter/impeller/geometry/path_builder.cc @@ -8,49 +8,45 @@ namespace impeller { -PathBuilder::PathBuilder() = default; +PathBuilder::PathBuilder() { + AddContourComponent({}); +} PathBuilder::~PathBuilder() = default; -Path PathBuilder::CopyPath(FillType fill) const { - auto path = prototype_.Clone(); - path.SetFillType(fill); - return path; +Path PathBuilder::CopyPath(FillType fill) { + prototype_.fill = fill; + return Path(prototype_); } Path PathBuilder::TakePath(FillType fill) { - auto path = std::move(prototype_); - path.SetFillType(fill); - path.SetConvexity(convexity_); - if (!did_compute_bounds_) { - path.ComputeBounds(); - } - did_compute_bounds_ = false; - return path; + prototype_.fill = fill; + UpdateBounds(); + return Path(prototype_); } void PathBuilder::Reserve(size_t point_size, size_t verb_size) { - prototype_.points_.reserve(point_size); - prototype_.points_.reserve(verb_size); + prototype_.points.reserve(point_size); + prototype_.components.reserve(verb_size); } PathBuilder& PathBuilder::MoveTo(Point point, bool relative) { current_ = relative ? current_ + point : point; subpath_start_ = current_; - prototype_.AddContourComponent(current_); + AddContourComponent(current_); return *this; } PathBuilder& PathBuilder::Close() { LineTo(subpath_start_); - prototype_.SetContourClosed(true); - prototype_.AddContourComponent(current_); + SetContourClosed(true); + AddContourComponent(current_); return *this; } PathBuilder& PathBuilder::LineTo(Point point, bool relative) { point = relative ? current_ + point : point; - prototype_.AddLinearComponent(current_, point); + AddLinearComponent(current_, point); current_ = point; return *this; } @@ -58,7 +54,7 @@ PathBuilder& PathBuilder::LineTo(Point point, bool relative) { PathBuilder& PathBuilder::HorizontalLineTo(Scalar x, bool relative) { Point endpoint = relative ? Point{current_.x + x, current_.y} : Point{x, current_.y}; - prototype_.AddLinearComponent(current_, endpoint); + AddLinearComponent(current_, endpoint); current_ = endpoint; return *this; } @@ -66,7 +62,7 @@ PathBuilder& PathBuilder::HorizontalLineTo(Scalar x, bool relative) { PathBuilder& PathBuilder::VerticalLineTo(Scalar y, bool relative) { Point endpoint = relative ? Point{current_.x, current_.y + y} : Point{current_.x, y}; - prototype_.AddLinearComponent(current_, endpoint); + AddLinearComponent(current_, endpoint); current_ = endpoint; return *this; } @@ -76,13 +72,13 @@ PathBuilder& PathBuilder::QuadraticCurveTo(Point controlPoint, bool relative) { point = relative ? current_ + point : point; controlPoint = relative ? current_ + controlPoint : controlPoint; - prototype_.AddQuadraticComponent(current_, controlPoint, point); + AddQuadraticComponent(current_, controlPoint, point); current_ = point; return *this; } PathBuilder& PathBuilder::SetConvexity(Convexity value) { - convexity_ = value; + prototype_.convexity = value; return *this; } @@ -93,14 +89,14 @@ PathBuilder& PathBuilder::CubicCurveTo(Point controlPoint1, controlPoint1 = relative ? current_ + controlPoint1 : controlPoint1; controlPoint2 = relative ? current_ + controlPoint2 : controlPoint2; point = relative ? current_ + point : point; - prototype_.AddCubicComponent(current_, controlPoint1, controlPoint2, point); + AddCubicComponent(current_, controlPoint1, controlPoint2, point); current_ = point; return *this; } PathBuilder& PathBuilder::AddQuadraticCurve(Point p1, Point cp, Point p2) { MoveTo(p1); - prototype_.AddQuadraticComponent(p1, cp, p2); + AddQuadraticComponent(p1, cp, p2); return *this; } @@ -109,7 +105,7 @@ PathBuilder& PathBuilder::AddCubicCurve(Point p1, Point cp2, Point p2) { MoveTo(p1); - prototype_.AddCubicComponent(p1, cp1, cp2, p2); + AddCubicComponent(p1, cp1, cp2, p2); return *this; } @@ -161,7 +157,7 @@ PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) { //---------------------------------------------------------------------------- // Top line. // - prototype_.AddLinearComponent( + AddLinearComponent( {rect_origin.x + radii.top_left.x, rect_origin.y}, {rect_origin.x + rect_size.width - radii.top_right.x, rect_origin.y}); @@ -173,7 +169,7 @@ PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) { //---------------------------------------------------------------------------- // Right line. // - prototype_.AddLinearComponent( + AddLinearComponent( {rect_origin.x + rect_size.width, rect_origin.y + radii.top_right.y}, {rect_origin.x + rect_size.width, rect_origin.y + rect_size.height - radii.bottom_right.y}); @@ -186,7 +182,7 @@ PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) { //---------------------------------------------------------------------------- // Bottom line. // - prototype_.AddLinearComponent( + AddLinearComponent( {rect_origin.x + rect_size.width - radii.bottom_right.x, rect_origin.y + rect_size.height}, {rect_origin.x + radii.bottom_left.x, rect_origin.y + rect_size.height}); @@ -199,7 +195,7 @@ PathBuilder& PathBuilder::AddRoundedRect(Rect rect, RoundingRadii radii) { //---------------------------------------------------------------------------- // Left line. // - prototype_.AddLinearComponent( + AddLinearComponent( {rect_origin.x, rect_origin.y + rect_size.height - radii.bottom_left.y}, {rect_origin.x, rect_origin.y + radii.top_left.y}); @@ -217,11 +213,10 @@ PathBuilder& PathBuilder::AddRoundedRectTopLeft(Rect rect, RoundingRadii radii) { const auto magic_top_left = radii.top_left * kArcApproximationMagic; const auto corner = rect.GetOrigin(); - prototype_.AddCubicComponent( - {corner.x, corner.y + radii.top_left.y}, - {corner.x, corner.y + radii.top_left.y - magic_top_left.y}, - {corner.x + radii.top_left.x - magic_top_left.x, corner.y}, - {corner.x + radii.top_left.x, corner.y}); + AddCubicComponent({corner.x, corner.y + radii.top_left.y}, + {corner.x, corner.y + radii.top_left.y - magic_top_left.y}, + {corner.x + radii.top_left.x - magic_top_left.x, corner.y}, + {corner.x + radii.top_left.x, corner.y}); return *this; } @@ -229,7 +224,7 @@ PathBuilder& PathBuilder::AddRoundedRectTopRight(Rect rect, RoundingRadii radii) { const auto magic_top_right = radii.top_right * kArcApproximationMagic; const auto corner = rect.GetOrigin() + Point{rect.GetWidth(), 0}; - prototype_.AddCubicComponent( + AddCubicComponent( {corner.x - radii.top_right.x, corner.y}, {corner.x - radii.top_right.x + magic_top_right.x, corner.y}, {corner.x, corner.y + radii.top_right.y - magic_top_right.y}, @@ -241,7 +236,7 @@ PathBuilder& PathBuilder::AddRoundedRectBottomRight(Rect rect, RoundingRadii radii) { const auto magic_bottom_right = radii.bottom_right * kArcApproximationMagic; const auto corner = rect.GetOrigin() + rect.GetSize(); - prototype_.AddCubicComponent( + AddCubicComponent( {corner.x, corner.y - radii.bottom_right.y}, {corner.x, corner.y - radii.bottom_right.y + magic_bottom_right.y}, {corner.x - radii.bottom_right.x + magic_bottom_right.x, corner.y}, @@ -253,7 +248,7 @@ PathBuilder& PathBuilder::AddRoundedRectBottomLeft(Rect rect, RoundingRadii radii) { const auto magic_bottom_left = radii.bottom_left * kArcApproximationMagic; const auto corner = rect.GetOrigin() + Point{0, rect.GetHeight()}; - prototype_.AddCubicComponent( + AddCubicComponent( {corner.x + radii.bottom_left.x, corner.y}, {corner.x + radii.bottom_left.x - magic_bottom_left.x, corner.y}, {corner.x, corner.y - radii.bottom_left.y + magic_bottom_left.y}, @@ -261,6 +256,60 @@ PathBuilder& PathBuilder::AddRoundedRectBottomLeft(Rect rect, return *this; } +void PathBuilder::AddContourComponent(const Point& destination, + bool is_closed) { + auto& components = prototype_.components; + auto& contours = prototype_.contours; + if (components.size() > 0 && + components.back().type == Path::ComponentType::kContour) { + // Never insert contiguous contours. + contours.back() = ContourComponent(destination, is_closed); + } else { + contours.emplace_back(ContourComponent(destination, is_closed)); + components.emplace_back(Path::ComponentType::kContour, contours.size() - 1); + } + prototype_.bounds.reset(); +} + +void PathBuilder::AddLinearComponent(const Point& p1, const Point& p2) { + auto& points = prototype_.points; + auto index = points.size(); + points.emplace_back(p1); + points.emplace_back(p2); + prototype_.components.emplace_back(Path::ComponentType::kLinear, index); + prototype_.bounds.reset(); +} + +void PathBuilder::AddQuadraticComponent(const Point& p1, + const Point& cp, + const Point& p2) { + auto& points = prototype_.points; + auto index = points.size(); + points.emplace_back(p1); + points.emplace_back(cp); + points.emplace_back(p2); + prototype_.components.emplace_back(Path::ComponentType::kQuadratic, index); + prototype_.bounds.reset(); +} + +void PathBuilder::AddCubicComponent(const Point& p1, + const Point& cp1, + const Point& cp2, + const Point& p2) { + auto& points = prototype_.points; + auto index = points.size(); + points.emplace_back(p1); + points.emplace_back(cp1); + points.emplace_back(cp2); + points.emplace_back(p2); + prototype_.components.emplace_back(Path::ComponentType::kCubic, index); + prototype_.bounds.reset(); +} + +void PathBuilder::SetContourClosed(bool is_closed) { + prototype_.contours.back().is_closed = is_closed; +} + PathBuilder& PathBuilder::AddArc(const Rect& oval_bounds, Radians start, Radians sweep, @@ -304,7 +353,7 @@ PathBuilder& PathBuilder::AddArc(const Rect& oval_bounds, Point cp1 = p1 + Vector2(-p1_unit.y, p1_unit.x) * arc_cp_lengths; Point cp2 = p2 + Vector2(p2_unit.y, -p2_unit.x) * arc_cp_lengths; - prototype_.AddCubicComponent(p1, cp1, cp2, p2); + AddCubicComponent(p1, cp1, cp2, p2); current_ = p2; start.radians += quadrant_angle; @@ -329,37 +378,37 @@ PathBuilder& PathBuilder::AddOval(const Rect& container) { //---------------------------------------------------------------------------- // Top right arc. // - prototype_.AddCubicComponent({c.x, c.y - r.y}, // p1 - {c.x + m.x, c.y - r.y}, // cp1 - {c.x + r.x, c.y - m.y}, // cp2 - {c.x + r.x, c.y} // p2 + AddCubicComponent({c.x, c.y - r.y}, // p1 + {c.x + m.x, c.y - r.y}, // cp1 + {c.x + r.x, c.y - m.y}, // cp2 + {c.x + r.x, c.y} // p2 ); //---------------------------------------------------------------------------- // Bottom right arc. // - prototype_.AddCubicComponent({c.x + r.x, c.y}, // p1 - {c.x + r.x, c.y + m.y}, // cp1 - {c.x + m.x, c.y + r.y}, // cp2 - {c.x, c.y + r.y} // p2 + AddCubicComponent({c.x + r.x, c.y}, // p1 + {c.x + r.x, c.y + m.y}, // cp1 + {c.x + m.x, c.y + r.y}, // cp2 + {c.x, c.y + r.y} // p2 ); //---------------------------------------------------------------------------- // Bottom left arc. // - prototype_.AddCubicComponent({c.x, c.y + r.y}, // p1 - {c.x - m.x, c.y + r.y}, // cp1 - {c.x - r.x, c.y + m.y}, // cp2 - {c.x - r.x, c.y} // p2 + AddCubicComponent({c.x, c.y + r.y}, // p1 + {c.x - m.x, c.y + r.y}, // cp1 + {c.x - r.x, c.y + m.y}, // cp2 + {c.x - r.x, c.y} // p2 ); //---------------------------------------------------------------------------- // Top left arc. // - prototype_.AddCubicComponent({c.x - r.x, c.y}, // p1 - {c.x - r.x, c.y - m.y}, // cp1 - {c.x - m.x, c.y - r.y}, // cp2 - {c.x, c.y - r.y} // p2 + AddCubicComponent({c.x - r.x, c.y}, // p1 + {c.x - r.x, c.y - m.y}, // cp1 + {c.x - m.x, c.y - r.y}, // cp2 + {c.x, c.y - r.y} // p2 ); Close(); @@ -369,40 +418,116 @@ PathBuilder& PathBuilder::AddOval(const Rect& container) { PathBuilder& PathBuilder::AddLine(const Point& p1, const Point& p2) { MoveTo(p1); - prototype_.AddLinearComponent(p1, p2); + AddLinearComponent(p1, p2); return *this; } -const Path& PathBuilder::GetCurrentPath() const { - return prototype_; -} - PathBuilder& PathBuilder::AddPath(const Path& path) { auto linear = [&](size_t index, const LinearPathComponent& l) { - prototype_.AddLinearComponent(l.p1, l.p2); + AddLinearComponent(l.p1, l.p2); }; auto quadratic = [&](size_t index, const QuadraticPathComponent& q) { - prototype_.AddQuadraticComponent(q.p1, q.cp, q.p2); + AddQuadraticComponent(q.p1, q.cp, q.p2); }; auto cubic = [&](size_t index, const CubicPathComponent& c) { - prototype_.AddCubicComponent(c.p1, c.cp1, c.cp2, c.p2); + AddCubicComponent(c.p1, c.cp1, c.cp2, c.p2); }; auto move = [&](size_t index, const ContourComponent& m) { - prototype_.AddContourComponent(m.destination); + AddContourComponent(m.destination); }; path.EnumerateComponents(linear, quadratic, cubic, move); return *this; } PathBuilder& PathBuilder::Shift(Point offset) { - prototype_.Shift(offset); + for (auto& point : prototype_.points) { + point += offset; + } + for (auto& contour : prototype_.contours) { + contour.destination += offset; + } + prototype_.bounds.reset(); return *this; } PathBuilder& PathBuilder::SetBounds(Rect bounds) { - prototype_.SetBounds(bounds); - did_compute_bounds_ = true; + prototype_.bounds = bounds; return *this; } +void PathBuilder::UpdateBounds() { + if (!prototype_.bounds.has_value()) { + auto min_max = GetMinMaxCoveragePoints(); + if (!min_max.has_value()) { + prototype_.bounds.reset(); + return; + } + auto min = min_max->first; + auto max = min_max->second; + const auto difference = max - min; + prototype_.bounds = + Rect::MakeXYWH(min.x, min.y, difference.x, difference.y); + } +} + +std::optional> PathBuilder::GetMinMaxCoveragePoints() + const { + auto& points = prototype_.points; + + if (points.empty()) { + return std::nullopt; + } + + std::optional min, max; + + auto clamp = [&min, &max](const Point& point) { + if (min.has_value()) { + min = min->Min(point); + } else { + min = point; + } + + if (max.has_value()) { + max = max->Max(point); + } else { + max = point; + } + }; + + for (const auto& component : prototype_.components) { + switch (component.type) { + case Path::ComponentType::kLinear: { + auto* linear = reinterpret_cast( + &points[component.index]); + clamp(linear->p1); + clamp(linear->p2); + break; + } + case Path::ComponentType::kQuadratic: + for (const auto& extrema : + reinterpret_cast( + &points[component.index]) + ->Extrema()) { + clamp(extrema); + } + break; + case Path::ComponentType::kCubic: + for (const auto& extrema : reinterpret_cast( + &points[component.index]) + ->Extrema()) { + clamp(extrema); + } + break; + case Path::ComponentType::kContour: + break; + } + } + + if (!min.has_value() || !max.has_value()) { + return std::nullopt; + } + + return std::make_pair(min.value(), max.value()); +} + } // namespace impeller diff --git a/engine/src/flutter/impeller/geometry/path_builder.h b/engine/src/flutter/impeller/geometry/path_builder.h index 826d2abfc29..0907b7b67f0 100644 --- a/engine/src/flutter/impeller/geometry/path_builder.h +++ b/engine/src/flutter/impeller/geometry/path_builder.h @@ -26,7 +26,7 @@ class PathBuilder { ~PathBuilder(); - Path CopyPath(FillType fill = FillType::kNonZero) const; + Path CopyPath(FillType fill = FillType::kNonZero); Path TakePath(FillType fill = FillType::kNonZero); @@ -34,8 +34,6 @@ class PathBuilder { /// path buffer. void Reserve(size_t point_size, size_t verb_size); - const Path& GetCurrentPath() const; - PathBuilder& SetConvexity(Convexity value); PathBuilder& MoveTo(Point point, bool relative = false); @@ -158,9 +156,7 @@ class PathBuilder { private: Point subpath_start_; Point current_; - Path prototype_; - Convexity convexity_; - bool did_compute_bounds_ = false; + Path::Data prototype_; PathBuilder& AddRoundedRectTopLeft(Rect rect, RoundingRadii radii); @@ -170,6 +166,26 @@ class PathBuilder { PathBuilder& AddRoundedRectBottomLeft(Rect rect, RoundingRadii radii); + void AddContourComponent(const Point& destination, bool is_closed = false); + + void SetContourClosed(bool is_closed); + + void AddLinearComponent(const Point& p1, const Point& p2); + + void AddQuadraticComponent(const Point& p1, const Point& cp, const Point& p2); + + void AddCubicComponent(const Point& p1, + const Point& cp1, + const Point& cp2, + const Point& p2); + + /// Compute the bounds of the path unless they are already computed or + /// set by an external source, such as |SetBounds|. Any call which mutates + /// the path data can invalidate the computed/set bounds. + void UpdateBounds(); + + std::optional> GetMinMaxCoveragePoints() const; + PathBuilder(const PathBuilder&) = delete; PathBuilder& operator=(const PathBuilder&&) = delete; }; diff --git a/engine/src/flutter/impeller/geometry/path_unittests.cc b/engine/src/flutter/impeller/geometry/path_unittests.cc index 6f626f69120..a848d2ecb3e 100644 --- a/engine/src/flutter/impeller/geometry/path_unittests.cc +++ b/engine/src/flutter/impeller/geometry/path_unittests.cc @@ -462,7 +462,8 @@ TEST(PathTest, CanBeCloned) { builder.SetConvexity(Convexity::kConvex); auto path_a = builder.TakePath(FillType::kAbsGeqTwo); - auto path_b = path_a.Clone(); + // NOLINTNEXTLINE(performance-unnecessary-copy-initialization) + auto path_b = path_a; EXPECT_EQ(path_a.GetBoundingBox(), path_b.GetBoundingBox()); EXPECT_EQ(path_a.GetFillType(), path_b.GetFillType()); @@ -485,5 +486,205 @@ TEST(PathTest, CanBeCloned) { } } +TEST(PathTest, PathBuilderDoesNotMutateTakenPaths) { + auto test_isolation = + [](const std::function& mutator, + bool will_close, Point mutation_offset, const std::string& label) { + PathBuilder builder; + builder.MoveTo({10, 10}); + builder.LineTo({20, 20}); + builder.LineTo({20, 10}); + + auto verify_path = [](const Path& path, bool is_mutated, bool is_closed, + Point offset, const std::string& label) { + if (is_mutated) { + // We can only test the initial state before the mutator did + // its work. We have >= 3 components and the first 3 components + // will match what we saw before the mutation. + EXPECT_GE(path.GetComponentCount(), 3u) << label; + } else { + EXPECT_EQ(path.GetComponentCount(), 3u) << label; + } + { + ContourComponent contour; + EXPECT_TRUE(path.GetContourComponentAtIndex(0, contour)) << label; + EXPECT_EQ(contour.destination, offset + Point(10, 10)) << label; + EXPECT_EQ(contour.is_closed, is_closed) << label; + } + { + LinearPathComponent line; + EXPECT_TRUE(path.GetLinearComponentAtIndex(1, line)) << label; + EXPECT_EQ(line.p1, offset + Point(10, 10)) << label; + EXPECT_EQ(line.p2, offset + Point(20, 20)) << label; + } + { + LinearPathComponent line; + EXPECT_TRUE(path.GetLinearComponentAtIndex(2, line)) << label; + EXPECT_EQ(line.p1, offset + Point(20, 20)) << label; + EXPECT_EQ(line.p2, offset + Point(20, 10)) << label; + } + }; + + auto path1 = builder.TakePath(); + verify_path(path1, false, false, {}, + "Initial Path1 state before " + label); + + for (int i = 0; i < 10; i++) { + auto path = builder.TakePath(); + verify_path( + path, false, false, {}, + "Extra TakePath #" + std::to_string(i + 1) + " for " + label); + } + mutator(builder); + verify_path(path1, false, false, {}, + "Path1 state after subsequent " + label); + + auto path2 = builder.TakePath(); + verify_path(path1, false, false, {}, + "Path1 state after subsequent " + label + " and TakePath"); + verify_path(path2, true, will_close, mutation_offset, + "Initial Path2 state with subsequent " + label); + }; + + test_isolation( + [](PathBuilder& builder) { // + builder.SetConvexity(Convexity::kConvex); + }, + false, {}, "SetConvex"); + + test_isolation( + [](PathBuilder& builder) { // + builder.SetConvexity(Convexity::kUnknown); + }, + false, {}, "SetUnknownConvex"); + + test_isolation( + [](PathBuilder& builder) { // + builder.Close(); + }, + true, {}, "Close"); + + test_isolation( + [](PathBuilder& builder) { + builder.MoveTo({20, 30}, false); + }, + false, {}, "Absolute MoveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.MoveTo({20, 30}, true); + }, + false, {}, "Relative MoveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.LineTo({20, 30}, false); + }, + false, {}, "Absolute LineTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.LineTo({20, 30}, true); + }, + false, {}, "Relative LineTo"); + + test_isolation( + [](PathBuilder& builder) { // + builder.HorizontalLineTo(100, false); + }, + false, {}, "Absolute HorizontalLineTo"); + + test_isolation( + [](PathBuilder& builder) { // + builder.HorizontalLineTo(100, true); + }, + false, {}, "Relative HorizontalLineTo"); + + test_isolation( + [](PathBuilder& builder) { // + builder.VerticalLineTo(100, false); + }, + false, {}, "Absolute VerticalLineTo"); + + test_isolation( + [](PathBuilder& builder) { // + builder.VerticalLineTo(100, true); + }, + false, {}, "Relative VerticalLineTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.QuadraticCurveTo({20, 30}, {30, 20}, false); + }, + false, {}, "Absolute QuadraticCurveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.QuadraticCurveTo({20, 30}, {30, 20}, true); + }, + false, {}, "Relative QuadraticCurveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.CubicCurveTo({20, 30}, {30, 20}, {30, 30}, false); + }, + false, {}, "Absolute CubicCurveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.CubicCurveTo({20, 30}, {30, 20}, {30, 30}, true); + }, + false, {}, "Relative CubicCurveTo"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddLine({100, 100}, {150, 100}); + }, + false, {}, "AddLine"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddRect(Rect::MakeLTRB(100, 100, 120, 120)); + }, + false, {}, "AddRect"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddOval(Rect::MakeLTRB(100, 100, 120, 120)); + }, + false, {}, "AddOval"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddCircle({100, 100}, 20); + }, + false, {}, "AddCircle"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddArc(Rect::MakeLTRB(100, 100, 120, 120), Degrees(10), + Degrees(170)); + }, + false, {}, "AddArc"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddQuadraticCurve({100, 100}, {150, 100}, {150, 150}); + }, + false, {}, "AddQuadraticCurve"); + + test_isolation( + [](PathBuilder& builder) { + builder.AddCubicCurve({100, 100}, {150, 100}, {100, 150}, {150, 150}); + }, + false, {}, "AddCubicCurve"); + + test_isolation( + [](PathBuilder& builder) { + builder.Shift({23, 42}); + }, + false, {23, 42}, "Shift"); +} + } // namespace testing } // namespace impeller