Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Rotation of AffineTransform seems to offset the image by 1 pixel #2753

Closed
4 tasks done
Socolin opened this issue Jun 19, 2024 · 29 comments · Fixed by #2795
Closed
4 tasks done

Rotation of AffineTransform seems to offset the image by 1 pixel #2753

Socolin opened this issue Jun 19, 2024 · 29 comments · Fixed by #2795

Comments

@Socolin
Copy link

Socolin commented Jun 19, 2024

Prerequisites

  • I have written a descriptive issue title
  • I have verified that I am running the latest version of ImageSharp
  • I have verified if the problem exist in both DEBUG and RELEASE mode
  • I have searched open and closed issues to ensure it has not already been reported

ImageSharp version

3.1.4

Other ImageSharp packages and versions

ImageSharp.Drawing 2.1.3

Environment (Operating system, version and so on)

Ubuntu 22.04, AMD Ryzen 9 7950X

.NET Framework version

8.0

Description

When using the rotation through the AffineTransformBuilder the result is not the same as the .Rotate() there seems to be 1 pixel offset.

Here the sample code of what I would expect

using (var img = new Image<Rgba32>(100, 100, Color.DimGray))
{
	img.Mutate(c => c
		.Rotate(180)
		.Fill(Color.Aqua, new RectangleF(49, 49, 2, 2))
	);
	img.Save("rotation-rotate.png");
}

Which give this result:
image

Steps to Reproduce

When I do the same with the .Transform() I'm getting this:

using (var img = new Image<Rgb24>(100, 100, Color.DimGray))
{
	img.Mutate(c => c
		.Transform(new  AffineTransformBuilder().AppendRotationDegrees(180))
		.Fill(Color.Aqua, new RectangleF(49, 49, 2, 2))
	);
	img.Save("rotation-center.png");
}

image
image

Another example with non centered rotation

for (int i = 0; i <= 10; i++)
{

	using (var img = new Image<Rgba32>(100, 100, Color.DimGray))
	{
		img.Mutate(c => c
			.Transform(new  AffineTransformBuilder().AppendTranslation(new Vector2(100, 100)))
			.Transform(new  AffineTransformBuilder().AppendRotationDegrees(i * 18, new Vector2(100, 100)))
			.Fill(Color.Aqua, new RectangleF(99, 99, 2, 2))
		);
		img.Save($"rotation-{i}.png");
	}
}

image
image

The rotation seems to be centered 1 pixel too much toward the bottom-right

Images

No response

@tocsoft
Copy link
Member

tocsoft commented Jun 19, 2024

I'm not 100% up on this area but wouldn't the bottom right location be (99, 99) as the pixels are zero indexed?

I can't remember fully correctly but it could be confusion caused by the difference in how Drawing vs pixel manipulation work where, I believe, one is pixel centre based on the other is pixel boundary based.

My caveat here is that I might be entirely wrong about all this as I'm trying to dredge up some memories from quite some time ago.

@JimBobSquarePants
Copy link
Member

Yeah, I think you’re diagnosis is correct; it should be zero based.

@Socolin
Copy link
Author

Socolin commented Jun 19, 2024

Just to clarify because I'm not sure about what you said, is the problem with my example ? or is it a bug in ImageSharp ?

@tocsoft
Copy link
Member

tocsoft commented Jun 19, 2024

Just to clarify because I'm not sure about what you said, is the problem with my example ? or is it a bug in ImageSharp ?

Taking a closer look at your code examples it looks like a bug for the
.Transform(new AffineTransformBuilder().AppendRotationDegrees(180)) to me.

The .Transform(new AffineTransformBuilder().AppendRotationDegrees(i * 18, new Vector2(100, 100))) case however looks more suspect as I would have expected that line to have been a rotation around [99, 99] but if there's a bug in of those call sites then there could very will be a bug that effects both.

@Socolin
Copy link
Author

Socolin commented Jun 19, 2024

Ok thanks.

