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 rounding in ImageOps contain() and pad() #6522

Conversation

bibinhashley
Copy link
Contributor

@bibinhashley bibinhashley commented Aug 22, 2022

Fixes ImageOps.contain function

Changes proposed in this pull request:

I tried to use ImageOps.pad on an image of size (4307, 6030) and I needed to resize it to (50,70). The image was resized but it had extended only on the right side. But actually, these two sizes don't need any expansion since both sizes are proportional.

So I checked the pillow code and found that ImageOps.pad is using ImageOps.contain and in contain function there is a code for calculating new height or new width based on the size.
new_height = int(image.height / image.width * size[0])
or
new_width = int(image.width / image.height * size[1])

But the problem here is when I ran that function with (50,70), it gave output (49,70).
code explanation:

size = (4307,6030)
new_width = int(image.width / image.height * size[1])
new_width = int(4307 / 6030 * 70)
new_width = 49

Expected output :

new_width = 50

But the new width should have been 50. So I changed " int " method with "round"

@radarhere radarhere changed the title ImageOps.contain function finding new size issue ImageOps.contain function finding new size Aug 23, 2022
@hugovk hugovk mentioned this pull request Aug 23, 2022
@radarhere
Copy link
Member

I've created bibinhashley#1 to suggest a test, and to also use round() instead of int() in ImageOps.pad().

@radarhere
Copy link
Member

radarhere commented Aug 26, 2022

@bibinhashley I find it slightly odd that you liked my comment, but haven't made any response to my pull request itself.

If you're taking a moment to review my suggestions, not to worry, but in case you're not familiar with this procedure - the intention is for you to look at bibinhashley#1, and if you're ok with my ideas, merge it. Those changes will then become a part of this PR.

@bibinhashley
Copy link
Contributor Author

Sorry I was busy and I couldn't follow the procedure. I will check the test and merge it.

…in-finding-new-size

Round position in pad()
@hugovk hugovk merged commit f98fde7 into python-pillow:main Sep 21, 2022
@hugovk
Copy link
Member

hugovk commented Sep 21, 2022

Thank you both for the PR!

@radarhere radarhere changed the title ImageOps.contain function finding new size Use rounding in ImageOps contain() and pad() Sep 21, 2022
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

Successfully merging this pull request may close these issues.

3 participants