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

Use transposed size after opening for TIFF images #8390

Merged
merged 5 commits into from
Sep 18, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 2 additions & 0 deletions Tests/test_image_copy.py
Original file line number Diff line number Diff line change
Expand Up @@ -49,5 +49,7 @@ def test_copy_zero() -> None:
@skip_unless_feature("libtiff")
def test_deepcopy() -> None:
with Image.open("Tests/images/g4_orientation_5.tif") as im:
assert im.size == (590, 88)

out = copy.deepcopy(im)
assert out.size == (590, 88)
4 changes: 1 addition & 3 deletions Tests/test_image_resize.py
Original file line number Diff line number Diff line change
Expand Up @@ -300,9 +300,7 @@ def resize(mode: str, size: tuple[int, int] | list[int]) -> None:
im.resize((10, 10), "unknown")

@skip_unless_feature("libtiff")
def test_load_first(self) -> None:
# load() may change the size of the image
# Test that resize() is calling it before getting the size
def test_transposed(self) -> None:
with Image.open("Tests/images/g4_orientation_5.tif") as im:
im = im.resize((64, 64))
assert im.size == (64, 64)
Expand Down
8 changes: 3 additions & 5 deletions Tests/test_image_thumbnail.py
Original file line number Diff line number Diff line change
Expand Up @@ -92,15 +92,13 @@ def test_no_resize() -> None:


@skip_unless_feature("libtiff")
def test_load_first() -> None:
# load() may change the size of the image
# Test that thumbnail() is calling it before performing size calculations
def test_transposed() -> None:
with Image.open("Tests/images/g4_orientation_5.tif") as im:
assert im.size == (590, 88)

im.thumbnail((64, 64))
assert im.size == (64, 10)

# Test thumbnail(), without draft(),
# on an image that is large enough once load() has changed the size
with Image.open("Tests/images/g4_orientation_5.tif") as im:
im.thumbnail((590, 88), reducing_gap=None)
assert im.size == (590, 88)
Expand Down
4 changes: 3 additions & 1 deletion Tests/test_numpy.py
Original file line number Diff line number Diff line change
Expand Up @@ -238,8 +238,10 @@ def test_zero_size() -> None:


@skip_unless_feature("libtiff")
def test_load_first() -> None:
def test_transposed() -> None:
with Image.open("Tests/images/g4_orientation_5.tif") as im:
assert im.size == (590, 88)

a = numpy.array(im)
assert a.shape == (88, 590)

Expand Down
20 changes: 5 additions & 15 deletions src/PIL/Image.py
Original file line number Diff line number Diff line change
Expand Up @@ -2332,7 +2332,6 @@ def resize(
msg = "reducing_gap must be 1.0 or greater"
raise ValueError(msg)

self.load()
if box is None:
box = (0, 0) + self.size

Expand Down Expand Up @@ -2781,27 +2780,18 @@ def round_aspect(number: float, key: Callable[[int], float]) -> int:
)
return x, y

preserved_size = preserve_aspect_ratio()
if preserved_size is None:
return
final_size = preserved_size

box = None
final_size: tuple[int, int]
if reducing_gap is not None:
preserved_size = preserve_aspect_ratio()
if preserved_size is None:
return
final_size = preserved_size

res = self.draft(
None, (int(size[0] * reducing_gap), int(size[1] * reducing_gap))
)
if res is not None:
box = res[1]
if box is None:
self.load()

# load() may have changed the size of the image
preserved_size = preserve_aspect_ratio()
if preserved_size is None:
return
final_size = preserved_size

if self.size != final_size:
im = self.resize(final_size, resample, box=box, reducing_gap=reducing_gap)
Expand Down
2 changes: 1 addition & 1 deletion src/PIL/ImageFile.py
Original file line number Diff line number Diff line change
Expand Up @@ -322,7 +322,7 @@ def load(self) -> Image.core.PixelAccess | None:

def load_prepare(self) -> None:
# create image memory if necessary
if self._im is None or self.im.mode != self.mode or self.im.size != self.size:
Copy link
Member

@homm homm Sep 18, 2024

Choose a reason for hiding this comment

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

Not sure how it affects multipage images with different page sizes

Copy link
Member Author

Choose a reason for hiding this comment

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

When seeking to a different TIFF frame, self.im is populated afresh.

def seek(self, frame: int) -> None:
"""Select a given frame as current image"""
if not self._seek_check(frame):
return
self._seek(frame)
# Create a new core image object on second and
# subsequent frames in the image. Image may be
# different size/mode.
Image._decompression_bomb_check(self.size)
self.im = Image.core.new(self.mode, self.size)

Copy link
Member

@homm homm Sep 18, 2024

Choose a reason for hiding this comment

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

Yeah, I saw this and tried to remove this initialization and relay on load_prepare instead, for some reasons it didn't work.

For me providing correct canvas (self.im) in the base class is more preferable than recreating it in the plugin. There was another approach which doesn't introduce difference between im.size and im._size.

Copy link
Member Author

Choose a reason for hiding this comment

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

Ah, I see now - my load_prepare() would create a new core image when the image was loaded after seek().

I've pushed a commit following your other approach.

if self._im is None:
self.im = Image.core.new(self.mode, self.size)
# create palette (optional)
if self.mode == "P":
Expand Down
22 changes: 16 additions & 6 deletions src/PIL/TiffImagePlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -1194,8 +1194,8 @@ def seek(self, frame: int) -> None:
# Create a new core image object on second and
# subsequent frames in the image. Image may be
# different size/mode.
Image._decompression_bomb_check(self.size)
self.im = Image.core.new(self.mode, self.size)
Image._decompression_bomb_check(self._tile_size)
self.im = Image.core.new(self.mode, self._tile_size)

def _seek(self, frame: int) -> None:
self.fp = self._fp
Expand Down Expand Up @@ -1275,6 +1275,11 @@ def load(self) -> Image.core.PixelAccess | None:
return self._load_libtiff()
return super().load()

def load_prepare(self) -> None:
if self._im is None:
self.im = Image.core.new(self.mode, self._tile_size)
ImageFile.ImageFile.load_prepare(self)

def load_end(self) -> None:
# allow closing if we're on the first frame, there's no next
# This is the ImageFile.load path only, libtiff specific below.
Expand Down Expand Up @@ -1416,7 +1421,12 @@ def _setup(self) -> None:
if not isinstance(xsize, int) or not isinstance(ysize, int):
msg = "Invalid dimensions"
raise ValueError(msg)
self._size = xsize, ysize
self._tile_size = xsize, ysize
orientation = self.tag_v2.get(ExifTags.Base.Orientation)
if orientation in (5, 6, 7, 8):
self._size = ysize, xsize
homm marked this conversation as resolved.
Show resolved Hide resolved
else:
self._size = xsize, ysize

logger.debug("- size: %s", self.size)

Expand Down Expand Up @@ -1559,7 +1569,7 @@ def _setup(self) -> None:
if STRIPOFFSETS in self.tag_v2:
offsets = self.tag_v2[STRIPOFFSETS]
h = self.tag_v2.get(ROWSPERSTRIP, ysize)
w = self.size[0]
w = xsize
else:
# tiled image
offsets = self.tag_v2[TILEOFFSETS]
Expand Down Expand Up @@ -1593,9 +1603,9 @@ def _setup(self) -> None:
)
)
x = x + w
if x >= self.size[0]:
if x >= xsize:
x, y = 0, y + h
if y >= self.size[1]:
if y >= ysize:
x = y = 0
layer += 1
else:
Expand Down
Loading