The .Transform(new AffineTransformBuilder().AppendRotationDegrees(i * 18, new Vector2(100, 100))) case however looks more suspect as I would have expected that line to have been a rotation around [99, 99] but if there's a bug in of those call sites then there could very will be a bug that effects both.

When it's rotating arround 100, 100, as I understand it, it should rotate arround the bottom-right edge of the pixel 99,99

So here another example, I resized it and added a cross to show where I think the rotation point should be

Before rotation
image
After rotation
image

using (var img = new Image<Rgba32>(8, 8, Color.DimGray))
{
	img.Mutate(c => c
		.Transform(new  AffineTransformBuilder().AppendTranslation(new Vector2(8, 8)))
		.Transform(new  AffineTransformBuilder().AppendRotationDegrees(180, new Vector2(8, 8)))
		.Fill(Color.Aqua, new RectangleF(7, 7, 2, 2))
		.Resize(new Size(800, 800), KnownResamplers.Box, true)
		.DrawLine(Color.Red, 1, new PointF(400, 350), new PointF(400, 450))
		.DrawLine(Color.Red, 1, new PointF(350, 400), new PointF(450, 400))
		.BackgroundColor(Color.DarkGreen)
	);
	img.Save($"rotation-180.png");
}

@JimBobSquarePants
Copy link
Member

I think I know what the issue is here. Will have a look over the weekend.

@JimBobSquarePants
Copy link
Member

I was right.... I was hoping I wasn't.

The result is offset by 1 pixel in each direction because the transformation matrix is centered using a 1-based coordinate system, which assumes the image's center is at (width * 0.5, height * 0.5). However, image pixels are zero-based, meaning their coordinates start from 0. This discrepancy causes a misalignment, as the matrix does not account for the zero-based nature of pixel indices, leading to a 1-pixel offset when the transformation is applied.

Fix:

To correct this issue, we need to use two separate matrices:

  1. Transformation Matrix for Pixel Operations: This matrix accounts for the zero-based nature of pixel indices. It adjusts the center calculation to ensure the transformations (translation, rotation, etc.) align correctly with the pixel grid. This matrix is used to perform the actual image transformation.

  2. Bounding Box Calculation Matrix: This matrix does not adjust for the zero-based pixel grid but instead accurately represents the intended transformation. It is used to compute the transformed image's size and bounds by transforming the image's corners and determining the extents.

By using these two matrices, we can ensure that both the pixel operations and the size/bounds calculations are handled correctly, resolving the 1-pixel offset issue and maintaining accurate and expected transformation results.

I'll get stuck in...

@JimBobSquarePants
Copy link
Member

PR opened

@JimBobSquarePants
Copy link
Member

Fixed with 3.1.5

@Socolin
Copy link
Author

Socolin commented Jul 30, 2024

@JimBobSquarePants I just tested and the problem is still present, AppendRotationDegrees(float degrees, Vector2 origin) is not using the new matrix

using (var img = new Image<Rgba32>(8, 8, Color.DimGray))
{
    img.Mutate(c => c
        .Transform(new AffineTransformBuilder().AppendTranslation(new Vector2(8, 8)))
        .Transform(new AffineTransformBuilder().AppendRotationDegrees(180, new Vector2(8, 8)))
        .Fill(Color.Aqua, new RectangleF(7, 7, 2, 2))
        .Resize(new Size(800, 800), KnownResamplers.Box, true)
        .DrawLine(Color.Red, 1, new PointF(400, 350), new PointF(400, 450))
        .DrawLine(Color.Red, 1, new PointF(350, 400), new PointF(450, 400))
        .BackgroundColor(Color.DarkGreen)
    );
    img.Save($"rotation-180.png");
}

@JimBobSquarePants
Copy link
Member

You're passing your own origin there. Nothing to do with us.

@Socolin
Copy link
Author

Socolin commented Jul 31, 2024

I ask it to rotate around a specific point and it does rotate around the point 1 pixel off, I think there is a bug.

Even more that If I offset the point by 1 to compensate this, I get a cropped image in the result (1 pixel missing on the border)

@JimBobSquarePants
Copy link
Member

