diff --git a/engine/src/flutter/display_list/geometry/dl_path.cc b/engine/src/flutter/display_list/geometry/dl_path.cc index f18f154dea4..98751b0d723 100644 --- a/engine/src/flutter/display_list/geometry/dl_path.cc +++ b/engine/src/flutter/display_list/geometry/dl_path.cc @@ -5,8 +5,8 @@ #include "flutter/display_list/geometry/dl_path.h" #include "flutter/display_list/geometry/dl_geometry_types.h" -#include "flutter/impeller/geometry/path.h" #include "flutter/impeller/geometry/path_builder.h" +#include "impeller/geometry/path.h" namespace { inline constexpr flutter::DlPathFillType ToDlFillType(SkPathFillType sk_type) { @@ -452,10 +452,9 @@ class ImpellerPathReceiver final : public DlPathReceiver { void QuadTo(const DlPoint& cp, const DlPoint& p2) override { builder_.QuadraticCurveTo(cp, p2); } - bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) override { - builder_.ConicCurveTo(cp, p2, weight); - return true; - } + // 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 { 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 97d179cec5c..79b03cb52ab 100644 --- a/engine/src/flutter/display_list/geometry/dl_path_unittests.cc +++ b/engine/src/flutter/display_list/geometry/dl_path_unittests.cc @@ -4,9 +4,9 @@ #include "flutter/display_list/geometry/dl_path.h" +#include "gmock/gmock.h" #include "gtest/gtest.h" -#include "flutter/display_list/testing/dl_test_mock_path_receiver.h" #include "flutter/third_party/skia/include/core/SkRRect.h" namespace flutter { @@ -595,6 +595,31 @@ TEST(DisplayListPath, IsLineFromImpellerPath) { } 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 @@ -900,104 +925,6 @@ TEST(DisplayListPath, DispatchImpellerPathConvexSpecified) { path.Dispatch(mock_receiver); } -TEST(DisplayListPath, DispatchSkiaPathConicToQuads) { - // If we execute conicTo with a weight of exactly 1.0, SkPath will turn - // it into a quadTo, so we avoid that by using 0.999 - SkScalar weights[4] = { - 0.02f, - 0.5f, - SK_ScalarSqrt2 * 0.5f, - 1.0f - kEhCloseEnough, - }; - - for (SkScalar weight : weights) { - SkPath sk_path; - sk_path.moveTo(10, 10); - sk_path.conicTo(20, 10, 20, 20, weight); - - std::array i_points; - impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10), - DlPoint(20, 20), weight); - i_conic.SubdivideToQuadraticPoints(i_points); - - ::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(2u, 3u)) // - .Times(AtMost(1)); - all_recommendations += - EXPECT_CALL(mock_receiver, - RecommendBounds(DlRect::MakeLTRB(10, 10, 20, 20))) - .Times(AtMost(1)); - EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, true)); - - { - ::testing::InSequence sequence; - - EXPECT_CALL(mock_receiver, MoveTo(DlPoint(10, 10))) - .After(all_recommendations); - EXPECT_CALL(mock_receiver, - ConicTo(DlPoint(20, 10), DlPoint(20, 20), weight)) - .WillOnce(Return(false)); - EXPECT_CALL(mock_receiver, QuadTo(i_points[1], i_points[2])); - EXPECT_CALL(mock_receiver, QuadTo(i_points[3], i_points[4])); - } - - DlPath(sk_path).Dispatch(mock_receiver); - } -} - -TEST(DisplayListPath, DispatchImpellerPathConicToQuads) { - // If we execute conicTo with a weight of exactly 1.0, SkPath will turn - // it into a quadTo, so we avoid that by using 0.999 - DlScalar weights[4] = { - 0.02f, - 0.5f, - SK_ScalarSqrt2 * 0.5f, - 1.0f - kEhCloseEnough, - }; - - for (DlScalar weight : weights) { - DlPathBuilder path_builder; - path_builder.MoveTo(DlPoint(10, 10)); - path_builder.ConicCurveTo(DlPoint(20, 10), DlPoint(20, 20), weight); - - std::array i_points; - impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10), - DlPoint(20, 20), weight); - i_conic.SubdivideToQuadraticPoints(i_points); - - ::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(2u, 6u)) // - .Times(AtMost(1)); - all_recommendations += - EXPECT_CALL(mock_receiver, - RecommendBounds(DlRect::MakeLTRB(10, 10, 20, 20))) - .Times(AtMost(1)); - EXPECT_CALL(mock_receiver, SetPathInfo(DlPathFillType::kNonZero, false)); - - { - ::testing::InSequence sequence; - - EXPECT_CALL(mock_receiver, MoveTo(DlPoint(10, 10))) - .After(all_recommendations); - EXPECT_CALL(mock_receiver, - ConicTo(DlPoint(20, 10), DlPoint(20, 20), weight)) - .WillOnce(Return(false)); - EXPECT_CALL(mock_receiver, QuadTo(i_points[1], i_points[2])); - EXPECT_CALL(mock_receiver, QuadTo(i_points[3], i_points[4])); - } - - DlPath(path_builder).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/display_list/skia/dl_sk_conversions_unittests.cc b/engine/src/flutter/display_list/skia/dl_sk_conversions_unittests.cc index fb093baa5a2..61235d29b8d 100644 --- a/engine/src/flutter/display_list/skia/dl_sk_conversions_unittests.cc +++ b/engine/src/flutter/display_list/skia/dl_sk_conversions_unittests.cc @@ -12,11 +12,10 @@ #include "flutter/display_list/effects/dl_image_filters.h" #include "flutter/display_list/skia/dl_sk_conversions.h" #include "flutter/impeller/geometry/path_component.h" -#include "flutter/third_party/skia/include/core/SkColorSpace.h" -#include "flutter/third_party/skia/include/core/SkSamplingOptions.h" -#include "flutter/third_party/skia/include/core/SkTileMode.h" - #include "gtest/gtest.h" +#include "third_party/skia/include/core/SkColorSpace.h" +#include "third_party/skia/include/core/SkSamplingOptions.h" +#include "third_party/skia/include/core/SkTileMode.h" namespace flutter { namespace testing { @@ -403,7 +402,9 @@ TEST(DisplayListSkConversions, ConicToQuads) { } } -TEST(DisplayListSkConversions, ConicPathToImpeller) { +// This tests the new conic subdivision code in the Impeller conic path +// component object vs the code we used to rely on inside Skia +TEST(DisplayListSkConversions, ConicPathToQuads) { // If we execute conicTo with a weight of exactly 1.0, SkPath will turn // it into a quadTo, so we avoid that by using 0.999 SkScalar weights[4] = { @@ -425,16 +426,35 @@ TEST(DisplayListSkConversions, ConicPathToImpeller) { ASSERT_EQ(it.type(), impeller::Path::ComponentType::kContour); ++it; - ASSERT_EQ(it.type(), impeller::Path::ComponentType::kConic); - auto conic = it.conic(); - ASSERT_NE(conic, nullptr); + ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic); + auto quad1 = it.quadratic(); + ASSERT_NE(quad1, nullptr); ++it; - EXPECT_EQ(conic->p1, DlPoint(10, 10)) << "weight: " << weight; - EXPECT_EQ(conic->cp, DlPoint(20, 10)) << "weight: " << weight; - EXPECT_EQ(conic->p2, DlPoint(20, 20)) << "weight: " << weight; - EXPECT_EQ(conic->weight.x, weight) << "weight: " << weight; - EXPECT_EQ(conic->weight.y, weight) << "weight: " << weight; + ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic); + auto quad2 = it.quadratic(); + ASSERT_NE(quad2, nullptr); + ++it; + + SkPoint sk_points[5]; + int ncurves = SkPath::ConvertConicToQuads( + SkPoint::Make(10, 10), SkPoint::Make(20, 10), SkPoint::Make(20, 20), + weight, sk_points, 1); + ASSERT_EQ(ncurves, 2); + + EXPECT_FLOAT_EQ(sk_points[0].fX, quad1->p1.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[0].fY, quad1->p1.y) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[1].fX, quad1->cp.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[1].fY, quad1->cp.y) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[2].fX, quad1->p2.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[2].fY, quad1->p2.y) << "weight: " << weight; + + EXPECT_FLOAT_EQ(sk_points[2].fX, quad2->p1.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[2].fY, quad2->p1.y) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[3].fX, quad2->cp.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[3].fY, quad2->cp.y) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[4].fX, quad2->p2.x) << "weight: " << weight; + EXPECT_FLOAT_EQ(sk_points[4].fY, quad2->p2.y) << "weight: " << weight; } } diff --git a/engine/src/flutter/display_list/testing/BUILD.gn b/engine/src/flutter/display_list/testing/BUILD.gn index 8e1a0536126..93a9458da3c 100644 --- a/engine/src/flutter/display_list/testing/BUILD.gn +++ b/engine/src/flutter/display_list/testing/BUILD.gn @@ -10,7 +10,6 @@ source_set("display_list_testing") { sources = [ "dl_test_equality.h", - "dl_test_mock_path_receiver.h", "dl_test_snippets.cc", "dl_test_snippets.h", ] diff --git a/engine/src/flutter/display_list/testing/dl_test_mock_path_receiver.h b/engine/src/flutter/display_list/testing/dl_test_mock_path_receiver.h deleted file mode 100644 index bbef3f999d9..00000000000 --- a/engine/src/flutter/display_list/testing/dl_test_mock_path_receiver.h +++ /dev/null @@ -1,42 +0,0 @@ -// Copyright 2013 The Flutter Authors. All rights reserved. -// Use of this source code is governed by a BSD-style license that can be -// found in the LICENSE file. - -#ifndef FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_ -#define FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_ - -#include "gmock/gmock.h" - -#include "flutter/display_list/geometry/dl_path.h" - -namespace flutter { -namespace testing { -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)); -}; - -} // namespace testing -} // namespace flutter - -#endif // FLUTTER_DISPLAY_LIST_TESTING_DL_TEST_MOCK_PATH_RECEIVER_H_ diff --git a/engine/src/flutter/impeller/geometry/path_component.cc b/engine/src/flutter/impeller/geometry/path_component.cc index cba5b58bb91..6d7d302c724 100644 --- a/engine/src/flutter/impeller/geometry/path_component.cc +++ b/engine/src/flutter/impeller/geometry/path_component.cc @@ -344,9 +344,7 @@ void ConicPathComponent::AppendPolylinePoints( Scalar scale_factor, std::vector& points) const { ToLinearPathComponents(scale_factor, [&points](const Point& point) { - if (point != points.back()) { - points.emplace_back(point); - } + points.emplace_back(point); }); } diff --git a/engine/src/flutter/testing/display_list_testing.cc b/engine/src/flutter/testing/display_list_testing.cc index b93569cd145..f921a68fe3f 100644 --- a/engine/src/flutter/testing/display_list_testing.cc +++ b/engine/src/flutter/testing/display_list_testing.cc @@ -59,7 +59,6 @@ using DlImageSampling = flutter::DlImageSampling; using SaveLayerOptions = flutter::SaveLayerOptions; using DisplayListOpType = flutter::DisplayListOpType; using DisplayListOpCategory = flutter::DisplayListOpCategory; -using DlPathFillType = flutter::DlPathFillType; using DlPath = flutter::DlPath; using DisplayListStreamDispatcher = flutter::testing::DisplayListStreamDispatcher; @@ -138,14 +137,6 @@ extern std::ostream& operator<<( return os << "DisplayListOpCategory::???"; } -extern std::ostream& operator<<( - std::ostream& os, const flutter::DlPathFillType& type) { - switch (type) { - DLT_OSTREAM_CASE(DlPathFillType, Odd); - DLT_OSTREAM_CASE(DlPathFillType, NonZero); - } -} - #undef DLT_OSTREAM_CASE std::ostream& operator<<(std::ostream& os, const SaveLayerOptions& options) { @@ -176,12 +167,6 @@ extern std::ostream& operator<<(std::ostream& os, const DlPath& path) { << ")"; } -extern std::ostream& operator<<(std::ostream& os, const flutter::testing::DlVerbosePath& path) { - DisplayListStreamDispatcher dispatcher(os, 0); - dispatcher.out(path); - return os; -} - std::ostream& operator<<(std::ostream& os, const flutter::DlClipOp& op) { switch (op) { case flutter::DlClipOp::kDifference: return os << "DlClipOp::kDifference"; @@ -640,75 +625,6 @@ void DisplayListStreamDispatcher::out(const DlImageFilter* filter) { outdent(1); } } -DisplayListStreamDispatcher::DlPathStreamer::~DlPathStreamer() { - if (done_with_info_) { - dispatcher_.outdent(2); - dispatcher_.startl() << "}" << std::endl; - } -} -void DisplayListStreamDispatcher::DlPathStreamer::RecommendSizes( - size_t verb_count, size_t point_count) { - FML_DCHECK(!done_with_info_); - dispatcher_.startl() << "sizes: " - << verb_count << " verbs, " << point_count << " points" << std::endl; -}; -void DisplayListStreamDispatcher::DlPathStreamer::RecommendBounds( - const DlRect& bounds) { - FML_DCHECK(!done_with_info_); - dispatcher_.startl() << "bounds: " << bounds << std::endl; -}; -void DisplayListStreamDispatcher::DlPathStreamer::SetPathInfo( - DlPathFillType fill_type, bool is_convex) { - FML_DCHECK(!done_with_info_); - dispatcher_.startl() << "info: " - << fill_type << ", convex: " << is_convex << std::endl; -} -void DisplayListStreamDispatcher::DlPathStreamer::MoveTo(const DlPoint& p2) { - if (!done_with_info_) { - done_with_info_ = true; - dispatcher_.startl() << "{" << std::endl; - dispatcher_.indent(2); - } - dispatcher_.startl() << "MoveTo(" << p2 << ")," << std::endl; -} -void DisplayListStreamDispatcher::DlPathStreamer::LineTo(const DlPoint& p2) { - FML_DCHECK(done_with_info_); - dispatcher_.startl() << "LineTo(" << p2 << ")," << std::endl; -} -void DisplayListStreamDispatcher::DlPathStreamer::QuadTo(const DlPoint& cp, - const DlPoint& p2) { - FML_DCHECK(done_with_info_); - dispatcher_.startl() << "QuadTo(" << cp << ", " << p2 << ")," << std::endl; -} -bool DisplayListStreamDispatcher::DlPathStreamer::ConicTo(const DlPoint& cp, - const DlPoint& p2, - DlScalar weight) { - FML_DCHECK(done_with_info_); - dispatcher_.startl() << "ConicTo(" << cp << ", " << p2 << ", " << weight - << ")," << std::endl; - return true; -} -void DisplayListStreamDispatcher::DlPathStreamer::CubicTo(const DlPoint& cp1, - const DlPoint& cp2, - const DlPoint& p2) { - FML_DCHECK(done_with_info_); - dispatcher_.startl() << "CubicTo(" << cp1 << ", " << cp2 << ", " << p2 << ", " - << p2 << ")," << std::endl; -} -void DisplayListStreamDispatcher::DlPathStreamer::Close() { - FML_DCHECK(done_with_info_); - dispatcher_.startl() << "Close()," << std::endl; -} -void DisplayListStreamDispatcher::out(const DlVerbosePath& path) { - os_ << "DlPath(" << std::endl; - indent(2); - { - DlPathStreamer streamer(*this); - path.path.Dispatch(streamer); - } - outdent(2); - os_ << ")"; -} void DisplayListStreamDispatcher::setImageFilter(const DlImageFilter* filter) { startl() << "setImageFilter("; indent(15); diff --git a/engine/src/flutter/testing/display_list_testing.h b/engine/src/flutter/testing/display_list_testing.h index 354c6cf9ca1..7b53354a57b 100644 --- a/engine/src/flutter/testing/display_list_testing.h +++ b/engine/src/flutter/testing/display_list_testing.h @@ -35,12 +35,6 @@ namespace flutter::testing { const sk_sp& b) { return DisplayListsNE_Verbose(a.get(), b.get()); } -class DlVerbosePath { - public: - explicit DlVerbosePath(const DlPath& path) : path(path) {} - - const DlPath& path; -}; } // namespace flutter::testing @@ -82,10 +76,6 @@ extern std::ostream& operator<<(std::ostream& os, extern std::ostream& operator<<(std::ostream& os, const flutter::DisplayListOpCategory& category); extern std::ostream& operator<<(std::ostream& os, const flutter::DlPath& path); -extern std::ostream& operator<<(std::ostream& os, - const flutter::testing::DlVerbosePath& path); -extern std::ostream& operator<<(std::ostream& os, - const flutter::DlPathFillType& type); extern std::ostream& operator<<(std::ostream& os, const flutter::DlImageFilter& type); extern std::ostream& operator<<(std::ostream& os, @@ -214,7 +204,6 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { void out(const DlColorFilter* filter); void out(const DlImageFilter& filter); void out(const DlImageFilter* filter); - void out(const DlVerbosePath& path); private: std::ostream& os_; @@ -230,29 +219,6 @@ class DisplayListStreamDispatcher final : public DlOpReceiver { std::ostream& out_array(std::string name, int count, const T array[]); std::ostream& startl(); - - class DlPathStreamer : public DlPathReceiver { - public: - ~DlPathStreamer(); - - explicit DlPathStreamer(DisplayListStreamDispatcher& dispatcher) - : dispatcher_(dispatcher) {} - - void RecommendSizes(size_t verb_count, size_t point_count); - void RecommendBounds(const DlRect& bounds); - void SetPathInfo(DlPathFillType fill_type, bool is_convex); - void MoveTo(const DlPoint& p2); - void LineTo(const DlPoint& p2); - void QuadTo(const DlPoint& cp, const DlPoint& p2); - bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight); - void CubicTo(const DlPoint& cp1, const DlPoint& cp2, const DlPoint& p2); - void Close(); - - private: - DisplayListStreamDispatcher& dispatcher_; - bool done_with_info_ = false; - }; - friend class DlPathStreamer; }; class DisplayListGeneralReceiver : public DlOpReceiver {