From 2666377728b64caa78db2f4a94d9fec8661bb2e6 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 25 Sep 2023 16:19:29 -0700 Subject: [PATCH 01/11] [Impeller] fallback to no index buffer for large tessellation. --- .../entity/geometry/fill_path_geometry.cc | 14 +++++-- impeller/tessellator/tessellator.cc | 40 +++++++++++++++---- 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index b608ed0428ae1..19d45b1c2debb 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/entity/geometry/fill_path_geometry.h" +#include "impeller/core/formats.h" namespace impeller { @@ -48,10 +49,15 @@ GeometryResult FillPathGeometry::GetPositionBuffer( size_t indices_count) { 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.vertex_count = vertices_count; + vertex_buffer.index_type = IndexType::kNone; + } return true; }); if (tesselation_result != Tessellator::Result::kSuccess) { diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index f6b27215cca31..e15df5af34447 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -3,6 +3,7 @@ // found in the LICENSE file. #include "impeller/tessellator/tessellator.h" +#include "flutter/fml/logging.h" #include "third_party/libtess2/Include/tesselator.h" @@ -115,14 +116,37 @@ Tessellator::Result Tessellator::Tessellate( 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 indices(elementItemCount); - for (int i = 0; i < elementItemCount; i++) { - indices[i] = static_cast(elements[i]); - } - if (!callback(vertices, vertexItemCount, indices.data(), elementItemCount)) { - return Result::kInputError; + + if (elementItemCount < 65535) { + // libtess uses an int index internally due to usage of -1 as a sentinel + // value. + std::vector indices(elementItemCount); + for (int i = 0; i < elementItemCount; i++) { + indices[i] = static_cast(elements[i]); + } + if (!callback(vertices, vertexItemCount, indices.data(), + elementItemCount)) { + return Result::kInputError; + } + } else { + std::vector points; + std::vector indices; + + 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(points.data()), points.size(), nullptr, 0u)) { + return Result::kInputError; + } } return Result::kSuccess; From 42d5fef089f9758728cfa8beccfa60259aa36a7e Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Mon, 25 Sep 2023 16:19:52 -0700 Subject: [PATCH 02/11] ++ --- impeller/tessellator/tessellator.cc | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index e15df5af34447..171afa2383c77 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -144,7 +144,8 @@ Tessellator::Result Tessellator::Tessellate( indices.emplace_back(elements[i]); } - if (!callback(reinterpret_cast(points.data()), points.size(), nullptr, 0u)) { + if (!callback(reinterpret_cast(points.data()), points.size(), + nullptr, 0u)) { return Result::kInputError; } } From 30682b5ed36fac5ea0b611c47bf39c2dd2681e00 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 26 Sep 2023 10:39:15 -0700 Subject: [PATCH 03/11] more work. --- .../entity/geometry/fill_path_geometry.cc | 9 +- impeller/tessellator/tessellator.cc | 178 ++++++++++++------ 2 files changed, 123 insertions(+), 64 deletions(-) diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 19d45b1c2debb..f237405001011 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -55,7 +55,8 @@ GeometryResult FillPathGeometry::GetPositionBuffer( vertex_buffer.vertex_count = indices_count; vertex_buffer.index_type = IndexType::k16bit; } else { - vertex_buffer.vertex_count = vertices_count; + vertex_buffer.index_buffer = {}; + vertex_buffer.vertex_count = indices_count; vertex_buffer.index_type = IndexType::kNone; } return true; @@ -128,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; }); diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 171afa2383c77..f11c4675ce9b8 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -3,7 +3,6 @@ // found in the LICENSE file. #include "impeller/tessellator/tessellator.h" -#include "flutter/fml/logging.h" #include "third_party/libtess2/Include/tesselator.h" @@ -79,74 +78,131 @@ 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 // - ); - } - - //---------------------------------------------------------------------------- - /// 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); - - if (elementItemCount < 65535) { - // libtess uses an int index internally due to usage of -1 as a sentinel - // value. - std::vector indices(elementItemCount); - for (int i = 0; i < elementItemCount; i++) { - indices[i] = static_cast(elements[i]); + if (polyline.contours.size() > 30 && fill_type == FillType::kNonZero) { + std::vector points; + std::vector 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; + 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(vertices, vertexItemCount, indices.data(), - elementItemCount)) { + if (!callback(data.data(), data.size(), nullptr, total)) { return Result::kInputError; } } else { - std::vector points; - std::vector indices; - - 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]); + //---------------------------------------------------------------------------- + /// 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 // + ); } - int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; - auto elements = tessGetElements(tessellator); - for (int i = 0; i < elementItemCount; i++) { - indices.emplace_back(elements[i]); + //---------------------------------------------------------------------------- + /// 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; } - if (!callback(reinterpret_cast(points.data()), points.size(), - nullptr, 0u)) { - return Result::kInputError; + 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 indices(elementItemCount); + for (int i = 0; i < elementItemCount; i++) { + indices[i] = static_cast(elements[i]); + } + if (!callback(vertices, vertexItemCount, indices.data(), + elementItemCount)) { + return Result::kInputError; + } + } else { + std::vector points; + std::vector indices; + + 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(points.data()), points.size(), + nullptr, elementItemCount)) { + return Result::kInputError; + } } } From da271365e247bd18cc270e5b770eed54b1c22088 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 26 Sep 2023 14:54:58 -0700 Subject: [PATCH 04/11] ++ --- impeller/tessellator/tessellator.cc | 25 +++++++--- impeller/tessellator/tessellator.h | 9 +++- impeller/tessellator/tessellator_unittests.cc | 46 +++++++++++++++++++ 3 files changed, 73 insertions(+), 7 deletions(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index f11c4675ce9b8..f503675dddf52 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -58,6 +58,10 @@ static int ToTessWindingRule(FillType fill_type) { return TESS_WINDING_ODD; } +/// @brief An arbitrary value to determine when a multi-contour non-zero fill +/// path should be split into multiple tessellations. +static constexpr size_t kMultiContourThreshold = 30u; + Tessellator::Result Tessellator::Tessellate( FillType fill_type, const Path::Polyline& polyline, @@ -78,7 +82,11 @@ Tessellator::Result Tessellator::Tessellate( constexpr int kVertexSize = 2; constexpr int kPolygonSize = 3; - if (polyline.contours.size() > 30 && fill_type == FillType::kNonZero) { + // If we have a larger polyline and the fill type is non-zero, we can split + // the tessellation up per contour. Since in general the complexity is at + // least nlog(n), this speeds up the processes substantially. + if (polyline.contours.size() > kMultiContourThreshold && + fill_type == FillType::kNonZero) { std::vector points; std::vector data; @@ -168,6 +176,12 @@ Tessellator::Result Tessellator::Tessellate( } int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; + + // We default to using a 16bit index buffer, but in cases where we generate + // more tessellated data than this can contain we need to fall back to + // dropping the index buffer entirely. Instead code could instead switch to + // a uint32 index buffer, but this is done for simplicity with the other + // fast path above. if (elementItemCount < 65535) { int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize; auto vertices = tessGetVertices(tessellator); @@ -185,7 +199,7 @@ Tessellator::Result Tessellator::Tessellate( } } else { std::vector points; - std::vector indices; + std::vector data; int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize; auto vertices = tessGetVertices(tessellator); @@ -196,11 +210,10 @@ Tessellator::Result Tessellator::Tessellate( int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; auto elements = tessGetElements(tessellator); for (int i = 0; i < elementItemCount; i++) { - indices.emplace_back(elements[i]); + data.emplace_back(points[elements[i]].x); + data.emplace_back(points[elements[i]].y); } - - if (!callback(reinterpret_cast(points.data()), points.size(), - nullptr, elementItemCount)) { + if (!callback(data.data(), data.size(), nullptr, elementItemCount)) { return Result::kInputError; } } diff --git a/impeller/tessellator/tessellator.h b/impeller/tessellator/tessellator.h index ff93b7091a3f6..97da500decbda 100644 --- a/impeller/tessellator/tessellator.h +++ b/impeller/tessellator/tessellator.h @@ -44,10 +44,17 @@ class Tessellator { ~Tessellator(); + /// @brief A callback that returns the results of the tessellation. + /// + /// The index buffer may not be populated, in which case [indices] will + /// be nullptr. The [vertices_size] is the size of the vertices array + /// and not the number of vertices, which is usually vertices_size / 2. + /// In cases where the indices is nullptr, vertex_or_index_count will + /// contain the count of indices. using BuilderCallback = std::function; + size_t vertex_or_index_count)>; //---------------------------------------------------------------------------- /// @brief Generates filled triangles from the polyline. A callback is diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 0b943d1391254..6054bbe8f1b7f 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -4,6 +4,7 @@ #include "flutter/testing/testing.h" #include "gtest/gtest.h" +#include "impeller/geometry/path.h" #include "impeller/geometry/path_builder.h" #include "impeller/tessellator/tessellator.h" @@ -82,6 +83,51 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { ASSERT_EQ(polyline.points.size(), 2u); ASSERT_EQ(result, Tessellator::Result::kInputError); } + + // More than 30 contours, non-zero fill mode. + { + Tessellator t; + PathBuilder builder = {}; + for (auto i = 0; i < 60; i++) { + builder.AddCircle(Point(i, i), 4); + } + auto polyline = builder.TakePath().CreatePolyline(1.0f); + bool no_indices = false; + Tessellator::Result result = t.Tessellate( + FillType::kNonZero, polyline, + [&no_indices](const float* vertices, size_t vertices_size, + const uint16_t* indices, size_t indices_size) { + no_indices = indices == nullptr; + return true; + }); + + ASSERT_TRUE(no_indices); + ASSERT_EQ(result, Tessellator::Result::kSuccess); + } + + // More than uint16 points, odd fill mode. + { + Tessellator t; + PathBuilder builder = {}; + for (auto i = 0; i < 1000; i++) { + builder.AddCircle(Point(i, i), 4); + } + auto polyline = builder.TakePath(FillType::kOdd).CreatePolyline(1.0f); + bool no_indices = false; + size_t indices_count = 0u; + Tessellator::Result result = t.Tessellate( + FillType::kOdd, polyline, + [&no_indices, &indices_count](const float* vertices, size_t vertices_size, + const uint16_t* indices, size_t indices_size) { + no_indices = indices == nullptr; + indices_count = indices_size; + return true; + }); + + ASSERT_TRUE(no_indices); + ASSERT_TRUE(indices_count >= 65535); + ASSERT_EQ(result, Tessellator::Result::kSuccess); + } } } // namespace testing From 8f60191e6cb55da3e5f7645d7f88a9ba62af4aa1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 26 Sep 2023 14:57:23 -0700 Subject: [PATCH 05/11] ++ --- impeller/tessellator/tessellator_unittests.cc | 17 +++++++++-------- 1 file changed, 9 insertions(+), 8 deletions(-) diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 6054bbe8f1b7f..a530c6511c828 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -115,14 +115,15 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { auto polyline = builder.TakePath(FillType::kOdd).CreatePolyline(1.0f); bool no_indices = false; size_t indices_count = 0u; - Tessellator::Result result = t.Tessellate( - FillType::kOdd, polyline, - [&no_indices, &indices_count](const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { - no_indices = indices == nullptr; - indices_count = indices_size; - return true; - }); + Tessellator::Result result = + t.Tessellate(FillType::kOdd, polyline, + [&no_indices, &indices_count]( + const float* vertices, size_t vertices_size, + const uint16_t* indices, size_t indices_size) { + no_indices = indices == nullptr; + indices_count = indices_size; + return true; + }); ASSERT_TRUE(no_indices); ASSERT_TRUE(indices_count >= 65535); From d5bc00a32621ffece4882dc776b797579ba1ec3a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 26 Sep 2023 15:44:26 -0700 Subject: [PATCH 06/11] cleanups --- .../entity/geometry/fill_path_geometry.cc | 8 +-- impeller/geometry/geometry_benchmarks.cc | 4 +- impeller/renderer/renderer_unittests.cc | 8 +-- impeller/tessellator/c/tessellator.cc | 8 +-- impeller/tessellator/tessellator.cc | 24 +++---- impeller/tessellator/tessellator.h | 9 +-- impeller/tessellator/tessellator_unittests.cc | 65 ++++++++++--------- 7 files changed, 63 insertions(+), 63 deletions(-) diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index f237405001011..1153572f25c08 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -48,7 +48,7 @@ GeometryResult FillPathGeometry::GetPositionBuffer( const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { vertex_buffer.vertex_buffer = host_buffer.Emplace( - vertices, vertices_count * sizeof(float), alignof(float)); + vertices, vertices_count * sizeof(float) * 2, alignof(float)); if (indices != nullptr) { vertex_buffer.index_buffer = host_buffer.Emplace( indices, indices_count * sizeof(uint16_t), alignof(uint16_t)); @@ -56,7 +56,7 @@ GeometryResult FillPathGeometry::GetPositionBuffer( vertex_buffer.index_type = IndexType::k16bit; } else { vertex_buffer.index_buffer = {}; - vertex_buffer.vertex_count = indices_count; + vertex_buffer.vertex_count = vertices_count; vertex_buffer.index_type = IndexType::kNone; } return true; @@ -119,7 +119,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( [&vertex_builder, &texture_coverage, &effect_transform]( const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { - for (auto i = 0u; i < vertices_count; i += 2) { + for (auto i = 0u; i < vertices_count * 2; i += 2) { VS::PerVertexData data; Point vtx = {vertices[i], vertices[i + 1]}; data.position = vtx; @@ -128,7 +128,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( texture_coverage.size; vertex_builder.AppendVertex(data); } - FML_DCHECK(vertex_builder.GetVertexCount() == vertices_count / 2); + FML_DCHECK(vertex_builder.GetVertexCount() == vertices_count); if (indices != nullptr) { for (auto i = 0u; i < indices_count; i++) { vertex_builder.AppendIndex(indices[i]); diff --git a/impeller/geometry/geometry_benchmarks.cc b/impeller/geometry/geometry_benchmarks.cc index d3c1f9ea525c5..8bff21f3282bf 100644 --- a/impeller/geometry/geometry_benchmarks.cc +++ b/impeller/geometry/geometry_benchmarks.cc @@ -35,8 +35,8 @@ static void BM_Polyline(benchmark::State& state, Args&&... args) { if (tessellate) { tess.Tessellate( FillType::kNonZero, polyline, - [](const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { return true; }); + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return true; }); } } state.counters["SinglePointCount"] = single_point_count; diff --git a/impeller/renderer/renderer_unittests.cc b/impeller/renderer/renderer_unittests.cc index 48772d09198e1..2ecc6fab2a004 100644 --- a/impeller/renderer/renderer_unittests.cc +++ b/impeller/renderer/renderer_unittests.cc @@ -404,14 +404,14 @@ TEST_P(RendererTest, CanRenderInstanced) { .AddRect(Rect::MakeXYWH(10, 10, 100, 100)) .TakePath() .CreatePolyline(1.0f), - [&builder](const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { - for (auto i = 0u; i < vertices_size; i += 2) { + [&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_size; i++) { + for (auto i = 0u; i < indices_count; i++) { builder.AppendIndex(indices[i]); } return true; diff --git a/impeller/tessellator/c/tessellator.cc b/impeller/tessellator/c/tessellator.cc index 1bdbfde66db84..b576d4393e809 100644 --- a/impeller/tessellator/c/tessellator.cc +++ b/impeller/tessellator/c/tessellator.cc @@ -45,14 +45,14 @@ struct Vertices* Tessellate(PathBuilder* builder, std::vector points; if (Tessellator{}.Tessellate( path.GetFillType(), polyline, - [&points](const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { + [&points](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { // Results are expected to be re-duplicated. std::vector raw_points; - for (auto i = 0u; i < vertices_size; i += 2) { + for (auto i = 0u; i < vertices_count * 2; i += 2) { raw_points.emplace_back(Point{vertices[i], vertices[i + 1]}); } - for (auto i = 0u; i < indices_size; i++) { + for (auto i = 0u; i < indices_count; i++) { auto point = raw_points[indices[i]]; points.push_back(point.x); points.push_back(point.y); diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index f503675dddf52..986cbb730b68b 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -123,22 +123,22 @@ Tessellator::Result Tessellator::Tessellate( return Result::kTessellationError; } - int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize; + int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; auto vertices = tessGetVertices(tessellator); - for (int i = 0; i < vertexItemCount; i += 2) { + for (int i = 0; i < vertex_item_count; i += 2) { points.emplace_back(vertices[i], vertices[i + 1]); } - int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; + int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; auto elements = tessGetElements(tessellator); - total += elementItemCount; - for (int i = 0; i < elementItemCount; i++) { + total += element_item_count; + for (int i = 0; i < element_item_count; 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)) { + if (!callback(data.data(), total, nullptr, 0u)) { return Result::kInputError; } } else { @@ -183,7 +183,7 @@ Tessellator::Result Tessellator::Tessellate( // a uint32 index buffer, but this is done for simplicity with the other // fast path above. if (elementItemCount < 65535) { - int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize; + int vertex_item_count = tessGetVertexCount(tessellator); auto vertices = tessGetVertices(tessellator); auto elements = tessGetElements(tessellator); @@ -193,7 +193,7 @@ Tessellator::Result Tessellator::Tessellate( for (int i = 0; i < elementItemCount; i++) { indices[i] = static_cast(elements[i]); } - if (!callback(vertices, vertexItemCount, indices.data(), + if (!callback(vertices, vertex_item_count, indices.data(), elementItemCount)) { return Result::kInputError; } @@ -201,19 +201,19 @@ Tessellator::Result Tessellator::Tessellate( std::vector points; std::vector data; - int vertexItemCount = tessGetVertexCount(tessellator) * kVertexSize; + int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; auto vertices = tessGetVertices(tessellator); - for (int i = 0; i < vertexItemCount; i += 2) { + for (int i = 0; i < vertex_item_count; i += 2) { points.emplace_back(vertices[i], vertices[i + 1]); } - int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; + int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; auto elements = tessGetElements(tessellator); for (int i = 0; i < elementItemCount; i++) { data.emplace_back(points[elements[i]].x); data.emplace_back(points[elements[i]].y); } - if (!callback(data.data(), data.size(), nullptr, elementItemCount)) { + if (!callback(data.data(), element_item_count, nullptr, 0u)) { return Result::kInputError; } } diff --git a/impeller/tessellator/tessellator.h b/impeller/tessellator/tessellator.h index 97da500decbda..7cd8690da9fb4 100644 --- a/impeller/tessellator/tessellator.h +++ b/impeller/tessellator/tessellator.h @@ -47,14 +47,11 @@ class Tessellator { /// @brief A callback that returns the results of the tessellation. /// /// The index buffer may not be populated, in which case [indices] will - /// be nullptr. The [vertices_size] is the size of the vertices array - /// and not the number of vertices, which is usually vertices_size / 2. - /// In cases where the indices is nullptr, vertex_or_index_count will - /// contain the count of indices. + /// be nullptr and indices_count will be 0. using BuilderCallback = std::function; + size_t indices_count)>; //---------------------------------------------------------------------------- /// @brief Generates filled triangles from the polyline. A callback is diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index a530c6511c828..8a7ab3e084eae 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -18,8 +18,8 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { auto polyline = PathBuilder{}.TakePath().CreatePolyline(1.0f); Tessellator::Result result = t.Tessellate( FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_size, const uint16_t* indices, - size_t indices_size) { return true; }); + [](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { return true; }); ASSERT_EQ(polyline.points.size(), 0u); ASSERT_EQ(result, Tessellator::Result::kInputError); @@ -30,10 +30,11 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { Tessellator t; auto polyline = PathBuilder{}.LineTo({0, 0}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = t.Tessellate( - FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_size, const uint16_t* indices, - size_t indices_size) { return true; }); + Tessellator::Result result = + t.Tessellate(FillType::kPositive, polyline, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices_count, + size_t indices_size) { return true; }); ASSERT_EQ(polyline.points.size(), 1u); ASSERT_EQ(result, Tessellator::Result::kSuccess); } @@ -43,10 +44,11 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { Tessellator t; auto polyline = PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = t.Tessellate( - FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_size, const uint16_t* indices, - size_t indices_size) { return true; }); + Tessellator::Result result = + t.Tessellate(FillType::kPositive, polyline, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices_count, + size_t indices_size) { return true; }); ASSERT_EQ(polyline.points.size(), 2u); ASSERT_EQ(result, Tessellator::Result::kSuccess); @@ -61,10 +63,11 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { builder.AddLine({coord, coord}, {coord + 1, coord + 1}); } auto polyline = builder.TakePath().CreatePolyline(1.0f); - Tessellator::Result result = t.Tessellate( - FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_size, const uint16_t* indices, - size_t indices_size) { return true; }); + Tessellator::Result result = + t.Tessellate(FillType::kPositive, polyline, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices_count, + size_t indices_size) { return true; }); ASSERT_EQ(polyline.points.size(), 2000u); ASSERT_EQ(result, Tessellator::Result::kSuccess); @@ -75,10 +78,11 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { Tessellator t; auto polyline = PathBuilder{}.AddLine({0, 0}, {0, 1}).TakePath().CreatePolyline(1.0f); - Tessellator::Result result = t.Tessellate( - FillType::kPositive, polyline, - [](const float* vertices, size_t vertices_size, const uint16_t* indices, - size_t indices_size) { return false; }); + Tessellator::Result result = + t.Tessellate(FillType::kPositive, polyline, + [](const float* vertices, size_t vertices_count, + const uint16_t* indices_count, + size_t indices_size) { return false; }); ASSERT_EQ(polyline.points.size(), 2u); ASSERT_EQ(result, Tessellator::Result::kInputError); @@ -95,8 +99,8 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { bool no_indices = false; Tessellator::Result result = t.Tessellate( FillType::kNonZero, polyline, - [&no_indices](const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { + [&no_indices](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { no_indices = indices == nullptr; return true; }); @@ -114,19 +118,18 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { } auto polyline = builder.TakePath(FillType::kOdd).CreatePolyline(1.0f); bool no_indices = false; - size_t indices_count = 0u; - Tessellator::Result result = - t.Tessellate(FillType::kOdd, polyline, - [&no_indices, &indices_count]( - const float* vertices, size_t vertices_size, - const uint16_t* indices, size_t indices_size) { - no_indices = indices == nullptr; - indices_count = indices_size; - return true; - }); + size_t count = 0u; + Tessellator::Result result = t.Tessellate( + FillType::kOdd, polyline, + [&no_indices, &count](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { + no_indices = indices == nullptr; + count = vertices_count; + return true; + }); ASSERT_TRUE(no_indices); - ASSERT_TRUE(indices_count >= 65535); + ASSERT_TRUE(count >= 65535); ASSERT_EQ(result, Tessellator::Result::kSuccess); } } From 2ea9c227f14b65ac8c1edb068807bc861f81e12a Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Tue, 26 Sep 2023 15:53:02 -0700 Subject: [PATCH 07/11] ++ --- impeller/tessellator/tessellator.cc | 4 ---- impeller/tessellator/tessellator.h | 4 ++++ impeller/tessellator/tessellator_unittests.cc | 2 +- 3 files changed, 5 insertions(+), 5 deletions(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 986cbb730b68b..bba7ee466a28d 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -58,10 +58,6 @@ static int ToTessWindingRule(FillType fill_type) { return TESS_WINDING_ODD; } -/// @brief An arbitrary value to determine when a multi-contour non-zero fill -/// path should be split into multiple tessellations. -static constexpr size_t kMultiContourThreshold = 30u; - Tessellator::Result Tessellator::Tessellate( FillType fill_type, const Path::Polyline& polyline, diff --git a/impeller/tessellator/tessellator.h b/impeller/tessellator/tessellator.h index 7cd8690da9fb4..b6d444c5a81ba 100644 --- a/impeller/tessellator/tessellator.h +++ b/impeller/tessellator/tessellator.h @@ -44,6 +44,10 @@ class Tessellator { ~Tessellator(); + /// @brief An arbitrary value to determine when a multi-contour non-zero fill + /// path should be split into multiple tessellations. + static constexpr size_t kMultiContourThreshold = 30u; + /// @brief A callback that returns the results of the tessellation. /// /// The index buffer may not be populated, in which case [indices] will diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 8a7ab3e084eae..67c74b701f4cd 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -92,7 +92,7 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { { Tessellator t; PathBuilder builder = {}; - for (auto i = 0; i < 60; i++) { + for (auto i = 0u; i < Tessellator::kMultiContourThreshold + 1; i++) { builder.AddCircle(Point(i, i), 4); } auto polyline = builder.TakePath().CreatePolyline(1.0f); From 21588071fe40dc9a45a00fe738a6f782c2789c3e Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 27 Sep 2023 09:12:40 -0700 Subject: [PATCH 08/11] Update impeller/tessellator/tessellator.cc Co-authored-by: gaaclarke <30870216+gaaclarke@users.noreply.github.com> --- impeller/tessellator/tessellator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index bba7ee466a28d..8123a10aecd43 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -171,7 +171,7 @@ Tessellator::Result Tessellator::Tessellate( return Result::kTessellationError; } - int elementItemCount = tessGetElementCount(tessellator) * kPolygonSize; + int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; // We default to using a 16bit index buffer, but in cases where we generate // more tessellated data than this can contain we need to fall back to From 364acc483c1a3d41404174344f52e5be5f24547b Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Wed, 27 Sep 2023 09:12:48 -0700 Subject: [PATCH 09/11] Update impeller/tessellator/tessellator.cc Co-authored-by: gaaclarke <30870216+gaaclarke@users.noreply.github.com> --- impeller/tessellator/tessellator.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 8123a10aecd43..6d29b107804dd 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -178,7 +178,7 @@ Tessellator::Result Tessellator::Tessellate( // dropping the index buffer entirely. Instead code could instead switch to // a uint32 index buffer, but this is done for simplicity with the other // fast path above. - if (elementItemCount < 65535) { + if (elementItemCount < USHRT_MAX) { int vertex_item_count = tessGetVertexCount(tessellator); auto vertices = tessGetVertices(tessellator); auto elements = tessGetElements(tessellator); From 8804000e3d3c9b3a4992c44c20db4df453453324 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 27 Sep 2023 09:39:08 -0700 Subject: [PATCH 10/11] fixes --- impeller/tessellator/tessellator.cc | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 6d29b107804dd..f7c80fe747ea0 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -178,19 +178,19 @@ Tessellator::Result Tessellator::Tessellate( // dropping the index buffer entirely. Instead code could instead switch to // a uint32 index buffer, but this is done for simplicity with the other // fast path above. - if (elementItemCount < USHRT_MAX) { + if (element_item_count < USHRT_MAX) { int vertex_item_count = tessGetVertexCount(tessellator); 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 indices(elementItemCount); - for (int i = 0; i < elementItemCount; i++) { + std::vector indices(element_item_count); + for (int i = 0; i < element_item_count; i++) { indices[i] = static_cast(elements[i]); } if (!callback(vertices, vertex_item_count, indices.data(), - elementItemCount)) { + element_item_count)) { return Result::kInputError; } } else { @@ -205,7 +205,7 @@ Tessellator::Result Tessellator::Tessellate( int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; auto elements = tessGetElements(tessellator); - for (int i = 0; i < elementItemCount; i++) { + for (int i = 0; i < element_item_count; i++) { data.emplace_back(points[elements[i]].x); data.emplace_back(points[elements[i]].y); } From f65c055dbf89264251d7fb4737c0fdb1185934f1 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Wed, 27 Sep 2023 09:39:30 -0700 Subject: [PATCH 11/11] ++ --- impeller/tessellator/tessellator_unittests.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index 67c74b701f4cd..8fca675a9a9a6 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -129,7 +129,7 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { }); ASSERT_TRUE(no_indices); - ASSERT_TRUE(count >= 65535); + ASSERT_TRUE(count >= USHRT_MAX); ASSERT_EQ(result, Tessellator::Result::kSuccess); } }