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

Affine transformation may be incorrect #8731

Closed
hyj1230 opened this issue Feb 4, 2025 · 5 comments
Closed

Affine transformation may be incorrect #8731

hyj1230 opened this issue Feb 4, 2025 · 5 comments

Comments

@hyj1230
Copy link

hyj1230 commented Feb 4, 2025

File: Pillow/src/libImaging/Geometry.c, Line: 1083, in function ImagingTransformAffine

xo = a[2] + a[1] * 0.5 + a[0] * 0.5;
yo = a[5] + a[4] * 0.5 + a[3] * 0.5;

The inverse matrix corresponding to array double a[6] should be used for the computation.

@radarhere
Copy link
Member

radarhere commented Feb 4, 2025

Hi. If I consider your change request from purely the perspective of the code, then ImagingTransformAffine only receives a[6], so accessing the 6th offset is not possible.

ImagingTransformAffine(
Imaging imOut,
Imaging imIn,
int x0,
int y0,
int x1,
int y1,
double a[6],

The documentation for AffineTransform describes the Pillow functionality as

Define an affine image transform.

This function takes a 6-tuple (a, b, c, d, e, f) which contain the first two rows from an affine transform matrix. For each pixel (x, y) in the output image, the new value is taken from a position (a x + b y + c, d x + e y + f) in the input image, rounded to nearest pixel.

This function can be used to scale, translate, rotate, and shear the original image.

See Image.transform()

Parameters:
matrix – A 6-tuple (a, b, c, d, e, f) containing the first two rows from an affine transform matrix.

So again, there is no data at the 6th offset - but hopefully that documentation clarifies the operation that is happening.

I understand though that you're just thinking about matrices. You may find https://stackabuse.com/affine-image-transformations-in-python-with-numpy-pillow-and-opencv/#affinetransformationswithpillow useful information, or https://stackoverflow.com/questions/17056209/python-pil-affine-transformation.

@hyj1230 hyj1230 closed this as completed Feb 5, 2025
@hyj1230
Copy link
Author

hyj1230 commented Feb 5, 2025

I found this sentence in the documentation of scipy's affine transformation. I believe the PIL documentation should also include this sentence.

This does ‘pull’ (or ‘backward’) resampling, transforming the output space to the input to locate data. Affine transformations are often described in the ‘push’ (or ‘forward’) direction, transforming input to output. If you have a matrix for the ‘push’ transformation, use its inverse (numpy.linalg.inv) in this function.

@radarhere
Copy link
Member

Could you explain your request in some more detail?

  • Why? Were you not able to understand our documentation as it currently exists?
  • The links that provided talk about using the inverse. Do you find "‘pull’ (or ‘backward’) resampling" understandable, but not 'inverse'?
  • Do you explicitly want a way to use Pillow without the inverse?

@hyj1230
Copy link
Author

hyj1230 commented Feb 5, 2025

  • Initially, I indeed misunderstood the meaning in the documentation. I once thought it was equivalent to ctx.setTransform, but later I discovered that "For each pixel (x, y) in the output image, the new value is taken from a position (a x + b y + c, d x + e y + f) in the input image."
  • I think both 'inverse' and 'pull' are understandable, but it seems that these terms are not explicitly mentioned in the PIL documentation.
  • If they could be added, it might be even better.

@radarhere
Copy link
Member

I've created #8735 to mention that it is the inverse matrix that is used.

Regarding a way to do this without the inverse, I think it only makes sense to explain that if you're using a 3x3 matrix, and describing the process of inverting the matrix, while still emphasising that it is only the first six values are used, sounds like it would only create further confusion.

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

No branches or pull requests

2 participants