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

Commit 0d5a79e

Browse files
author
Jonah Williams
authored
[Impeller] remove incremental allocation during filled path tessellation. (#52401)
Don't allocate a polyline (and resulting std::vectors) when performing convex tessellation. Instead use a listener pattern and iterate through path verbs.
1 parent 3087ec1 commit 0d5a79e

File tree

14 files changed

+346
-175
lines changed

14 files changed

+346
-175
lines changed

impeller/entity/geometry/fill_path_geometry.cc

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -36,15 +36,8 @@ GeometryResult FillPathGeometry::GetPositionBuffer(
3636
};
3737
}
3838

39-
VertexBuffer vertex_buffer;
40-
41-
auto points = renderer.GetTessellator()->TessellateConvex(
42-
path_, entity.GetTransform().GetMaxBasisLength());
43-
44-
vertex_buffer.vertex_buffer = host_buffer.Emplace(
45-
points.data(), points.size() * sizeof(Point), alignof(Point));
46-
vertex_buffer.index_buffer = {}, vertex_buffer.vertex_count = points.size();
47-
vertex_buffer.index_type = IndexType::kNone;
39+
VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
40+
path_, host_buffer, entity.GetTransform().GetMaxBasisLength());
4841

4942
return GeometryResult{
5043
.type = PrimitiveType::kTriangleStrip,

impeller/geometry/geometry_benchmarks.cc

Lines changed: 16 additions & 33 deletions
Original file line numberDiff line numberDiff line change
@@ -9,7 +9,6 @@
99
#include "impeller/entity/geometry/stroke_path_geometry.h"
1010
#include "impeller/geometry/path.h"
1111
#include "impeller/geometry/path_builder.h"
12-
#include "impeller/tessellator/tessellator.h"
1312
#include "impeller/tessellator/tessellator_libtess.h"
1413

1514
namespace impeller {
@@ -44,39 +43,22 @@ template <class... Args>
4443
static void BM_Polyline(benchmark::State& state, Args&&... args) {
4544
auto args_tuple = std::make_tuple(std::move(args)...);
4645
auto path = std::get<Path>(args_tuple);
47-
bool tessellate = std::get<bool>(args_tuple);
4846

4947
size_t point_count = 0u;
5048
size_t single_point_count = 0u;
5149
auto points = std::make_unique<std::vector<Point>>();
5250
points->reserve(2048);
5351
while (state.KeepRunning()) {
54-
if (tessellate) {
55-
tess.Tessellate(path, 1.0f,
56-
[&point_count, &single_point_count](
57-
const float* vertices, size_t vertices_count,
58-
const uint16_t* indices, size_t indices_count) {
59-
if (indices_count > 0) {
60-
single_point_count = indices_count;
61-
point_count += indices_count;
62-
} else {
63-
single_point_count = vertices_count;
64-
point_count += vertices_count;
65-
}
66-
return true;
67-
});
68-
} else {
69-
auto polyline = path.CreatePolyline(
70-
// Clang-tidy doesn't know that the points get moved back before
71-
// getting moved again in this loop.
72-
// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
73-
1.0f, std::move(points),
74-
[&points](Path::Polyline::PointBufferPtr reclaimed) {
75-
points = std::move(reclaimed);
76-
});
77-
single_point_count = polyline.points->size();
78-
point_count += single_point_count;
79-
}
52+
auto polyline = path.CreatePolyline(
53+
// Clang-tidy doesn't know that the points get moved back before
54+
// getting moved again in this loop.
55+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
56+
1.0f, std::move(points),
57+
[&points](Path::Polyline::PointBufferPtr reclaimed) {
58+
points = std::move(reclaimed);
59+
});
60+
single_point_count = polyline.points->size();
61+
point_count += single_point_count;
8062
}
8163
state.counters["SinglePointCount"] = single_point_count;
8264
state.counters["TotalPointCount"] = point_count;
@@ -121,11 +103,14 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
121103
size_t point_count = 0u;
122104
size_t single_point_count = 0u;
123105
auto points = std::make_unique<std::vector<Point>>();
106+
auto indices = std::make_unique<std::vector<uint16_t>>();
124107
points->reserve(2048);
125108
while (state.KeepRunning()) {
126-
auto points = tess.TessellateConvex(path, 1.0f);
127-
single_point_count = points.size();
128-
point_count += points.size();
109+
points->clear();
110+
indices->clear();
111+
Tessellator::TessellateConvexInternal(path, *points, *indices, 1.0f);
112+
single_point_count = indices->size();
113+
point_count += indices->size();
129114
}
130115
state.counters["SinglePointCount"] = single_point_count;
131116
state.counters["TotalPointCount"] = point_count;
@@ -143,14 +128,12 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
143128
MAKE_STROKE_BENCHMARK_CAPTURE(path, Round, Bevel, false, uvname, uvtype)
144129

145130
BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline, CreateCubic(true), false);
146-
BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline_tess, CreateCubic(true), true);
147131
BENCHMARK_CAPTURE(BM_Polyline,
148132
unclosed_cubic_polyline,
149133
CreateCubic(false),
150134
false);
151135

152136
BENCHMARK_CAPTURE(BM_Polyline, quad_polyline, CreateQuadratic(true), false);
153-
BENCHMARK_CAPTURE(BM_Polyline, quad_polyline_tess, CreateQuadratic(true), true);
154137
BENCHMARK_CAPTURE(BM_Polyline,
155138
unclosed_quad_polyline,
156139
CreateQuadratic(false),

impeller/geometry/path.cc

Lines changed: 66 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -103,6 +103,72 @@ void Path::EnumerateComponents(
103103
}
104104
}
105105

106+
void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
107+
auto& path_components = data_->components;
108+
auto& path_points = data_->points;
109+
bool started_contour = false;
110+
bool first_point = true;
111+
112+
for (size_t component_i = 0; component_i < path_components.size();
113+
component_i++) {
114+
const auto& path_component = path_components[component_i];
115+
switch (path_component.type) {
116+
case ComponentType::kLinear: {
117+
const LinearPathComponent* linear =
118+
reinterpret_cast<const LinearPathComponent*>(
119+
&path_points[path_component.index]);
120+
if (first_point) {
121+
writer.Write(linear->p1);
122+
first_point = false;
123+
}
124+
writer.Write(linear->p2);
125+
break;
126+
}
127+
case ComponentType::kQuadratic: {
128+
const QuadraticPathComponent* quad =
129+
reinterpret_cast<const QuadraticPathComponent*>(
130+
&path_points[path_component.index]);
131+
if (first_point) {
132+
writer.Write(quad->p1);
133+
first_point = false;
134+
}
135+
quad->ToLinearPathComponents(scale, writer);
136+
break;
137+
}
138+
case ComponentType::kCubic: {
139+
const CubicPathComponent* cubic =
140+
reinterpret_cast<const CubicPathComponent*>(
141+
&path_points[path_component.index]);
142+
if (first_point) {
143+
writer.Write(cubic->p1);
144+
first_point = false;
145+
}
146+
cubic->ToLinearPathComponents(scale, writer);
147+
break;
148+
}
149+
case ComponentType::kContour:
150+
if (component_i == path_components.size() - 1) {
151+
// If the last component is a contour, that means it's an empty
152+
// contour, so skip it.
153+
continue;
154+
}
155+
// The contour component type is the first segment in a contour.
156+
// Since this should contain the destination (if closed), we
157+
// can start with this point. If there was already an open
158+
// contour, or we've reached the end of the verb list, we
159+
// also close the contour.
160+
if (started_contour) {
161+
writer.EndContour();
162+
}
163+
started_contour = true;
164+
first_point = true;
165+
}
166+
}
167+
if (started_contour) {
168+
writer.EndContour();
169+
}
170+
}
171+
106172
bool Path::GetLinearComponentAtIndex(size_t index,
107173
LinearPathComponent& linear) const {
108174
auto& components = data_->components;

impeller/geometry/path.h

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
#include <vector>
1212

1313
#include "impeller/geometry/path_component.h"
14+
#include "impeller/geometry/rect.h"
1415

1516
namespace impeller {
1617

@@ -172,6 +173,13 @@ class Path {
172173

173174
std::optional<Rect> GetTransformedBoundingBox(const Matrix& transform) const;
174175

176+
/// Generate a polyline into the temporary storage held by the [writer].
177+
///
178+
/// It is suitable to use the max basis length of the matrix used to transform
179+
/// the path. If the provided scale is 0, curves will revert to straight
180+
/// lines.
181+
void WritePolyline(Scalar scale, VertexWriter& writer) const;
182+
175183
private:
176184
friend class PathBuilder;
177185

impeller/geometry/path_component.cc

Lines changed: 77 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,64 @@
1010

1111
namespace impeller {
1212

13+
VertexWriter::VertexWriter(std::vector<Point>& points,
14+
std::vector<uint16_t>& indices)
15+
: points_(points), indices_(indices) {}
16+
17+
void VertexWriter::EndContour() {
18+
if (points_.size() == 0u || contour_start_ == points_.size() - 1) {
19+
// Empty or first contour.
20+
return;
21+
}
22+
23+
auto start = contour_start_;
24+
auto end = points_.size() - 1;
25+
// All filled paths are drawn as if they are closed, but if
26+
// there is an explicit close then a lineTo to the origin
27+
// is inserted. This point isn't strictly necesary to
28+
// correctly render the shape and can be dropped.
29+
if (points_[end] == points_[start]) {
30+
end--;
31+
}
32+
33+
// Triangle strip break for subsequent contours
34+
if (contour_start_ != 0) {
35+
auto back = indices_.back();
36+
indices_.push_back(back);
37+
indices_.push_back(start);
38+
indices_.push_back(start);
39+
40+
// If the contour has an odd number of points, insert an extra point when
41+
// bridging to the next contour to preserve the correct triangle winding
42+
// order.
43+
if (previous_contour_odd_points_) {
44+
indices_.push_back(start);
45+
}
46+
} else {
47+
indices_.push_back(start);
48+
}
49+
50+
size_t a = start + 1;
51+
size_t b = end;
52+
while (a < b) {
53+
indices_.push_back(a);
54+
indices_.push_back(b);
55+
a++;
56+
b--;
57+
}
58+
if (a == b) {
59+
indices_.push_back(a);
60+
previous_contour_odd_points_ = false;
61+
} else {
62+
previous_contour_odd_points_ = true;
63+
}
64+
contour_start_ = points_.size();
65+
}
66+
67+
void VertexWriter::Write(Point point) {
68+
points_.push_back(point);
69+
}
70+
1371
/*
1472
* Based on: https://en.wikipedia.org/wiki/B%C3%A9zier_curve#Specific_cases
1573
*/
@@ -100,6 +158,16 @@ Point QuadraticPathComponent::SolveDerivative(Scalar time) const {
100158
};
101159
}
102160

161+
void QuadraticPathComponent::ToLinearPathComponents(
162+
Scalar scale,
163+
VertexWriter& writer) const {
164+
Scalar line_count = std::ceilf(ComputeQuadradicSubdivisions(scale, *this));
165+
for (size_t i = 1; i < line_count; i += 1) {
166+
writer.Write(Solve(i / line_count));
167+
}
168+
writer.Write(p2);
169+
}
170+
103171
void QuadraticPathComponent::AppendPolylinePoints(
104172
Scalar scale_factor,
105173
std::vector<Point>& points) const {
@@ -165,6 +233,15 @@ void CubicPathComponent::AppendPolylinePoints(
165233
scale, [&points](const Point& point) { points.emplace_back(point); });
166234
}
167235

236+
void CubicPathComponent::ToLinearPathComponents(Scalar scale,
237+
VertexWriter& writer) const {
238+
Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this));
239+
for (size_t i = 1; i < line_count; i++) {
240+
writer.Write(Solve(i / line_count));
241+
}
242+
writer.Write(p2);
243+
}
244+
168245
inline QuadraticPathComponent CubicPathComponent::Lower() const {
169246
return QuadraticPathComponent(3.0 * (cp1 - p1), 3.0 * (cp2 - cp1),
170247
3.0 * (p2 - cp2));

impeller/geometry/path_component.h

Lines changed: 25 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -6,16 +6,36 @@
66
#define FLUTTER_IMPELLER_GEOMETRY_PATH_COMPONENT_H_
77

88
#include <functional>
9+
#include <optional>
910
#include <type_traits>
1011
#include <variant>
1112
#include <vector>
1213

1314
#include "impeller/geometry/point.h"
14-
#include "impeller/geometry/rect.h"
1515
#include "impeller/geometry/scalar.h"
1616

1717
namespace impeller {
1818

19+
/// @brief An interface for generating a multi contour polyline as a triangle
20+
/// strip.
21+
class VertexWriter {
22+
public:
23+
explicit VertexWriter(std::vector<Point>& points,
24+
std::vector<uint16_t>& indices);
25+
26+
~VertexWriter() = default;
27+
28+
void EndContour();
29+
30+
void Write(Point point);
31+
32+
private:
33+
bool previous_contour_odd_points_ = false;
34+
size_t contour_start_ = 0u;
35+
std::vector<Point>& points_;
36+
std::vector<uint16_t>& indices_;
37+
};
38+
1939
struct LinearPathComponent {
2040
Point p1;
2141
Point p2;
@@ -64,6 +84,8 @@ struct QuadraticPathComponent {
6484

6585
void ToLinearPathComponents(Scalar scale_factor, const PointProc& proc) const;
6686

87+
void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
88+
6789
std::vector<Point> Extrema() const;
6890

6991
bool operator==(const QuadraticPathComponent& other) const {
@@ -109,6 +131,8 @@ struct CubicPathComponent {
109131

110132
void ToLinearPathComponents(Scalar scale, const PointProc& proc) const;
111133

134+
void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
135+
112136
CubicPathComponent Subsegment(Scalar t0, Scalar t1) const;
113137

114138
bool operator==(const CubicPathComponent& other) const {

impeller/renderer/renderer_unittests.cc

Lines changed: 8 additions & 20 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,6 @@
3535
#include "impeller/renderer/render_target.h"
3636
#include "impeller/renderer/renderer.h"
3737
#include "impeller/renderer/vertex_buffer_builder.h"
38-
#include "impeller/tessellator/tessellator_libtess.h"
3938
#include "third_party/imgui/imgui.h"
4039

4140
// TODO(zanderso): https://github.com/flutter/flutter/issues/127701
@@ -389,25 +388,14 @@ TEST_P(RendererTest, CanRenderInstanced) {
389388
using FS = InstancedDrawFragmentShader;
390389

391390
VertexBufferBuilder<VS::PerVertexData> builder;
392-
393-
ASSERT_EQ(Tessellator::Result::kSuccess,
394-
TessellatorLibtess{}.Tessellate(
395-
PathBuilder{}
396-
.AddRect(Rect::MakeXYWH(10, 10, 100, 100))
397-
.TakePath(FillType::kOdd),
398-
1.0f,
399-
[&builder](const float* vertices, size_t vertices_count,
400-
const uint16_t* indices, size_t indices_count) {
401-
for (auto i = 0u; i < vertices_count * 2; i += 2) {
402-
VS::PerVertexData data;
403-
data.vtx = {vertices[i], vertices[i + 1]};
404-
builder.AppendVertex(data);
405-
}
406-
for (auto i = 0u; i < indices_count; i++) {
407-
builder.AppendIndex(indices[i]);
408-
}
409-
return true;
410-
}));
391+
builder.AddVertices({
392+
VS::PerVertexData{Point{10, 10}},
393+
VS::PerVertexData{Point{10, 110}},
394+
VS::PerVertexData{Point{110, 10}},
395+
VS::PerVertexData{Point{10, 110}},
396+
VS::PerVertexData{Point{110, 10}},
397+
VS::PerVertexData{Point{110, 110}},
398+
});
411399

412400
ASSERT_NE(GetContext(), nullptr);
413401
auto pipeline =

0 commit comments

Comments
 (0)