Remove break corner cases, simplify strokes, and generate closed path joins (flutter#41)

Author: bdero

impeller/entity/contents.cc

@@ -5,6 +5,7 @@
 #include "impeller/entity/contents.h"
 
 #include <memory>
+#include <tuple>
 
 #include "flutter/fml/logging.h"
 #include "impeller/entity/content_context.h"
@@ -324,62 +325,60 @@ static VertexBuffer CreateSolidStrokeVertices(const Path& path,
   VertexBufferBuilder<VS::PerVertexData> vtx_builder;
   auto polyline = path.CreatePolyline();
 
-  size_t point_i = 0;
   if (polyline.points.size() < 2) {
     return {};  // Nothing to render.
   }
 
   VS::PerVertexData vtx;
 
-  // Cursor state.
-  Point direction;
+  // Normal state.
   Point normal;
   Point previous_normal;  // Used for computing joins.
-  auto compute_normals = [&](size_t point_i) {
+
+  auto compute_normal = [&polyline, &normal, &previous_normal](size_t point_i) {
     previous_normal = normal;
-    direction =
+    Point direction =
         (polyline.points[point_i] - polyline.points[point_i - 1]).Normalize();
     normal = {-direction.y, direction.x};
   };
-  compute_normals(1);
 
-  // Break state.
-  auto breaks_it = polyline.breaks.begin();
-  size_t break_end =
-      breaks_it != polyline.breaks.end() ? *breaks_it : polyline.points.size();
+  for (size_t contour_i = 0; contour_i < polyline.contours.size();
+       contour_i++) {
+    size_t contour_start_point_i, contour_end_point_i;
+    std::tie(contour_start_point_i, contour_end_point_i) =
+        polyline.GetContourPointBounds(contour_i);
+
+    if (contour_end_point_i - contour_start_point_i < 2) {
+      continue;  // This contour has no renderable content.
+    }
 
-  while (point_i < polyline.points.size()) {
-    if (point_i > 0) {
-      compute_normals(point_i);
+    // The first point's normal is always the same as
+    compute_normal(contour_start_point_i + 1);
+    const Point contour_first_normal = normal;
 
+    if (contour_i > 0) {
       // This branch only executes when we've just finished drawing a contour
       // and are switching to a new one.
       // We're drawing a triangle strip, so we need to "pick up the pen" by
       // appending transparent vertices between the end of the previous contour
       // and the beginning of the new contour.
-      vtx.vertex_position = polyline.points[point_i - 1];
+      vtx.vertex_position = polyline.points[contour_start_point_i - 1];
       vtx.vertex_normal = {};
       vtx.pen_down = 0.0;
       vtx_builder.AppendVertex(vtx);
-      vtx.vertex_position = polyline.points[point_i];
+      vtx.vertex_position = polyline.points[contour_start_point_i];
       vtx_builder.AppendVertex(vtx);
     }
 
     // Generate start cap.
-    CreateCap(vtx_builder, polyline.points[point_i], -direction);
+    if (!polyline.contours[contour_i].is_closed) {
+      CreateCap(vtx_builder, polyline.points[contour_start_point_i], -normal);
+    }
 
     // Generate contour geometry.
-    size_t contour_point_i = 0;
-    while (point_i < break_end) {
-      if (contour_point_i > 0) {
-        if (contour_point_i > 1) {
-          // Generate join from the previous line to the current line.
-          CreateJoin(vtx_builder, polyline.points[point_i - 1], previous_normal,
-                     normal);
-        } else {
-          compute_normals(point_i);
-        }
-
+    for (size_t point_i = contour_start_point_i; point_i < contour_end_point_i;
+         point_i++) {
+      if (point_i > contour_start_point_i) {
         // Generate line rect.
         vtx.vertex_position = polyline.points[point_i - 1];
         vtx.pen_down = 1.0;
@@ -393,20 +392,22 @@ static VertexBuffer CreateSolidStrokeVertices(const Path& path,
         vtx.vertex_normal = -normal;
         vtx_builder.AppendVertex(vtx);
 
-        compute_normals(point_i + 1);
-      }
+        if (point_i < contour_end_point_i - 1) {
+          compute_normal(point_i + 1);
 
-      ++contour_point_i;
-      ++point_i;
+          // Generate join from the current line to the next line.
+          CreateJoin(vtx_builder, polyline.points[point_i], previous_normal,
+                     normal);
+        }
+      }
     }
 
-    // Generate end cap.
-    CreateCap(vtx_builder, polyline.points[point_i - 1], -direction);
-
-    if (break_end < polyline.points.size()) {
-      ++breaks_it;
-      break_end = breaks_it != polyline.breaks.end() ? *breaks_it
-                                                     : polyline.points.size();
+    // Generate end cap or join.
+    if (!polyline.contours[contour_i].is_closed) {
+      CreateCap(vtx_builder, polyline.points[contour_end_point_i - 1], normal);
+    } else {
+      CreateJoin(vtx_builder, polyline.points[contour_start_point_i], normal,
+                 contour_first_normal);
     }
   }
 

impeller/geometry/geometry_unittests.cc

@@ -149,21 +149,21 @@ TEST(GeometryTest, SimplePath) {
       .AddQuadraticComponent({100, 100}, {200, 200}, {300, 300})
       .AddCubicComponent({300, 300}, {400, 400}, {500, 500}, {600, 600});
 
-  ASSERT_EQ(path.GetComponentCount(), 3u);
+  ASSERT_EQ(path.GetComponentCount(), 4u);
 
   path.EnumerateComponents(
       [](size_t index, const LinearPathComponent& linear) {
         Point p1(0, 0);
         Point p2(100, 100);
-        ASSERT_EQ(index, 0u);
+        ASSERT_EQ(index, 1u);
         ASSERT_EQ(linear.p1, p1);
         ASSERT_EQ(linear.p2, p2);
       },
       [](size_t index, const QuadraticPathComponent& quad) {
         Point p1(100, 100);
         Point cp(200, 200);
         Point p2(300, 300);
-        ASSERT_EQ(index, 1u);
+        ASSERT_EQ(index, 2u);
         ASSERT_EQ(quad.p1, p1);
         ASSERT_EQ(quad.cp, cp);
         ASSERT_EQ(quad.p2, p2);
@@ -173,13 +173,18 @@ TEST(GeometryTest, SimplePath) {
         Point cp1(400, 400);
         Point cp2(500, 500);
         Point p2(600, 600);
-        ASSERT_EQ(index, 2u);
+        ASSERT_EQ(index, 3u);
         ASSERT_EQ(cubic.p1, p1);
         ASSERT_EQ(cubic.cp1, cp1);
         ASSERT_EQ(cubic.cp2, cp2);
         ASSERT_EQ(cubic.p2, p2);
       },
-      [](size_t index, const MovePathComponent& move) { ASSERT_TRUE(false); });
+      [](size_t index, const ContourComponent& contour) {
+        Point p1(0, 0);
+        ASSERT_EQ(index, 0u);
+        ASSERT_EQ(contour.destination, p1);
+        ASSERT_FALSE(contour.is_closed);
+      });
 }
 
 TEST(GeometryTest, BoundingBoxCubic) {
@@ -638,15 +643,15 @@ TEST(GeometryTest, CubicPathComponentPolylineDoesNotIncludePointOne) {
 
 TEST(GeometryTest, PathCreatePolyLineDoesNotDuplicatePoints) {
   Path path;
-  path.AddMoveComponent({10, 10});
+  path.AddContourComponent({10, 10});
   path.AddLinearComponent({10, 10}, {20, 20});
   path.AddLinearComponent({20, 20}, {30, 30});
-  path.AddMoveComponent({40, 40});
+  path.AddContourComponent({40, 40});
   path.AddLinearComponent({40, 40}, {50, 50});
 
   auto polyline = path.CreatePolyline();
 
-  ASSERT_EQ(polyline.breaks.size(), 2u);
+  ASSERT_EQ(polyline.contours.size(), 2u);
   ASSERT_EQ(polyline.points.size(), 5u);
   ASSERT_EQ(polyline.points[0].x, 10);
   ASSERT_EQ(polyline.points[1].x, 20);
@@ -655,5 +660,107 @@ TEST(GeometryTest, PathCreatePolyLineDoesNotDuplicatePoints) {
   ASSERT_EQ(polyline.points[4].x, 50);
 }
 
+TEST(GeometryTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) {
+  // Closed shapes.
+  {
+    Path path = PathBuilder{}.AddCircle({100, 100}, 50).TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(100, 50));
+    ASSERT_TRUE(contour.is_closed);
+  }
+
+  {
+    Path path = PathBuilder{}.AddOval(Rect(100, 100, 100, 100)).TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(150, 100));
+    ASSERT_TRUE(contour.is_closed);
+  }
+
+  {
+    Path path = PathBuilder{}.AddRect(Rect(100, 100, 100, 100)).TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(100, 100));
+    ASSERT_TRUE(contour.is_closed);
+  }
+
+  {
+    Path path =
+        PathBuilder{}.AddRoundedRect(Rect(100, 100, 100, 100), 10).TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(110, 100));
+    ASSERT_TRUE(contour.is_closed);
+  }
+
+  // Open shapes.
+  {
+    Point p(100, 100);
+    Path path = PathBuilder{}.AddLine(p, {200, 100}).TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, p);
+    ASSERT_FALSE(contour.is_closed);
+  }
+
+  {
+    Path path =
+        PathBuilder{}
+            .AddCubicCurve({100, 100}, {100, 50}, {100, 150}, {200, 100})
+            .TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(100, 100));
+    ASSERT_FALSE(contour.is_closed);
+  }
+
+  {
+    Path path = PathBuilder{}
+                    .AddQuadraticCurve({100, 100}, {100, 50}, {200, 100})
+                    .TakePath();
+    ContourComponent contour;
+    path.GetContourComponentAtIndex(0, contour);
+    ASSERT_POINT_NEAR(contour.destination, Point(100, 100));
+    ASSERT_FALSE(contour.is_closed);
+  }
+}
+
+TEST(GeometryTest, PathCreatePolylineGeneratesCorrectContourData) {
+  Path::Polyline polyline = PathBuilder{}
+                                .AddLine({100, 100}, {200, 100})
+                                .MoveTo({100, 200})
+                                .LineTo({150, 250})
+                                .LineTo({200, 200})
+                                .Close()
+                                .TakePath()
+                                .CreatePolyline();
+  ASSERT_EQ(polyline.points.size(), 6u);
+  ASSERT_EQ(polyline.contours.size(), 2u);
+  ASSERT_EQ(polyline.contours[0].is_closed, false);
+  ASSERT_EQ(polyline.contours[0].start_index, 0u);
+  ASSERT_EQ(polyline.contours[1].is_closed, true);
+  ASSERT_EQ(polyline.contours[1].start_index, 2u);
+}
+
+TEST(GeometryTest, PolylineGetContourPointBoundsReturnsCorrectRanges) {
+  Path::Polyline polyline = PathBuilder{}
+                                .AddLine({100, 100}, {200, 100})
+                                .MoveTo({100, 200})
+                                .LineTo({150, 250})
+                                .LineTo({200, 200})
+                                .Close()
+                                .TakePath()
+                                .CreatePolyline();
+  size_t a1, a2, b1, b2;
+  std::tie(a1, a2) = polyline.GetContourPointBounds(0);
+  std::tie(b1, b2) = polyline.GetContourPointBounds(1);
+  ASSERT_EQ(a1, 0u);
+  ASSERT_EQ(a2, 2u);
+  ASSERT_EQ(b1, 2u);
+  ASSERT_EQ(b2, 6u);
+}
+
 }  // namespace testing
 }  // namespace impeller