diff --git a/impeller/entity/entity_unittests.cc b/impeller/entity/entity_unittests.cc index ea564dfa8e029..20665eb56982c 100644 --- a/impeller/entity/entity_unittests.cc +++ b/impeller/entity/entity_unittests.cc @@ -2354,42 +2354,6 @@ TEST_P(EntityTest, TiledTextureContentsIsOpaque) { ASSERT_FALSE(contents.IsOpaque()); } -TEST_P(EntityTest, TessellateConvex) { - { - // Sanity check simple rectangle. - auto [pts, indices] = - TessellateConvex(PathBuilder{} - .AddRect(Rect::MakeLTRB(0, 0, 10, 10)) - .TakePath() - .CreatePolyline(1.0)); - - std::vector expected = { - {0, 0}, {10, 0}, {10, 10}, {0, 10}, // - }; - std::vector expected_indices = {0, 1, 2, 0, 2, 3}; - ASSERT_EQ(pts, expected); - ASSERT_EQ(indices, expected_indices); - } - - { - auto [pts, indices] = - TessellateConvex(PathBuilder{} - .AddRect(Rect::MakeLTRB(0, 0, 10, 10)) - .AddRect(Rect::MakeLTRB(20, 20, 30, 30)) - .TakePath() - .CreatePolyline(1.0)); - - std::vector expected = { - {0, 0}, {10, 0}, {10, 10}, {0, 10}, // - {20, 20}, {30, 20}, {30, 30}, {20, 30} // - }; - std::vector expected_indices = {0, 1, 2, 0, 2, 3, - 0, 6, 7, 0, 7, 8}; - ASSERT_EQ(pts, expected); - ASSERT_EQ(indices, expected_indices); - } -} - TEST_P(EntityTest, PointFieldGeometryDivisions) { // Square always gives 4 divisions. ASSERT_EQ(PointFieldGeometry::ComputeCircleDivisions(24.0, false), 4u); diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 4cd025aef0422..a734395f00736 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -22,8 +22,8 @@ GeometryResult FillPathGeometry::GetPositionBuffer( if (path_.GetFillType() == FillType::kNonZero && // path_.IsConvex()) { - auto [points, indices] = TessellateConvex( - path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength())); + auto [points, indices] = renderer.GetTessellator()->TessellateConvex( + path_, entity.GetTransformation().GetMaxBasisLength()); vertex_buffer.vertex_buffer = host_buffer.Emplace( points.data(), points.size() * sizeof(Point), alignof(Point)); @@ -42,8 +42,7 @@ GeometryResult FillPathGeometry::GetPositionBuffer( } auto tesselation_result = renderer.GetTessellator()->Tessellate( - path_.GetFillType(), - path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength()), + path_, entity.GetTransformation().GetMaxBasisLength(), [&vertex_buffer, &host_buffer]( const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { @@ -87,8 +86,8 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( if (path_.GetFillType() == FillType::kNonZero && // path_.IsConvex()) { - auto [points, indices] = TessellateConvex( - path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength())); + auto [points, indices] = renderer.GetTessellator()->TessellateConvex( + path_, entity.GetTransformation().GetMaxBasisLength()); VertexBufferBuilder vertex_builder; vertex_builder.Reserve(points.size()); @@ -115,8 +114,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( VertexBufferBuilder vertex_builder; auto tesselation_result = renderer.GetTessellator()->Tessellate( - path_.GetFillType(), - path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength()), + path_, entity.GetTransformation().GetMaxBasisLength(), [&vertex_builder, &uv_transform]( const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { diff --git a/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index 29d880a86c86b..80656d1dc2cb6 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/impeller/entity/geometry/geometry.cc @@ -15,37 +15,6 @@ namespace impeller { -/// Given a convex polyline, create a triangle fan structure. -std::pair, std::vector> TessellateConvex( - Path::Polyline polyline) { - std::vector output; - std::vector indices; - - for (auto j = 0u; j < polyline.contours.size(); j++) { - auto [start, end] = polyline.GetContourPointBounds(j); - auto center = polyline.points[start]; - - // Some polygons will not self close and an additional triangle - // must be inserted, others will self close and we need to avoid - // inserting an extra triangle. - if (polyline.points[end - 1] == polyline.points[start]) { - end--; - } - output.emplace_back(center); - output.emplace_back(polyline.points[start + 1]); - - for (auto i = start + 2; i < end; i++) { - const auto& point_b = polyline.points[i]; - output.emplace_back(point_b); - - indices.emplace_back(0); - indices.emplace_back(i - 1); - indices.emplace_back(i); - } - } - return std::make_pair(output, indices); -} - VertexBufferBuilder ComputeUVGeometryCPU( VertexBufferBuilder& input, diff --git a/impeller/entity/geometry/geometry.h b/impeller/entity/geometry/geometry.h index c0743626953ae..f336d4717ef55 100644 --- a/impeller/entity/geometry/geometry.h +++ b/impeller/entity/geometry/geometry.h @@ -46,11 +46,6 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect, const Entity& entity, RenderPass& pass); -/// @brief Given a polyline created from a convex filled path, perform a -/// tessellation. -std::pair, std::vector> TessellateConvex( - Path::Polyline polyline); - class Geometry { public: Geometry(); diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index 9f857d5808af2..48eebb6a9286a 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -128,9 +128,9 @@ StrokePathGeometry::JoinProc StrokePathGeometry::GetJoinProc(Join stroke_join) { PathBuilder::kArcApproximationMagic * alignment * dir; - auto arc_points = CubicPathComponent(start_offset, start_handle, - middle_handle, middle) - .CreatePolyline(scale); + std::vector arc_points; + CubicPathComponent(start_offset, start_handle, middle_handle, middle) + .AppendPolylinePoints(scale, arc_points); VS::PerVertexData vtx; for (const auto& point : arc_points) { @@ -192,7 +192,9 @@ StrokePathGeometry::CapProc StrokePathGeometry::GetCapProc(Cap stroke_cap) { vtx_builder.AppendVertex(vtx); vtx.position = position - orientation; vtx_builder.AppendVertex(vtx); - for (const auto& point : arc.CreatePolyline(scale)) { + std::vector arc_points; + arc.AppendPolylinePoints(scale, arc_points); + for (const auto& point : arc_points) { vtx.position = position + point; vtx_builder.AppendVertex(vtx); vtx.position = position + (-point).Reflect(forward_normal); @@ -234,7 +236,12 @@ StrokePathGeometry::CreateSolidStrokeVertices( const StrokePathGeometry::CapProc& cap_proc, Scalar scale) { VertexBufferBuilder vtx_builder; - auto polyline = path.CreatePolyline(scale); + auto point_buffer = std::make_unique>(); + // 512 is an arbitrary choice that should be big enough for most paths without + // needing to reallocate. If we have motivating benchmarks we should raise or + // lower this number, cause dnfield just made it up! + point_buffer->reserve(512); + auto polyline = path.CreatePolyline(scale, std::move(point_buffer)); VS::PerVertexData vtx; @@ -256,8 +263,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( } else if (point_i <= contour_start_point_i) { direction = -contour.start_direction; } else { - direction = - (polyline.points[point_i] - polyline.points[point_i - 1]).Normalize(); + direction = (polyline.GetPoint(point_i) - polyline.GetPoint(point_i - 1)) + .Normalize(); } previous_offset = offset; offset = Vector2{-direction.y, direction.x} * stroke_width * 0.5; @@ -276,23 +283,23 @@ StrokePathGeometry::CreateSolidStrokeVertices( for (size_t point_i = component_start_index; point_i < component_end_index; point_i++) { auto is_end_of_component = point_i == component_end_index - 1; - vtx.position = polyline.points[point_i] + offset; + vtx.position = polyline.GetPoint(point_i) + offset; vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i] - offset; + vtx.position = polyline.GetPoint(point_i) - offset; vtx_builder.AppendVertex(vtx); // For line components, two additional points need to be appended // prior to appending a join connecting the next component. - vtx.position = polyline.points[point_i + 1] + offset; + vtx.position = polyline.GetPoint(point_i + 1) + offset; vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i + 1] - offset; + vtx.position = polyline.GetPoint(point_i + 1) - offset; vtx_builder.AppendVertex(vtx); compute_offset(point_i + 2, contour_start_point_i, contour_end_point_i, contour); if (!is_last_component && is_end_of_component) { // Generate join from the current line to the next line. - join_proc(vtx_builder, polyline.points[point_i + 1], + join_proc(vtx_builder, polyline.GetPoint(point_i + 1), previous_offset, offset, scaled_miter_limit, scale); } } @@ -312,9 +319,9 @@ StrokePathGeometry::CreateSolidStrokeVertices( point_i < component_end_index; point_i++) { auto is_end_of_component = point_i == component_end_index - 1; - vtx.position = polyline.points[point_i] + offset; + vtx.position = polyline.GetPoint(point_i) + offset; vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i] - offset; + vtx.position = polyline.GetPoint(point_i) - offset; vtx_builder.AppendVertex(vtx); compute_offset(point_i + 2, contour_start_point_i, @@ -322,13 +329,13 @@ StrokePathGeometry::CreateSolidStrokeVertices( // For curve components, the polyline is detailed enough such that // it can avoid worrying about joins altogether. if (is_end_of_component) { - vtx.position = polyline.points[point_i + 1] + offset; + vtx.position = polyline.GetPoint(point_i + 1) + offset; vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[point_i + 1] - offset; + vtx.position = polyline.GetPoint(point_i + 1) - offset; vtx_builder.AppendVertex(vtx); // Generate join from the current line to the next line. if (!is_last_component) { - join_proc(vtx_builder, polyline.points[point_i + 1], + join_proc(vtx_builder, polyline.GetPoint(point_i + 1), previous_offset, offset, scaled_miter_limit, scale); } } @@ -344,7 +351,7 @@ StrokePathGeometry::CreateSolidStrokeVertices( switch (contour_end_point_i - contour_start_point_i) { case 1: { - Point p = polyline.points[contour_start_point_i]; + Point p = polyline.GetPoint(contour_start_point_i); cap_proc(vtx_builder, p, {-stroke_width * 0.5f, 0}, scale, false); cap_proc(vtx_builder, p, {stroke_width * 0.5f, 0}, scale, false); continue; @@ -367,14 +374,14 @@ StrokePathGeometry::CreateSolidStrokeVertices( // vertices at the start of the new contour (thus connecting the two // contours with two zero volume triangles, which will be discarded by // the rasterizer). - vtx.position = polyline.points[contour_start_point_i - 1]; + vtx.position = polyline.GetPoint(contour_start_point_i - 1); // Append two vertices when "picking up" the pen so that the triangle // drawn when moving to the beginning of the new contour will have zero // volume. vtx_builder.AppendVertex(vtx); vtx_builder.AppendVertex(vtx); - vtx.position = polyline.points[contour_start_point_i]; + vtx.position = polyline.GetPoint(contour_start_point_i); // Append two vertices at the beginning of the new contour, which // appends two triangles of zero area. vtx_builder.AppendVertex(vtx); @@ -386,8 +393,8 @@ StrokePathGeometry::CreateSolidStrokeVertices( auto cap_offset = Vector2(-contour.start_direction.y, contour.start_direction.x) * stroke_width * 0.5; // Counterclockwise normal - cap_proc(vtx_builder, polyline.points[contour_start_point_i], cap_offset, - scale, true); + cap_proc(vtx_builder, polyline.GetPoint(contour_start_point_i), + cap_offset, scale, true); } for (size_t contour_component_i = 0; @@ -418,10 +425,10 @@ StrokePathGeometry::CreateSolidStrokeVertices( auto cap_offset = Vector2(-contour.end_direction.y, contour.end_direction.x) * stroke_width * 0.5; // Clockwise normal - cap_proc(vtx_builder, polyline.points[contour_end_point_i - 1], + cap_proc(vtx_builder, polyline.GetPoint(contour_end_point_i - 1), cap_offset, scale, false); } else { - join_proc(vtx_builder, polyline.points[contour_start_point_i], offset, + join_proc(vtx_builder, polyline.GetPoint(contour_start_point_i), offset, contour_first_offset, scaled_miter_limit, scale); } } diff --git a/impeller/geometry/geometry_benchmarks.cc b/impeller/geometry/geometry_benchmarks.cc index 8bff21f3282bf..ca97d7b18177f 100644 --- a/impeller/geometry/geometry_benchmarks.cc +++ b/impeller/geometry/geometry_benchmarks.cc @@ -28,15 +28,34 @@ static void BM_Polyline(benchmark::State& state, Args&&... args) { size_t point_count = 0u; size_t single_point_count = 0u; + auto points = std::make_unique>(); + points->reserve(2048); while (state.KeepRunning()) { - auto polyline = path.CreatePolyline(1.0f); - single_point_count = polyline.points.size(); - point_count += single_point_count; if (tessellate) { - tess.Tessellate( - FillType::kNonZero, polyline, - [](const float* vertices, size_t vertices_count, - const uint16_t* indices, size_t indices_count) { return true; }); + tess.Tessellate(path, 1.0f, + [&point_count, &single_point_count]( + const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { + if (indices_count > 0) { + single_point_count = indices_count; + point_count += indices_count; + } else { + single_point_count = vertices_count; + point_count += vertices_count; + } + return true; + }); + } else { + auto polyline = path.CreatePolyline( + // Clang-tidy doesn't know that the points get moved back before + // getting moved again in this loop. + // NOLINTNEXTLINE(clang-analyzer-cplusplus.Move) + 1.0f, std::move(points), + [&points](Path::Polyline::PointBufferPtr reclaimed) { + points = std::move(reclaimed); + }); + single_point_count = polyline.points->size(); + point_count += single_point_count; } } state.counters["SinglePointCount"] = single_point_count; diff --git a/impeller/geometry/geometry_unittests.cc b/impeller/geometry/geometry_unittests.cc index d5ee6d423002d..4c5ea75435e95 100644 --- a/impeller/geometry/geometry_unittests.cc +++ b/impeller/geometry/geometry_unittests.cc @@ -600,7 +600,7 @@ TEST(GeometryTest, EmptyPath) { ASSERT_POINT_NEAR(c.destination, Point()); Path::Polyline polyline = path.CreatePolyline(1.0f); - ASSERT_TRUE(polyline.points.empty()); + ASSERT_TRUE(polyline.points->empty()); ASSERT_TRUE(polyline.contours.empty()); } @@ -2099,7 +2099,8 @@ TEST(GeometryTest, RectRoundOut) { TEST(GeometryTest, CubicPathComponentPolylineDoesNotIncludePointOne) { CubicPathComponent component({10, 10}, {20, 35}, {35, 20}, {40, 40}); - auto polyline = component.CreatePolyline(1.0f); + std::vector polyline; + component.AppendPolylinePoints(1.0f, polyline); ASSERT_NE(polyline.front().x, 10); ASSERT_NE(polyline.front().y, 10); ASSERT_EQ(polyline.back().x, 40); @@ -2117,12 +2118,12 @@ TEST(GeometryTest, PathCreatePolyLineDoesNotDuplicatePoints) { auto polyline = builder.TakePath().CreatePolyline(1.0f); 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); - ASSERT_EQ(polyline.points[2].x, 30); - ASSERT_EQ(polyline.points[3].x, 40); - ASSERT_EQ(polyline.points[4].x, 50); + ASSERT_EQ(polyline.points->size(), 5u); + ASSERT_EQ(polyline.GetPoint(0).x, 10); + ASSERT_EQ(polyline.GetPoint(1).x, 20); + ASSERT_EQ(polyline.GetPoint(2).x, 30); + ASSERT_EQ(polyline.GetPoint(3).x, 40); + ASSERT_EQ(polyline.GetPoint(4).x, 50); } TEST(GeometryTest, PathBuilderSetsCorrectContourPropertiesForAddCommands) { @@ -2204,7 +2205,7 @@ TEST(GeometryTest, PathCreatePolylineGeneratesCorrectContourData) { .Close() .TakePath() .CreatePolyline(1.0f); - ASSERT_EQ(polyline.points.size(), 6u); + 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); @@ -2238,12 +2239,12 @@ TEST(GeometryTest, PathAddRectPolylineHasCorrectContourData) { ASSERT_EQ(polyline.contours.size(), 1u); ASSERT_TRUE(polyline.contours[0].is_closed); ASSERT_EQ(polyline.contours[0].start_index, 0u); - ASSERT_EQ(polyline.points.size(), 5u); - ASSERT_EQ(polyline.points[0], Point(50, 60)); - ASSERT_EQ(polyline.points[1], Point(70, 60)); - ASSERT_EQ(polyline.points[2], Point(70, 80)); - ASSERT_EQ(polyline.points[3], Point(50, 80)); - ASSERT_EQ(polyline.points[4], Point(50, 60)); + ASSERT_EQ(polyline.points->size(), 5u); + ASSERT_EQ(polyline.GetPoint(0), Point(50, 60)); + ASSERT_EQ(polyline.GetPoint(1), Point(70, 60)); + ASSERT_EQ(polyline.GetPoint(2), Point(70, 80)); + ASSERT_EQ(polyline.GetPoint(3), Point(50, 80)); + ASSERT_EQ(polyline.GetPoint(4), Point(50, 60)); } TEST(GeometryTest, PathPolylineDuplicatesAreRemovedForSameContour) { @@ -2265,14 +2266,41 @@ TEST(GeometryTest, PathPolylineDuplicatesAreRemovedForSameContour) { ASSERT_TRUE(polyline.contours[0].is_closed); ASSERT_EQ(polyline.contours[1].start_index, 4u); ASSERT_FALSE(polyline.contours[1].is_closed); - ASSERT_EQ(polyline.points.size(), 7u); - ASSERT_EQ(polyline.points[0], Point(50, 50)); - ASSERT_EQ(polyline.points[1], Point(100, 50)); - ASSERT_EQ(polyline.points[2], Point(100, 100)); - ASSERT_EQ(polyline.points[3], Point(50, 50)); - ASSERT_EQ(polyline.points[4], Point(50, 50)); - ASSERT_EQ(polyline.points[5], Point(0, 50)); - ASSERT_EQ(polyline.points[6], Point(0, 100)); + ASSERT_EQ(polyline.points->size(), 7u); + ASSERT_EQ(polyline.GetPoint(0), Point(50, 50)); + ASSERT_EQ(polyline.GetPoint(1), Point(100, 50)); + ASSERT_EQ(polyline.GetPoint(2), Point(100, 100)); + ASSERT_EQ(polyline.GetPoint(3), Point(50, 50)); + ASSERT_EQ(polyline.GetPoint(4), Point(50, 50)); + ASSERT_EQ(polyline.GetPoint(5), Point(0, 50)); + ASSERT_EQ(polyline.GetPoint(6), Point(0, 100)); +} + +TEST(GeometryTest, PolylineBufferReuse) { + auto point_buffer = std::make_unique>(); + auto point_buffer_address = reinterpret_cast(point_buffer.get()); + Path::Polyline polyline = + PathBuilder{} + .MoveTo({50, 50}) + .LineTo({100, 100}) + .TakePath() + .CreatePolyline( + 1.0f, std::move(point_buffer), + [point_buffer_address]( + Path::Polyline::PointBufferPtr point_buffer) { + ASSERT_EQ(point_buffer->size(), 0u); + ASSERT_EQ(point_buffer_address, + reinterpret_cast(point_buffer.get())); + }); +} + +TEST(GeometryTest, PolylineFailsWithNullptrBuffer) { + EXPECT_DEATH_IF_SUPPORTED(PathBuilder{} + .MoveTo({50, 50}) + .LineTo({100, 100}) + .TakePath() + .CreatePolyline(1.0f, nullptr), + ""); } TEST(GeometryTest, MatrixPrinting) { diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 5bd197d039a04..5f99a5cfeb6f9 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -7,7 +7,9 @@ #include #include +#include "flutter/fml/logging.h" #include "impeller/geometry/path_component.h" +#include "impeller/geometry/point.h" namespace impeller { @@ -20,11 +22,11 @@ Path::~Path() = default; std::tuple Path::Polyline::GetContourPointBounds( size_t contour_index) const { if (contour_index >= contours.size()) { - return {points.size(), points.size()}; + return {points->size(), points->size()}; } const size_t start_index = contours.at(contour_index).start_index; const size_t end_index = (contour_index >= contours.size() - 1) - ? points.size() + ? points->size() : contours.at(contour_index + 1).start_index; return std::make_tuple(start_index, end_index); } @@ -212,26 +214,30 @@ bool Path::GetContourComponentAtIndex(size_t index, return true; } -Path::Polyline Path::CreatePolyline(Scalar scale) const { - Polyline polyline; +Path::Polyline::Polyline(Path::Polyline::PointBufferPtr point_buffer, + Path::Polyline::ReclaimPointBufferCallback reclaim) + : points(std::move(point_buffer)), reclaim_points(std::move(reclaim)) { + FML_DCHECK(points); +} - std::optional previous_contour_point; - auto collect_points = [&polyline, &previous_contour_point]( - const std::vector& collection) { - if (collection.empty()) { - return; - } +Path::Polyline::Polyline(Path::Polyline&& other) { + points = std::move(other.points); + reclaim_points = std::move(other.reclaim_points); + contours = std::move(other.contours); +} - for (const auto& point : collection) { - if (previous_contour_point.has_value() && - previous_contour_point.value() == point) { - // Skip over duplicate points in the same contour. - continue; - } - previous_contour_point = point; - polyline.points.push_back(point); - } - }; +Path::Polyline::~Polyline() { + if (reclaim_points) { + points->clear(); + reclaim_points(std::move(points)); + } +} + +Path::Polyline Path::CreatePolyline( + Scalar scale, + Path::Polyline::PointBufferPtr point_buffer, + Path::Polyline::ReclaimPointBufferCallback reclaim) const { + Polyline polyline(std::move(point_buffer), std::move(reclaim)); auto get_path_component = [this](size_t component_i) -> PathComponentVariant { if (component_i >= components_.size()) { @@ -271,8 +277,8 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { std::optional previous_path_component_index; auto end_contour = [&polyline, &previous_path_component_index, &get_path_component, &components]() { - // Whenever a contour has ended, extract the exact end direction from the - // last component. + // Whenever a contour has ended, extract the exact end direction from + // the last component. if (polyline.contours.empty()) { return; } @@ -310,26 +316,26 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { switch (component.type) { case ComponentType::kLinear: components.push_back({ - .component_start_index = polyline.points.size() - 1, + .component_start_index = polyline.points->size() - 1, .is_curve = false, }); - collect_points(linears_[component.index].CreatePolyline()); + linears_[component.index].AppendPolylinePoints(*polyline.points); previous_path_component_index = component_i; break; case ComponentType::kQuadratic: components.push_back({ - .component_start_index = polyline.points.size() - 1, + .component_start_index = polyline.points->size() - 1, .is_curve = true, }); - collect_points(quads_[component.index].CreatePolyline(scale)); + quads_[component.index].AppendPolylinePoints(scale, *polyline.points); previous_path_component_index = component_i; break; case ComponentType::kCubic: components.push_back({ - .component_start_index = polyline.points.size() - 1, + .component_start_index = polyline.points->size() - 1, .is_curve = true, }); - collect_points(cubics_[component.index].CreatePolyline(scale)); + cubics_[component.index].AppendPolylinePoints(scale, *polyline.points); previous_path_component_index = component_i; break; case ComponentType::kContour: @@ -342,12 +348,12 @@ Path::Polyline Path::CreatePolyline(Scalar scale) const { Vector2 start_direction = compute_contour_start_direction(component_i); const auto& contour = contours_[component.index]; - polyline.contours.push_back({.start_index = polyline.points.size(), + polyline.contours.push_back({.start_index = polyline.points->size(), .is_closed = contour.is_closed, .start_direction = start_direction, .components = components}); - previous_contour_point = std::nullopt; - collect_points({contour.destination}); + + polyline.points->push_back(contour.destination); break; } } diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index 62c2d21cc9db6..0733ad833cac9 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -90,10 +90,31 @@ class Path { /// One or more contours represented as a series of points and indices in /// the point vector representing the start of a new contour. + /// + /// Polylines are ephemeral and meant to be used by the tessellator. They do + /// not allocate their own point vectors to allow for optimizations around + /// allocation and reuse of arenas. struct Polyline { + /// The signature of a method called when it is safe to reclaim the point + /// buffer provided to the constructor of this object. + using PointBufferPtr = std::unique_ptr>; + using ReclaimPointBufferCallback = std::function; + + /// The buffer will be cleared and returned at the destruction of this + /// polyline. + Polyline(PointBufferPtr point_buffer, ReclaimPointBufferCallback reclaim); + + Polyline(Polyline&& other); + ~Polyline(); + /// Points in the polyline, which may represent multiple contours specified - /// by indices in |breaks|. - std::vector points; + /// by indices in |contours|. + PointBufferPtr points; + + Point& GetPoint(size_t index) const { return (*points)[index]; } + + /// Contours are disconnected pieces of a polyline, such as when a MoveTo + /// was issued on a PathBuilder. std::vector contours; /// Convenience method to compute the start (inclusive) and end (exclusive) @@ -102,6 +123,9 @@ class Path { /// The contour_index parameter is clamped to contours.size(). std::tuple GetContourPointBounds( size_t contour_index) const; + + private: + ReclaimPointBufferCallback reclaim_points; }; Path(); @@ -137,8 +161,13 @@ class Path { /// transformed. /// /// It is suitable to use the max basis length of the matrix used to transform - /// the path. If the provided scale is 0, curves will revert to lines. - Polyline CreatePolyline(Scalar scale) const; + /// the path. If the provided scale is 0, curves will revert to straight + /// lines. + Polyline CreatePolyline( + Scalar scale, + Polyline::PointBufferPtr point_buffer = + std::make_unique>(), + Polyline::ReclaimPointBufferCallback reclaim = nullptr) const; std::optional GetBoundingBox() const; diff --git a/impeller/geometry/path_component.cc b/impeller/geometry/path_component.cc index 14848ccbea9d4..96cc8bc78ce40 100644 --- a/impeller/geometry/path_component.cc +++ b/impeller/geometry/path_component.cc @@ -59,8 +59,11 @@ Point LinearPathComponent::Solve(Scalar time) const { }; } -std::vector LinearPathComponent::CreatePolyline() const { - return {p2}; +void LinearPathComponent::AppendPolylinePoints( + std::vector& points) const { + if (points.size() == 0 || points.back() != p2) { + points.push_back(p2); + } } std::vector LinearPathComponent::Extrema() const { @@ -100,14 +103,9 @@ static Scalar ApproximateParabolaIntegral(Scalar x) { return x / (1.0 - d + sqrt(sqrt(pow(d, 4) + 0.25 * x * x))); } -std::vector QuadraticPathComponent::CreatePolyline(Scalar scale) const { - std::vector points; - FillPointsForPolyline(points, scale); - return points; -} - -void QuadraticPathComponent::FillPointsForPolyline(std::vector& points, - Scalar scale_factor) const { +void QuadraticPathComponent::AppendPolylinePoints( + Scalar scale_factor, + std::vector& points) const { auto tolerance = kDefaultCurveTolerance / scale_factor; auto sqrt_tolerance = sqrt(tolerance); @@ -187,13 +185,13 @@ Point CubicPathComponent::SolveDerivative(Scalar time) const { }; } -std::vector CubicPathComponent::CreatePolyline(Scalar scale) const { +void CubicPathComponent::AppendPolylinePoints( + Scalar scale, + std::vector& points) const { auto quads = ToQuadraticPathComponents(.1); - std::vector points; for (const auto& quad : quads) { - quad.FillPointsForPolyline(points, scale); + quad.AppendPolylinePoints(scale, points); } - return points; } inline QuadraticPathComponent CubicPathComponent::Lower() const { @@ -231,7 +229,7 @@ CubicPathComponent::ToQuadraticPathComponents(Scalar accuracy) const { auto p = p2x2 - p1x2; auto err = p.Dot(p); auto quad_count = std::max(1., ceil(pow(err / max_hypot2, 1. / 6.0))); - + quads.reserve(quad_count); for (size_t i = 0; i < quad_count; i++) { auto t0 = i / quad_count; auto t1 = (i + 1) / quad_count; diff --git a/impeller/geometry/path_component.h b/impeller/geometry/path_component.h index a8b0d4fa9bd68..c721afd8c0826 100644 --- a/impeller/geometry/path_component.h +++ b/impeller/geometry/path_component.h @@ -14,9 +14,9 @@ namespace impeller { -// The default tolerance value for QuadraticCurveComponent::CreatePolyline and -// CubicCurveComponent::CreatePolyline. It also impacts the number of quadratics -// created when flattening a cubic curve to a polyline. +// The default tolerance value for QuadraticCurveComponent::AppendPolylinePoints +// and CubicCurveComponent::AppendPolylinePoints. It also impacts the number of +// quadratics created when flattening a cubic curve to a polyline. // // Smaller numbers mean more points. This number seems suitable for particularly // curvy curves at scales close to 1.0. As the scale increases, this number @@ -34,7 +34,7 @@ struct LinearPathComponent { Point Solve(Scalar time) const; - std::vector CreatePolyline() const; + void AppendPolylinePoints(std::vector& points) const; std::vector Extrema() const; @@ -75,10 +75,8 @@ struct QuadraticPathComponent { // making it trivially parallelizable. // // See also the implementation in kurbo: https://github.com/linebender/kurbo. - std::vector CreatePolyline(Scalar scale) const; - - void FillPointsForPolyline(std::vector& points, - Scalar scale_factor) const; + void AppendPolylinePoints(Scalar scale_factor, + std::vector& points) const; std::vector Extrema() const; @@ -120,8 +118,9 @@ struct CubicPathComponent { // This method approximates the cubic component with quadratics, and then // generates a polyline from those quadratics. // - // See the note on QuadraticPathComponent::CreatePolyline for references. - std::vector CreatePolyline(Scalar scale) const; + // See the note on QuadraticPathComponent::AppendPolylinePoints for + // references. + void AppendPolylinePoints(Scalar scale, std::vector& points) const; std::vector Extrema() const; diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 2ecc6fab2a004..749d611bb322a 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -399,11 +399,10 @@ TEST_P(RendererTest, CanRenderInstanced) { ASSERT_EQ(Tessellator::Result::kSuccess, Tessellator{}.Tessellate( - FillType::kPositive, PathBuilder{} .AddRect(Rect::MakeXYWH(10, 10, 100, 100)) - .TakePath() - .CreatePolyline(1.0f), + .TakePath(FillType::kPositive), + 1.0f, [&builder](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { for (auto i = 0u; i < vertices_count * 2; i += 2) { diff --git a/impeller/tessellator/c/tessellator.cc b/impeller/tessellator/c/tessellator.cc index b576d4393e809..f5300dcba1dee 100644 --- a/impeller/tessellator/c/tessellator.cc +++ b/impeller/tessellator/c/tessellator.cc @@ -41,10 +41,9 @@ struct Vertices* Tessellate(PathBuilder* builder, int fill_type, Scalar tolerance) { auto path = builder->CopyPath(static_cast(fill_type)); - auto polyline = path.CreatePolyline(tolerance); std::vector points; if (Tessellator{}.Tessellate( - path.GetFillType(), polyline, + path, tolerance, [&points](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { // Results are expected to be re-duplicated. diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index f7c80fe747ea0..15069a6d2cbf2 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -31,7 +31,10 @@ static const TESSalloc kAlloc = { 0 /* =extraVertices */ }; -Tessellator::Tessellator() : c_tessellator_(nullptr, &DestroyTessellator) { +Tessellator::Tessellator() + : point_buffer_(std::make_unique>()), + c_tessellator_(nullptr, &DestroyTessellator) { + point_buffer_->reserve(2048); TESSalloc alloc = kAlloc; { // libTess2 copies the TESSalloc despite the non-const argument. @@ -58,15 +61,23 @@ static int ToTessWindingRule(FillType fill_type) { return TESS_WINDING_ODD; } -Tessellator::Result Tessellator::Tessellate( - FillType fill_type, - const Path::Polyline& polyline, - const BuilderCallback& callback) const { +Tessellator::Result Tessellator::Tessellate(const Path& path, + Scalar tolerance, + const BuilderCallback& callback) { if (!callback) { return Result::kInputError; } - if (polyline.points.empty()) { + point_buffer_->clear(); + auto polyline = + path.CreatePolyline(tolerance, std::move(point_buffer_), + [this](Path::Polyline::PointBufferPtr point_buffer) { + point_buffer_ = std::move(point_buffer); + }); + + auto fill_type = path.GetFillType(); + + if (polyline.points->empty()) { return Result::kInputError; } @@ -99,9 +110,9 @@ Tessellator::Result Tessellator::Tessellate( ::tessAddContour(tessellator, // the C tessellator kVertexSize, // - polyline.points.data() + start_point_index, // - sizeof(Point), // - end_point_index - start_point_index // + polyline.points->data() + start_point_index, // + sizeof(Point), // + end_point_index - start_point_index // ); //---------------------------------------------------------------------------- @@ -150,9 +161,9 @@ Tessellator::Result Tessellator::Tessellate( ::tessAddContour(tessellator, // the C tessellator kVertexSize, // - polyline.points.data() + start_point_index, // - sizeof(Point), // - end_point_index - start_point_index // + polyline.points->data() + start_point_index, // + sizeof(Point), // + end_point_index - start_point_index // ); } @@ -199,12 +210,14 @@ Tessellator::Result Tessellator::Tessellate( int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; auto vertices = tessGetVertices(tessellator); + points.reserve(vertex_item_count); for (int i = 0; i < vertex_item_count; i += 2) { points.emplace_back(vertices[i], vertices[i + 1]); } int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; auto elements = tessGetElements(tessellator); + data.reserve(element_item_count); for (int i = 0; i < element_item_count; i++) { data.emplace_back(points[elements[i]].x); data.emplace_back(points[elements[i]].y); @@ -218,6 +231,43 @@ Tessellator::Result Tessellator::Tessellate( return Result::kSuccess; } +std::pair, std::vector> +Tessellator::TessellateConvex(const Path& path, Scalar tolerance) { + std::vector output; + std::vector indices; + + point_buffer_->clear(); + auto polyline = + path.CreatePolyline(tolerance, std::move(point_buffer_), + [this](Path::Polyline::PointBufferPtr point_buffer) { + point_buffer_ = std::move(point_buffer); + }); + + for (auto j = 0u; j < polyline.contours.size(); j++) { + auto [start, end] = polyline.GetContourPointBounds(j); + auto center = polyline.GetPoint(start); + + // Some polygons will not self close and an additional triangle + // must be inserted, others will self close and we need to avoid + // inserting an extra triangle. + if (polyline.GetPoint(end - 1) == polyline.GetPoint(start)) { + end--; + } + output.emplace_back(center); + output.emplace_back(polyline.GetPoint(start + 1)); + + for (auto i = start + 2; i < end; i++) { + const auto& point_b = polyline.GetPoint(i); + output.emplace_back(point_b); + + indices.emplace_back(0); + indices.emplace_back(i - 1); + indices.emplace_back(i); + } + } + return std::make_pair(output, indices); +} + void DestroyTessellator(TESStesselator* tessellator) { if (tessellator != nullptr) { ::tessDeleteTess(tessellator); diff --git a/impeller/tessellator/tessellator.h b/impeller/tessellator/tessellator.h index fd86d29edfa95..7733c25a5aec0 100644 --- a/impeller/tessellator/tessellator.h +++ b/impeller/tessellator/tessellator.h @@ -30,7 +30,8 @@ enum class WindingOrder { /// @brief A utility that generates triangles of the specified fill type /// given a polyline. This happens on the CPU. /// -/// @bug This should just be called a triangulator. +/// This object is not thread safe, and its methods must not be +/// called from multiple threads. /// class Tessellator { public: @@ -58,20 +59,41 @@ class Tessellator { size_t indices_count)>; //---------------------------------------------------------------------------- - /// @brief Generates filled triangles from the polyline. A callback is + /// @brief Generates filled triangles from the path. A callback is /// invoked once for the entire tessellation. /// - /// @param[in] fill_type The fill rule to use when filling. - /// @param[in] polyline The polyline + /// @param[in] path The path to tessellate. + /// @param[in] tolerance The tolerance value for conversion of the path to + /// a polyline. This value is often derived from the + /// Matrix::GetMaxBasisLength of the CTM applied to the + /// path for rendering. /// @param[in] callback The callback, return false to indicate failure. /// /// @return The result status of the tessellation. /// - Tessellator::Result Tessellate(FillType fill_type, - const Path::Polyline& polyline, - const BuilderCallback& callback) const; + Tessellator::Result Tessellate(const Path& path, + Scalar tolerance, + const BuilderCallback& callback); + + //---------------------------------------------------------------------------- + /// @brief Given a convex path, create a triangle fan structure. + /// + /// @param[in] path The path to tessellate. + /// @param[in] tolerance The tolerance value for conversion of the path to + /// a polyline. This value is often derived from the + /// Matrix::GetMaxBasisLength of the CTM applied to the + /// path for rendering. + /// + /// @return A point vector containing the vertices and a vector of indices + /// into the point vector. + /// + std::pair, std::vector> TessellateConvex( + const Path& path, + Scalar tolerance); private: + /// Used for polyline generation. + std::unique_ptr> point_buffer_; CTessellator c_tessellator_; Tessellator(const Tessellator&) = delete; diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 8fca675a9a9a6..3ef6bd163691a 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -15,42 +15,37 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { // Zero points. { Tessellator t; - auto polyline = PathBuilder{}.TakePath().CreatePolyline(1.0f); + auto path = PathBuilder{}.TakePath(FillType::kPositive); Tessellator::Result result = t.Tessellate( - FillType::kPositive, polyline, + path, 1.0f, [](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { return true; }); - ASSERT_EQ(polyline.points.size(), 0u); ASSERT_EQ(result, Tessellator::Result::kInputError); } // One point. { Tessellator t; - auto polyline = - PathBuilder{}.LineTo({0, 0}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = - t.Tessellate(FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_count, - const uint16_t* indices_count, - size_t indices_size) { return true; }); - ASSERT_EQ(polyline.points.size(), 1u); + auto path = PathBuilder{}.LineTo({0, 0}).TakePath(FillType::kPositive); + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return true; }); + ASSERT_EQ(result, Tessellator::Result::kSuccess); } // Two points. { Tessellator t; - auto polyline = - PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = - t.Tessellate(FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_count, - const uint16_t* indices_count, - size_t indices_size) { return true; }); - - ASSERT_EQ(polyline.points.size(), 2u); + auto path = + PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath(FillType::kPositive); + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return true; }); + ASSERT_EQ(result, Tessellator::Result::kSuccess); } @@ -62,29 +57,25 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { auto coord = i * 1.0f; builder.AddLine({coord, coord}, {coord + 1, coord + 1}); } - auto polyline = builder.TakePath().CreatePolyline(1.0f); - Tessellator::Result result = - t.Tessellate(FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_count, - const uint16_t* indices_count, - size_t indices_size) { return true; }); - - ASSERT_EQ(polyline.points.size(), 2000u); + auto path = builder.TakePath(FillType::kPositive); + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return true; }); + ASSERT_EQ(result, Tessellator::Result::kSuccess); } // Closure fails. { Tessellator t; - auto polyline = - PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = - t.Tessellate(FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_count, - const uint16_t* indices_count, - size_t indices_size) { return false; }); - - ASSERT_EQ(polyline.points.size(), 2u); + auto path = + PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath(FillType::kPositive); + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return false; }); + ASSERT_EQ(result, Tessellator::Result::kInputError); } @@ -95,10 +86,10 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { for (auto i = 0u; i < Tessellator::kMultiContourThreshold + 1; i++) { builder.AddCircle(Point(i, i), 4); } - auto polyline = builder.TakePath().CreatePolyline(1.0f); + auto path = builder.TakePath(FillType::kNonZero); bool no_indices = false; Tessellator::Result result = t.Tessellate( - FillType::kNonZero, polyline, + path, 1.0f, [&no_indices](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { no_indices = indices == nullptr; @@ -116,11 +107,11 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { for (auto i = 0; i < 1000; i++) { builder.AddCircle(Point(i, i), 4); } - auto polyline = builder.TakePath(FillType::kOdd).CreatePolyline(1.0f); + auto path = builder.TakePath(FillType::kOdd); bool no_indices = false; size_t count = 0u; Tessellator::Result result = t.Tessellate( - FillType::kOdd, polyline, + path, 1.0f, [&no_indices, &count](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { no_indices = indices == nullptr; @@ -134,5 +125,40 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { } } +TEST(TessellatorTest, TessellateConvex) { + { + Tessellator t; + // Sanity check simple rectangle. + auto [pts, indices] = t.TessellateConvex( + PathBuilder{}.AddRect(Rect::MakeLTRB(0, 0, 10, 10)).TakePath(), 1.0); + + std::vector expected = { + {0, 0}, {10, 0}, {10, 10}, {0, 10}, // + }; + std::vector expected_indices = {0, 1, 2, 0, 2, 3}; + ASSERT_EQ(pts, expected); + ASSERT_EQ(indices, expected_indices); + } + + { + Tessellator t; + auto [pts, indices] = + t.TessellateConvex(PathBuilder{} + .AddRect(Rect::MakeLTRB(0, 0, 10, 10)) + .AddRect(Rect::MakeLTRB(20, 20, 30, 30)) + .TakePath(), + 1.0); + + std::vector expected = { + {0, 0}, {10, 0}, {10, 10}, {0, 10}, // + {20, 20}, {30, 20}, {30, 30}, {20, 30} // + }; + std::vector expected_indices = {0, 1, 2, 0, 2, 3, + 0, 6, 7, 0, 7, 8}; + ASSERT_EQ(pts, expected); + ASSERT_EQ(indices, expected_indices); + } +} + } // namespace testing } // namespace impeller