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

Commit 6f5a3b3

Browse files
Reverts "[Impeller] remove most temporary allocation during polyline generation. (#52131)" (#52177)
Reverts: #52131 Initiated by: jonahwilliams Reason for reverting: breaking flutter logo rendering ![image](https://github.com/flutter/engine/assets/8975114/90a6d70f-db22-4684-80f9-1cea3dc21ac5) Original PR Author: jonahwilliams Reviewed By: {chinmaygarde} This change reverts the following previous change: Part of flutter/flutter#143077 Only allocate into reused arenas instead of allocating a new vector of data. Fixes flutter/flutter#133348 Also moves tessellation logic into the c/tessellator and out of the impeller tessellator. This was necessary to fix a compilation error. introduced by including host_buffer -> allocator -> fml mapping -> window.h include which has a function definition that conflicts with the c tessellator definition.
1 parent 599c385 commit 6f5a3b3

File tree

14 files changed

+429
-480
lines changed

14 files changed

+429
-480
lines changed

impeller/core/range.h

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -7,6 +7,8 @@
77

88
#include <cstddef>
99

10+
#include "flutter/fml/macros.h"
11+
1012
namespace impeller {
1113

1214
struct Range {

impeller/entity/geometry/fill_path_geometry.cc

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

39-
VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
40-
path_, host_buffer, entity.GetTransform().GetMaxBasisLength());
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;
4148

4249
return GeometryResult{
4350
.type = PrimitiveType::kTriangleStrip,
@@ -54,6 +61,8 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
5461
const ContentContext& renderer,
5562
const Entity& entity,
5663
RenderPass& pass) const {
64+
using VS = TextureFillVertexShader;
65+
5766
const auto& bounding_box = path_.GetBoundingBox();
5867
if (bounding_box.has_value() && bounding_box->IsEmpty()) {
5968
return GeometryResult{
@@ -71,13 +80,22 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
7180
auto uv_transform =
7281
texture_coverage.GetNormalizingTransform() * effect_transform;
7382

74-
VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
75-
path_, renderer.GetTransientsBuffer(),
76-
entity.GetTransform().GetMaxBasisLength(), uv_transform);
83+
auto points = renderer.GetTessellator()->TessellateConvex(
84+
path_, entity.GetTransform().GetMaxBasisLength());
85+
86+
VertexBufferBuilder<VS::PerVertexData> vertex_builder;
87+
vertex_builder.Reserve(points.size());
88+
for (auto i = 0u; i < points.size(); i++) {
89+
VS::PerVertexData data;
90+
data.position = points[i];
91+
data.texture_coords = uv_transform * points[i];
92+
vertex_builder.AppendVertex(data);
93+
}
7794

7895
return GeometryResult{
7996
.type = PrimitiveType::kTriangleStrip,
80-
.vertex_buffer = vertex_buffer,
97+
.vertex_buffer =
98+
vertex_builder.CreateVertexBuffer(renderer.GetTransientsBuffer()),
8199
.transform = entity.GetShaderTransform(pass),
82100
.mode = GetResultMode(),
83101
};

impeller/geometry/geometry_benchmarks.cc

Lines changed: 40 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -59,22 +59,39 @@ template <class... Args>
5959
static void BM_Polyline(benchmark::State& state, Args&&... args) {
6060
auto args_tuple = std::make_tuple(std::move(args)...);
6161
auto path = std::get<Path>(args_tuple);
62+
bool tessellate = std::get<bool>(args_tuple);
6263

6364
size_t point_count = 0u;
6465
size_t single_point_count = 0u;
6566
auto points = std::make_unique<std::vector<Point>>();
6667
points->reserve(2048);
6768
while (state.KeepRunning()) {
68-
auto polyline = path.CreatePolyline(
69-
// Clang-tidy doesn't know that the points get moved back before
70-
// getting moved again in this loop.
71-
// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
72-
1.0f, std::move(points),
73-
[&points](Path::Polyline::PointBufferPtr reclaimed) {
74-
points = std::move(reclaimed);
75-
});
76-
single_point_count = polyline.points->size();
77-
point_count += single_point_count;
69+
if (tessellate) {
70+
tess.Tessellate(path, 1.0f,
71+
[&point_count, &single_point_count](
72+
const float* vertices, size_t vertices_count,
73+
const uint16_t* indices, size_t indices_count) {
74+
if (indices_count > 0) {
75+
single_point_count = indices_count;
76+
point_count += indices_count;
77+
} else {
78+
single_point_count = vertices_count;
79+
point_count += vertices_count;
80+
}
81+
return true;
82+
});
83+
} else {
84+
auto polyline = path.CreatePolyline(
85+
// Clang-tidy doesn't know that the points get moved back before
86+
// getting moved again in this loop.
87+
// NOLINTNEXTLINE(clang-analyzer-cplusplus.Move)
88+
1.0f, std::move(points),
89+
[&points](Path::Polyline::PointBufferPtr reclaimed) {
90+
points = std::move(reclaimed);
91+
});
92+
single_point_count = polyline.points->size();
93+
point_count += single_point_count;
94+
}
7895
}
7996
state.counters["SinglePointCount"] = single_point_count;
8097
state.counters["TotalPointCount"] = point_count;
@@ -138,13 +155,11 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
138155
size_t point_count = 0u;
139156
size_t single_point_count = 0u;
140157
auto points = std::make_unique<std::vector<Point>>();
141-
auto indices = std::make_unique<std::vector<uint16_t>>();
142158
points->reserve(2048);
143-
indices->reserve(2048);
144159
while (state.KeepRunning()) {
145-
tess.TessellateConvexInternal(path, *points, *indices, 1.0f);
146-
single_point_count = indices->size();
147-
point_count += indices->size();
160+
auto points = tess.TessellateConvex(path, 1.0f);
161+
single_point_count = points.size();
162+
point_count += points.size();
148163
}
149164
state.counters["SinglePointCount"] = single_point_count;
150165
state.counters["TotalPointCount"] = point_count;
@@ -167,17 +182,27 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
167182
MAKE_STROKE_BENCHMARK_CAPTURE_CAPS_JOINS(path, _uvNoTx, UVMode::kUVRect)
168183

169184
BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline, CreateCubic(true), false);
185+
BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline_tess, CreateCubic(true), true);
170186
BENCHMARK_CAPTURE(BM_Polyline,
171187
unclosed_cubic_polyline,
172188
CreateCubic(false),
173189
false);
190+
BENCHMARK_CAPTURE(BM_Polyline,
191+
unclosed_cubic_polyline_tess,
192+
CreateCubic(false),
193+
true);
174194
MAKE_STROKE_BENCHMARK_CAPTURE_UVS(Cubic);
175195

176196
BENCHMARK_CAPTURE(BM_Polyline, quad_polyline, CreateQuadratic(true), false);
197+
BENCHMARK_CAPTURE(BM_Polyline, quad_polyline_tess, CreateQuadratic(true), true);
177198
BENCHMARK_CAPTURE(BM_Polyline,
178199
unclosed_quad_polyline,
179200
CreateQuadratic(false),
180201
false);
202+
BENCHMARK_CAPTURE(BM_Polyline,
203+
unclosed_quad_polyline_tess,
204+
CreateQuadratic(false),
205+
true);
181206
MAKE_STROKE_BENCHMARK_CAPTURE_UVS(Quadratic);
182207

183208
BENCHMARK_CAPTURE(BM_Convex, rrect_convex, CreateRRect(), true);

impeller/geometry/path.cc

Lines changed: 0 additions & 41 deletions
Original file line numberDiff line numberDiff line change
@@ -349,45 +349,4 @@ std::optional<Rect> Path::GetTransformedBoundingBox(
349349
return bounds->TransformBounds(transform);
350350
}
351351

352-
void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
353-
auto& path_components = data_->components;
354-
auto& path_points = data_->points;
355-
356-
for (size_t component_i = 0; component_i < path_components.size();
357-
component_i++) {
358-
const auto& path_component = path_components[component_i];
359-
switch (path_component.type) {
360-
case ComponentType::kLinear: {
361-
const LinearPathComponent* linear =
362-
reinterpret_cast<const LinearPathComponent*>(
363-
&path_points[path_component.index]);
364-
writer.Write(linear->p2);
365-
break;
366-
}
367-
case ComponentType::kQuadratic: {
368-
const QuadraticPathComponent* quad =
369-
reinterpret_cast<const QuadraticPathComponent*>(
370-
&path_points[path_component.index]);
371-
quad->ToLinearPathComponents(scale, writer);
372-
break;
373-
}
374-
case ComponentType::kCubic: {
375-
const CubicPathComponent* cubic =
376-
reinterpret_cast<const CubicPathComponent*>(
377-
&path_points[path_component.index]);
378-
cubic->ToLinearPathComponents(scale, writer);
379-
break;
380-
}
381-
case ComponentType::kContour:
382-
if (component_i == path_components.size() - 1) {
383-
// If the last component is a contour, that means it's an empty
384-
// contour, so skip it.
385-
continue;
386-
}
387-
writer.EndContour();
388-
break;
389-
}
390-
}
391-
}
392-
393352
} // namespace impeller

impeller/geometry/path.h

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

1313
#include "impeller/geometry/path_component.h"
14-
#include "impeller/geometry/rect.h"
1514

1615
namespace impeller {
1716

@@ -169,13 +168,6 @@ class Path {
169168
std::make_unique<std::vector<Point>>(),
170169
Polyline::ReclaimPointBufferCallback reclaim = nullptr) const;
171170

172-
/// Generate a polyline into the temporary storage held by the [writer].
173-
///
174-
/// It is suitable to use the max basis length of the matrix used to transform
175-
/// the path. If the provided scale is 0, curves will revert to straight
176-
/// lines.
177-
void WritePolyline(Scalar scale, VertexWriter& writer) const;
178-
179171
std::optional<Rect> GetBoundingBox() const;
180172

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

impeller/geometry/path_component.cc

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

1111
namespace impeller {
1212

13-
VertexWriter::VertexWriter(std::vector<Point>& points,
14-
std::vector<uint16_t>& indices,
15-
std::optional<Matrix> uv_transform)
16-
: points_(points), indices_(indices), uv_transform_(uv_transform) {}
17-
18-
void VertexWriter::EndContour() {
19-
if (points_.size() == 0u || contour_start_ == points_.size() - 1) {
20-
// Empty or first contour.
21-
return;
22-
}
23-
24-
auto start = contour_start_;
25-
auto end = points_.size() - 1;
26-
// Some polygons will not self close and an additional triangle
27-
// must be inserted, others will self close and we need to avoid
28-
// inserting an extra triangle.
29-
if (points_[end] == points_[start]) {
30-
end--;
31-
}
32-
33-
if (contour_start_ > 0) {
34-
// Triangle strip break.
35-
indices_.emplace_back(indices_.back());
36-
indices_.emplace_back(start);
37-
indices_.emplace_back(start);
38-
39-
// If the contour has an odd number of points, insert an extra point when
40-
// bridging to the next contour to preserve the correct triangle winding
41-
// order.
42-
if (previous_contour_odd_points_) {
43-
indices_.emplace_back(start);
44-
}
45-
} else {
46-
indices_.emplace_back(start);
47-
}
48-
49-
size_t a = start + 1;
50-
size_t b = end;
51-
while (a < b) {
52-
indices_.emplace_back(a);
53-
indices_.emplace_back(b);
54-
a++;
55-
b--;
56-
}
57-
if (a == b) {
58-
indices_.emplace_back(a);
59-
previous_contour_odd_points_ = false;
60-
} else {
61-
previous_contour_odd_points_ = true;
62-
}
63-
contour_start_ = points_.size();
64-
}
65-
66-
void VertexWriter::Write(Point point) {
67-
points_.emplace_back(point);
68-
if (uv_transform_.has_value()) {
69-
points_.emplace_back(*uv_transform_ * point);
70-
}
71-
}
72-
7313
/*
7414
* Based on: https://en.wikipedia.org/wiki/B%C3%A9zier_curve#Specific_cases
7515
*/
@@ -179,16 +119,6 @@ void QuadraticPathComponent::ToLinearPathComponents(
179119
proc(p2);
180120
}
181121

182-
void QuadraticPathComponent::ToLinearPathComponents(
183-
Scalar scale,
184-
VertexWriter& writer) const {
185-
Scalar line_count = std::ceilf(ComputeQuadradicSubdivisions(scale, *this));
186-
for (size_t i = 1; i < line_count; i += 1) {
187-
writer.Write(Solve(i / line_count));
188-
}
189-
writer.Write(p2);
190-
}
191-
192122
std::vector<Point> QuadraticPathComponent::Extrema() const {
193123
CubicPathComponent elevated(*this);
194124
return elevated.Extrema();
@@ -259,15 +189,6 @@ void CubicPathComponent::ToLinearPathComponents(Scalar scale,
259189
proc(p2);
260190
}
261191

262-
void CubicPathComponent::ToLinearPathComponents(Scalar scale,
263-
VertexWriter& writer) const {
264-
Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this));
265-
for (size_t i = 1; i < line_count; i++) {
266-
writer.Write(Solve(i / line_count));
267-
}
268-
writer.Write(p2);
269-
}
270-
271192
static inline bool NearEqual(Scalar a, Scalar b, Scalar epsilon) {
272193
return (a > (b - epsilon)) && (a < (b + epsilon));
273194
}

impeller/geometry/path_component.h

Lines changed: 1 addition & 27 deletions
Original file line numberDiff line numberDiff line change
@@ -10,34 +10,12 @@
1010
#include <variant>
1111
#include <vector>
1212

13-
#include "impeller/geometry/matrix.h"
1413
#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-
std::optional<Matrix> uv_transform);
26-
27-
~VertexWriter() = default;
28-
29-
void EndContour();
30-
31-
void Write(Point point);
32-
33-
private:
34-
bool previous_contour_odd_points_ = false;
35-
size_t contour_start_ = 0u;
36-
std::vector<Point>& points_;
37-
std::vector<uint16_t>& indices_;
38-
std::optional<Matrix> uv_transform_;
39-
};
40-
4119
struct LinearPathComponent {
4220
Point p1;
4321
Point p2;
@@ -86,8 +64,6 @@ struct QuadraticPathComponent {
8664

8765
void ToLinearPathComponents(Scalar scale_factor, const PointProc& proc) const;
8866

89-
void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
90-
9167
std::vector<Point> Extrema() const;
9268

9369
bool operator==(const QuadraticPathComponent& other) const {
@@ -133,8 +109,6 @@ struct CubicPathComponent {
133109

134110
void ToLinearPathComponents(Scalar scale, const PointProc& proc) const;
135111

136-
void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;
137-
138112
CubicPathComponent Subsegment(Scalar t0, Scalar t1) const;
139113

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

0 commit comments

Comments
 (0)