Think about it like this. All the methods you are using are syntactic sugar around creating two Matrix3x2 transformations.

Transform(new AffineTransformBuilder().AppendTranslation(new Vector2(8, 8)))

Creates a new translation matrix that exactly matches:

Matrix3x2.CreateTranslation(new Vector2(8, 8))
 M11         | 1
-------------|-----
 M12         | 0
 M21         | 0
 M22         | 1
 M31         | 8
 M32         | 8
 IsIdentity  | False
 Translation | <8, 8>

The translation moves the image by 8 pixels along both the X and Y axes. We expand the canvas to compensate.

Next, you apply a rotation:

Transform(new AffineTransformBuilder().AppendRotationDegrees(180, new Vector2(8, 8)))

This creates a new rotation matrix that exactly matches:

Matrix3x2.CreateRotation(GeometryUtilities.DegreeToRadian(180), new Vector2(8,8))
 M11         | -1
-------------|-----
 M12         |  0
 M21         | -0
 M22         | -1
 M31         | 16
 M32         | 16
 IsIdentity  | False
 Translation | <16, 16>

This rotates the image 180 degrees around the point (8, 8). Let's see what happens when we apply this rotation to the four corners of your 8x8 square, which now sits at position [8, 8]:

Matrix3x2 r180 = Matrix3x2.CreateRotation(GeometryUtilities.DegreeToRadian(180), new Vector2(8, 8));

// TL
Vector2.Transform(new Vector2(8, 8), r180); // [8, 8]

// TR
Vector2.Transform(new Vector2(15, 8), r180); // [1, 8]

// BR
Vector2.Transform(new Vector2(15, 15), r180); // [1, 1]

// BL
Vector2.Transform(new Vector2(8, 15), r180); // [8, 1]

Given that image pixel locations are zero-based, you can see how the rotation shifts the coordinates to unexpected positions.

Remember:

  • Translation moves the image by the specified number of pixels.
  • Rotation rotates the image around a specified point, based on zero-based coordinates.

Since your image is 16x16, to rotate it around the geometric center, you should use:

.Transform(new AffineTransformBuilder().AppendRotationDegrees(180, new Vector2(7.5F, 7.5F)))

This adjustment rotates the image around its true center, ensuring it stays properly aligned within its bounds.

Does this now make sense?

@Socolin
Copy link
Author

Socolin commented Jul 31, 2024

I think, now, I'm getting what is happening.

What I was expecting with AppendRotationDegrees and a rotation center was something like this

output.mp4

My understanding was that pixels were 1x1 squares, rotating around the specified axis. For example, when rotating around (0, 0), I expected the pixel at (0, 0) (a square between (0, 0) and (1, 1)) to end up between (0, 0) and (-1, -1).

However, it appears that in ImageSharp, pixels are treated as points. Thus, rotating the pixel (0, 0) around (0, 0) results in no movement. This makes sense if we consider pixels as being drawn from (-0.5, -0.5) to (0.5, 0.5) on the plane.

To illustrate:
I was seeing the pixels like this :

image

But it seems they're actually treated more like this:

image

Assuming this is right I have new questions:

  1. Would this be worth mentioning in the documentation of the function? I feel if I was confused by this, it could confuse other people.
  2. I'm using this as a workaround, but my use case is to draw an image on another with a rotation (I might do a PR at some point). If such a function were to be added, would it make sense to consider the pasted image as a surface on a plane (like in the video)?

And thanks a lot for taking the time for the explanations.

@christianrondeau
Copy link

christianrondeau commented Aug 1, 2024

