[Impeller] Fix incorrect rendering path when duplicated point exists.

impeller/display_list/display_list_unittests.cc

@@ -188,49 +188,173 @@ TEST_P(DisplayListTest, CanDrawArc) {
 }
 
 TEST_P(DisplayListTest, StrokedPathsDrawCorrectly) {
-  flutter::DisplayListBuilder builder;
-  builder.setColor(SK_ColorRED);
-  builder.setStyle(flutter::DlDrawStyle::kStroke);
-  builder.setStrokeWidth(10);
+  auto callback = [&]() {
+    flutter::DisplayListBuilder builder;
+    builder.setColor(SK_ColorRED);
+    builder.setStyle(flutter::DlDrawStyle::kStroke);
 
-  // Rectangle
-  builder.translate(100, 100);
-  builder.drawRect(SkRect::MakeSize({100, 100}));
+    static float stroke_width = 10.0f;
+    static int selected_stroke_type = 0;
+    static int selected_join_type = 0;
+    const char* stroke_types[] = {"Butte", "Round", "Square"};
+    const char* join_type[] = {"kMiter", "Round", "kBevel"};
 
-  // Rounded rectangle
-  builder.translate(150, 0);
-  builder.drawRRect(SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10));
+    ImGui::Begin("Controls", nullptr, ImGuiWindowFlags_AlwaysAutoResize);
+    ImGui::Combo("Cap", &selected_stroke_type, stroke_types,
+                 sizeof(stroke_types) / sizeof(char*));
+    ImGui::Combo("Join", &selected_join_type, join_type,
+                 sizeof(join_type) / sizeof(char*));
+    ImGui::SliderFloat("Stroke Width", &stroke_width, 10.0f, 50.0f);
+    ImGui::End();
 
-  // Double rounded rectangle
-  builder.translate(150, 0);
-  builder.drawDRRect(
-      SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10),
-      SkRRect::MakeRectXY(SkRect::MakeXYWH(10, 10, 80, 30), 10, 10));
+    flutter::DlStrokeCap cap;
+    flutter::DlStrokeJoin join;
+    switch (selected_stroke_type) {
+      case 0:
+        cap = flutter::DlStrokeCap::kButt;
+        break;
+      case 1:
+        cap = flutter::DlStrokeCap::kRound;
+        break;
+      case 2:
+        cap = flutter::DlStrokeCap::kSquare;
+        break;
+      default:
+        cap = flutter::DlStrokeCap::kButt;
+        break;
+    }
+    switch (selected_join_type) {
+      case 0:
+        join = flutter::DlStrokeJoin::kMiter;
+        break;
+      case 1:
+        join = flutter::DlStrokeJoin::kRound;
+        break;
+      case 2:
+        join = flutter::DlStrokeJoin::kBevel;
+        break;
+      default:
+        join = flutter::DlStrokeJoin::kMiter;
+        break;
+    }
+    builder.setStrokeCap(cap);
+    builder.setStrokeJoin(join);
+    builder.setStrokeWidth(stroke_width);
 
-  // Contour with duplicate join points
-  {
+    // Make rendering better to watch.
+    builder.scale(1.5f, 1.5f);
+
+    // Rectangle
+    builder.translate(100, 100);
+    builder.drawRect(SkRect::MakeSize({100, 100}));
+
+    // Rounded rectangle
     builder.translate(150, 0);
-    SkPath path;
-    path.lineTo({100, 0});
-    path.lineTo({100, 0});
-    path.lineTo({100, 100});
-    builder.drawPath(path);
-  }
+    builder.drawRRect(SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10));
 
-  // Contour with duplicate end points
-  {
-    builder.setStrokeCap(flutter::DlStrokeCap::kRound);
+    // Double rounded rectangle
     builder.translate(150, 0);
-    SkPath path;
-    path.moveTo(0, 0);
-    path.lineTo({0, 0});
-    path.lineTo({50, 50});
-    path.lineTo({100, 0});
-    path.lineTo({100, 0});
-    builder.drawPath(path);
-  }
+    builder.drawDRRect(
+        SkRRect::MakeRectXY(SkRect::MakeSize({100, 50}), 10, 10),
+        SkRRect::MakeRectXY(SkRect::MakeXYWH(10, 10, 80, 30), 10, 10));
 
