From c4d8870f50df71a445e50e49aba97ae881f528b2 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 18 Mar 2025 13:42:06 -0700 Subject: [PATCH] [DisplayList] DlPath supports generic path dispatching (#164753) There are different ways to iterate over an SkPath or an impeller::Path and various points in the engine source tree we have boilerplate duplicates of this code to transfer the contents of the DlPath wrapper object into some platform-specific path. This PR adds a dispatch/receiver mechanism to read back the contents of a DlPath - independent of whether it is backed by an SkPath or an impeller::Path - in a simpler form that avoids potential mistakes in the various conversion methods. See DlPathReceiver and DlPath::Dispatch in the dl_path.h file --- .../flutter/display_list/geometry/dl_path.cc | 250 ++++++++++--- .../flutter/display_list/geometry/dl_path.h | 69 ++-- .../geometry/dl_path_unittests.cc | 333 ++++++++++++++++++ engine/src/flutter/impeller/geometry/path.cc | 4 + engine/src/flutter/impeller/geometry/path.h | 2 + .../android/platform_view_android_jni_impl.cc | 123 +++---- .../framework/Source/FlutterPlatformViews.mm | 101 ++---- 7 files changed, 665 insertions(+), 217 deletions(-) diff --git a/engine/src/flutter/display_list/geometry/dl_path.cc b/engine/src/flutter/display_list/geometry/dl_path.cc index 471470f5c8..51bd8cca63 100644 --- a/engine/src/flutter/display_list/geometry/dl_path.cc +++ b/engine/src/flutter/display_list/geometry/dl_path.cc @@ -8,6 +8,29 @@ #include "flutter/impeller/geometry/path_builder.h" #include "impeller/geometry/path.h" +namespace { +inline constexpr flutter::DlPathFillType ToDlFillType(SkPathFillType sk_type) { + switch (sk_type) { + case SkPathFillType::kEvenOdd: + return impeller::FillType::kOdd; + case SkPathFillType::kWinding: + return impeller::FillType::kNonZero; + case SkPathFillType::kInverseEvenOdd: + case SkPathFillType::kInverseWinding: + FML_UNREACHABLE(); + } +} + +inline constexpr SkPathFillType ToSkFillType(flutter::DlPathFillType dl_type) { + switch (dl_type) { + case impeller::FillType::kOdd: + return SkPathFillType::kEvenOdd; + case impeller::FillType::kNonZero: + return SkPathFillType::kWinding; + } +} +} // namespace + namespace flutter { using Path = impeller::Path; @@ -120,6 +143,22 @@ const Path& DlPath::GetPath() const { return path.value(); } +void DlPath::Dispatch(DlPathReceiver& receiver) const { + if (data_->sk_path_original) { + auto& sk_path = data_->sk_path; + FML_DCHECK(sk_path.has_value()); + if (sk_path.has_value()) { + DispatchFromSkiaPath(sk_path.value(), receiver); + } + } else { + auto& path = data_->path; + FML_DCHECK(path.has_value()); + if (path.has_value()) { + DispatchFromImpellerPath(path.value(), receiver); + } + } +} + void DlPath::WillRenderSkPath() const { if (data_->render_count >= kMaxVolatileUses) { auto& sk_path = data_->sk_path; @@ -252,26 +291,94 @@ DlPath DlPath::operator+(const DlPath& other) const { return DlPath(path); } -SkPath DlPath::ConvertToSkiaPath(const Path& path, const DlPoint& shift) { - SkPath sk_path; - sk_path.setFillType(ToSkFillType(path.GetFillType())); +static void ReduceConic(DlPathReceiver& receiver, + const DlPoint& p1, + const DlPoint& cp, + const DlPoint& p2, + DlScalar weight) { + // We might eventually have conic conversion math that deals with + // degenerate conics gracefully (or have all receivers just handle + // them directly). But, until then, we will just convert them to a + // pair of quads and accept the results as "close enough". + if (p1 != cp) { + if (cp != p2) { + std::array points; + impeller::ConicPathComponent conic(p1, cp, p2, weight); + conic.SubdivideToQuadraticPoints(points); + receiver.QuadTo(points[1], points[2]); + receiver.QuadTo(points[3], points[4]); + } else { + receiver.LineTo(cp); + } + } else if (cp != p2) { + receiver.LineTo(p2); + } +} + +namespace { +class SkiaPathReceiver final : public DlPathReceiver { + public: + void SetPathInfo(DlPathFillType fill_type, bool is_convex) override { + sk_path_.setFillType(ToSkFillType(fill_type)); + } + void MoveTo(const DlPoint& p2) override { sk_path_.moveTo(ToSkPoint(p2)); } + void LineTo(const DlPoint& p2) override { sk_path_.lineTo(ToSkPoint(p2)); } + void QuadTo(const DlPoint& cp, const DlPoint& p2) override { + sk_path_.quadTo(ToSkPoint(cp), ToSkPoint(p2)); + } + bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) override { + sk_path_.conicTo(ToSkPoint(cp), ToSkPoint(p2), weight); + return true; + } + void CubicTo(const DlPoint& cp1, + const DlPoint& cp2, + const DlPoint& p2) override { + sk_path_.cubicTo(ToSkPoint(cp1), ToSkPoint(cp2), ToSkPoint(p2)); + } + void Close() override { sk_path_.close(); } + + SkPath TakePath() { return sk_path_; } + + private: + SkPath sk_path_; +}; +} // namespace + +SkPath DlPath::ConvertToSkiaPath(const Path& path) { + SkiaPathReceiver receiver; + + DispatchFromImpellerPath(path, receiver); + + return receiver.TakePath(); +} + +void DlPath::DispatchFromImpellerPath(const impeller::Path& path, + DlPathReceiver& receiver) { bool subpath_needs_close = false; std::optional pending_moveto; - auto resolve_moveto = [&pending_moveto, &sk_path]() { + auto resolve_moveto = [&receiver, &pending_moveto]() { if (pending_moveto.has_value()) { - sk_path.moveTo(ToSkPoint(pending_moveto.value())); + receiver.MoveTo(pending_moveto.value()); pending_moveto.reset(); } }; + // The Impeller Point Count is way overestimated due to duplicate + // points between elements. + receiver.RecommendSizes(path.GetComponentCount(), path.GetPointCount()); + std::optional bounds = path.GetBoundingBox(); + if (bounds.has_value()) { + receiver.RecommendBounds(bounds.value()); + } + receiver.SetPathInfo(path.GetFillType(), path.IsConvex()); for (auto it = path.begin(), end = path.end(); it != end; ++it) { switch (it.type()) { case ComponentType::kContour: { const impeller::ContourComponent* contour = it.contour(); FML_DCHECK(contour != nullptr); if (subpath_needs_close) { - sk_path.close(); + receiver.Close(); } pending_moveto = contour->destination; subpath_needs_close = contour->IsClosed(); @@ -281,45 +388,96 @@ SkPath DlPath::ConvertToSkiaPath(const Path& path, const DlPoint& shift) { const impeller::LinearPathComponent* linear = it.linear(); FML_DCHECK(linear != nullptr); resolve_moveto(); - sk_path.lineTo(ToSkPoint(linear->p2)); + receiver.LineTo(linear->p2); break; } case ComponentType::kQuadratic: { const impeller::QuadraticPathComponent* quadratic = it.quadratic(); FML_DCHECK(quadratic != nullptr); resolve_moveto(); - sk_path.quadTo(ToSkPoint(quadratic->cp), ToSkPoint(quadratic->p2)); + receiver.QuadTo(quadratic->cp, quadratic->p2); break; } case ComponentType::kConic: { const impeller::ConicPathComponent* conic = it.conic(); FML_DCHECK(conic != nullptr); resolve_moveto(); - sk_path.conicTo(ToSkPoint(conic->cp), ToSkPoint(conic->p2), - conic->weight.x); + if (!receiver.ConicTo(conic->cp, conic->p2, conic->weight.x)) { + ReduceConic(receiver, conic->p1, conic->cp, conic->p2, + conic->weight.x); + } break; } case ComponentType::kCubic: { const impeller::CubicPathComponent* cubic = it.cubic(); FML_DCHECK(cubic != nullptr); resolve_moveto(); - sk_path.cubicTo(ToSkPoint(cubic->cp1), ToSkPoint(cubic->cp2), - ToSkPoint(cubic->p2)); + receiver.CubicTo(cubic->cp1, cubic->cp2, cubic->p2); break; } } } if (subpath_needs_close) { - sk_path.close(); + receiver.Close(); } - - return sk_path; } -Path DlPath::ConvertToImpellerPath(const SkPath& path, const DlPoint& shift) { - if (path.isEmpty() || !shift.IsFinite()) { +namespace { +class ImpellerPathReceiver final : public DlPathReceiver { + public: + void RecommendSizes(size_t verb_count, size_t point_count) override { + // Reserve a path size with some arbitrarily additional padding. + builder_.Reserve(point_count + 8, verb_count + 8); + } + void RecommendBounds(const DlRect& bounds) override { + builder_.SetBounds(bounds); + } + void SetPathInfo(DlPathFillType fill_type, bool is_convex) override { + this->fill_type_ = fill_type; + builder_.SetConvexity(is_convex ? Convexity::kConvex // + : Convexity::kUnknown); + } + void MoveTo(const DlPoint& p2) override { builder_.MoveTo(p2); } + void LineTo(const DlPoint& p2) override { builder_.LineTo(p2); } + void QuadTo(const DlPoint& cp, const DlPoint& p2) override { + builder_.QuadraticCurveTo(cp, p2); + } + // For legacy compatibility we do not override ConicTo to let the dispatcher + // convert conics to quads until we update Impeller for full support of + // rational quadratics + void CubicTo(const DlPoint& cp1, + const DlPoint& cp2, + const DlPoint& p2) override { + builder_.CubicCurveTo(cp1, cp2, p2); + } + void Close() override { builder_.Close(); } + + impeller::Path TakePath() { return builder_.TakePath(fill_type_); } + + private: + PathBuilder builder_; + DlPathFillType fill_type_; +}; +} // namespace + +Path DlPath::ConvertToImpellerPath(const SkPath& path) { + if (path.isEmpty()) { return Path{}; } + + ImpellerPathReceiver receiver; + + DispatchFromSkiaPath(path, receiver); + + return receiver.TakePath(); +} + +void DlPath::DispatchFromSkiaPath(const SkPath& path, + DlPathReceiver& receiver) { + if (path.isEmpty()) { + return; + } + auto iterator = SkPath::Iter(path, false); struct PathData { @@ -328,67 +486,47 @@ Path DlPath::ConvertToImpellerPath(const SkPath& path, const DlPoint& shift) { }; }; - PathBuilder builder; PathData data; - // Reserve a path size with some arbitrarily additional padding. - builder.Reserve(path.countPoints() + 8, path.countVerbs() + 8); + + receiver.RecommendSizes(path.countVerbs(), path.countPoints()); + receiver.RecommendBounds(ToDlRect(path.getBounds())); + receiver.SetPathInfo(ToDlFillType(path.getFillType()), path.isConvex()); auto verb = SkPath::Verb::kDone_Verb; do { verb = iterator.next(data.points); switch (verb) { case SkPath::kMove_Verb: - builder.MoveTo(ToDlPoint(data.points[0])); + receiver.MoveTo(ToDlPoint(data.points[0])); break; case SkPath::kLine_Verb: - builder.LineTo(ToDlPoint(data.points[1])); + receiver.LineTo(ToDlPoint(data.points[1])); break; case SkPath::kQuad_Verb: - builder.QuadraticCurveTo(ToDlPoint(data.points[1]), - ToDlPoint(data.points[2])); + receiver.QuadTo(ToDlPoint(data.points[1]), ToDlPoint(data.points[2])); break; case SkPath::kConic_Verb: - // We might eventually have conic conversion math that deals with - // degenerate conics gracefully (or just handle them directly), - // but until then, we will detect and ignore them. - if (data.points[0] != data.points[1]) { - if (data.points[1] != data.points[2]) { - std::array points; - impeller::ConicPathComponent conic( - ToDlPoint(data.points[0]), ToDlPoint(data.points[1]), - ToDlPoint(data.points[2]), iterator.conicWeight()); - conic.SubdivideToQuadraticPoints(points); - builder.QuadraticCurveTo(points[1], points[2]); - builder.QuadraticCurveTo(points[3], points[4]); - } else { - builder.LineTo(ToDlPoint(data.points[1])); - } - } else if (data.points[1] != data.points[2]) { - builder.LineTo(ToDlPoint(data.points[2])); + if (!receiver.ConicTo(ToDlPoint(data.points[1]), + ToDlPoint(data.points[2]), + iterator.conicWeight())) { + ReduceConic(receiver, // + ToDlPoint(data.points[0]), // + ToDlPoint(data.points[1]), // + ToDlPoint(data.points[2]), // + iterator.conicWeight()); } break; case SkPath::kCubic_Verb: - builder.CubicCurveTo(ToDlPoint(data.points[1]), - ToDlPoint(data.points[2]), - ToDlPoint(data.points[3])); + receiver.CubicTo(ToDlPoint(data.points[1]), // + ToDlPoint(data.points[2]), // + ToDlPoint(data.points[3])); break; case SkPath::kClose_Verb: - builder.Close(); + receiver.Close(); break; case SkPath::kDone_Verb: break; } } while (verb != SkPath::Verb::kDone_Verb); - - DlRect bounds = ToDlRect(path.getBounds()); - if (!shift.IsZero()) { - builder.Shift(shift); - bounds = bounds.Shift(shift); - } - - builder.SetConvexity(path.isConvex() ? Convexity::kConvex - : Convexity::kUnknown); - builder.SetBounds(bounds); - return builder.TakePath(ToDlFillType(path.getFillType())); } } // namespace flutter diff --git a/engine/src/flutter/display_list/geometry/dl_path.h b/engine/src/flutter/display_list/geometry/dl_path.h index b00011e93f..9dc9115979 100644 --- a/engine/src/flutter/display_list/geometry/dl_path.h +++ b/engine/src/flutter/display_list/geometry/dl_path.h @@ -5,6 +5,8 @@ #ifndef FLUTTER_DISPLAY_LIST_GEOMETRY_DL_PATH_H_ #define FLUTTER_DISPLAY_LIST_GEOMETRY_DL_PATH_H_ +#include + #include "flutter/display_list/geometry/dl_geometry_types.h" #include "flutter/impeller/geometry/path.h" #include "flutter/impeller/geometry/path_builder.h" @@ -15,6 +17,38 @@ namespace flutter { using DlPathFillType = impeller::FillType; using DlPathBuilder = impeller::PathBuilder; +/// @brief Collection of functions to receive path segments from the +/// underlying path representation via the DlPath::Dispatch method. +/// +/// The conic_to function is optional. If the receiver understands rational +/// quadratic Bezier curve forms then it should accept the curve parameters +/// and return true, otherwise it can return false and the dispatcher will +/// provide the path segment in a different form via the other methods. +/// +/// The dispatcher might not call the recommend_size or recommend_bounds +/// functions if the original path does not contain such information. +/// +/// The dispatcher will always call the path_info function, though the +/// is_convex parameter may be conservatively reported as false if the +/// original path does not contain such info. +class DlPathReceiver { + public: + virtual ~DlPathReceiver() = default; + virtual void RecommendSizes(size_t verb_count, size_t point_count) {}; + virtual void RecommendBounds(const DlRect& bounds) {}; + virtual void SetPathInfo(DlPathFillType fill_type, bool is_convex) = 0; + virtual void MoveTo(const DlPoint& p2) = 0; + virtual void LineTo(const DlPoint& p2) = 0; + virtual void QuadTo(const DlPoint& cp, const DlPoint& p2) = 0; + virtual bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) { + return false; + }; + virtual void CubicTo(const DlPoint& cp1, + const DlPoint& cp2, + const DlPoint& p2) = 0; + virtual void Close() = 0; +}; + class DlPath { public: static constexpr uint32_t kMaxVolatileUses = 2; @@ -69,6 +103,8 @@ class DlPath { const SkPath& GetSkPath() const; const impeller::Path& GetPath() const; + void Dispatch(DlPathReceiver& receiver) const; + /// Intent to render an SkPath multiple times will make the path /// non-volatile to enable caching in Skia. Calling this method /// before every rendering call that uses the SkPath will count @@ -116,34 +152,17 @@ class DlPath { const bool sk_path_original; }; - inline constexpr static DlPathFillType ToDlFillType(SkPathFillType sk_type) { - switch (sk_type) { - case SkPathFillType::kEvenOdd: - return impeller::FillType::kOdd; - case SkPathFillType::kWinding: - return impeller::FillType::kNonZero; - case SkPathFillType::kInverseEvenOdd: - case SkPathFillType::kInverseWinding: - FML_UNREACHABLE(); - } - } - - inline constexpr static SkPathFillType ToSkFillType(DlPathFillType dl_type) { - switch (dl_type) { - case impeller::FillType::kOdd: - return SkPathFillType::kEvenOdd; - case impeller::FillType::kNonZero: - return SkPathFillType::kWinding; - } - } - std::shared_ptr data_; - static SkPath ConvertToSkiaPath(const impeller::Path& path, - const DlPoint& shift = DlPoint()); + static void DispatchFromSkiaPath(const SkPath& path, + DlPathReceiver& receiver); - static impeller::Path ConvertToImpellerPath(const SkPath& path, - const DlPoint& shift = DlPoint()); + static void DispatchFromImpellerPath(const impeller::Path& path, + DlPathReceiver& receiver); + + static SkPath ConvertToSkiaPath(const impeller::Path& path); + + static impeller::Path ConvertToImpellerPath(const SkPath& path); }; } // namespace flutter diff --git a/engine/src/flutter/display_list/geometry/dl_path_unittests.cc b/engine/src/flutter/display_list/geometry/dl_path_unittests.cc index c48189776a..7008e6cedc 100644 --- a/engine/src/flutter/display_list/geometry/dl_path_unittests.cc +++ b/engine/src/flutter/display_list/geometry/dl_path_unittests.cc @@ -3,6 +3,8 @@ // found in the LICENSE file. #include "flutter/display_list/geometry/dl_path.h" + +#include "gmock/gmock.h" #include "gtest/gtest.h" #include "flutter/third_party/skia/include/core/SkRRect.h" @@ -553,6 +555,337 @@ TEST(DisplayListPath, ConstructFromImpellerEqualsConstructFromSkia) { EXPECT_EQ(DlPath(path_builder, DlPathFillType::kNonZero), DlPath(sk_path)); } +namespace { +class DlPathReceiverMock : public DlPathReceiver { + public: + MOCK_METHOD(void, + RecommendSizes, + (size_t verb_count, size_t point_count), + (override)); + MOCK_METHOD(void, RecommendBounds, (const DlRect& bounds), (override)); + MOCK_METHOD(void, + SetPathInfo, + (DlPathFillType fill_type, bool is_convex), + (override)); + MOCK_METHOD(void, MoveTo, (const DlPoint& p2), (override)); + MOCK_METHOD(void, LineTo, (const DlPoint& p2), (override)); + MOCK_METHOD(void, QuadTo, (const DlPoint& cp, const DlPoint& p2), (override)); + MOCK_METHOD(bool, + ConicTo, + (const DlPoint& cp, const DlPoint& p2, DlScalar weight), + (override)); + MOCK_METHOD(void, + CubicTo, + (const DlPoint& cp1, const DlPoint& cp2, const DlPoint& p2), + (override)); + MOCK_METHOD(void, Close, (), (override)); +}; + +using ::testing::AtMost; +using ::testing::Return; +} // namespace + +TEST(DisplayListPath, DispatchSkiaPathEvenOdd) { + SkPath path; + + path.setFillType(SkPathFillType::kEvenOdd); + path.moveTo(100, 200); + path.lineTo(101, 201); + path.quadTo(110, 202, 102, 210); + path.conicTo(150, 240, 250, 140, 0.5); + path.cubicTo(300, 300, 350, 300, 300, 350); + path.close(); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(6u, 9u)) // + .Times(AtMost(1)); + all_recommendations += + EXPECT_CALL(mock_receiver, + RecommendBounds(DlRect::MakeLTRB(100, 140, 350, 350))) + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kOdd, false)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(101, 201))); + EXPECT_CALL(mock_receiver, QuadTo(DlPoint(110, 202), DlPoint(102, 210))); + EXPECT_CALL(mock_receiver, + ConicTo(DlPoint(150, 240), DlPoint(250, 140), 0.5f)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_receiver, CubicTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + DlPath(path).Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchSkiaPathNonZero) { + SkPath path; + + path.setFillType(SkPathFillType::kWinding); + path.moveTo(100, 200); + path.lineTo(101, 201); + path.quadTo(110, 202, 102, 210); + path.conicTo(150, 240, 250, 140, 0.5); + path.cubicTo(300, 300, 350, 300, 300, 350); + path.close(); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(6u, 9u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, + RecommendBounds(DlRect::MakeLTRB(100, 140, 350, 350))) + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, false)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(101, 201))); + EXPECT_CALL(mock_receiver, QuadTo(DlPoint(110, 202), DlPoint(102, 210))); + EXPECT_CALL(mock_receiver, + ConicTo(DlPoint(150, 240), DlPoint(250, 140), 0.5f)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_receiver, CubicTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + DlPath(path).Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchSkiaPathConvex) { + SkPath path; + + path.setFillType(SkPathFillType::kWinding); + // Keep it simple - a triangle is obviously convex + path.moveTo(100, 200); + path.lineTo(200, 200); + path.lineTo(100, 300); + path.close(); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(4u, 3u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, + RecommendBounds(DlRect::MakeLTRB(100, 200, 200, 300))) + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, true)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(200, 200))); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 300))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + DlPath(path).Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchImpellerPathEvenOdd) { + DlPathBuilder path_builder; + + path_builder.MoveTo(DlPoint(100, 200)); + path_builder.LineTo(DlPoint(101, 201)); + path_builder.QuadraticCurveTo(DlPoint(110, 202), DlPoint(102, 210)); + path_builder.ConicCurveTo(DlPoint(150, 240), DlPoint(250, 140), 0.5); + path_builder.CubicCurveTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350)); + path_builder.Close(); + + DlPath path(path_builder, DlPathFillType::kOdd); + // Impeller computes tight bounds so it is difficult to hard-code the + // answer for the bounds of the above path... + auto bounds = path.GetBounds(); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(7u, 19u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendBounds(bounds)) // + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kOdd, false)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(101, 201))); + EXPECT_CALL(mock_receiver, QuadTo(DlPoint(110, 202), DlPoint(102, 210))); + EXPECT_CALL(mock_receiver, + ConicTo(DlPoint(150, 240), DlPoint(250, 140), 0.5f)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_receiver, CubicTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + path.Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchImpellerPathNonZero) { + DlPathBuilder path_builder; + + path_builder.MoveTo(DlPoint(100, 200)); + path_builder.LineTo(DlPoint(101, 201)); + path_builder.QuadraticCurveTo(DlPoint(110, 202), DlPoint(102, 210)); + path_builder.ConicCurveTo(DlPoint(150, 240), DlPoint(250, 140), 0.5); + path_builder.CubicCurveTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350)); + path_builder.Close(); + + DlPath path(path_builder, DlPathFillType::kNonZero); + // Impeller computes tight bounds so it is difficult to hard-code the + // answer for the bounds of the above path... + auto bounds = path.GetBounds(); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(7u, 19u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendBounds(bounds)) // + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, false)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(101, 201))); + EXPECT_CALL(mock_receiver, QuadTo(DlPoint(110, 202), DlPoint(102, 210))); + EXPECT_CALL(mock_receiver, + ConicTo(DlPoint(150, 240), DlPoint(250, 140), 0.5f)) + .WillOnce(Return(true)); + EXPECT_CALL(mock_receiver, CubicTo(DlPoint(300, 300), DlPoint(350, 300), + DlPoint(300, 350))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + path.Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchImpellerPathConvexUnspecified) { + DlPathBuilder path_builder; + + // Keep it simple - a triangle is obviously convex + path_builder.MoveTo(DlPoint(100, 200)); + path_builder.LineTo(DlPoint(200, 200)); + path_builder.LineTo(DlPoint(100, 300)); + path_builder.Close(); + + DlPath path(path_builder, DlPathFillType::kNonZero); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(5u, 10u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, + RecommendBounds(DlRect::MakeLTRB(100, 200, 200, 300))) + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, false)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(200, 200))); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 300))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + path.Dispatch(mock_receiver); +} + +TEST(DisplayListPath, DispatchImpellerPathConvexSpecified) { + DlPathBuilder path_builder; + + // Keep it simple - a triangle is obviously convex + path_builder.MoveTo(DlPoint(100, 200)); + path_builder.LineTo(DlPoint(200, 200)); + path_builder.LineTo(DlPoint(100, 300)); + path_builder.Close(); + path_builder.SetConvexity(impeller::Convexity::kConvex); + + DlPath path(path_builder, DlPathFillType::kNonZero); + + ::testing::StrictMock mock_receiver; + + // Recommendations must happen before any of the path segments is dispatched + ::testing::ExpectationSet all_recommendations; + all_recommendations += // + EXPECT_CALL(mock_receiver, RecommendSizes(5u, 10u)) // + .Times(AtMost(1)); + all_recommendations += // + EXPECT_CALL(mock_receiver, + RecommendBounds(DlRect::MakeLTRB(100, 200, 200, 300))) + .Times(AtMost(1)); + EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, true)); + + { + ::testing::InSequence sequence; + + EXPECT_CALL(mock_receiver, MoveTo(DlPoint(100, 200))) + .After(all_recommendations); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(200, 200))); + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 300))); + // Closing LineTo added implicitly to return to first point + EXPECT_CALL(mock_receiver, LineTo(DlPoint(100, 200))); + EXPECT_CALL(mock_receiver, Close()); + } + + path.Dispatch(mock_receiver); +} + #ifndef NDEBUG // Tests that verify we don't try to use inverse path modes as they aren't // supported by either Flutter public APIs or Impeller diff --git a/engine/src/flutter/impeller/geometry/path.cc b/engine/src/flutter/impeller/geometry/path.cc index 385fe0b90a..35539c543b 100644 --- a/engine/src/flutter/impeller/geometry/path.cc +++ b/engine/src/flutter/impeller/geometry/path.cc @@ -103,6 +103,10 @@ size_t Path::GetComponentCount(std::optional type) const { return count; } +size_t Path::GetPointCount() const { + return data_->points.size(); +} + FillType Path::GetFillType() const { return data_->fill; } diff --git a/engine/src/flutter/impeller/geometry/path.h b/engine/src/flutter/impeller/geometry/path.h index 978e9f3559..ce00424db0 100644 --- a/engine/src/flutter/impeller/geometry/path.h +++ b/engine/src/flutter/impeller/geometry/path.h @@ -183,6 +183,8 @@ class Path { size_t GetComponentCount(std::optional type = {}) const; + size_t GetPointCount() const; + FillType GetFillType() const; bool IsConvex() const; diff --git a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc index 5cc4f40ea7..400d714fe3 100644 --- a/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc +++ b/engine/src/flutter/shell/platform/android/platform_view_android_jni_impl.cc @@ -2023,6 +2023,54 @@ void PlatformViewAndroidJNIImpl::destroyOverlaySurface2() { FML_CHECK(fml::jni::CheckException(env)); } +namespace { +class AndroidPathReceiver final : public DlPathReceiver { + public: + explicit AndroidPathReceiver(JNIEnv* env) + : env_(env), + android_path_(env->NewObject(path_class->obj(), path_constructor)) {} + + void SetPathInfo(DlPathFillType type, bool is_convex) override { + // Need to convert the fill type to the Android enum and + // call setFillType on the path... + // see https://github.com/flutter/flutter/issues/164808 + } + void MoveTo(const DlPoint& p2) override { + env_->CallVoidMethod(android_path_, path_move_to_method, p2.x, p2.y); + } + void LineTo(const DlPoint& p2) override { + env_->CallVoidMethod(android_path_, path_line_to_method, p2.x, p2.y); + } + void QuadTo(const DlPoint& cp, const DlPoint& p2) override { + env_->CallVoidMethod(android_path_, path_quad_to_method, // + cp.x, cp.y, p2.x, p2.y); + } + bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) override { + if (!path_conic_to_method) { + return false; + } + env_->CallVoidMethod(android_path_, path_conic_to_method, // + cp.x, cp.y, p2.x, p2.y, weight); + return true; + }; + void CubicTo(const DlPoint& cp1, + const DlPoint& cp2, + const DlPoint& p2) override { + env_->CallVoidMethod(android_path_, path_cubic_to_method, // + cp1.x, cp1.y, cp2.x, cp2.y, p2.x, p2.y); + } + void Close() override { + env_->CallVoidMethod(android_path_, path_close_method); + } + + jobject TakePath() { return android_path_; } + + private: + JNIEnv* env_; + jobject android_path_; +}; +} // namespace + void PlatformViewAndroidJNIImpl::onDisplayPlatformView2( int32_t view_id, int32_t x, @@ -2132,79 +2180,14 @@ void PlatformViewAndroidJNIImpl::onDisplayPlatformView2( FML_DCHECK(!dlPath.IsRoundRect()); // Define and populate an Android Path with data from the DlPath - jobject androidPath = - env->NewObject(path_class->obj(), path_constructor); + AndroidPathReceiver receiver(env); - bool subpath_needs_close = false; - std::optional pending_moveto; - - auto resolve_moveto = [&env, &pending_moveto, &androidPath]() { - if (pending_moveto.has_value()) { - env->CallVoidMethod(androidPath, path_move_to_method, - pending_moveto->x, pending_moveto->y); - pending_moveto.reset(); - } - }; - - auto& path = dlPath.GetPath(); - for (auto it = path.begin(), end = path.end(); it != end; ++it) { - switch (it.type()) { - case impeller::Path::ComponentType::kContour: { - const impeller::ContourComponent* contour = it.contour(); - FML_DCHECK(contour != nullptr); - if (subpath_needs_close) { - env->CallVoidMethod(androidPath, path_close_method); - } - pending_moveto = contour->destination; - subpath_needs_close = contour->IsClosed(); - break; - } - case impeller::Path::ComponentType::kLinear: { - const impeller::LinearPathComponent* linear = it.linear(); - FML_DCHECK(linear != nullptr); - resolve_moveto(); - env->CallVoidMethod(androidPath, path_line_to_method, - linear->p2.x, linear->p2.y); - break; - } - case impeller::Path::ComponentType::kQuadratic: { - const impeller::QuadraticPathComponent* quadratic = - it.quadratic(); - FML_DCHECK(quadratic != nullptr); - resolve_moveto(); - env->CallVoidMethod(androidPath, path_quad_to_method, - quadratic->cp.x, quadratic->cp.y, - quadratic->p2.x, quadratic->p2.y); - break; - } - case impeller::Path::ComponentType::kConic: { - const impeller::ConicPathComponent* conic = it.conic(); - FML_DCHECK(conic != nullptr); - resolve_moveto(); - FML_DCHECK(path_conic_to_method != nullptr); - env->CallVoidMethod(androidPath, path_conic_to_method, - conic->cp.x, conic->cp.y, // - conic->p2.x, conic->p2.y, conic->weight); - break; - } - case impeller::Path::ComponentType::kCubic: { - const impeller::CubicPathComponent* cubic = it.cubic(); - FML_DCHECK(cubic != nullptr); - resolve_moveto(); - env->CallVoidMethod(androidPath, path_cubic_to_method, - cubic->cp1.x, cubic->cp1.y, // - cubic->cp2.x, cubic->cp2.y, // - cubic->p2.x, cubic->p2.y); - break; - } - } - } - if (subpath_needs_close) { - env->CallVoidMethod(androidPath, path_close_method); - } + dlPath.Dispatch(receiver); env->CallVoidMethod(mutatorsStack, - g_mutators_stack_push_clippath_method, androidPath); + g_mutators_stack_push_clippath_method, + receiver.TakePath()); + break; } // TODO(cyanglaz): Implement other mutators. // https://github.com/flutter/flutter/issues/58426 diff --git a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm index b4137fadc1..efac6ae54f 100644 --- a/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm +++ b/engine/src/flutter/shell/platform/darwin/ios/framework/Source/FlutterPlatformViews.mm @@ -43,6 +43,37 @@ CATransform3D GetCATransform3DFromDlMatrix(const flutter::DlMatrix& matrix) { transform.m44 = matrix.m[15]; return transform; } + +class CGPathReceiver final : public flutter::DlPathReceiver { + public: + void SetPathInfo(flutter::DlPathFillType type, bool is_convex) override { + // CGPaths do not have an inherit fill type, we would need to remember + // the fill type and employ it when we use the path. + // see https://github.com/flutter/flutter/issues/164826 + } + void MoveTo(const flutter::DlPoint& p2) override { // + CGPathMoveToPoint(path_ref_, nil, p2.x, p2.y); + } + void LineTo(const flutter::DlPoint& p2) override { + CGPathAddLineToPoint(path_ref_, nil, p2.x, p2.y); + } + void QuadTo(const flutter::DlPoint& cp, const flutter::DlPoint& p2) override { + CGPathAddQuadCurveToPoint(path_ref_, nil, cp.x, cp.y, p2.x, p2.y); + } + // bool conic_to(...) { CGPath has no equivalent to the conic curve type } + void CubicTo(const flutter::DlPoint& cp1, + const flutter::DlPoint& cp2, + const flutter::DlPoint& p2) override { + CGPathAddCurveToPoint(path_ref_, nil, // + cp1.x, cp1.y, cp2.x, cp2.y, p2.x, p2.y); + } + void Close() override { CGPathCloseSubpath(path_ref_); } + + CGMutablePathRef TakePath() { return path_ref_; } + + private: + CGMutablePathRef path_ref_ = CGPathCreateMutable(); +}; } // namespace @interface PlatformViewFilter () @@ -382,77 +413,15 @@ static BOOL _preparedOnce = NO; - (void)clipPath:(const flutter::DlPath&)dlPath matrix:(const flutter::DlMatrix&)matrix { containsNonRectPath_ = YES; - CGMutablePathRef pathRef = CGPathCreateMutable(); - bool subpath_needs_close = false; - std::optional pending_moveto; - auto resolve_moveto = [&pending_moveto, &pathRef]() { - if (pending_moveto.has_value()) { - CGPathMoveToPoint(pathRef, nil, pending_moveto->x, pending_moveto->y); - pending_moveto.reset(); - } - }; + CGPathReceiver receiver; + + dlPath.Dispatch(receiver); - auto& path = dlPath.GetPath(); - for (auto it = path.begin(), end = path.end(); it != end; ++it) { - switch (it.type()) { - case impeller::Path::ComponentType::kContour: { - const impeller::ContourComponent* contour = it.contour(); - FML_DCHECK(contour != nullptr); - if (subpath_needs_close) { - CGPathCloseSubpath(pathRef); - } - pending_moveto = contour->destination; - subpath_needs_close = contour->IsClosed(); - break; - } - case impeller::Path::ComponentType::kLinear: { - const impeller::LinearPathComponent* linear = it.linear(); - FML_DCHECK(linear != nullptr); - resolve_moveto(); - CGPathAddLineToPoint(pathRef, nil, linear->p2.x, linear->p2.y); - break; - } - case impeller::Path::ComponentType::kQuadratic: { - const impeller::QuadraticPathComponent* quadratic = it.quadratic(); - FML_DCHECK(quadratic != nullptr); - resolve_moveto(); - CGPathAddQuadCurveToPoint(pathRef, nil, // - quadratic->cp.x, quadratic->cp.y, // - quadratic->p2.x, quadratic->p2.y); - break; - } - case impeller::Path::ComponentType::kConic: { - const impeller::ConicPathComponent* conic = it.conic(); - FML_DCHECK(conic != nullptr); - resolve_moveto(); - // Conic is not available in quartz, we use quad to approximate. - // TODO(cyanglaz): Better approximate the conic path. - // https://github.com/flutter/flutter/issues/35062 - CGPathAddQuadCurveToPoint(pathRef, nil, // - conic->cp.x, conic->cp.y, // - conic->p2.x, conic->p2.y); - break; - } - case impeller::Path::ComponentType::kCubic: { - const impeller::CubicPathComponent* cubic = it.cubic(); - FML_DCHECK(cubic != nullptr); - resolve_moveto(); - CGPathAddCurveToPoint(pathRef, nil, // - cubic->cp1.x, cubic->cp1.y, // - cubic->cp2.x, cubic->cp2.y, // - cubic->p2.x, cubic->p2.y); - break; - } - } - } - if (subpath_needs_close) { - CGPathCloseSubpath(pathRef); - } // The `matrix` is based on the physical pixels, convert it to UIKit points. CATransform3D matrixInPoints = CATransform3DConcat(GetCATransform3DFromDlMatrix(matrix), _reverseScreenScale); - paths_.push_back([self getTransformedPath:pathRef matrix:matrixInPoints]); + paths_.push_back([self getTransformedPath:receiver.TakePath() matrix:matrixInPoints]); } - (CGAffineTransform)affineWithMatrix:(CATransform3D)matrix {