One way I think I understand this (I'm saying the same thing as @Socolin but differently), the maths are good but because they are rotating the top-left corner of pixels, but they are still drawn on the bottom right of the resulting point after rotation, this creates a confusing behavior. What is expected is that the pixel being drawn would also be rotated, so the square shape of the pixel would be drawn with the right rotation.

So, right now, rotating a pixel at 0, 0 by 180 degrees would stay at 0, 0, that makes sense. But the bottom right corner of that pixel was initially at 1, 1, and that corner would now be expected to be at -1, -1. But it's not the case because the pixel is still drawn on the bottom right of the new point.

So, one "solution" (I'm not nearly competent enough to say if that makes sense, but maybe this will trigger ideas) would be to carry to rotation information, so the matrix transform does not have to change, but to calculate the new pixels, that information would be used to determine where to draw the pixel in relation to the new position and that pixel's rotation. The difference with just applying a -0.5, -0.5 transform on the pixel would be that the blending of the pixel could use that information.

Example of using 0.5 offset v.s. pixel rotation

For example, in this image if we do a translation to compensate for the pixel being drawn on the bottom right, a 45 degrees rotation would consider all four pixels surrounding the rotation point to be blended equally, but in reality we'd want only the bottom two pixels (and a little more underneath) to have the pixel's color, not the top two.

image

In other words, I think the 0.5 pixel offset kind of works but does not actually do what would be expected of actually rotating the plane.

Here is an example of the current rotation as I understand it (red) and the desired rotation (green).

image

The point I'm making here is not to actually rotate the pixel, but rather to show how this ends up doing a 1 pixel off on x or y for 90 and 270 degrees, and 1 pixel off for x and y for 180 degrees, despite the coordinates themselves being correct.

@JimBobSquarePants
Copy link
Member

In other words, I think the 0.5 pixel offset kind of works but does not actually do what would be expected of actually rotating the plane.

@christianrondeau The .5F offset is not applied as a hack. It's the consequence of the zero-based indexing.

Translation and Rotation are fundamentally separate operations and the assumptions on behavior are incorrect.

  • Translation - Move by a given number of pixels.
  • Rotation - Rotate at a given pixel location.

When you perform transformations using Matrix3x2, such as rotations, the rotation is counterclockwise when the angle is positive, which is consistent with a right-handed coordinate system. In 2D graphics however, the Y-axis points downwards but this is due to screen space conventions, not the underlying mathematical system. The rotation is drawn at the top-left corner.

Imagine your image as a grid of tiles on a floor, where each tile represents a pixel. Now, let’s say you want to rotate something around a specific tile. If you tell the system to rotate around tile (8, 8), it’s like telling someone to stand at the exact center of that tile and spin around.

However, it’s important to understand that when you specify a position like (8, 8), you’re actually giving the index of the tile, not a precise point within the tile itself. The system assumes that the rotation point is at the top-left corner of that tile. This is different from thinking of it as the geometric center of the tile. (@Socolin your original image is the correct pixel representation)

So, if you want to rotate around the exact center of the image, you need to specify a position that takes into account the subpixel area, or the point between tiles, like (7.5, 7.5) in a zero-based coordinate system. This ensures that the rotation happens smoothly around the true center, rather than causing the image to "jump" or shift unexpectedly.

In short, specifying a whole pixel position (like 8, 8) for rotation treats that value as the starting index of the tile, not as the precise center of the area you want to rotate around.

I'm confident ImageSharp performs accurate transforms based upon the provided input. In fact, if the transform was dictated by the following lines the output would be expected.

.Transform(new AffineTransformBuilder().AppendTranslation(new Vector2(8, 8)))
.Transform(new AffineTransformBuilder().AppendRotationDegrees(180))

@Socolin
Copy link
Author

Socolin commented Aug 1, 2024

I found another bug with the rotation, kind of the same as the original issue:

When I apply 90/270 rotation on this 4x4

image

with

.Transform(new  AffineTransformBuilder().AppendRotationDegrees(270, new Vector2(3.5f, 3.5f)))

I'm getting this:
image
And this for a 90 degree rotation
image
The output is offset by one pixel

To reproduce

const int width = 4;
const int height = 4;
using (var img = new Image<Rgba32>(width, height, Color.DimGray))
{
	const int scaleFactor = 80;;
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(0, 0), new PointF(width - 1, 0)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(width - 1, 0), new PointF(width - 1, height - 1)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(width - 1, height - 1), new PointF(0, height - 1)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(0, height - 1), new PointF(0, 0)]));
	img[0, 0] = Color.DarkRed;
	img[width - 1, 0] = Color.Blue;
	img[width - 1, height - 1] = Color.Pink;
	img[0, height - 1] = Color.Azure;

	var centerX = width - 0.5f;
	var centerY = height - 0.5f;
	img.Mutate(c => c
		// .Transform(new  AffineTransformBuilder().AppendTranslation(new Vector2(width, height)))
		//.Transform(new  AffineTransformBuilder().AppendTranslation(new Vector2(width, height)))
		.Transform(new  AffineTransformBuilder().AppendRotationDegrees(90, new Vector2( centerX, centerY)))

		//.Transform(new  AffineTransformBuilder().AppendRotationDegrees(90, new Vector2(width - 0.5F, width - 0.5F)))
		//.Fill(Color.Coral, new RectangleF(PointF.Empty, new SizeF(2.0f, 2.0f)))
		//.Fill(Color.Aqua, new RectangleF(size - 1, size - 1, 2, 2))
		.Resize(new Size(img.Width * scaleFactor, img.Height * scaleFactor), KnownResamplers.Box, true)
		.BackgroundColor(Color.DarkGreen)
	);

	var font = SystemFonts.CreateFont("Arial", 15);

	img.Mutate(c =>
		{
			for (var x = 0; x < width * 2; x++)
				c.DrawLine(Pens.Dash(Color.Black, 1), [new PointF(x * scaleFactor + scaleFactor, 0), new PointF(x * scaleFactor + scaleFactor, height * 2 * scaleFactor + scaleFactor)]);
			for (var y = 0; y < height * 2; y++)
				c.DrawLine(Pens.Dash(Color.Black, 1), [new PointF(0, y * scaleFactor + scaleFactor), new PointF(width * 2 * scaleFactor + scaleFactor, y * scaleFactor + scaleFactor)]);


			for (var x = 0; x < width * 2; x++)
			for (var y = 0; y < height * 2; y++)
			{
				c.DrawLine(Color.Black, 1, [new PointF(
					x * scaleFactor + scaleFactor / 4f,
					y * scaleFactor + scaleFactor / 2f
				), new PointF(
					x * scaleFactor + 3 * scaleFactor / 4f,
					y * scaleFactor + scaleFactor / 2f
				)]);
				c.DrawLine(Color.Black, 1, [new PointF(
					x * scaleFactor + scaleFactor / 2f,
					y * scaleFactor + scaleFactor / 4f
				), new PointF(
					x * scaleFactor + scaleFactor / 2f,
					y * scaleFactor + 3 * scaleFactor / 4f
				)]);
				c.DrawText($"({x}, {y})", font, Color.Black, new PointF(x * scaleFactor + scaleFactor / 2f + 3, y * scaleFactor + scaleFactor / 2f + 3));
			}
		}
	);
	img.Mutate(c => c.DrawLine(Color.Red, 3, new PointF(centerX * scaleFactor, (centerY - 1) * scaleFactor), new PointF(centerX * scaleFactor, scaleFactor * (centerY + 1)))
		.DrawLine(Color.Red, 3, new PointF((centerX - 1) * scaleFactor, centerY * scaleFactor), new PointF((centerX + 1) * scaleFactor, centerY * scaleFactor))
	);
	img.Save($"rotation-180.png");
}

