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

GIF has transparency issues, not all fixed with PR #6176 #6260

Closed
raygard opened this issue May 2, 2022 · 4 comments · Fixed by #6176
Closed

GIF has transparency issues, not all fixed with PR #6176 #6260

raygard opened this issue May 2, 2022 · 4 comments · Fixed by #6176
Labels

Comments

@raygard
Copy link
Contributor

raygard commented May 2, 2022

What did you do?

Copied an animated GIF:

with Image.open('aniship.gif') as im:
    im.save(f'new.gif', save_all=True, optimize=True, interlace=0)

What did you expect to happen?

Hoped the copy would be something like the original aniship.gif:

aniship

What actually happened?

With PR #6176 applied, I get an artifact of (apropos of the nautical theme) aqua background on first frame:

new

What are your OS, Python and Pillow versions?

Discussion:

Issues of transparency continue to vex GIF handling. Not sure if I have a complete handle on this yet but will try.

In _write_multiple_frames() there is a call to _normalize_palette(), which calls .remap_palette() which calls convert("L") which changes .info["transparency"]. This happens after the fix of PR #6176. Also, .remap_palette() changes the palette but does not make a corresponding change to the transparency index. So here is a proposal (not yet a PR because it conflicts with PR #6176 and because I don't know if it's a good fix):

Add a function:

def _find_new_transparency(im, used_palette_colors):
    try:
        return used_palette_colors.index(im.info.get("transparency"))
    except ValueError:
        return None

Then in _normalize_palette() change

        im = im.remap_palette(used_palette_colors)
    else:
        used_palette_colors = _get_optimize(im, info)
        if used_palette_colors is not None:
            return im.remap_palette(used_palette_colors, source_palette)

to:

        transparency = _find_new_transparency(im, used_palette_colors)
        im = im.remap_palette(used_palette_colors)
        if transparency is not None:
            im.info["transparency"] = transparency
    else:
        used_palette_colors = _get_optimize(im, info)
        if used_palette_colors is not None:
            transparency = _find_new_transparency(im, used_palette_colors)
            im = im.remap_palette(used_palette_colors, source_palette)
            if transparency is not None:
                im.info["transparency"] = transparency
            return im

This changes .info["transparency"] to the remapped transparency index, if it was in the used palette to begin with. (More about this below... still a problem.)

Then in _write_multiple_frames() put the call to _normalize_palette() ahead of the set of encoderinfo:

            im_frame = _normalize_palette(im_frame, palette, encoderinfo)
            if "transparency" in im_frame.info:
                encoderinfo.setdefault("transparency", im_frame.info["transparency"])

I don't know if changing the order of these statements from PR #6176 cause a problem or not.

Another problem is: what if there is a transparency index (i.e. the Transparent Color Flag is set) but the index is to an unused color? My "fix" above to _normalize_palette() will not change im.info["transparency"] and may leave it referring to a color that is used and should not be transparent. This happens with that Tazspin.gif I used in Issue #6259. Only affects about 3 pixels there so it is not visible but could be a problem for some GIFs. Maybe there's an easy way to force the index to an unused color in this case?

BTW aniship.gif has no GCT, only LCTs (and they're all the same). The current 9.1.0 either with or without PR #6176 will create a reduced GCT of 8 slots (of used colors) but leaves the transparent color index at 14, pointing to a non-existent entry.

@raygard
Copy link
Contributor Author

raygard commented May 9, 2022

I’ll work on this.

@radarhere
Copy link
Member

For your first problem, I've pushed another commit to #6176, along the same lines of thoughts that you have. With that, and disposal=2, your image saves correctly.

from PIL import Image
with Image.open('aniship.gif') as im:
    im.save(f'new.gif', save_all=True, optimize=True, interlace=0, disposal=2)

@radarhere
Copy link
Member

I've now pushed another commit to #6176 to remove transparency if it is not used.

So this issue should now be resolved by that PR. Let me know if you find otherwise.

@raygard
Copy link
Contributor Author

raygard commented May 24, 2022

Thank you. I must apologize about the code I posted at the top of this. I neglected the disposal=2 argument that I certainly did use to get the posted image with odd artifact. Your improved #6176 seems to do the trick, and more cleanly than what I was proposing.

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

Successfully merging a pull request may close this issue.

2 participants