Skip to content

Commit

Permalink
[Impeller] remove incremental allocation during filled path tessellat…
Browse files Browse the repository at this point in the history
…ion. (#52401)

Don't allocate a polyline (and resulting std::vectors) when performing convex tessellation. Instead use a listener pattern and iterate through path verbs.
  • Loading branch information
jonahwilliams authored May 2, 2024
1 parent 3087ec1 commit 0d5a79e
Show file tree
Hide file tree
Showing 14 changed files with 346 additions and 175 deletions.
11 changes: 2 additions & 9 deletions impeller/entity/geometry/fill_path_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -36,15 +36,8 @@ GeometryResult FillPathGeometry::GetPositionBuffer(
};
}

VertexBuffer vertex_buffer;

auto points = renderer.GetTessellator()->TessellateConvex(
path_, entity.GetTransform().GetMaxBasisLength());

vertex_buffer.vertex_buffer = host_buffer.Emplace(
points.data(), points.size() * sizeof(Point), alignof(Point));
vertex_buffer.index_buffer = {}, vertex_buffer.vertex_count = points.size();
vertex_buffer.index_type = IndexType::kNone;
VertexBuffer vertex_buffer = renderer.GetTessellator()->TessellateConvex(
path_, host_buffer, entity.GetTransform().GetMaxBasisLength());