@JimBobSquarePants
Copy link
Member

Rather than bombard me with a wall of text can you please acknowledge my explanation as to why your expectations were incorrect above

@Socolin
Copy link
Author

Socolin commented Aug 1, 2024

Rather than bombard me with a wall of text can you please acknowledge my explanation as to why your expectations were incorrect above

Sorry if you saw my repro as a wall of text, I just paste the code so you can easily reproduce this.

I acknowledge I understand why the value is offseted by 0.5, it make sense (Sorry if this was not clear in my previous message)

I'm sorry in my previous message I made a typo and I wrongly place the rotation point, so the bug is not an offset but only that the image is cropped by 1 pixel

Do you prefer I open an new issue for this one since it's not exactly the same problem ?

using (var img = new Image<Rgba32>(4, 4, Color.DimGray))
{
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(0, 0), new PointF(3, 0)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(3, 0), new PointF(3, 3)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(3, 3), new PointF(0, 3)]));
	img.Mutate(c => c.DrawLine(Color.Orange, 1, [new PointF(0, 3), new PointF(0, 0)]));

	img.Mutate(c => c.Transform(new AffineTransformBuilder().AppendRotationDegrees(270, new Vector2(3.5f, 3.5f))));
	img.Save("rotation-cropped.png");
}

image

