From 23e60b7794bb0060a413a6996fe9659c83383b0a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 7 Nov 2023 16:36:20 -0800 Subject: [PATCH 1/2] [Impeller] Add Rect::GetNormalizingTransform to handle UV coordinate conversion --- .../entity/geometry/fill_path_geometry.cc | 13 +- impeller/entity/geometry/geometry.cc | 5 +- impeller/entity/geometry/vertices_geometry.cc | 8 +- impeller/geometry/rect.h | 29 ++++ impeller/geometry/rect_unittests.cc | 150 +++++++++++++++++- 5 files changed, 185 insertions(+), 20 deletions(-) diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 1153572f25c08..5eade4f5d8502 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -82,6 +82,9 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( RenderPass& pass) { using VS = TextureFillVertexShader; + auto uv_transform = + effect_transform * texture_coverage.GetNormalizingTransform(); + if (path_.GetFillType() == FillType::kNonZero && // path_.IsConvex()) { auto [points, indices] = TessellateConvex( @@ -93,9 +96,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( for (auto i = 0u; i < points.size(); i++) { VS::PerVertexData data; data.position = points[i]; - data.texture_coords = effect_transform * - (points[i] - texture_coverage.origin) / - texture_coverage.size; + data.texture_coords = uv_transform * points[i]; vertex_builder.AppendVertex(data); } for (auto i = 0u; i < indices.size(); i++) { @@ -116,16 +117,14 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( auto tesselation_result = renderer.GetTessellator()->Tessellate( path_.GetFillType(), path_.CreatePolyline(entity.GetTransformation().GetMaxBasisLength()), - [&vertex_builder, &texture_coverage, &effect_transform]( + [&vertex_builder, &uv_transform]( 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; Point vtx = {vertices[i], vertices[i + 1]}; data.position = vtx; - data.texture_coords = effect_transform * - (vtx - texture_coverage.origin) / - texture_coverage.size; + data.texture_coords = uv_transform * vtx; vertex_builder.AppendVertex(data); } FML_DCHECK(vertex_builder.GetVertexCount() == vertices_count); diff --git a/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index cff1770f134fb..2bcea6f589a2b 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/impeller/entity/geometry/geometry.cc @@ -75,12 +75,13 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect, RenderPass& pass) { auto& host_buffer = pass.GetTransientsBuffer(); + auto uv_transform = + effect_transform * texture_coverage.GetNormalizingTransform(); std::vector data(8); auto points = source_rect.GetPoints(); for (auto i = 0u, j = 0u; i < 8; i += 2, j++) { data[i] = points[j]; - data[i + 1] = effect_transform * (points[j] - texture_coverage.origin) / - texture_coverage.size; + data[i + 1] = uv_transform * points[j]; } return GeometryResult{ diff --git a/impeller/entity/geometry/vertices_geometry.cc b/impeller/entity/geometry/vertices_geometry.cc index 23a1c32a5f0b1..0709f6c292992 100644 --- a/impeller/entity/geometry/vertices_geometry.cc +++ b/impeller/entity/geometry/vertices_geometry.cc @@ -227,8 +227,8 @@ GeometryResult VerticesGeometry::GetPositionUVBuffer( auto index_count = indices_.size(); auto vertex_count = vertices_.size(); - auto size = texture_coverage.size; - auto origin = texture_coverage.origin; + auto uv_transform = + effect_transform * texture_coverage.GetNormalizingTransform(); auto has_texture_coordinates = HasTextureCoordinates(); std::vector vertex_data(vertex_count); { @@ -236,9 +236,7 @@ GeometryResult VerticesGeometry::GetPositionUVBuffer( auto vertex = vertices_[i]; auto texture_coord = has_texture_coordinates ? texture_coordinates_[i] : vertices_[i]; - auto uv = - effect_transform * Point((texture_coord.x - origin.x) / size.width, - (texture_coord.y - origin.y) / size.height); + auto uv = uv_transform * texture_coord; // From experimentation we need to clamp these values to < 1.0 or else // there can be flickering. vertex_data[i] = { diff --git a/impeller/geometry/rect.h b/impeller/geometry/rect.h index fa33757318d17..4c5fbcefffc12 100644 --- a/impeller/geometry/rect.h +++ b/impeller/geometry/rect.h @@ -214,6 +214,35 @@ struct TRect { return TRect::MakePointBounds(points.begin(), points.end()).value(); } + /// @brief Constructs a Matrix that will map all points in the coordinate + /// space of the rectangle into a new normalized coordinate space + /// where the upper left corner of the rectangle maps to (0, 0) + /// and the lower right corner of the rectangle maps to (1, 1). + /// + /// Empty and non-finite rectangles will return a zero-scaling + /// transform that maps all points to (0, 0). + constexpr Matrix GetNormalizingTransform() const { + if (!IsEmpty()) { + Scalar sx = 1.0 / size.width; + Scalar sy = 1.0 / size.height; + Scalar tx = origin.x * -sx; + Scalar ty = origin.y * -sy; + + // Exclude NaN and infinities and either scale underflowing to zero + if (sx != 0.0 && sy != 0.0 && 0.0 * sx * sy * tx * ty == 0.0) { + // clang-format off + return Matrix( sx, 0.0f, 0.0f, 0.0f, + 0.0f, sy, 0.0f, 0.0f, + 0.0f, 0.0f, 1.0f, 0.0f, + tx, ty, 0.0f, 1.0f); + // clang-format on + } + } + + // Map all coordinates to the origin. + return Matrix::MakeScale({0.0f, 0.0f, 1.0f}); + } + constexpr TRect Union(const TRect& o) const { auto this_ltrb = GetLTRB(); auto other_ltrb = o.GetLTRB(); diff --git a/impeller/geometry/rect_unittests.cc b/impeller/geometry/rect_unittests.cc index 27bedfab38f09..c9abe94b1244b 100644 --- a/impeller/geometry/rect_unittests.cc +++ b/impeller/geometry/rect_unittests.cc @@ -14,14 +14,14 @@ namespace testing { TEST(RectTest, RectOriginSizeGetters) { { Rect r = Rect::MakeOriginSize({10, 20}, {50, 40}); - ASSERT_EQ(r.GetOrigin(), Point(10, 20)); - ASSERT_EQ(r.GetSize(), Size(50, 40)); + EXPECT_EQ(r.GetOrigin(), Point(10, 20)); + EXPECT_EQ(r.GetSize(), Size(50, 40)); } { Rect r = Rect::MakeLTRB(10, 20, 50, 40); - ASSERT_EQ(r.GetOrigin(), Point(10, 20)); - ASSERT_EQ(r.GetSize(), Size(40, 20)); + EXPECT_EQ(r.GetOrigin(), Point(10, 20)); + EXPECT_EQ(r.GetSize(), Size(40, 20)); } } @@ -44,14 +44,152 @@ TEST(RectTest, RectMakeSize) { Size s(100, 200); IRect r = IRect::MakeSize(s); IRect expected = IRect::MakeLTRB(0, 0, 100, 200); - ASSERT_EQ(r, expected); + EXPECT_EQ(r, expected); } { ISize s(100, 200); IRect r = IRect::MakeSize(s); IRect expected = IRect::MakeLTRB(0, 0, 100, 200); - ASSERT_EQ(r, expected); + EXPECT_EQ(r, expected); + } +} + +TEST(RectTest, RectGetNormalizingTransform) { + { + // Checks for expected matrix values + + auto r = Rect::MakeXYWH(100, 200, 200, 400); + + EXPECT_EQ(r.GetNormalizingTransform(), + Matrix::MakeScale({0.005, 0.0025, 1.0}) * + Matrix::MakeTranslation({-100, -200})); + } + + { + // Checks for expected transformation of points relative to the rect + + auto r = Rect::MakeLTRB(300, 500, 400, 700); + auto m = r.GetNormalizingTransform(); + + // The 4 corners of the rect => (0, 0) to (1, 1) + EXPECT_EQ(m * Point(300, 500), Point(0, 0)); + EXPECT_EQ(m * Point(400, 500), Point(1, 0)); + EXPECT_EQ(m * Point(400, 700), Point(1, 1)); + EXPECT_EQ(m * Point(300, 700), Point(0, 1)); + + // The center => (0.5, 0.5) + EXPECT_EQ(m * Point(350, 600), Point(0.5, 0.5)); + + // Outside the 4 corners => (-1, -1) to (2, 2) + EXPECT_EQ(m * Point(200, 300), Point(-1, -1)); + EXPECT_EQ(m * Point(500, 300), Point(2, -1)); + EXPECT_EQ(m * Point(500, 900), Point(2, 2)); + EXPECT_EQ(m * Point(200, 900), Point(-1, 2)); + } + + { + // Checks for behavior with empty rects + + auto zero = Matrix::MakeScale({0.0, 0.0, 1.0}); + + // Empty for width and/or height == 0 + EXPECT_EQ(Rect::MakeXYWH(10, 10, 0, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, 0).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 0, 0).GetNormalizingTransform(), zero); + + // Empty for width and/or height < 0 + EXPECT_EQ(Rect::MakeXYWH(10, 10, -1, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, -1).GetNormalizingTransform(), zero); + EXPECT_EQ(Rect::MakeXYWH(10, 10, -1, -1).GetNormalizingTransform(), zero); + } + + { + // Checks for behavior with non-finite rects + + auto z = Matrix::MakeScale({0.0, 0.0, 1.0}); + auto nan = std::numeric_limits::quiet_NaN(); + auto inf = std::numeric_limits::infinity(); + + // Non-finite for width and/or height == nan + EXPECT_EQ(Rect::MakeXYWH(10, 10, nan, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, nan).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, nan, nan).GetNormalizingTransform(), z); + + // Non-finite for width and/or height == inf + EXPECT_EQ(Rect::MakeXYWH(10, 10, inf, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, inf).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, inf, inf).GetNormalizingTransform(), z); + + // Non-finite for width and/or height == -inf + EXPECT_EQ(Rect::MakeXYWH(10, 10, -inf, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, 10, -inf).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, 10, -inf, -inf).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == nan + EXPECT_EQ(Rect::MakeXYWH(nan, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, nan, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(nan, nan, 10, 10).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == inf + EXPECT_EQ(Rect::MakeXYWH(inf, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, inf, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(inf, inf, 10, 10).GetNormalizingTransform(), z); + + // Non-finite for origin X and/or Y == -inf + EXPECT_EQ(Rect::MakeXYWH(-inf, 10, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(10, -inf, 10, 10).GetNormalizingTransform(), z); + EXPECT_EQ(Rect::MakeXYWH(-inf, -inf, 10, 10).GetNormalizingTransform(), z); + } +} + +TEST(RectTest, IRectGetNormalizingTransform) { + { + // Checks for expected matrix values + + auto r = IRect::MakeXYWH(100, 200, 200, 400); + + EXPECT_EQ(r.GetNormalizingTransform(), + Matrix::MakeScale({0.005, 0.0025, 1.0}) * + Matrix::MakeTranslation({-100, -200})); + } + + { + // Checks for expected transformation of points relative to the rect + + auto r = IRect::MakeLTRB(300, 500, 400, 700); + auto m = r.GetNormalizingTransform(); + + // The 4 corners of the rect => (0, 0) to (1, 1) + EXPECT_EQ(m * Point(300, 500), Point(0, 0)); + EXPECT_EQ(m * Point(400, 500), Point(1, 0)); + EXPECT_EQ(m * Point(400, 700), Point(1, 1)); + EXPECT_EQ(m * Point(300, 700), Point(0, 1)); + + // The center => (0.5, 0.5) + EXPECT_EQ(m * Point(350, 600), Point(0.5, 0.5)); + + // Outside the 4 corners => (-1, -1) to (2, 2) + EXPECT_EQ(m * Point(200, 300), Point(-1, -1)); + EXPECT_EQ(m * Point(500, 300), Point(2, -1)); + EXPECT_EQ(m * Point(500, 900), Point(2, 2)); + EXPECT_EQ(m * Point(200, 900), Point(-1, 2)); + } + + { + // Checks for behavior with empty rects + + auto zero = Matrix::MakeScale({0.0, 0.0, 1.0}); + + // Empty for width and/or height == 0 + EXPECT_EQ(IRect::MakeXYWH(10, 10, 0, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 10, 0).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 0, 0).GetNormalizingTransform(), zero); + + // Empty for width and/or height < 0 + EXPECT_EQ(IRect::MakeXYWH(10, 10, -1, 10).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, 10, -1).GetNormalizingTransform(), zero); + EXPECT_EQ(IRect::MakeXYWH(10, 10, -1, -1).GetNormalizingTransform(), zero); } } From 4e1604fdb6339bed67f7908ba5fc5f04bbff010a Mon Sep 17 00:00:00 2001 From: Jim Graham Date: Tue, 7 Nov 2023 19:33:58 -0800 Subject: [PATCH 2/2] update playground tests to include all modified cases and fix matrix ordering --- impeller/aiks/aiks_unittests.cc | 46 ++++++++++++++++--- .../entity/geometry/fill_path_geometry.cc | 2 +- impeller/entity/geometry/geometry.cc | 2 +- impeller/entity/geometry/vertices_geometry.cc | 2 +- 4 files changed, 43 insertions(+), 9 deletions(-) diff --git a/impeller/aiks/aiks_unittests.cc b/impeller/aiks/aiks_unittests.cc index b646c6dbe0bfc..3191257a14d44 100644 --- a/impeller/aiks/aiks_unittests.cc +++ b/impeller/aiks/aiks_unittests.cc @@ -206,12 +206,23 @@ void CanRenderTiledTexture(AiksTest* aiks_test, canvas.DrawRect(Rect::MakeXYWH(0, 0, 600, 600), paint); } - // Should not change the image. - PathBuilder path_builder; - path_builder.AddCircle({150, 150}, 150); - path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); - paint.style = Paint::Style::kFill; - canvas.DrawPath(path_builder.TakePath(), paint); + { + // Should not change the image. + PathBuilder path_builder; + path_builder.AddCircle({150, 150}, 150); + path_builder.AddRoundedRect(Rect::MakeLTRB(300, 300, 600, 600), 10); + paint.style = Paint::Style::kFill; + canvas.DrawPath(path_builder.TakePath(), paint); + } + + { + // Should not change the image. Tests the Convex short-cut code. + PathBuilder path_builder; + path_builder.AddCircle({150, 450}, 150); + path_builder.SetConvexity(Convexity::kConvex); + paint.style = Paint::Style::kFill; + canvas.DrawPath(path_builder.TakePath(), paint); + } ASSERT_TRUE(aiks_test->OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } @@ -3744,6 +3755,29 @@ TEST_P(AiksTest, VerticesGeometryUVPositionData) { ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); } +// Regression test for https://github.com/flutter/flutter/issues/135441 . +TEST_P(AiksTest, VerticesGeometryUVPositionDataWithTranslate) { + Canvas canvas; + Paint paint; + auto texture = CreateTextureForFixture("table_mountain_nx.png"); + + paint.color_source = ColorSource::MakeImage( + texture, Entity::TileMode::kClamp, Entity::TileMode::kClamp, {}, + Matrix::MakeTranslation({100.0, 100.0})); + + auto vertices = {Point(0, 0), Point(texture->GetSize().width, 0), + Point(0, texture->GetSize().height)}; + std::vector indices = {0u, 1u, 2u}; + std::vector texture_coordinates = {}; + std::vector vertex_colors = {}; + auto geometry = std::make_shared( + vertices, indices, texture_coordinates, vertex_colors, + Rect::MakeLTRB(0, 0, 1, 1), VerticesGeometry::VertexMode::kTriangleStrip); + + canvas.DrawVertices(geometry, BlendMode::kSourceOver, paint); + ASSERT_TRUE(OpenPlaygroundHere(canvas.EndRecordingAsPicture())); +} + TEST_P(AiksTest, ClearBlendWithBlur) { Canvas canvas; Paint white; diff --git a/impeller/entity/geometry/fill_path_geometry.cc b/impeller/entity/geometry/fill_path_geometry.cc index 5eade4f5d8502..4cd025aef0422 100644 --- a/impeller/entity/geometry/fill_path_geometry.cc +++ b/impeller/entity/geometry/fill_path_geometry.cc @@ -83,7 +83,7 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer( using VS = TextureFillVertexShader; auto uv_transform = - effect_transform * texture_coverage.GetNormalizingTransform(); + texture_coverage.GetNormalizingTransform() * effect_transform; if (path_.GetFillType() == FillType::kNonZero && // path_.IsConvex()) { diff --git a/impeller/entity/geometry/geometry.cc b/impeller/entity/geometry/geometry.cc index 2bcea6f589a2b..29d880a86c86b 100644 --- a/impeller/entity/geometry/geometry.cc +++ b/impeller/entity/geometry/geometry.cc @@ -76,7 +76,7 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect, auto& host_buffer = pass.GetTransientsBuffer(); auto uv_transform = - effect_transform * texture_coverage.GetNormalizingTransform(); + texture_coverage.GetNormalizingTransform() * effect_transform; std::vector data(8); auto points = source_rect.GetPoints(); for (auto i = 0u, j = 0u; i < 8; i += 2, j++) { diff --git a/impeller/entity/geometry/vertices_geometry.cc b/impeller/entity/geometry/vertices_geometry.cc index 0709f6c292992..42af1e49c0a46 100644 --- a/impeller/entity/geometry/vertices_geometry.cc +++ b/impeller/entity/geometry/vertices_geometry.cc @@ -228,7 +228,7 @@ GeometryResult VerticesGeometry::GetPositionUVBuffer( auto index_count = indices_.size(); auto vertex_count = vertices_.size(); auto uv_transform = - effect_transform * texture_coverage.GetNormalizingTransform(); + texture_coverage.GetNormalizingTransform() * effect_transform; auto has_texture_coordinates = HasTextureCoordinates(); std::vector vertex_data(vertex_count); {