Skip to content

Navigation Menu

Sign in
Appearance settings

Search code, repositories, users, issues, pull requests...

Provide feedback

We read every piece of feedback, and take your input very seriously.

Saved searches

Use saved searches to filter your results more quickly
Appearance settings

Better thumbnail aspect ratio preservation #4256

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.

Sign up for GitHub

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

Merged
radarhere merged 1 commit into from
Dec 16, 2019
Merged

Better thumbnail aspect ratio preservation #4256

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
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
Better thumbnail aspect ratio preservation
  • Loading branch information
@homm
homm committed Dec 7, 2019
commit 791d1a2846e28955e964cfa38ac4632fc457fa30
40 changes: 23 additions & 17 deletions 40 Tests/test_image_thumbnail.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -5,37 +5,43 @@

class TestImageThumbnail(PillowTestCase):
def test_sanity(self):

im = hopper()
im.thumbnail((100, 100))
self.assertIsNone(im.thumbnail((100, 100)))

self.assert_image(im, im.mode, (100, 100))
self.assertEqual(im.size, (100, 100))

def test_aspect(self):

im = hopper()
im = Image.new("L", (128, 128))
im.thumbnail((100, 100))
self.assert_image(im, im.mode, (100, 100))
self.assertEqual(im.size, (100, 100))

im = hopper().resize((128, 256))
im = Image.new("L", (128, 256))
im.thumbnail((100, 100))
self.assert_image(im, im.mode, (50, 100))
self.assertEqual(im.size, (50, 100))

im = hopper().resize((128, 256))
im = Image.new("L", (128, 256))
im.thumbnail((50, 100))
self.assert_image(im, im.mode, (50, 100))
self.assertEqual(im.size, (50, 100))

im = hopper().resize((256, 128))
im = Image.new("L", (256, 128))
im.thumbnail((100, 100))
self.assert_image(im, im.mode, (100, 50))
self.assertEqual(im.size, (100, 50))

im = hopper().resize((256, 128))
im = Image.new("L", (256, 128))
im.thumbnail((100, 50))
self.assert_image(im, im.mode, (100, 50))
self.assertEqual(im.size, (100, 50))

im = hopper().resize((128, 128))
im = Image.new("L", (128, 128))
im.thumbnail((100, 100))
self.assert_image(im, im.mode, (100, 100))
self.assertEqual(im.size, (100, 100))

im = Image.new("L", (256, 162)) # ratio is 1.5802469136
im.thumbnail((33, 33))
self.assertEqual(im.size, (33, 21)) # ratio is 1.5714285714

im = Image.new("L", (162, 256)) # ratio is 0.6328125
im.thumbnail((33, 33))
self.assertEqual(im.size, (21, 33)) # ratio is 0.6363636364

def test_no_resize(self):
# Check that draft() can resize the image to the destination size
Expand All @@ -46,4 +52,4 @@ def test_no_resize(self):
# Test thumbnail(), where only draft() is necessary to resize the image
with Image.open("Tests/images/hopper.jpg") as im:
im.thumbnail((64, 64))
self.assert_image(im, im.mode, (64, 64))
self.assertEqual(im.size, (64, 64))
8 changes: 4 additions & 4 deletions 8 src/PIL/Image.py
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -2137,11 +2137,11 @@ def thumbnail(self, size, resample=BICUBIC):
# preserve aspect ratio
x, y = self.size
if x > size[0]:
y = int(max(y * size[0] / x, 1))
x = int(size[0])
y = max(round(y * size[0] / x), 1)
x = size[0]
Copy link
Member

@radarhere radarhere Dec 16, 2019

Choose a reason for hiding this comment

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

Just to be aware - by removing the int rounding on these second lines, if someone had existing code im.thumbnail((150.2, 150.2)), it would now break.

That's not unreasonable though.

Copy link
Member

@hugovk hugovk Dec 16, 2019

Choose a reason for hiding this comment

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

Example:

from PIL import Image

im = Image.open("Tests/images/hopper.png")
print(im.size)

im.thumbnail((15.2, 15.2))
print(im.size)

im.thumbnail((10, 10))
print(im.size)

On 6.2.1:

(128, 128)
(15, 15)
(10, 10)

On master with this PR:

(128, 128)
Traceback (most recent call last):
  File "1.py", line 6, in <module>
    im.thumbnail((15.2, 15.2))
  File "/Users/hugo/github/Pillow/src/PIL/Image.py", line 2153, in thumbnail
    im = self.resize(size, resample)
  File "/Users/hugo/github/Pillow/src/PIL/Image.py", line 1822, in resize
    return self._new(self.im.resize(size, resample, box))
TypeError: integer argument expected, got float

Should we fix it something like this?

diff --git a/src/PIL/Image.py b/src/PIL/Image.py
index 613b3a36..981c8c81 100644
--- a/src/PIL/Image.py
+++ b/src/PIL/Image.py
@@ -2138,10 +2138,10 @@ class Image:
         x, y = self.size
         if x > size[0]:
             y = max(round(y * size[0] / x), 1)
-            x = size[0]
+            x = round(size[0])
         if y > size[1]:
             x = max(round(x * size[1] / y), 1)
-            y = size[1]
+            y = round(size[1])
         size = x, y

         if size == self.size:
(128, 128)
(15, 15)
(10, 10)

Copy link
Member

@radarhere radarhere Dec 16, 2019

Choose a reason for hiding this comment

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

Okay, I've created PR #4272

if y > size[1]:
x = int(max(x * size[1] / y, 1))
y = int(size[1])
x = max(round(x * size[1] / y), 1)
y = size[1]
size = x, y

if size == self.size:
Expand Down
Morty Proxy This is a proxified and sanitized view of the page, visit original site.