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

Added alpha_only argument to getbbox() #7123

Merged
merged 10 commits into from
Jun 30, 2023
Merged

Conversation

radarhere
Copy link
Member

@radarhere radarhere commented Apr 29, 2023

Resolves #7207
Resolves #7122

When saving APNG frames, APNG converts the individual frames to RGB to check if there are any differences. If there aren't, then it is a duplicate frame, and can be combined with the previous one to save space.

delta = ImageChops.subtract_modulo(
im_frame.convert("RGB"), base_im.convert("RGB")
)
bbox = delta.getbbox()
if (
not bbox
and prev_disposal == encoderinfo.get("disposal")
and prev_blend == encoderinfo.get("blend")
):
if isinstance(duration, (list, tuple)):
previous["encoderinfo"]["duration"] += encoderinfo["duration"]
continue

However, the issue tries to save RGBA frames that become identical when converted to RGB. So instead, this PR changes the code to convert the frames to RGBA.

Except that then introduces a new problem. If two frames have the same alpha values, then ImageChops.subtract_modulo() returns a image showing that there is 0 alpha difference... when is then interpreted by getbbox() as a fully transparent image, meaning that as per #4454, it ignores whatever RGB information there might be.

So this PR adds an argument to the C getbbox() to allow it to turn off the special treatment of the alpha channel.

@Yay295
Copy link
Contributor

Yay295 commented Apr 29, 2023

consider_alpha doesn't sound quite right to me. I think alpha_only fits a bit better, or reverse it and call it consider_all_bands.

@radarhere
Copy link
Member Author

I've pushed a commit to rename it to alpha_only.

@Yay295
Copy link
Contributor

Yay295 commented Apr 30, 2023

It can be a separate change, but should this new parameter be exposed to the public api?

@radarhere
Copy link
Member Author

Ok, I've added an alpha_only argument to the Python getbbox(). This was originally requested in #4849

@radarhere radarhere changed the title When saving APNG, allow alpha differences to indicate different frames Added alpha_only argument to getbbox() Apr 30, 2023
@gsingh93
Copy link

I've been using a local build with this patch for a few weeks now and it's been working well, hopefully we can get this merged soon :)

@radarhere
Copy link
Member Author

Be aware that even if it is merged, Pillow 10.0.0 is not due out until July 1.

src/PIL/Image.py Outdated Show resolved Hide resolved
src/PIL/Image.py Outdated Show resolved Hide resolved
@radarhere
Copy link
Member Author

I've added a commit to resolve #7207 - the change involves the same logic presented initially for PngImagePlugin, just this time for GifImagePlugin.

@radarhere radarhere added the GIF label Jun 21, 2023
@hugovk hugovk merged commit be4bfaa into python-pillow:main Jun 30, 2023
@radarhere radarhere deleted the apng branch June 30, 2023 06:50
radarhere added a commit to radarhere/Pillow that referenced this pull request Jun 30, 2023
@radarhere
Copy link
Member Author

Pillow 10.0.0 has now been released with this fix.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
4 participants