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

Add append_images support for ICO #4568

Merged
merged 9 commits into from Dec 24, 2020
Merged

Add append_images support for ICO #4568

merged 9 commits into from Dec 24, 2020

Conversation

@ghost
Copy link

@ghost ghost commented Apr 19, 2020

This adds support for the append_images parameter when saving .ICO files. Its behavior is much the same as for .ICNS files, and can be used to provide alternative sprites for particular icon sizes instead of simply resizing the main image to fit that size.

@radarhere radarhere changed the title Add append_images support for ico Add append_images support for ICO Apr 20, 2020
src/PIL/IcoImagePlugin.py Outdated Show resolved Hide resolved
Co-Authored-By: Andrew Murray <3112309+radarhere@users.noreply.github.com>
alt_images = {im.size: im for im in im.encoderinfo.get("append_images", [])}
for size in sizes:
Copy link
Contributor

@nulano nulano Apr 20, 2020

It looks like if someone were to add an unusual sized image to append_images (e.g. (20,20)), it would not be saved unless it is added to the sizes parameter as well. I feel like this could be confusing behaviour. Perhaps adding sizes.extend(alt_images.keys()) would be a good idea (sizes would have to be changed to a set to avoid duplicates).

Copy link
Author

@ghost ghost Apr 20, 2020

That is correct. I believe the ICNS saving currently suffers from the same issue. I don't know if it should be fixed for .ICOs; the documentation would need to mention the role of append_images either way, since if the behavior is changed as you suggest, it would then also save other sizes than the ones the user requested (via the sizes parameter).

@radarhere
Copy link
Member

@radarhere radarhere commented Nov 4, 2020

I've created https://github.com/ziplantil/Pillow/pull/1 with some suggestions.

Tests/test_file_ico.py Outdated Show resolved Hide resolved
@radarhere radarhere merged commit e37a8a2 into python-pillow:master Dec 24, 2020
50 checks passed
@ghost ghost deleted the ico-append-images branch Dec 24, 2020
radarhere added a commit to radarhere/Pillow that referenced this issue Dec 24, 2020
hugovk added a commit that referenced this issue Dec 24, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

None yet

3 participants