@JimBobSquarePants
Copy link
Member

@Socolin can you please check my working here. I've applied an equivalent transform at both the index and bounds to see what's going on and applied a simplified form of the bounds calculation.

    // Create the matrix.
    Matrix3x2 mt = Matrix3x2.CreateRotation(GeometryUtilities.DegreeToRadian(270), new Vector2(3.5f, 3.5f));

    // Transform indices. Rectangle of size 4x4 at position [0, 0]
    // TL
    Vector2.Transform(new Vector2(0, 0), mt); // [0, 7]

    // TR
    Vector2.Transform(new Vector2(3, 0), mt); // [0, 4]

    // BR
    Vector2.Transform(new Vector2(3, 3), mt); // [3, 4]

    // BL
    Vector2.Transform(new Vector2(0, 3), mt); // [3, 7]

    // Transform Bounds. Rectangle of size 4x4 at position [0, 0]
    // TL
    var tl = Vector2.Transform(new Vector2(0, 0), mt); // [0, 7]

    // TR
    var tr = Vector2.Transform(new Vector2(4, 0), mt); // [0, 3]

    // BR
    var br = Vector2.Transform(new Vector2(4, 4), mt); // [4, 3]

    // BL
    var bl = Vector2.Transform(new Vector2(0, 4), mt); // [4, 7]
    
     // Find the minimum and maximum "corners" based on the given vectors
    float left = MathF.Min(tl.X, MathF.Min(tr.X, MathF.Min(bl.X, br.X))); // 0
    float top = MathF.Min(tl.Y, MathF.Min(tr.Y, MathF.Min(bl.Y, br.Y))); // 3
    float right = MathF.Max(tl.X, MathF.Max(tr.X, MathF.Max(bl.X, br.X))); // 4
    float bottom = MathF.Max(tl.Y, MathF.Max(tr.Y, MathF.Max(bl.Y, br.Y))); // 7

    var bounds = Rectangle.Round(RectangleF.FromLTRB(left, top, right, bottom)); [0, 3, 4, 7] @ 3x4
    
    // Take the max of height vs bottom and width vs right to house our transform.
    // ignore negative values for now since everything is positive.
    bounds  = new Rectangle(0, 0, right, bottom);  // [0, 0, 4, 7] @ 4x7    

We end up with a rectangle of dimensions 4x7. However, if you look at the results of applying the transform when sampling at a given index you can see that the values exceed the given bounds.

Now.... What I'm curious about is whether we should in, fact be calculating the bounds based upon the maximum index of the given rectangle. 🤔

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 2, 2024

OK, what seems to get the expected result 4x8 for this test would be to update
TransformUtils.GetTransformedSize(Size size, Matrix3x2 matrix) to do the following.

public static Size GetTransformedSize(Size size, Matrix3x2 matrix)
{
    if (matrix.Equals(default) || matrix.Equals(Matrix3x2.Identity))
    {
        return size;
    }

    // Calculate the transform based upon the max zero-based indices
    Rectangle rectangle = GetTransformedRectangle(new Rectangle(Point.Empty, size - new Size(1, 1)), matrix);

    size = ConstrainSize(rectangle);

    // Pad out by 1 to transform to one-based dimensions
    return size + new Size(1, 1);
}

I'd have to double check this doesn't negatively affect the size calculation for automatically centered rotations but it shouldn't...

@Socolin
Copy link
Author

Socolin commented Aug 3, 2024

