Custom GIF emoji - alpha color

[julievalet]

Hello,

After the resizing function for the custom gif was added, every time I upload a new custom GIF emoji, the background, which should be transparent, is now black.

Thanks @HarshitOnGitHub, for the resizing function, do you think this issue could be related ?

[zulipbot]

Hello @zulip/server-emoji members, this issue was labeled with the "area: emoji" label, so you may want to check it out!

[timabbott]

Tagging as a priority, since it'd be really good to have this fixed before the 1.9 release next week.

[trueskawka]

I noticed it as well, here are some examples.

I had a .gif with a transparent background, which worked fine in a message body.

But when I added it as a custom emoji, it showed with a black background. I also used an example of a different emoji I added a while ago, that has a transparent background, so it's not breaking the behaviour for existing custom emoji.

It got a temporary fix through adding a white background.

I'm also putting the original .gif for reference here, maybe there's something off with the file itself.

[moagstar]

I did some investigation on this issue and it seems that the problem lies within Pillow itself. There are two things that lead me to believe this...

1. If you change the code in upload.py to save the frames as pngs rather than all as a gif then the alpha is correctly preserved.
2. If you change the call to save in upload.py to use optimize=False then the alpha is preserved in the gif (but it looks horrible).

So it looks like there is something in GifImagePlugin optimization that breaks the alpha. How to properly fix this with pillow is beyond my understanding, perhaps a gif or pillow wizard might be able to figure out what is going on. However perhaps there a couple of other solutions...

1. Do the files actually need to be resized? Is this a space optimization or something or is there some technical reason why they need to be 25 x 25?
2. Perhaps using an imagemagick binding (e.g. wand) could work, since resizing using ezgif.com doesn't have this problem)
3. At the moment the image is always resized, even if it is the correct size. Perhaps if it is already in the correct size the resizing could be skipped, so users can at least resize the image themselves first to preserve the alpha.

[HarshitOnGitHub]

@moagstar Thanks for investing time in this, your investigation is exactly correct, this is a problem with pillow's optimization related code. I had tried various solutions for fixing it but none of them worked(related PR #10679). Probably the next step on this is to do some research on adding an external library that can handle GIFs(and would be best if we could also add svg support using it). :)

[timabbott]

We should open a bug upstream if we believe there's something wrong with Pillow -- they may be able to fix that. I'm not super excited about adding additional code pathways for editing images from a security perspective.

On the sizing question, we need them to be square, but 25x25 resizing is more focused on ensuring we don't consume too much bandwidth. We could consider changing the logic to not do the resizing for square animated GIFs under a certain size.

[stretch4x4]

We have just migrated and come across this too, having an option to disable or customise the resizing would be good as we have a local install and bandwidth isn't too much of an issue at this point in time.
Most of the images we have used in our old hipchat server previously are getting crushed pretty badly so far.

[moagstar]

I found a number of upstream issues related to gif and transparency...

https://github.com/python-pillow/Pillow/issues?utf8=%E2%9C%93&q=is%3Aissue+is%3Aopen+gif

So perhaps it's not one single issue, it looks like the handling of transparency is a bit flaky.

@timabbott I understand the concern about adding more code paths and dependencies. So I agree that this is probably not the best approach.

I think if the reason for resizing is bandwidth then it might actually be making things worse trying to resize, for example consider the following gifs (original downloaded from https://cultofthepartyparrot.com/, resized by uploading to zulip):

The original is actually smaller than the resized (15kb vs 15.5kb). Perhaps the best approach would be to have a file size limit on images which can be used as gifs?

[moagstar]

Actually I'm not sure where I got the 25x25 value from since it looks like the gifs are resized to 64x64, apologies if I cause any confusion!

[moagstar]

@zulipbot claim

[zulipbot]

Welcome to Zulip, @moagstar! We just sent you an invite to collaborate on this repository at https://github.com/zulip/zulip/invitations. Please accept this invite in order to claim this issue and begin a fun, rewarding experience contributing to Zulip!

Here's some tips to get you off to a good start:

• Join me on the Zulip developers' server, to get help, chat about this issue, and meet the other developers.
• Sign the Dropbox Contributor License Agreement, so that Zulip can use your code.
• Unwatch this repository, so that you don't get 100 emails a day.

As you work on this issue, you'll also want to refer to the Zulip code contribution guide, as well as the rest of the developer documentation on that site.

See you on the other side (that is, the pull request side)!

[sjuxax]

This is still impacting a pretty large number of emoji imported from slackmojis.com. The issue does indeed seem to originate in Pillow, see python-pillow/Pillow#3665, where there is ongoing discussion around how and/or if to fix.

The docs mention potentially moving this task to Thumbor some day, which may be a good idea since Thumbor appears to use either OpenCV or GraphicsMagick, either of which presumably better understand how to handle animated GIFs.

[sjuxax]

Looking into this more, it seems that it'd require some involved logic to implement something that works reasonably well for the general case.

I hacked up the code to use Wand, and then to use a subprocess call out to the imagemagick binary itself, but there are still some unsolved issues. The long and short of it is, no surprise, getting decent transparency handling with GIF is really annoying. Matters are substantially complicated by night mode, since GIFs must display properly against two nearly-opposite background colors.

Potential solutions would be:
a) not use GIF and use an animation format that supports real transparency, which doesn't seem very plausible;
b) generate two gifs that are transparency-optimized for the different modes (one for night, one for day);
c) attempt to shave/dither enough of the matting off that things look acceptable against either background.

It'd take some work to do this elegantly for any image uploaded. We can certainly fix the massive wreck that results from pushing a GIF through Pillow by just using Wand or something similar instead (which, e.g., it seems that Wagtail does through its wrapper Willow), but the issue of transparency in both night and day mode will be harder.

[timabbott]

Thanks for the investigation @sjuxax! I think in our ideal world, Pillow would just fix the bug here, since I'm not super excited about adding additional dependencies just for animated GIFs. Using thumbor or one if its dependencies is certainly an option as we already have that.

I suppose we could also just allow uploading raw animated GIFs more generally, and just warn users if the GIF is larger that it'll make their Zulip use extra bandwidth when people use it.

I don't see a good general solution to transparency for both day and night mode that doesn't require some care in the design of the emoji itself; the basic issue is e.g. that if you have an all-black emoji, it won't look good on a night theme.

[timabbott]

The upstream Pillow bug was fixed via python-pillow/Pillow#3665 (comment) a couple weeks ago, and https://github.com/python-pillow/Pillow/releases suggests there's a new Pillow release containing the fix.

@HarshitOnGitHub do you have time to investigate whether upgrading Pillow fixes this issue of Pillow mangling animated GIFs?

(If not, other folks should feel free to take this).

[HarshitOnGitHub]

@timabbott I think we have already migrated to the latest Pillow library in 0b35bb9. I can verify that it is working great! 🎉

[timabbott]

Awesome! So sounds like we can fully close this now.

[alfonsrv]

Still getting a black background when uploading GIFs like these:

Example 1: with optimize=True:

Example 2 with optimize=False:

[alfonsrv]

@timabbott – still happening on the latest version

[timabbott]

@HarshitOnGitHub would you be up for investigating this?

[timabbott]

Likely we'll want to file this as a bug with Pillow upstream.

[Xenophilicy]

@timabbott I am having this same exact issue with the black background. If you do create an issue for a bug report with Pillow upstream, would you mind linking it here, please? Thanks!

[alfonsrv]

This issue should be reopened @timabbott

[timabbott]

Let's open a new issue linking to this one -- that'll get more visibility on it.