Relands "[Impeller] Render conics without conversion from Flutter apps (#166305)" (#166598)

Reverts flutter/flutter#166591

Golden diffs were not discovered the first time this was submitted.
Re-submitting to double check the goldens.

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,9 +452,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 {
@@ -595,31 +595,6 @@ 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
@@ -925,6 +900,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);
+    ASSERT_EQ(it.type(), impeller::Path::ComponentType::kConic);
+    auto conic = it.conic();
+    ASSERT_NE(conic, nullptr);
     ++it;
 
-    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;
+    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);
+    }
   });
 }
 

engine/src/flutter/testing/display_list_testing.cc

@@ -59,6 +59,7 @@ 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;
@@ -137,6 +138,14 @@ 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) {
@@ -167,6 +176,12 @@ 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";
@@ -625,6 +640,75 @@ 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);

engine/src/flutter/testing/display_list_testing.h

@@ -35,6 +35,12 @@ namespace flutter::testing {
     const sk_sp<const DisplayList>& b) {
   return DisplayListsNE_Verbose(a.get(), b.get());
 }
+class DlVerbosePath {
+ public:
+  explicit DlVerbosePath(const DlPath& path) : path(path) {}
+
+  const DlPath& path;
+};
 
 }  // namespace flutter::testing
 
@@ -76,6 +82,10 @@ 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,
@@ -204,6 +214,7 @@ 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_;
@@ -219,6 +230,29 @@ 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 {