As far as I can tell the math are all right with the transform. I'm very visual so I draw a bunch of schema to understand this, I'm including them for clarity

The way the current bounding box is computed, I think, would work if it were using the plan coordinate (If I rotate a 4x4 square on a plan (0, 0, 4, 4) from (3.5, 3.5) to top coordinate is 7)

image

Since the pixel are centered on their coordinate (in the plan, the pixel 0, 0, is [-0.5, -0.5, 0.5, 0.5])

image
And after the rotation
image

it seems normal to have an offset of 0.5 to align the pixels with the plan, and then add another 0.5 to get the position of the edge of the pixel.

image

I'm not sure I understand the - new Size(1, 1) then +new Size(1, 1)` I understand that we offset everything to do the computation and then offset it back, from my understanding it should only be offset by 0.5 (But I'm not that good with math so I'm really not sure about this and probably I'm missing something).

I now understand why there is 2 matrix, and I wonder if instead of doing the -1 / +1 the center of rotation should be offset by (0.5, 0.5) instead when computing the bounding box matrix

    public AffineTransformBuilder AppendRotationRadians(float radians, Vector2 origin)
        => this.Append(
            _ => Matrix3x2.CreateRotation(radians, origin),
            _ => Matrix3x2.CreateRotation(radians, new Vector2(origin.X + 0.5f, origin.Y + 0.5f)));

(If this is a solution, private AffineTransformBuilder Append may need to be made public so people doing custom math can give both matrix)

@Socolin
Copy link
Author

Socolin commented Aug 3, 2024

Also while reading the code I'm also have a question regarding GetBoundingRectangle (I did no test so it's purely speculative at this point)

if after a rotation one of the border is slightly over a pixel (like a bounding box of 0.7, 0.7, 10.2, 10.2), seems like it's going to cut the pixels a bit (1,1, 10, 10) instead of (0,0, 11, 11) in (GetBoundingRectangle it's using Rectangle.Round) ?

@JimBobSquarePants
Copy link
Member

@Socolin Your observation regarding GetBoundingRectangle is correct. Rounding should not be taking place and I've already updated my local code to reflect this.

private static Rectangle GetBoundingRectangle(Vector2 tl, Vector2 tr, Vector2 bl, Vector2 br)
{
    // Find the minimum and maximum "corners" based on the given vectors
    float left = MathF.Min(tl.X, MathF.Min(tr.X, MathF.Min(bl.X, br.X)));
    float top = MathF.Min(tl.Y, MathF.Min(tr.Y, MathF.Min(bl.Y, br.Y)));
    float right = MathF.Max(tl.X, MathF.Max(tr.X, MathF.Max(bl.X, br.X)));
    float bottom = MathF.Max(tl.Y, MathF.Max(tr.Y, MathF.Max(bl.Y, br.Y)));

    return Rectangle.FromLTRB(
        (int)Math.Floor(left),
        (int)Math.Floor(top),
        (int)Math.Ceiling(right),
        (int)Math.Ceiling(bottom));
}

Since the pixel are centered on their coordinate (in the plan, the pixel 0, 0, is [-0.5, -0.5, 0.5, 0.5])

We've touched upon this a few times and I think there's a slight misunderstanding there. Pixels are not centered on their coordinates as you have drawn, rather a pixel's location is always considered the top-left corner.

What we have here is two separate coordinate spaces.

  1. Coordinate Space (Geometric or Mathematical Space):
    This refers to the continuous mathematical representation of a shape or region, often defined by vertices or boundaries like left (L), top (T), right (R), and bottom (B) positions of a rectangle.
    Transforming in coordinate space means you are applying the transformation to these geometric boundaries or control points directly.

  2. Pixel Space (Discrete or Raster Space):
    This refers to the discrete grid of pixels that make up an image. The pixel indices (like (0,0) to (width-1, height-1)) are discrete and aligned with the grid's extents.
    Applying a transformation in pixel space means you are transforming the actual grid points or pixel indices, which can result in different outcomes due to the discrete nature of the pixel grid.

