Skip to content

Conversation

@homm
Copy link
Member

@homm homm commented Sep 16, 2024

No description provided.

@homm homm added the TIFF label Sep 16, 2024

@property
def size(self) -> tuple[int, int]:
if hasattr(self, "tag_v2"):
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There is a cases after copying or pickling, when the class is actually TiffImageFile, but it is loaded and doesn't have tag_v2 attribute

return super().load()

@property
def size(self) -> tuple[int, int]:
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This implementation is differ from uploadcare@1d77bfe since for me it looks less magical, but if you can't agree, I can copy the previous as is

@radarhere
Copy link
Member

This PR was created in an effort to ensure that load() does not change the size of an image.

It naturally sounds like a good idea for images to have the same size after opening as after loading. However, Pillow also aims to open an image without loading it, for the sake of performance, and the rotation data might not appear early on in the file.

For the PNG format, https://ftp-osl.osuosl.org/pub/libpng/documents/pngext-1.5.0.html#C.eXIf states

The eXIf chunk may appear anywhere between the IHDR and IEND chunks except between IDAT chunks.

So I don't think it is possible to simultaneously achieve those two goals.

@homm
Copy link
Member Author

homm commented Sep 17, 2024

There is no conflict between the presence of eXIf chunks in PNG files and the goal of "having the same size after opening as after loading," because when loading a PNG (by calling the .load() method or any dependent method), the Exif data is populated, but the image is not automatically rotated. This differs from the behavior of TIFF, where calling .load() actually rotates the image and can change its size. This is exactly the issue I aim to address in this PR.

@radarhere
Copy link
Member

Ah, I see now, thanks.

I think introducing the idea that im.size might not match im._size complicates things. How about #8390 instead?

@hugovk
Copy link
Member

hugovk commented Sep 18, 2024

Closed in favour of #8390.

@hugovk hugovk closed this Sep 18, 2024
@hugovk hugovk deleted the tiff-correct-size branch September 18, 2024 19:35
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants