From c2b8314d6b74f3af70e0da4a17639743ef8152ea Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 28 Jan 2024 11:07:36 -0800 Subject: [PATCH 1/5] [Impeller] stash tessellated data. --- impeller/geometry/path.cc | 25 +++++ impeller/geometry/path.h | 40 +++++++- .../renderer/backend/vulkan/context_vk.cc | 2 +- impeller/tessellator/tessellator.cc | 8 ++ impeller/tessellator/tessellator_unittests.cc | 99 +++++++++++++++++++ 5 files changed, 169 insertions(+), 5 deletions(-) diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 1e175decec81c..56276367d10da 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -353,4 +353,29 @@ std::optional Path::GetTransformedBoundingBox( return bounds->TransformBounds(transform); } +bool Path::HasTessellatedData(Scalar scale) const { + return ScalarNearlyEqual(scale, data_->tessellated_data.scale); +} + +const Path::TessellatedData& Path::GetTessellatedData() const { + return data_->tessellated_data; +} + +void Path::StoreTessellatedData(Scalar scale, + const float* vertices, + size_t vertices_count, + const uint16_t* indices, + size_t indices_count) const { + auto data = data_->tessellated_data; + data.scale = scale; + data.points.resize(vertices_count * 2); + data.indices.resize(indices_count); + for (auto i = 0u; i < vertices_count * 2; i++) { + data.points[i] = vertices[i]; + } + for (auto i = 0u; i < indices_count; i++) { + data.indices[i] = indices[i]; + } +} + } // namespace impeller diff --git a/impeller/geometry/path.h b/impeller/geometry/path.h index ccca96aa36391..a0af4c3a223ee 100644 --- a/impeller/geometry/path.h +++ b/impeller/geometry/path.h @@ -5,16 +5,21 @@ #ifndef FLUTTER_IMPELLER_GEOMETRY_PATH_H_ #define FLUTTER_IMPELLER_GEOMETRY_PATH_H_ +#include #include #include -#include #include #include #include "impeller/geometry/path_component.h" +#include "impeller/geometry/scalar.h" namespace impeller { +namespace testing { +class TessellatorAccess; +} + enum class Cap { kButt, kRound, @@ -178,6 +183,35 @@ class Path { private: friend class PathBuilder; + friend class Tessellator; + friend class testing::TessellatorAccess; + + struct TessellatedData { + Scalar scale = 0.0; + std::vector points; + std::vector indices; + }; + + // The following methods are not thread safe and should only be called + // by the tessellator component. This functionality should be removed + // after https://github.com/flutter/flutter/issues/123671 is fixed. + + /// @brief Whether or not there is tessellated data at or close to the + /// provided [scale]. + bool HasTessellatedData(Scalar scale) const; + + /// @brief Retrieve the tessellated data for the current path. + /// + /// You must check that this data matches the current scale with + /// [HasTessellatedData] first. + const TessellatedData& GetTessellatedData() const; + + /// @brief Update the tessellated data for the given [scale]. + void StoreTessellatedData(Scalar scale, + const float* vertices, + size_t vertices_count, + const uint16_t* indices, + size_t indices_count) const; struct ComponentIndexPair { ComponentType type = ComponentType::kLinear; @@ -209,10 +243,8 @@ class Path { std::vector components; std::vector points; std::vector contours; - std::optional bounds; - - bool locked = false; + mutable TessellatedData tessellated_data; }; explicit Path(const Data& data); diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index e573d6aa5bfb9..bb0d3cb191004 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -156,7 +156,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = settings.enable_validation; + auto enable_validation = false; //settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; diff --git a/impeller/tessellator/tessellator.cc b/impeller/tessellator/tessellator.cc index 952569f85adc8..6ca0a1aaf0369 100644 --- a/impeller/tessellator/tessellator.cc +++ b/impeller/tessellator/tessellator.cc @@ -67,6 +67,12 @@ Tessellator::Result Tessellator::Tessellate(const Path& path, if (!callback) { return Result::kInputError; } + if (path.HasTessellatedData(tolerance)) { + auto& data = path.GetTessellatedData(); + callback(data.points.data(), data.points.size() / 2, data.indices.data(), + data.indices.size()); + return Result::kSuccess; + } point_buffer_->clear(); auto polyline = @@ -144,6 +150,8 @@ Tessellator::Result Tessellator::Tessellate(const Path& path, element_item_count)) { return Result::kInputError; } + path.StoreTessellatedData(tolerance, vertices, vertex_item_count, + indices.data(), element_item_count); } else { std::vector points; std::vector data; diff --git a/impeller/tessellator/tessellator_unittests.cc b/impeller/tessellator/tessellator_unittests.cc index ec6afc069ec9f..97800bbf3a1e8 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -2,17 +2,32 @@ // Use of this source code is governed by a BSD-style license that can be // found in the LICENSE file. +#include #include "flutter/testing/testing.h" #include "gtest/gtest.h" #include "impeller/geometry/geometry_asserts.h" #include "impeller/geometry/path.h" #include "impeller/geometry/path_builder.h" +#include "impeller/geometry/scalar.h" #include "impeller/tessellator/tessellator.h" namespace impeller { namespace testing { +class TessellatorAccess { + public: + static bool HasData(const Path& path, Scalar scale) { + return path.HasTessellatedData(scale); + } + + static std::pair, std::vector> GetData( + const Path& path) { + auto& result = path.GetTessellatedData(); + return std::make_pair(result.points, result.indices); + } +}; + TEST(TessellatorTest, TessellatorBuilderReturnsCorrectResultStatus) { // Zero points. { @@ -484,5 +499,89 @@ TEST(TessellatorTest, FilledRoundRectTessellationVertices) { Rect::MakeXYWH(5000, 10000, 2000, 3000), {50, 70}); } +TEST(TessellatorTest, CachesPathData) { + Tessellator t; + auto path = PathBuilder{} + .AddCircle(Point{100, 100}, 10) + .TakePath(FillType::kPositive); + + EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); + + std::vector test_vertices; + std::vector test_indicies; + Tessellator::Result result = + t.Tessellate(path, 1.0f, + [&](const float* vertices, size_t vertices_count, + const uint16_t* indices, size_t indices_count) { + for (auto i = 0u; i < vertices_count * 2; i++) { + test_vertices.push_back(vertices[i]); + } + for (auto i = 0u; i < indices_count; i++) { + test_indicies.push_back(indices[i]); + } + return true; + }); + + EXPECT_EQ(result, Tessellator::Result::kSuccess); + EXPECT_TRUE(TessellatorAccess::HasData(path, 1.0)); + EXPECT_TRUE(TessellatorAccess::HasData(path, 1.0000001)); + EXPECT_FALSE(TessellatorAccess::HasData(path, 2.0)); + + auto [points, indices] = TessellatorAccess::GetData(path); + + ASSERT_EQ(points.size(), test_vertices.size()); + ASSERT_EQ(indices.size(), test_indicies.size()); + + for (auto i = 0u; i < test_vertices.size(); i++) { + EXPECT_EQ(test_vertices[i], points[i]); + } + for (auto i = 0u; i < test_indicies.size(); i++) { + EXPECT_EQ(test_indicies[i], indices[i]); + } +} + +TEST(TessellatorTest, CachesSingleScalarData) { + Tessellator t; + auto path = PathBuilder{} + .AddCircle(Point{100, 100}, 10) + .TakePath(FillType::kPositive); + + EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); + + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, const uint16_t* indices, + size_t indices_count) { return true; }); + + EXPECT_EQ(result, Tessellator::Result::kSuccess); + EXPECT_TRUE(TessellatorAccess::HasData(path, 1.0)); + + result = t.Tessellate( + path, 2.0f, + [](const float* vertices, size_t vertices_count, const uint16_t* indices, + size_t indices_count) { return true; }); + + EXPECT_EQ(result, Tessellator::Result::kSuccess); + EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); + EXPECT_TRUE(TessellatorAccess::HasData(path, 2.0)); +} + +TEST(TessellatorTest, DoesNotCacheFailedTessellation) { + Tessellator t; + auto path = PathBuilder{} + .AddCircle(Point{100, 100}, 10) + .TakePath(FillType::kPositive); + + EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); + + Tessellator::Result result = t.Tessellate( + path, 1.0f, + [](const float* vertices, size_t vertices_count, const uint16_t* indices, + size_t indices_count) { return false; }); + + EXPECT_EQ(result, Tessellator::Result::kTessellationError); + EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); +} + } // namespace testing } // namespace impeller From b0e1676c2147122234bb3b6ee1cc7575facbd816 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Fri, 9 Feb 2024 10:35:23 -0800 Subject: [PATCH 2/5] ++ --- impeller/renderer/backend/vulkan/context_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index af58ede5d826f..199bfc36fdb0c 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -158,7 +158,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = false; //settings.enable_validation; + auto enable_validation = false; // settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; From e384d9f5b9973b55a319969e0199ffed314a4981 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sat, 10 Feb 2024 12:21:35 -0800 Subject: [PATCH 3/5] add missing reference. --- impeller/geometry/path.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/geometry/path.cc b/impeller/geometry/path.cc index 153672f3222fd..3214a7e6a6b20 100644 --- a/impeller/geometry/path.cc +++ b/impeller/geometry/path.cc @@ -362,7 +362,7 @@ void Path::StoreTessellatedData(Scalar scale, size_t vertices_count, const uint16_t* indices, size_t indices_count) const { - auto data = data_->tessellated_data; + auto& data = data_->tessellated_data; data.scale = scale; data.points.resize(vertices_count * 2); data.indices.resize(indices_count); From 717c3e7c89fe7bba4c4b8c3630a51d5c2af348a8 Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 11 Feb 2024 11:11:49 -0800 Subject: [PATCH 4/5] enable validation. --- impeller/renderer/backend/vulkan/context_vk.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/impeller/renderer/backend/vulkan/context_vk.cc b/impeller/renderer/backend/vulkan/context_vk.cc index 973291ac13c82..d12e0dd6519f1 100644 --- a/impeller/renderer/backend/vulkan/context_vk.cc +++ b/impeller/renderer/backend/vulkan/context_vk.cc @@ -152,7 +152,7 @@ void ContextVK::Setup(Settings settings) { // 1. The user has explicitly enabled it. // 2. We are in a combination of debug mode, and running on Android. // (It's possible 2 is overly conservative and we can simplify this) - auto enable_validation = false; // settings.enable_validation; + auto enable_validation = settings.enable_validation; #if defined(FML_OS_ANDROID) && !defined(NDEBUG) enable_validation = true; From 753f6fb565ebc50d71c75b503ef70889d62f511f Mon Sep 17 00:00:00 2001 From: jonahwilliams Date: Sun, 11 Feb 2024 13:47:39 -0800 Subject: [PATCH 5/5] fix expectation. --- 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 7e3248b312205..ce99fdcb73528 100644 --- a/impeller/tessellator/tessellator_unittests.cc +++ b/impeller/tessellator/tessellator_unittests.cc @@ -579,7 +579,7 @@ TEST(TessellatorTest, DoesNotCacheFailedTessellation) { [](const float* vertices, size_t vertices_count, const uint16_t* indices, size_t indices_count) { return false; }); - EXPECT_EQ(result, Tessellator::Result::kTessellationError); + EXPECT_EQ(result, Tessellator::Result::kInputError); EXPECT_FALSE(TessellatorAccess::HasData(path, 1.0)); }