return GeometryResult{
.type = PrimitiveType::kTriangleStrip,
Expand Down
49 changes: 16 additions & 33 deletions impeller/geometry/geometry_benchmarks.cc
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,6 @@
#include "impeller/entity/geometry/stroke_path_geometry.h"
#include "impeller/geometry/path.h"
#include "impeller/geometry/path_builder.h"
#include "impeller/tessellator/tessellator.h"
#include "impeller/tessellator/tessellator_libtess.h"

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

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()) {
if (tessellate) {
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;
}
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;
state.counters["TotalPointCount"] = point_count;
Expand Down Expand Up @@ -121,11 +103,14 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
size_t point_count = 0u;
size_t single_point_count = 0u;
auto points = std::make_unique<std::vector<Point>>();
auto indices = std::make_unique<std::vector<uint16_t>>();
points->reserve(2048);
while (state.KeepRunning()) {
auto points = tess.TessellateConvex(path, 1.0f);
single_point_count = points.size();
point_count += points.size();
points->clear();
indices->clear();
Tessellator::TessellateConvexInternal(path, *points, *indices, 1.0f);
single_point_count = indices->size();
point_count += indices->size();
}
state.counters["SinglePointCount"] = single_point_count;
state.counters["TotalPointCount"] = point_count;
Expand All @@ -143,14 +128,12 @@ static void BM_Convex(benchmark::State& state, Args&&... args) {
MAKE_STROKE_BENCHMARK_CAPTURE(path, Round, Bevel, false, uvname, uvtype)

BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline, CreateCubic(true), false);
BENCHMARK_CAPTURE(BM_Polyline, cubic_polyline_tess, CreateCubic(true), true);
BENCHMARK_CAPTURE(BM_Polyline,
unclosed_cubic_polyline,
CreateCubic(false),
false);

BENCHMARK_CAPTURE(BM_Polyline, quad_polyline, CreateQuadratic(true), false);
BENCHMARK_CAPTURE(BM_Polyline, quad_polyline_tess, CreateQuadratic(true), true);
BENCHMARK_CAPTURE(BM_Polyline,
unclosed_quad_polyline,
CreateQuadratic(false),
Expand Down
66 changes: 66 additions & 0 deletions impeller/geometry/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -103,6 +103,72 @@ void Path::EnumerateComponents(
}
}

void Path::WritePolyline(Scalar scale, VertexWriter& writer) const {
auto& path_components = data_->components;
auto& path_points = data_->points;
bool started_contour = false;
bool first_point = true;

for (size_t component_i = 0; component_i < path_components.size();
component_i++) {
const auto& path_component = path_components[component_i];
switch (path_component.type) {
case ComponentType::kLinear: {
const LinearPathComponent* linear =
reinterpret_cast<const LinearPathComponent*>(
&path_points[path_component.index]);
if (first_point) {
writer.Write(linear->p1);
first_point = false;
}
writer.Write(linear->p2);
break;
}
case ComponentType::kQuadratic: {
const QuadraticPathComponent* quad =
reinterpret_cast<const QuadraticPathComponent*>(
&path_points[path_component.index]);
if (first_point) {
writer.Write(quad->p1);
first_point = false;
}
quad->ToLinearPathComponents(scale, writer);
break;
}
case ComponentType::kCubic: {
const CubicPathComponent* cubic =
reinterpret_cast<const CubicPathComponent*>(
&path_points[path_component.index]);
if (first_point) {
writer.Write(cubic->p1);
first_point = false;
}
cubic->ToLinearPathComponents(scale, writer);
break;
}
case ComponentType::kContour:
if (component_i == path_components.size() - 1) {
// If the last component is a contour, that means it's an empty
// contour, so skip it.
continue;
}
// The contour component type is the first segment in a contour.
// Since this should contain the destination (if closed), we
// can start with this point. If there was already an open
// contour, or we've reached the end of the verb list, we
// also close the contour.
if (started_contour) {
writer.EndContour();
}
started_contour = true;
first_point = true;
}
}
if (started_contour) {
writer.EndContour();
}
}

bool Path::GetLinearComponentAtIndex(size_t index,
LinearPathComponent& linear) const {
auto& components = data_->components;
Expand Down
8 changes: 8 additions & 0 deletions impeller/geometry/path.h
Original file line number Diff line number Diff line change
Expand Up @@ -11,6 +11,7 @@
#include <vector>

#include "impeller/geometry/path_component.h"
#include "impeller/geometry/rect.h"

namespace impeller {

Expand Down Expand Up @@ -172,6 +173,13 @@ class Path {

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

/// Generate a polyline into the temporary storage held by the [writer].
///
/// 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 straight
/// lines.
void WritePolyline(Scalar scale, VertexWriter& writer) const;

private:
friend class PathBuilder;

Expand Down
77 changes: 77 additions & 0 deletions impeller/geometry/path_component.cc
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,64 @@

namespace impeller {

VertexWriter::VertexWriter(std::vector<Point>& points,
std::vector<uint16_t>& indices)
: points_(points), indices_(indices) {}

void VertexWriter::EndContour() {
if (points_.size() == 0u || contour_start_ == points_.size() - 1) {
// Empty or first contour.
return;
}

auto start = contour_start_;
auto end = points_.size() - 1;
// All filled paths are drawn as if they are closed, but if
// there is an explicit close then a lineTo to the origin
// is inserted. This point isn't strictly necesary to
// correctly render the shape and can be dropped.
if (points_[end] == points_[start]) {
end--;
}

// Triangle strip break for subsequent contours
if (contour_start_ != 0) {
auto back = indices_.back();
indices_.push_back(back);
indices_.push_back(start);
indices_.push_back(start);

// If the contour has an odd number of points, insert an extra point when
// bridging to the next contour to preserve the correct triangle winding
// order.
if (previous_contour_odd_points_) {
indices_.push_back(start);
}
} else {
indices_.push_back(start);
}

size_t a = start + 1;
size_t b = end;
while (a < b) {
indices_.push_back(a);
indices_.push_back(b);
a++;
b--;
}
if (a == b) {
indices_.push_back(a);
previous_contour_odd_points_ = false;
} else {
previous_contour_odd_points_ = true;
}
contour_start_ = points_.size();
}

void VertexWriter::Write(Point point) {
points_.push_back(point);
}

/*
* Based on: https://en.wikipedia.org/wiki/B%C3%A9zier_curve#Specific_cases
*/
Expand Down Expand Up @@ -100,6 +158,16 @@ Point QuadraticPathComponent::SolveDerivative(Scalar time) const {
};
}

void QuadraticPathComponent::ToLinearPathComponents(
Scalar scale,
VertexWriter& writer) const {
Scalar line_count = std::ceilf(ComputeQuadradicSubdivisions(scale, *this));
for (size_t i = 1; i < line_count; i += 1) {
writer.Write(Solve(i / line_count));
}
writer.Write(p2);
}

void QuadraticPathComponent::AppendPolylinePoints(
Scalar scale_factor,
std::vector<Point>& points) const {
Expand Down Expand Up @@ -165,6 +233,15 @@ void CubicPathComponent::AppendPolylinePoints(
scale, [&points](const Point& point) { points.emplace_back(point); });
}

void CubicPathComponent::ToLinearPathComponents(Scalar scale,
VertexWriter& writer) const {
Scalar line_count = std::ceilf(ComputeCubicSubdivisions(scale, *this));
for (size_t i = 1; i < line_count; i++) {
writer.Write(Solve(i / line_count));
}
writer.Write(p2);
}

inline QuadraticPathComponent CubicPathComponent::Lower() const {
return QuadraticPathComponent(3.0 * (cp1 - p1), 3.0 * (cp2 - cp1),
3.0 * (p2 - cp2));
Expand Down
26 changes: 25 additions & 1 deletion impeller/geometry/path_component.h
Original file line number Diff line number Diff line change
Expand Up @@ -6,16 +6,36 @@
#define FLUTTER_IMPELLER_GEOMETRY_PATH_COMPONENT_H_

#include <functional>
#include <optional>
#include <type_traits>
#include <variant>
#include <vector>

#include "impeller/geometry/point.h"
#include "impeller/geometry/rect.h"
#include "impeller/geometry/scalar.h"

namespace impeller {

/// @brief An interface for generating a multi contour polyline as a triangle
/// strip.
class VertexWriter {
public:
explicit VertexWriter(std::vector<Point>& points,
std::vector<uint16_t>& indices);

~VertexWriter() = default;

void EndContour();

void Write(Point point);

private:
bool previous_contour_odd_points_ = false;
size_t contour_start_ = 0u;
std::vector<Point>& points_;
std::vector<uint16_t>& indices_;
};

struct LinearPathComponent {
Point p1;
Point p2;
Expand Down Expand Up @@ -64,6 +84,8 @@ struct QuadraticPathComponent {

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

void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;

std::vector<Point> Extrema() const;

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

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

void ToLinearPathComponents(Scalar scale, VertexWriter& writer) const;

CubicPathComponent Subsegment(Scalar t0, Scalar t1) const;

bool operator==(const CubicPathComponent& other) const {
Expand Down
28 changes: 8 additions & 20 deletions impeller/renderer/renderer_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@
#include "impeller/renderer/render_target.h"
#include "impeller/renderer/renderer.h"
#include "impeller/renderer/vertex_buffer_builder.h"
#include "impeller/tessellator/tessellator_libtess.h"
#include "third_party/imgui/imgui.h"

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

VertexBufferBuilder<VS::PerVertexData> builder;

ASSERT_EQ(Tessellator::Result::kSuccess,
TessellatorLibtess{}.Tessellate(
PathBuilder{}
.AddRect(Rect::MakeXYWH(10, 10, 100, 100))
.TakePath(FillType::kOdd),
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) {
VS::PerVertexData data;
data.vtx = {vertices[i], vertices[i + 1]};
builder.AppendVertex(data);
}
for (auto i = 0u; i < indices_count; i++) {
builder.AppendIndex(indices[i]);
}
return true;
}));
builder.AddVertices({
VS::PerVertexData{Point{10, 10}},
VS::PerVertexData{Point{10, 110}},
VS::PerVertexData{Point{110, 10}},
VS::PerVertexData{Point{10, 110}},
VS::PerVertexData{Point{110, 10}},
VS::PerVertexData{Point{110, 110}},
});

ASSERT_NE(GetContext(), nullptr);
auto pipeline =
Expand Down
Loading

0 comments on commit 0d5a79e

Please sign in to comment.