From 889110523591364d5b35acd95562041dff0a5ca3 Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 13 Feb 2024 19:09:24 -0800 Subject: [PATCH 1/2] [Impeller] consolidate transforms in PositionUVWriter --- .../entity/geometry/geometry_unittests.cc | 150 ++++++++++++++++++ .../entity/geometry/stroke_path_geometry.cc | 12 +- .../entity/geometry/stroke_path_geometry.h | 2 + impeller/geometry/size.h | 2 +- 4 files changed, 158 insertions(+), 8 deletions(-) diff --git a/impeller/entity/geometry/geometry_unittests.cc b/impeller/entity/geometry/geometry_unittests.cc index c2c5c74767125..b87291c5cee2e 100644 --- a/impeller/entity/geometry/geometry_unittests.cc +++ b/impeller/entity/geometry/geometry_unittests.cc @@ -4,9 +4,77 @@ #include "flutter/testing/testing.h" #include "impeller/entity/geometry/geometry.h" +#include "impeller/entity/geometry/stroke_path_geometry.h" +#include "impeller/geometry/geometry_asserts.h" #include "impeller/geometry/path_builder.h" +inline ::testing::AssertionResult SolidVerticesNear( + std::vector a, + std::vector b) { + if (a.size() != b.size()) { + return ::testing::AssertionFailure() << "Colors length does not match"; + } + for (auto i = 0u; i < b.size(); i++) { + if (!PointNear(a[i].position, b[i].position)) { + return ::testing::AssertionFailure() << "Positions are not equal."; + } + } + return ::testing::AssertionSuccess(); +} + +inline ::testing::AssertionResult TextureVerticesNear( + std::vector a, + std::vector b) { + if (a.size() != b.size()) { + return ::testing::AssertionFailure() << "Colors length does not match"; + } + for (auto i = 0u; i < b.size(); i++) { + if (!PointNear(a[i].position, b[i].position)) { + return ::testing::AssertionFailure() << "Positions are not equal."; + } + if (!PointNear(a[i].texture_coords, b[i].texture_coords)) { + return ::testing::AssertionFailure() << "Texture coords are not equal."; + } + } + return ::testing::AssertionSuccess(); +} + +#define EXPECT_SOLID_VERTICES_NEAR(a, b) \ + EXPECT_PRED2(&::SolidVerticesNear, a, b) +#define EXPECT_TEXTURE_VERTICES_NEAR(a, b) \ + EXPECT_PRED2(&::TextureVerticesNear, a, b) + namespace impeller { + +class ImpellerEntityUnitTestAccessor { + public: + static std::vector + GenerateSolidStrokeVertices(const Path::Polyline& polyline, + Scalar stroke_width, + Scalar miter_limit, + Join stroke_join, + Cap stroke_cap, + Scalar scale) { + return StrokePathGeometry::GenerateSolidStrokeVertices( + polyline, stroke_width, miter_limit, stroke_join, stroke_cap, scale); + } + + static std::vector + GenerateSolidStrokeVerticesUV(const Path::Polyline& polyline, + Scalar stroke_width, + Scalar miter_limit, + Join stroke_join, + Cap stroke_cap, + Scalar scale, + Point texture_origin, + Size texture_size, + const Matrix& effect_transform) { + return StrokePathGeometry::GenerateSolidStrokeVerticesUV( + polyline, stroke_width, miter_limit, stroke_join, stroke_cap, scale, + texture_origin, texture_size, effect_transform); + } +}; + namespace testing { TEST(EntityGeometryTest, RectGeometryCoversArea) { @@ -71,5 +139,87 @@ TEST(EntityGeometryTest, RoundRectGeometryCoversArea) { EXPECT_TRUE(geometry->CoversArea({}, Rect::MakeLTRB(1, 30, 99, 70))); } +TEST(EntityGeometryTest, StrokePathGeometryTransformOfLine) { + auto path = + PathBuilder().AddLine(Point(100, 100), Point(200, 100)).TakePath(); + auto points = std::make_unique>(); + auto polyline = + path.CreatePolyline(1.0f, std::move(points), + [&points](Path::Polyline::PointBufferPtr reclaimed) { + points = std::move(reclaimed); + }); + + auto vertices = ImpellerEntityUnitTestAccessor::GenerateSolidStrokeVertices( + polyline, 10.0f, 10.0f, Join::kBevel, Cap::kButt, 1.0); + + std::vector expected = { + {.position = Point(100.0f, 105.0f)}, // + {.position = Point(100.0f, 95.0f)}, // + {.position = Point(100.0f, 105.0f)}, // + {.position = Point(100.0f, 95.0f)}, // + {.position = Point(200.0f, 105.0f)}, // + {.position = Point(200.0f, 95.0f)}, // + {.position = Point(200.0f, 105.0f)}, // + {.position = Point(200.0f, 95.0f)}, // + }; + + EXPECT_SOLID_VERTICES_NEAR(vertices, expected); +} + +TEST(EntityGeometryTest, StrokePathGeometryUVTransformOfLine) { + auto path = + PathBuilder().AddLine(Point(100, 100), Point(200, 100)).TakePath(); + auto points = std::make_unique>(); + auto polyline = + path.CreatePolyline(1.0f, std::move(points), + [&points](Path::Polyline::PointBufferPtr reclaimed) { + points = std::move(reclaimed); + }); + + auto vertices = ImpellerEntityUnitTestAccessor::GenerateSolidStrokeVerticesUV( + polyline, 10.0f, 10.0f, Join::kBevel, Cap::kButt, 1.0, // + Point(50.0f, 40.0f), Size(20.0f, 40.0f), + Matrix::MakeScale({8.0f, 4.0f, 1.0f})); + + // uvx = (x - 50) / 20 * 8 + // uvy = (y - 40) / 40 * 4 + std::vector expected = { + { + .position = Point(100.0f, 105.0f), + .texture_coords = Point(20.0f, 6.5f), + }, + { + .position = Point(100.0f, 95.0f), + .texture_coords = Point(20.0f, 5.5f), + }, + { + .position = Point(100.0f, 105.0f), + .texture_coords = Point(20.0f, 6.5f), + }, + { + .position = Point(100.0f, 95.0f), + .texture_coords = Point(20.0f, 5.5f), + }, + { + .position = Point(200.0f, 105.0f), + .texture_coords = Point(60.0f, 6.5f), + }, + { + .position = Point(200.0f, 95.0f), + .texture_coords = Point(60.0f, 5.5f), + }, + { + .position = Point(200.0f, 105.0f), + .texture_coords = Point(60.0f, 6.5f), + }, + { + .position = Point(200.0f, 95.0f), + .texture_coords = Point(60.0f, 5.5f), + }, + }; + + EXPECT_TEXTURE_VERTICES_NEAR(vertices, expected); +} + } // namespace testing } // namespace impeller diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index a079c9ba2d25c..f3e2ca8fef52a 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -49,9 +49,9 @@ class PositionUVWriter { PositionUVWriter(Point texture_origin, Size texture_coverage, const Matrix& effect_transform) - : texture_origin_(texture_origin), - texture_coverage_(texture_coverage), - effect_transform_(effect_transform) {} + : effect_transform_(effect_transform // + .Scale(1.0f / texture_coverage) + .Translate(-texture_origin)) {} const std::vector& GetData() const { return data_; @@ -60,14 +60,12 @@ class PositionUVWriter { void AppendVertex(const Point& point) { data_.emplace_back(TextureFillVertexShader::PerVertexData{ .position = point, - .texture_coords = - effect_transform_ * (point - texture_origin_) / texture_coverage_}); + .texture_coords = effect_transform_ * point, + }); } private: std::vector data_ = {}; - const Point texture_origin_; - const Size texture_coverage_; const Matrix effect_transform_; }; diff --git a/impeller/entity/geometry/stroke_path_geometry.h b/impeller/entity/geometry/stroke_path_geometry.h index 15281cb0a6ad8..08be408ab80ff 100644 --- a/impeller/entity/geometry/stroke_path_geometry.h +++ b/impeller/entity/geometry/stroke_path_geometry.h @@ -66,7 +66,9 @@ class StrokePathGeometry final : public Geometry { Point texture_origin, Size texture_size, const Matrix& effect_transform); + friend class ImpellerBenchmarkAccessor; + friend class ImpellerEntityUnitTestAccessor; bool SkipRendering() const; diff --git a/impeller/geometry/size.h b/impeller/geometry/size.h index 5c1c9c96c9528..9841b63d32642 100644 --- a/impeller/geometry/size.h +++ b/impeller/geometry/size.h @@ -131,7 +131,7 @@ constexpr TSize operator*(U s, const TSize& p) { template >> constexpr TSize operator/(U s, const TSize& p) { - return {static_cast(s) / p.x, static_cast(s) / p.y}; + return {static_cast(s) / p.width, static_cast(s) / p.height}; } using Size = TSize; From 203869ca433336a4a04df095afef6098b69d7b8a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Wed, 14 Feb 2024 11:35:44 -0800 Subject: [PATCH 2/2] fix transform math and update unit test --- .../entity/geometry/geometry_unittests.cc | 91 ++++++++----------- .../entity/geometry/stroke_path_geometry.cc | 10 +- 2 files changed, 44 insertions(+), 57 deletions(-) diff --git a/impeller/entity/geometry/geometry_unittests.cc b/impeller/entity/geometry/geometry_unittests.cc index b87291c5cee2e..6bc8c018cfa9b 100644 --- a/impeller/entity/geometry/geometry_unittests.cc +++ b/impeller/entity/geometry/geometry_unittests.cc @@ -164,61 +164,48 @@ TEST(EntityGeometryTest, StrokePathGeometryTransformOfLine) { }; EXPECT_SOLID_VERTICES_NEAR(vertices, expected); -} -TEST(EntityGeometryTest, StrokePathGeometryUVTransformOfLine) { - auto path = - PathBuilder().AddLine(Point(100, 100), Point(200, 100)).TakePath(); - auto points = std::make_unique>(); - auto polyline = - path.CreatePolyline(1.0f, std::move(points), - [&points](Path::Polyline::PointBufferPtr reclaimed) { - points = std::move(reclaimed); - }); + { + auto uv_vertices = + ImpellerEntityUnitTestAccessor::GenerateSolidStrokeVerticesUV( + polyline, 10.0f, 10.0f, Join::kBevel, Cap::kButt, 1.0, // + Point(50.0f, 40.0f), Size(20.0f, 40.0f), + Matrix::MakeScale({8.0f, 4.0f, 1.0f})); + // uvx = ((x * 8) - 50) / 20 + // uvy = ((y * 4) - 40) / 40 + auto uv = [](const Point& p) { + return Point(((p.x * 8.0f) - 50.0f) / 20.0f, + ((p.y * 4.0f) - 40.0f) / 40.0f); + }; + std::vector uv_expected; + for (size_t i = 0; i < expected.size(); i++) { + auto p = expected[i].position; + uv_expected.push_back({.position = p, .texture_coords = uv(p)}); + } - auto vertices = ImpellerEntityUnitTestAccessor::GenerateSolidStrokeVerticesUV( - polyline, 10.0f, 10.0f, Join::kBevel, Cap::kButt, 1.0, // - Point(50.0f, 40.0f), Size(20.0f, 40.0f), - Matrix::MakeScale({8.0f, 4.0f, 1.0f})); - - // uvx = (x - 50) / 20 * 8 - // uvy = (y - 40) / 40 * 4 - std::vector expected = { - { - .position = Point(100.0f, 105.0f), - .texture_coords = Point(20.0f, 6.5f), - }, - { - .position = Point(100.0f, 95.0f), - .texture_coords = Point(20.0f, 5.5f), - }, - { - .position = Point(100.0f, 105.0f), - .texture_coords = Point(20.0f, 6.5f), - }, - { - .position = Point(100.0f, 95.0f), - .texture_coords = Point(20.0f, 5.5f), - }, - { - .position = Point(200.0f, 105.0f), - .texture_coords = Point(60.0f, 6.5f), - }, - { - .position = Point(200.0f, 95.0f), - .texture_coords = Point(60.0f, 5.5f), - }, - { - .position = Point(200.0f, 105.0f), - .texture_coords = Point(60.0f, 6.5f), - }, - { - .position = Point(200.0f, 95.0f), - .texture_coords = Point(60.0f, 5.5f), - }, - }; + EXPECT_TEXTURE_VERTICES_NEAR(uv_vertices, uv_expected); + } - EXPECT_TEXTURE_VERTICES_NEAR(vertices, expected); + { + auto uv_vertices = + ImpellerEntityUnitTestAccessor::GenerateSolidStrokeVerticesUV( + polyline, 10.0f, 10.0f, Join::kBevel, Cap::kButt, 1.0, // + Point(50.0f, 40.0f), Size(20.0f, 40.0f), + Matrix::MakeTranslation({8.0f, 4.0f})); + // uvx = ((x + 8) - 50) / 20 + // uvy = ((y + 4) - 40) / 40 + auto uv = [](const Point& p) { + return Point(((p.x + 8.0f) - 50.0f) / 20.0f, + ((p.y + 4.0f) - 40.0f) / 40.0f); + }; + std::vector uv_expected; + for (size_t i = 0; i < expected.size(); i++) { + auto p = expected[i].position; + uv_expected.push_back({.position = p, .texture_coords = uv(p)}); + } + + EXPECT_TEXTURE_VERTICES_NEAR(uv_vertices, uv_expected); + } } } // namespace testing diff --git a/impeller/entity/geometry/stroke_path_geometry.cc b/impeller/entity/geometry/stroke_path_geometry.cc index f3e2ca8fef52a..dc3ffae5ab1e6 100644 --- a/impeller/entity/geometry/stroke_path_geometry.cc +++ b/impeller/entity/geometry/stroke_path_geometry.cc @@ -49,9 +49,9 @@ class PositionUVWriter { PositionUVWriter(Point texture_origin, Size texture_coverage, const Matrix& effect_transform) - : effect_transform_(effect_transform // - .Scale(1.0f / texture_coverage) - .Translate(-texture_origin)) {} + : uv_transform_(Matrix::MakeScale(1 / texture_coverage) * + Matrix::MakeTranslation(-texture_origin) * + effect_transform) {} const std::vector& GetData() const { return data_; @@ -60,13 +60,13 @@ class PositionUVWriter { void AppendVertex(const Point& point) { data_.emplace_back(TextureFillVertexShader::PerVertexData{ .position = point, - .texture_coords = effect_transform_ * point, + .texture_coords = uv_transform_ * point, }); } private: std::vector data_ = {}; - const Matrix effect_transform_; + const Matrix uv_transform_; }; template