Skip to content

Commit

Permalink
[Impeller] Add Rect::GetNormalizingTransform to handle UV coordinate …
Browse files Browse the repository at this point in the history
…conversion (flutter#47775)

Three places in the code were manually computing the UV coordinates relative to a texture coverage rectangle while also transforming the points. This change will make it easier both to compute the UV conversion matrix and also to consolidate it with the transform that was already being applied to streamline the total computations.
  • Loading branch information
flar authored Nov 8, 2023
1 parent 9250de9 commit b5a151f
Show file tree
Hide file tree
Showing 6 changed files with 225 additions and 26 deletions.
46 changes: 40 additions & 6 deletions impeller/aiks/aiks_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
}
Expand Down Expand Up @@ -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<uint16_t> indices = {0u, 1u, 2u};
std::vector<Point> texture_coordinates = {};
std::vector<Color> vertex_colors = {};
auto geometry = std::make_shared<VerticesGeometry>(
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;
Expand Down
13 changes: 6 additions & 7 deletions impeller/entity/geometry/fill_path_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -82,6 +82,9 @@ GeometryResult FillPathGeometry::GetPositionUVBuffer(
RenderPass& pass) {
using VS = TextureFillVertexShader;

auto uv_transform =
texture_coverage.GetNormalizingTransform() * effect_transform;

if (path_.GetFillType() == FillType::kNonZero && //
path_.IsConvex()) {
auto [points, indices] = TessellateConvex(
Expand All @@ -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++) {
Expand All @@ -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);
Expand Down
5 changes: 3 additions & 2 deletions impeller/entity/geometry/geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -75,12 +75,13 @@ GeometryResult ComputeUVGeometryForRect(Rect source_rect,
RenderPass& pass) {
auto& host_buffer = pass.GetTransientsBuffer();

auto uv_transform =
texture_coverage.GetNormalizingTransform() * effect_transform;
std::vector<Point> 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{
Expand Down
8 changes: 3 additions & 5 deletions impeller/entity/geometry/vertices_geometry.cc
Original file line number Diff line number Diff line change
Expand Up @@ -227,18 +227,16 @@ 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 =
texture_coverage.GetNormalizingTransform() * effect_transform;
auto has_texture_coordinates = HasTextureCoordinates();
std::vector<VS::PerVertexData> vertex_data(vertex_count);
{
for (auto i = 0u; i < vertex_count; i++) {
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] = {
Expand Down
29 changes: 29 additions & 0 deletions impeller/geometry/rect.h
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
150 changes: 144 additions & 6 deletions impeller/geometry/rect_unittests.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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));
}
}

Expand All @@ -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<Scalar>::quiet_NaN();
auto inf = std::numeric_limits<Scalar>::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);
}
}

Expand Down

0 comments on commit b5a151f

Please sign in to comment.