Reverts "[Impeller] Render conics without conversion from Flutter apps (flutter#166305)" (flutter#166591)

<!-- start_original_pr_link -->
Reverts: flutter#166305
<!-- end_original_pr_link -->
<!-- start_initiating_author -->
Initiated by: gaaclarke
<!-- end_initiating_author -->
<!-- start_revert_reason -->
Reason for reverting: Integration golden test failures. Please reland it
this and address golden diffs.
<!-- end_revert_reason -->
<!-- start_original_pr_author -->
Original PR Author: flar
<!-- end_original_pr_author -->

<!-- start_reviewers -->
Reviewed By: {jonahwilliams}
<!-- end_reviewers -->

<!-- start_revert_body -->
This change reverts the following previous change:
Now that Impeller performs high fidelity tessellation of Conic curves we
will no longer convert Flutter app's conic curves into approximated
quadratic curves.
<!-- end_revert_body -->

Co-authored-by: auto-submit[bot] <[email protected]>

Author: 2 people

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 {

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<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,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;
+
+    ASSERT_EQ(it.type(), impeller::Path::ComponentType::kQuadratic);
+    auto quad2 = it.quadratic();
+    ASSERT_NE(quad2, 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;
+    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;
   }
 }
 

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",
   ]

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

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