From 0b707db40bb6b545d2a4c57be46f060fcac3a904 Mon Sep 17 00:00:00 2001 From: Brandon DeRosier Date: Fri, 25 Feb 2022 13:24:14 -0800 Subject: [PATCH] Remove duplicate points between connected components when generating polylines (flutter/engine#34) --- .../impeller/geometry/geometry_unittests.cc | 19 +++++++++++++++++++ engine/src/flutter/impeller/geometry/path.cc | 16 +++++++++++++--- 2 files changed, 32 insertions(+), 3 deletions(-) diff --git a/engine/src/flutter/impeller/geometry/geometry_unittests.cc b/engine/src/flutter/impeller/geometry/geometry_unittests.cc index 4c0f2e33404..2d551d32ee9 100644 --- a/engine/src/flutter/impeller/geometry/geometry_unittests.cc +++ b/engine/src/flutter/impeller/geometry/geometry_unittests.cc @@ -636,5 +636,24 @@ TEST(GeometryTest, CubicPathComponentPolylineDoesNotIncludePointOne) { ASSERT_EQ(polyline.back().y, 40); } +TEST(GeometryTest, PathCreatePolyLineDoesNotDuplicatePoints) { + Path path; + path.AddMoveComponent({10, 10}); + path.AddLinearComponent({10, 10}, {20, 20}); + path.AddLinearComponent({20, 20}, {30, 30}); + path.AddMoveComponent({40, 40}); + path.AddLinearComponent({40, 40}, {50, 50}); + + auto polyline = path.CreatePolyline(); + + ASSERT_EQ(polyline.breaks.size(), 2u); + ASSERT_EQ(polyline.points.size(), 5u); + ASSERT_EQ(polyline.points[0].x, 10); + ASSERT_EQ(polyline.points[1].x, 20); + ASSERT_EQ(polyline.points[2].x, 30); + ASSERT_EQ(polyline.points[3].x, 40); + ASSERT_EQ(polyline.points[4].x, 50); +} + } // namespace testing } // namespace impeller diff --git a/engine/src/flutter/impeller/geometry/path.cc b/engine/src/flutter/impeller/geometry/path.cc index a5c0268bfc1..4bacb04ea9b 100644 --- a/engine/src/flutter/impeller/geometry/path.cc +++ b/engine/src/flutter/impeller/geometry/path.cc @@ -197,9 +197,18 @@ bool Path::UpdateMoveComponentAtIndex(size_t index, Path::Polyline Path::CreatePolyline( const SmoothingApproximation& approximation) const { Polyline polyline; - auto collect_points = [&polyline](const std::vector& collection) { - polyline.points.reserve(polyline.points.size() + collection.size()); - polyline.points.insert(polyline.points.end(), collection.begin(), + // TODO(99177): Refactor this to have component polyline creation always + // exclude the first point, and append the destination point for + // move components. See issue for details. + bool new_contour = true; + auto collect_points = [&polyline, + &new_contour](const std::vector& collection) { + size_t offset = new_contour ? 0 : 1; + new_contour = false; + + polyline.points.reserve(polyline.points.size() + collection.size() - + offset); + polyline.points.insert(polyline.points.end(), collection.begin() + offset, collection.end()); }; for (const auto& component : components_) { @@ -215,6 +224,7 @@ Path::Polyline Path::CreatePolyline( break; case ComponentType::kMove: polyline.breaks.insert(polyline.points.size()); + new_contour = true; break; } }