Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.

[Impeller] Reduce allocations for polyline generation #47837

Merged
merged 15 commits into from
Nov 10, 2023
Merged

[Impeller] Reduce allocations for polyline generation #47837

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
15 commits
Select commit Hold shift + click to select a range
1bc0c4d
1
dnfield Nov 8, 2023
ebfaabb
[Impeller] Reduce allocations in polyline generation
dnfield Nov 8, 2023
84d2e20
merge
dnfield Nov 8, 2023
a92b757
fixes for tests, minor cleanup/rename
dnfield Nov 8, 2023
aa280ec
docs
dnfield Nov 8, 2023
cd60b9f
one more reserve
dnfield Nov 8, 2023
07eee60
No TLS, unique_ptr for vector with callback
dnfield Nov 9, 2023
761be29
more review
dnfield Nov 9, 2023
4d9fe4e
merge
dnfield Nov 9, 2023
720a0ee
names
dnfield Nov 9, 2023
615f8fd
tests
dnfield Nov 9, 2023
71afaa0
lint
dnfield Nov 9, 2023
da16afe
lint
dnfield Nov 9, 2023
d9a0ab9
Merge branch 'main' into alloc
dnfield Nov 10, 2023
eaa07be
Merge branch 'main' into alloc
dnfield Nov 10, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
36 changes: 0 additions & 36 deletions impeller/entity/entity_unittests.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Point> expected = {
{0, 0}, {10, 0}, {10, 10}, {0, 10}, //
};
std::vector<uint16_t> 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<Point> expected = {
{0, 0}, {10, 0}, {10, 10}, {0, 10}, //
{20, 20}, {30, 20}, {30, 30}, {20, 30} //
};
std::vector<uint16_t> 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);
Expand Down
14 changes: 6 additions & 8 deletions impeller/entity/geometry/fill_path_geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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));
Expand All @@ -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) {
Expand Down Expand Up @@ -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<VS::PerVertexData> vertex_builder;
vertex_builder.Reserve(points.size());
Expand All @@ -115,8 +114,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(

VertexBufferBuilder<VS::PerVertexData> 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) {
Expand Down
31 changes: 0 additions & 31 deletions impeller/entity/geometry/geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,37 +15,6 @@

namespace impeller {

/// Given a convex polyline, create a triangle fan structure.
std::pair<std::vector<Point>, std::vector<uint16_t>> TessellateConvex(
Path::Polyline polyline) {
std::vector<Point> output;
std::vector<uint16_t> 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<TextureFillVertexShader::PerVertexData>
ComputeUVGeometryCPU(
VertexBufferBuilder<SolidFillVertexShader::PerVertexData>& input,
Expand Down
5 changes: 0 additions & 5 deletions impeller/entity/geometry/geometry.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Point>, std::vector<uint16_t>> TessellateConvex(
Path::Polyline polyline);

class Geometry {
public:
Geometry();
Expand Down
55 changes: 31 additions & 24 deletions impeller/entity/geometry/stroke_path_geometry.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<Point> arc_points;
CubicPathComponent(start_offset, start_handle, middle_handle, middle)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can the reserve size be calculated here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These are small and shouldn't result in lots of reallocation. I'd feel more comfortable having a benchmark showing this before trying to place a number in it.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I ask because I'm turning on a linter for this in #47868. I don't think it will flag this though since it can't trivially know what the reserve size is.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Reserving incorrect sizes can make things worse. If we don't know let's use the standard vec reallocation startegy

.AppendPolylinePoints(scale, arc_points);

VS::PerVertexData vtx;
for (const auto& point : arc_points) {
Expand Down Expand Up @@ -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<Point> arc_points;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Or here?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Same deal. This shouldn't typically result in a large number of 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);
Expand Down Expand Up @@ -234,7 +236,12 @@ StrokePathGeometry::CreateSolidStrokeVertices(
const StrokePathGeometry::CapProc& cap_proc,
Scalar scale) {
VertexBufferBuilder<VS::PerVertexData> vtx_builder;
auto polyline = path.CreatePolyline(scale);
auto point_buffer = std::make_unique<std::vector<Point>>();
// 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;

Expand All @@ -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;
Expand All @@ -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);
}
}
Expand All @@ -312,23 +319,23 @@ 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,
contour_end_point_i, contour);
// 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);
}
}
Expand All @@ -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;
Expand All @@ -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);
Expand All @@ -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;
Expand Down Expand Up @@ -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);
}
}
Expand Down
33 changes: 26 additions & 7 deletions impeller/geometry/geometry_benchmarks.cc
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -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<std::vector<Point>>();
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;
Expand Down
Loading