Now the current code blurs those spaces and adds complexity. Everything we do should actually be based upon the Pixel Space.

This is why the suggested solution code subtracts [1, 1] from the given size then adds it on again at the end. It's a simple translation.

I'm going to do some experimentation, but I think I may be able to use a single transform matrix and refactor the bounds calculations to use the correct space.

@JimBobSquarePants
Copy link
Member

It's more complicated calculating the correct transform than it first appears as you need to consider linear scaling in affine transforms and non-linear scaling in perspective ones. I think I have everything looking good locally though now.

@JimBobSquarePants
Copy link
Member

@Socolin I've pushed my fixes to a branch but it's very late now so I haven't updated the reference output for the tests.

e1555fd

Please have a good look. I've added a LOT of comments to describe how things work and why.

@Socolin
Copy link
Author

Socolin commented Aug 5, 2024

I took a look, and it looks good. I'm sorry I'm really having a hard time understanding all this.

I think the main confusion for me is that almost all operation works the same between the Coordinate space / Pixel space except the rotation. I'm bad with abstract math, I generally need to visualize things or it take me a long time to understand a new concept. In this case trying to visualize this with what I knew was not working.

I think I now got the differences between both space:

  1. The size of a 4x4 image is
    • 5x5 wide in the coordinate space
    • 4x4 in pixel space
  2. When applying the matrix:
    • in coordinate space, pixel would be square of 1x1
    • In pixel space: pixels are only point

So the -1 make sense, to go from the 5x5 to 4x4 before working on it :)

And about the commit the only remark I would have, is that in the comments the Coordinate / Pixel space are not clearly explained. I think it would be nice to have this explained somewhere (the last explanation you gave here was good :) ) Either in the code directly, or linked to a doc somewhere.

Also I would suggest to update the doc of AppendRotationRadians() (like in <remarks>) to add that the rotation is done in the Pixel space and thus the rotation are done around the pixel/point and not around the coordinate.

And thanks a lot for the explanation and for your patience :)

@JimBobSquarePants
Copy link
Member

JimBobSquarePants commented Aug 5, 2024

Thanks for your feedback! I understand the confusion between coordinate space and pixel space, especially when it comes to transformations like rotation and scaling. Let's break it down:

Coordinate Space vs. Pixel Space:

  • Coordinate Space: This is a continuous grid where positions can be any value, even fractions. When you define a 4x4 image here, it spans from (0,0) to (4,4), but this includes the boundaries and represents exactly 4 units in each direction.
  • Pixel Space: This is a discrete grid where each position is a whole number, corresponding to specific pixels on the screen. In pixel space, a 4x4 image means 4 pixels wide and 4 pixels tall, covering positions (0,0) to (3,3).

Why the Offset Matters:

  • When you move from coordinate space to pixel space, there's a key difference: in pixel space, you’re working with exact grid points, while coordinate space can be more flexible. To align these properly, especially after transformations like scaling and rotation, we sometimes use an offset.
  • The Offset: This offset (which you can think of as 1/scale in each direction) ensures that after scaling, the transformed image aligns correctly with the pixel grid. Without this, you could end up with pixels slightly misaligned, causing visual artifacts like clipping.

When the Offset is Applied:

  • The offset is crucial when dealing with standard 2D transformations (like scaling or rotation) because it keeps everything lined up in pixel space. However, it’s only used when the transformation doesn’t involve more complex effects like perspective or non-linear scaling, where the transformation is less predictable.

In Simple Terms:

  • Coordinate space lets you place things with precision, but pixel space is where you make sure everything lines up on your screen. The offset helps bridge these two spaces when you scale or rotate, ensuring that your image looks right without any parts being cut off.

I’ll make sure to add a clearer explanation in the code comments to help others understand this distinction. I think I'll actually have to add an overload to both builders that takes an enum defining whether to use pixel or coordinate space as with all the changes the builders do not work well when drawing shapes since they operate in the coordinate space.

@JimBobSquarePants
Copy link
Member

New PR #2791 has been opened to fix all known issues.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants