[Impeller] Render conics without conversion from Flutter apps

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) {
@@ -442,9 +442,10 @@ class ImpellerPathReceiver final : public DlPathReceiver {
   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
+  bool ConicTo(const DlPoint& cp, const DlPoint& p2, DlScalar weight) override {
+    builder_.ConicCurveTo(cp, p2, weight);
+    return true;
+  }
   void CubicTo(const DlPoint& cp1,
                const DlPoint& cp2,
                const DlPoint& p2) override {

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 {
@@ -556,31 +556,6 @@ TEST(DisplayListPath, ConstructFromImpellerEqualsConstructFromSkia) {
 }
 
 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
@@ -886,6 +861,104 @@ 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<DlPoint, 5> i_points;
+    impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10),
+                                         DlPoint(20, 20), weight);
+    i_conic.SubdivideToQuadraticPoints(i_points);
+
+    ::testing::StrictMock<DlPathReceiverMock> 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<DlPoint, 5> i_points;
+    impeller::ConicPathComponent i_conic(DlPoint(10, 10), DlPoint(20, 10),
+                                         DlPoint(20, 20), weight);
+    i_conic.SubdivideToQuadraticPoints(i_points);
+
+    ::testing::StrictMock<DlPathReceiverMock> 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

engine/src/flutter/display_list/skia/dl_sk_conversions_unittests.cc

@@ -12,10 +12,11 @@
 #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 {
@@ -402,9 +403,7 @@ TEST(DisplayListSkConversions, ConicToQuads) {
   }
 }
 
-// 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) {
+TEST(DisplayListSkConversions, ConicPathToImpeller) {
   // 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] = {
@@ -426,35 +425,16 @@ TEST(DisplayListSkConversions, ConicPathToQuads) {
     ASSERT_EQ(it.type(), impeller::Path::ComponentType::kContour);
     ++it;
 
-    ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic);
-    auto quad1 = it.quadratic();
-    ASSERT_NE(quad1, nullptr);
-    ++it;
-
-    ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic);
-    auto quad2 = it.quadratic();
-    ASSERT_NE(quad2, nullptr);
+    ASSERT_EQ(it.type(), impeller::Path::ComponentType::kConic);
+    auto conic = it.conic();
+    ASSERT_NE(conic, 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;
+    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;
   }
 }
 

engine/src/flutter/display_list/testing/BUILD.gn

@@ -10,6 +10,7 @@ source_set("display_list_testing") {
 
   sources = [
     "dl_test_equality.h",
+    "dl_test_mock_path_receiver.h",
     "dl_test_snippets.cc",
     "dl_test_snippets.h",
   ]

engine/src/flutter/display_list/testing/dl_test_mock_path_receiver.h

@@ -0,0 +1,42 @@
+// 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_

engine/src/flutter/impeller/geometry/path_component.cc

@@ -344,7 +344,9 @@ void ConicPathComponent::AppendPolylinePoints(
     Scalar scale_factor,
     std::vector<Point>& points) const {
   ToLinearPathComponents(scale_factor, [&points](const Point& point) {
-    points.emplace_back(point);
+    if (point != points.back()) {
+      points.emplace_back(point);
+    }
   });
 }