diff --git a/display_list/display_list_unittests.cc b/display_list/display_list_unittests.cc index 729897cb3531f..8845ce3ac6065 100644 --- a/display_list/display_list_unittests.cc +++ b/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/display_list/dl_op_receiver.h b/display_list/dl_op_receiver.h index d15cbba4d8744..dff97c3c15b7a 100644 --- a/display_list/dl_op_receiver.h +++ b/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/display_list/dl_op_records.h b/display_list/dl_op_records.h index fa9aeaafa0034..259102c14251e 100644 --- a/display_list/dl_op_records.h +++ b/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/display_list/testing/dl_test_snippets.cc b/display_list/testing/dl_test_snippets.cc index c831005482cbb..c26d95ab91e99 100644 --- a/display_list/testing/dl_test_snippets.cc +++ b/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/impeller/aiks/aiks_gradient_unittests.cc b/impeller/aiks/aiks_gradient_unittests.cc index bfecaac904e63..70fe173daaf2c 100644 --- a/impeller/aiks/aiks_gradient_unittests.cc +++ b/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/impeller/aiks/aiks_path_unittests.cc b/impeller/aiks/aiks_path_unittests.cc index 47b835b1c45e9..6f194d0623e0a 100644 --- a/impeller/aiks/aiks_path_unittests.cc +++ b/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/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index 9e36f2e3ed9a8..b8ace127462c5 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/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/impeller/aiks/canvas.cc b/impeller/aiks/canvas.cc index cb40eb802ab1f..6c72e097db7a1 100644 --- a/impeller/aiks/canvas.cc +++ b/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/impeller/aiks/canvas.h b/impeller/aiks/canvas.h index 2067ebe7937b2..6285a2cf13039 100644 --- a/impeller/aiks/canvas.h +++ b/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/impeller/aiks/canvas_unittests.cc b/impeller/aiks/canvas_unittests.cc index 8dfe051ed6bb7..a2deabcb9006b 100644 --- a/impeller/aiks/canvas_unittests.cc +++ b/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/impeller/display_list/dl_dispatcher.cc b/impeller/display_list/dl_dispatcher.cc index dca79aba9ee85..d999c5d9eaae4 100644 --- a/impeller/display_list/dl_dispatcher.cc +++ b/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/impeller/display_list/dl_dispatcher.h b/impeller/display_list/dl_dispatcher.h index a0a1fcfb8d3ad..4ef1da1c29aea 100644 --- a/impeller/display_list/dl_dispatcher.h +++ b/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/impeller/display_list/dl_unittests.cc b/impeller/display_list/dl_unittests.cc index 258141e240010..112c8563bf350 100644 --- a/impeller/display_list/dl_unittests.cc +++ b/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/impeller/entity/contents/solid_color_contents.cc b/impeller/entity/contents/solid_color_contents.cc index d7c3f985a81ae..c2545fc0a0164 100644 --- a/impeller/entity/contents/solid_color_contents.cc +++ b/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/impeller/entity/contents/solid_color_contents.h b/impeller/entity/contents/solid_color_contents.h index 6d0b54359a29c..59358ac68f3f4 100644 --- a/impeller/entity/contents/solid_color_contents.h +++ b/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/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index f3d40b009cf98..17d83085de404 100644 --- a/impeller/entity/entity_unittests.cc +++ b/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/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index a906a661636de..8069ef9aa87ac 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/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/impeller/entity/geometry/fill_path_geometry.h b/impeller/entity/geometry/fill_path_geometry.h index a1210846b311b..c93a04b90cc8d 100644 --- a/impeller/entity/geometry/fill_path_geometry.h +++ b/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/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index 0690cc5abb696..65b2b43c583c0 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/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/impeller/entity/geometry/geometry.h b/impeller/entity/geometry/geometry.h index 7c639e6b35c9e..ef574fd727877 100644 --- a/impeller/entity/geometry/geometry.h +++ b/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/impeller/entity/geometry/geometry_unittests.cc b/impeller/entity/geometry/geometry_unittests.cc index 170a01ff98e85..c2c5c74767125 100644 --- a/impeller/entity/geometry/geometry_unittests.cc +++ b/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/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 1237845261544..bd06de87ffdca 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/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/impeller/entity/geometry/stroke_path_geometry.h b/impeller/entity/geometry/stroke_path_geometry.h index ef08a98ed5fe6..7b0d044326107 100644 --- a/impeller/entity/geometry/stroke_path_geometry.h +++ b/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/impeller/geometry/geometry_benchmarks.cc b/impeller/geometry/geometry_benchmarks.cc index 52064eca8b43e..c4801e09fa630 100644 --- a/impeller/geometry/geometry_benchmarks.cc +++ b/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/impeller/geometry/path.cc b/impeller/geometry/path.cc index b68e9b28456e2..1e175decec81c 100644 --- a/impeller/geometry/path.cc +++ b/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; -} - -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; + return data_->convexity == Convexity::kConvex; } -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/impeller/geometry/path.h b/impeller/geometry/path.h index f4f5800ab12e3..ccca96aa36391 100644 --- a/impeller/geometry/path.h +++ b/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; + + ~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::optional computed_bounds_; + std::shared_ptr data_; }; +static_assert(sizeof(Path) == sizeof(std::shared_ptr)); + } // namespace impeller #endif // FLUTTER_IMPELLER_GEOMETRY_PATH_H_ diff --git a/impeller/geometry/path_builder.cc b/impeller/geometry/path_builder.cc index 599e8f2e75d25..c70a04958fc20 100644 --- a/impeller/geometry/path_builder.cc +++ b/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/impeller/geometry/path_builder.h b/impeller/geometry/path_builder.h index 826d2abfc2990..0907b7b67f0f4 100644 --- a/impeller/geometry/path_builder.h +++ b/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/impeller/geometry/path_unittests.cc b/impeller/geometry/path_unittests.cc index 6f626f691205d..a848d2ecb3e1c 100644 --- a/impeller/geometry/path_unittests.cc +++ b/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