-  ASSERT_TRUE(OpenPlaygroundHere(builder.Build()));
+    // Contour with duplicate join points
+    {
+      builder.translate(150, 0);
+      SkPath path;
+      path.moveTo(0, 0);
+      path.lineTo(0, 0);
+      path.lineTo({100, 0});
+      path.lineTo({100, 0});
+      path.lineTo({100, 100});
+      builder.drawPath(path);
+    }
+
+    // Contour with duplicate start and end points
+
+    // Line.
+    builder.translate(200, 0);
+    {
+      builder.save();
+
+      SkPath line_path;
+      line_path.moveTo(0, 0);
+      line_path.moveTo(0, 0);
+      line_path.lineTo({0, 0});
+      line_path.lineTo({0, 0});
+      line_path.lineTo({50, 50});
+      line_path.lineTo({50, 50});
+      line_path.lineTo({100, 0});
+      line_path.lineTo({100, 0});
+      builder.drawPath(line_path);
+
+      builder.translate(0, 100);
+      builder.drawPath(line_path);
+
+      builder.translate(0, 100);
+      SkPath line_path2;
+      line_path2.moveTo(0, 0);
+      line_path2.lineTo(0, 0);
+      line_path2.lineTo(0, 0);
+      builder.drawPath(line_path2);
+
+      builder.restore();
+    }
+
+    // Cubic.
+    builder.translate(150, 0);
+    {
+      builder.save();
+
+      SkPath cubic_path;
+      cubic_path.moveTo({0, 0});
+      cubic_path.cubicTo(0, 0, 140.0, 100.0, 140, 20);
+      builder.drawPath(cubic_path);
+
+      builder.translate(0, 100);
+      SkPath cubic_path2;
+      cubic_path2.moveTo({0, 0});
+      cubic_path2.cubicTo(0, 0, 0, 0, 150, 150);
+      builder.drawPath(cubic_path2);
+
+      builder.translate(0, 100);
+      SkPath cubic_path3;
+      cubic_path3.moveTo({0, 0});
+      cubic_path3.cubicTo(0, 0, 0, 0, 0, 0);
+      builder.drawPath(cubic_path3);
+
+      builder.restore();
+    }
+
+    // Quad.
+    builder.translate(200, 0);
+    {
+      builder.save();
+
+      SkPath quad_path;
+      quad_path.moveTo(0, 0);
+      quad_path.moveTo(0, 0);
+      quad_path.quadTo({100, 40}, {50, 80});
+      builder.drawPath(quad_path);
+
+      builder.translate(0, 150);
+      SkPath quad_path2;
+      quad_path2.moveTo(0, 0);
+      quad_path2.moveTo(0, 0);
+      quad_path2.quadTo({0, 0}, {100, 100});
+      builder.drawPath(quad_path2);
+
+      builder.translate(0, 100);
+      SkPath quad_path3;
+      quad_path3.moveTo(0, 0);
+      quad_path3.quadTo({0, 0}, {0, 0});
+      builder.drawPath(quad_path3);
+
+      builder.restore();
+    }
+    return builder.Build();
+  };
+  ASSERT_TRUE(OpenPlaygroundHere(callback));
 }
 
 TEST_P(DisplayListTest, CanDrawWithOddPathWinding) {

impeller/geometry/path.cc

@@ -245,7 +245,7 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
 
   auto get_path_component =
       [this](size_t component_i) -> std::optional<const PathComponent*> {
-    if (component_i >= components_.size()) {
+    if (component_i < 0 || component_i >= components_.size()) {
       return std::nullopt;
     }
     const auto& component = components_[component_i];
@@ -261,20 +261,47 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
     }
   };
 
-  std::optional<const PathComponent*> previous_path_component;
-  auto end_contour = [&polyline, &previous_path_component]() {
+  auto compute_contour_start_direction = [&get_path_component](
+                                             int current_path_component_index) {
+    int next_component_index = current_path_component_index + 1;
+    while (get_path_component(next_component_index).has_value()) {
+      auto next_component = get_path_component(next_component_index).value();
+      if (next_component->GetStartDirection().has_value()) {
+        return next_component->GetStartDirection().value();
+      } else {
+        next_component_index++;
+      }
+    }
+    return Vector2(0, -1);
+  };
+
+  std::optional<int> previous_path_component_index;
+  auto end_contour = [&polyline, &previous_path_component_index,
+                      &get_path_component]() {
     // Whenever a contour has ended, extract the exact end direction from the
     // last component.
     if (polyline.contours.empty()) {
       return;
     }
-    if (!previous_path_component.has_value()) {
+
+    if (!previous_path_component_index.has_value()) {
       return;
     }
+
     auto& contour = polyline.contours.back();
-    contour.end_direction =
-        previous_path_component.value()->GetEndDirection().value_or(
-            Vector2(0, 1));
+    contour.end_direction = Vector2(0, 1);
+
+    int previous_index = previous_path_component_index.value();
+    while (get_path_component(previous_index).has_value()) {
+      auto previous_path_component = get_path_component(previous_index).value();
+      if (previous_path_component->GetEndDirection().has_value()) {
+        contour.end_direction =
+            previous_path_component->GetEndDirection().value();
+        break;
+      } else {
+        previous_index--;
+      }
+    }
   };
 
   for (size_t component_i = 0; component_i < components_.size();
@@ -283,15 +310,15 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
     switch (component.type) {
       case ComponentType::kLinear:
         collect_points(linears_[component.index].CreatePolyline());
-        previous_path_component = &linears_[component.index];
+        previous_path_component_index = component_i;
         break;
       case ComponentType::kQuadratic:
         collect_points(quads_[component.index].CreatePolyline(scale));
-        previous_path_component = &quads_[component.index];
+        previous_path_component_index = component_i;
         break;
       case ComponentType::kCubic:
         collect_points(cubics_[component.index].CreatePolyline(scale));
-        previous_path_component = &cubics_[component.index];
+        previous_path_component_index = component_i;
         break;
       case ComponentType::kContour:
         if (component_i == components_.size() - 1) {
@@ -301,14 +328,7 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const {
         }
         end_contour();
 
-        Vector2 start_direction(0, -1);
-        auto first_component = get_path_component(component_i + 1);
-        if (first_component.has_value()) {
-          start_direction =
-              first_component.value()->GetStartDirection().value_or(
-                  Vector2(0, -1));
-        }
-
+        Vector2 start_direction = compute_contour_start_direction(component_i);
         const auto& contour = contours_[component.index];
         polyline.contours.push_back({.start_index = polyline.points.size(),
                                      .is_closed = contour.is_closed,

impeller/geometry/path_component.cc

@@ -70,10 +70,16 @@ std::vector<Point> LinearPathComponent::Extrema() const {
 }
 
 std::optional<Vector2> LinearPathComponent::GetStartDirection() const {
+  if (p1 == p2) {
+    return std::nullopt;
+  }
   return (p1 - p2).Normalize();
 }
 
 std::optional<Vector2> LinearPathComponent::GetEndDirection() const {
+  if (p1 == p2) {
+    return std::nullopt;
+  }
   return (p2 - p1).Normalize();
 }
 
@@ -150,11 +156,23 @@ std::vector<Point> QuadraticPathComponent::Extrema() const {
 }
 
 std::optional<Vector2> QuadraticPathComponent::GetStartDirection() const {
-  return (p1 - cp).Normalize();
+  if (p1 != cp) {
+    return (p1 - cp).Normalize();
+  }
+  if (p1 != p2) {
+    return (p1 - p2).Normalize();
+  }
+  return std::nullopt;
 }
 
 std::optional<Vector2> QuadraticPathComponent::GetEndDirection() const {
-  return (p2 - cp).Normalize();
+  if (p2 != cp) {
+    return (p2 - cp).Normalize();
+  }
+  if (p2 != p1) {
+    return (p2 - p1).Normalize();
+  }
+  return std::nullopt;
 }
 
 Point CubicPathComponent::Solve(Scalar time) const {
@@ -302,11 +320,29 @@ std::vector<Point> CubicPathComponent::Extrema() const {
 }
 
 std::optional<Vector2> CubicPathComponent::GetStartDirection() const {
-  return (p1 - cp1).Normalize();
+  if (p1 != cp1) {
+    return (p1 - cp1).Normalize();
+  }
+  if (p1 != cp2) {
+    return (p1 - cp2).Normalize();
+  }
+  if (p1 != p2) {
+    return (p1 - p2).Normalize();
+  }
+  return std::nullopt;
 }
 
 std::optional<Vector2> CubicPathComponent::GetEndDirection() const {
-  return (p2 - cp2).Normalize();
+  if (p2 != cp2) {
+    return (p2 - cp2).Normalize();
+  }
+  if (p2 != cp1) {
+    return (p2 - cp1).Normalize();
+  }
+  if (p2 != p1) {
+    return (p2 - p1).Normalize();
+  }
+  return std::nullopt;
 }
 
 }  // namespace impeller