Skip to content
This repository was archived by the owner on Feb 25, 2025. It is now read-only.
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
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
21 changes: 15 additions & 6 deletions impeller/entity/geometry/fill_path_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@
// found in the LICENSE file.

#include "impeller/entity/geometry/fill_path_geometry.h"
#include "impeller/core/formats.h"

namespace impeller {

Expand Down Expand Up @@ -48,10 +49,16 @@ GeometryResult FillPathGeometry::GetPositionBuffer(
size_t indices_count) {
Copy link
Member

Choose a reason for hiding this comment

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

The variable name here belies the fact that it actually can represent 2 different things.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

vertex_buffer.vertex_buffer = host_buffer.Emplace(
vertices, vertices_count * sizeof(float), alignof(float));
vertex_buffer.index_buffer = host_buffer.Emplace(
indices, indices_count * sizeof(uint16_t), alignof(uint16_t));
vertex_buffer.vertex_count = indices_count;
vertex_buffer.index_type = IndexType::k16bit;
if (indices != nullptr) {
vertex_buffer.index_buffer = host_buffer.Emplace(
indices, indices_count * sizeof(uint16_t), alignof(uint16_t));
vertex_buffer.vertex_count = indices_count;
vertex_buffer.index_type = IndexType::k16bit;
} else {
vertex_buffer.index_buffer = {};
vertex_buffer.vertex_count = indices_count;
vertex_buffer.index_type = IndexType::kNone;
}
return true;
});
if (tesselation_result != Tessellator::Result::kSuccess) {
Expand Down Expand Up @@ -122,8 +129,10 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
vertex_builder.AppendVertex(data);
}
FML_DCHECK(vertex_builder.GetVertexCount() == vertices_count / 2);
for (auto i = 0u; i < indices_count; i++) {
vertex_builder.AppendIndex(indices[i]);
if (indices != nullptr) {
for (auto i = 0u; i < indices_count; i++) {
vertex_builder.AppendIndex(indices[i]);
}
}
return true;
});
Expand Down
167 changes: 124 additions & 43 deletions impeller/tessellator/tessellator.cc
Original file line number Diff line number Diff line change
Expand Up @@ -78,51 +78,132 @@ Tessellator::Result Tessellator::Tessellate(
constexpr int kVertexSize = 2;
constexpr int kPolygonSize = 3;

//----------------------------------------------------------------------------
/// Feed contour information to the tessellator.
///
static_assert(sizeof(Point) == 2 * sizeof(float));
for (size_t contour_i = 0; contour_i < polyline.contours.size();
contour_i++) {
size_t start_point_index, end_point_index;
std::tie(start_point_index, end_point_index) =
polyline.GetContourPointBounds(contour_i);

::tessAddContour(tessellator, // the C tessellator
kVertexSize, //
polyline.points.data() + start_point_index, //
sizeof(Point), //
end_point_index - start_point_index //
if (polyline.contours.size() > 30 && fill_type == FillType::kNonZero) {
Copy link
Member

Choose a reason for hiding this comment

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

Consider pulling 30 out to a named constant, and as much as might be possible, explain why 30 was chosen.

Copy link
Member

Choose a reason for hiding this comment

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

I'd also add a comment about not using indices since there is a lot of code to read through to understand that's the case.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Added more comments here.

std::vector<Point> points;
std::vector<float> data;

//----------------------------------------------------------------------------
/// Feed contour information to the tessellator.
///
size_t total = 0u;
static_assert(sizeof(Point) == 2 * sizeof(float));
for (size_t contour_i = 0; contour_i < polyline.contours.size();
contour_i++) {
size_t start_point_index, end_point_index;
std::tie(start_point_index, end_point_index) =
polyline.GetContourPointBounds(contour_i);

::tessAddContour(tessellator, // the C tessellator
kVertexSize, //
polyline.points.data() + start_point_index, //
sizeof(Point), //
end_point_index - start_point_index //
);

//----------------------------------------------------------------------------
/// Let's tessellate.
///
auto result = ::tessTesselate(tessellator, // tessellator
ToTessWindingRule(fill_type), // winding
TESS_POLYGONS, // element type
kPolygonSize, // polygon size
kVertexSize, // vertex size
nullptr // normal (null is automatic)
);

if (result != 1) {
return Result::kTessellationError;
}

int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize;
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize;
int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize;

These variable names don't conform to the c++ style guide.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Done

auto vertices = tessGetVertices(tessellator);
for (int i = 0; i < vertexItemCount; i += 2) {
points.emplace_back(vertices[i], vertices[i + 1]);
}

int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize;
auto elements = tessGetElements(tessellator);
total += elementItemCount;
for (int i = 0; i < elementItemCount; i++) {
data.emplace_back(points[elements[i]].x);
data.emplace_back(points[elements[i]].y);
}
points.clear();
}
if (!callback(data.data(), data.size(), nullptr, total)) {
return Result::kInputError;
}
} else {
//----------------------------------------------------------------------------
/// Feed contour information to the tessellator.
///
static_assert(sizeof(Point) == 2 * sizeof(float));
for (size_t contour_i = 0; contour_i < polyline.contours.size();
contour_i++) {
size_t start_point_index, end_point_index;
std::tie(start_point_index, end_point_index) =
polyline.GetContourPointBounds(contour_i);

::tessAddContour(tessellator, // the C tessellator
kVertexSize, //
polyline.points.data() + start_point_index, //
sizeof(Point), //
end_point_index - start_point_index //
);
}

//----------------------------------------------------------------------------
/// Let's tessellate.
///
auto result = ::tessTesselate(tessellator, // tessellator
ToTessWindingRule(fill_type), // winding
TESS_POLYGONS, // element type
kPolygonSize, // polygon size
kVertexSize, // vertex size
nullptr // normal (null is automatic)
);
}

//----------------------------------------------------------------------------
/// Let's tessellate.
///
auto result = ::tessTesselate(tessellator, // tessellator
ToTessWindingRule(fill_type), // winding
TESS_POLYGONS, // element type
kPolygonSize, // polygon size
kVertexSize, // vertex size
nullptr // normal (null is automatic)
);

if (result != 1) {
return Result::kTessellationError;
}

int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize;
auto vertices = tessGetVertices(tessellator);
int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize;
auto elements = tessGetElements(tessellator);
// libtess uses an int index internally due to usage of -1 as a sentinel
// value.
std::vector<uint16_t> indices(elementItemCount);
for (int i = 0; i < elementItemCount; i++) {
indices[i] = static_cast<uint16_t>(elements[i]);
}
if (!callback(vertices, vertexItemCount, indices.data(), elementItemCount)) {
return Result::kInputError;
if (result != 1) {
return Result::kTessellationError;
}

int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize;
if (elementItemCount < 65535) {
int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize;
auto vertices = tessGetVertices(tessellator);
auto elements = tessGetElements(tessellator);

// libtess uses an int index internally due to usage of -1 as a sentinel
// value.
std::vector<uint16_t> indices(elementItemCount);
for (int i = 0; i < elementItemCount; i++) {
indices[i] = static_cast<uint16_t>(elements[i]);
}
if (!callback(vertices, vertexItemCount, indices.data(),
elementItemCount)) {
return Result::kInputError;
}
} else {
std::vector<Point> points;
std::vector<uint16_t> indices;
Copy link
Member

Choose a reason for hiding this comment

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

This value is generated, but never used.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed


int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize;
auto vertices = tessGetVertices(tessellator);
for (int i = 0; i < vertexItemCount; i += 2) {
points.emplace_back(vertices[i], vertices[i + 1]);
}

int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize;
auto elements = tessGetElements(tessellator);
for (int i = 0; i < elementItemCount; i++) {
indices.emplace_back(elements[i]);
}

if (!callback(reinterpret_cast<float*>(points.data()), points.size(),
nullptr, elementItemCount)) {
return Result::kInputError;
}
}
}

return Result::kSuccess;
Expand Down