From 6624cfb967c7f1f13a98bfca816958ab620643fe Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Mon, 27 Nov 2023 09:26:36 -0800 Subject: [PATCH 1/3] [Impeller] revert non-zero tessellation optimization. (#48234) Forgot about the winding order. partially reverts https://github.com/flutter/engine/pull/46282 See https://github.com/flutter/flutter/issues/138598 Reopens https://github.com/flutter/flutter/issues/135458 --- impeller/tessellator/tessellator.cc | 191 ++++++------------ impeller/tessellator/tessellator.h | 4 - impeller/tessellator/tessellator_unittests.cc | 21 -- 3 files changed, 66 insertions(+), 150 deletions(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index f7c80fe747ea0..c8c0b19b4d912 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -78,140 +78,81 @@ Tessellator::Result Tessellator::Tessellate( constexpr int kVertexSize = 2; constexpr int kPolygonSize = 3; - // 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; + //---------------------------------------------------------------------------- + /// 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 // + ); + } - //---------------------------------------------------------------------------- - /// 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 vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; - auto vertices = tessGetVertices(tessellator); - for (int i = 0; i < vertex_item_count; i += 2) { - points.emplace_back(vertices[i], vertices[i + 1]); - } - - int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; - auto elements = tessGetElements(tessellator); - 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(); + //---------------------------------------------------------------------------- + /// 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 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 + // 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 (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(element_item_count); + for (int i = 0; i < element_item_count; i++) { + indices[i] = static_cast(elements[i]); } - if (!callback(data.data(), total, nullptr, 0u)) { + if (!callback(vertices, vertex_item_count, indices.data(), + element_item_count)) { 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) - ); + std::vector points; + std::vector data; - if (result != 1) { - return Result::kTessellationError; + int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; + auto vertices = tessGetVertices(tessellator); + points.reserve(vertex_item_count); + for (int i = 0; i < vertex_item_count; i += 2) { + points.emplace_back(vertices[i], vertices[i + 1]); } 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 - // 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 (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(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(), - element_item_count)) { - return Result::kInputError; - } - } else { - std::vector points; - std::vector data; - - int vertex_item_count = tessGetVertexCount(tessellator) * kVertexSize; - auto vertices = tessGetVertices(tessellator); - for (int i = 0; i < vertex_item_count; i += 2) { - points.emplace_back(vertices[i], vertices[i + 1]); - } - - int element_item_count = tessGetElementCount(tessellator) * kPolygonSize; - auto elements = tessGetElements(tessellator); - for (int i = 0; i < element_item_count; i++) { - data.emplace_back(points[elements[i]].x); - data.emplace_back(points[elements[i]].y); - } - if (!callback(data.data(), element_item_count, nullptr, 0u)) { - return Result::kInputError; - } + auto elements = tessGetElements(tessellator); + data.reserve(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); + } + 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 b6d444c5a81ba..7cd8690da9fb4 100644 --- a/impeller/tessellator/tessellator.h +++ b/impeller/tessellator/tessellator.h @@ -44,10 +44,6 @@ 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 8fca675a9a9a6..f3af64c7ff4cb 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -88,27 +88,6 @@ TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { ASSERT_EQ(result, Tessellator::Result::kInputError); } - // More than 30 contours, non-zero fill mode. - { - Tessellator t; - PathBuilder builder = {}; - for (auto i = 0u; i < Tessellator::kMultiContourThreshold + 1; 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_count, - const uint16_t* indices, size_t indices_count) { - 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; From 3af839bb5d424030887abc206c8cb8f86c867901 Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 28 Nov 2023 08:57:46 -0800 Subject: [PATCH 2/3] Update tessellator.cc --- 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 c8c0b19b4d912..c3d47f1d4eeab 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -90,7 +90,7 @@ Tessellator::Result Tessellator::Tessellate( ::tessAddContour(tessellator, // the C tessellator kVertexSize, // - polyline.points->data() + start_point_index, // + polyline.points.data() + start_point_index, // sizeof(Point), // end_point_index - start_point_index // ); From f9ca33178b4d8de33eb70f9ac9afab0dfd8992bd Mon Sep 17 00:00:00 2001 From: Jonah Williams Date: Tue, 28 Nov 2023 10:05:03 -0800 Subject: [PATCH 3/3] Update tessellator.cc --- impeller/tessellator/tessellator.cc | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index c3d47f1d4eeab..c8cfdbeff819f 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -91,8 +91,8 @@ Tessellator::Result Tessellator::Tessellate( ::tessAddContour(tessellator, // the C tessellator kVertexSize, // polyline.points.data() + start_point_index, // - sizeof(Point), // - end_point_index - start_point_index // + sizeof(Point), // + end_point_index - start_point_index // ); }