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

Custom GIF emoji - alpha color v2 #16370

Open
alfonsrv opened this issue Sep 17, 2020 · 6 comments
Open

Custom GIF emoji - alpha color v2 #16370

alfonsrv opened this issue Sep 17, 2020 · 6 comments

Comments

@alfonsrv
Copy link

alfonsrv commented Sep 17, 2020

As per #10351, the error still occurs with Zulip 3.2. Uploading custom emojis GIFs results in unexpected behavior. Turning on or off the optimize= flag does not change anything about it.

Example: uploading a Giphy sticker, mostly results in a black background – here, on the official Zulip server.

nice-normal, ref
nice-black
Side note: It would also be nice if extra padding was added to the top and bottom to convert the image to 1:1 ratio rather than cut it off.

Another artifact I encountered was uploading another sticker, resulting in not only a black background but also some other artifacts.

nice-normal, ref
nice-black

@timabbott
Copy link
Sponsor Member

#10351 has useful context; debugging this will likely involve spending some type looking at Pillow and debugging, and potentially reporting an upstream bug. @HarshitOnGitHub did this with the previous iteration of this sort of bug, and may have some context as well.

@akshatdalton
Copy link
Member

akshatdalton commented Sep 21, 2020

@alfonsrv @timabbott can you help me reproduce this bug as this seems to be working fine with me.
Uploaded GIFs do not show any black background.

custom emoji

@alfonsrv
Copy link
Author

@akshatdalton
What I do is:

  1. Download the image via Chrome
  2. Go to chat.zulip.org, click Settings -> Organization -> Custom Emoji
  3. Choose the GIF I just downloaded, name it e.g. akshat1 and upload.

In the preview pane you should now see the same result as shown above. Not sure what you're doing, but this clearly doesn't seem like an upload to Zulip, as it would cut off the sides of the Nice-GIF in the current way Zulip handles GIF conversion, as outlined above.

@akshatdalton
Copy link
Member

I am working on this issue.

akshatdalton added a commit to akshatdalton/zulip that referenced this issue Oct 2, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes zulip#16370 (partially).
akshatdalton added a commit to akshatdalton/zulip that referenced this issue Oct 3, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes part of zulip#16370.
akshatdalton added a commit to akshatdalton/zulip that referenced this issue Oct 3, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes part of zulip#16370.
timabbott pushed a commit that referenced this issue Oct 7, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes part of #16370.
AstonBraham pushed a commit to AstonBraham/zulip that referenced this issue Oct 9, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes part of zulip#16370.
timabbott pushed a commit that referenced this issue Oct 11, 2020
This preserves the alpha layer on GIF images that need to be resized
before being uploaded.  Two important changes occur here:

1. The new frame is a *copy* of the original image, which preserves the
   GIF info.
2. The disposal method of the original GIF is preserved.  This
   essentially determines what state each frame of the GIF starts from
   when it is drawn; see PIL's docs:
   https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#saving
   for more info.

This resolves some but not all of the test cases in #16370.
@timabbott
Copy link
Sponsor Member

Several classes of this form of issue were fixed in #16456 thanks to excellent work by @codypiersall; the remaining ones appear to be an upstream PIL bug (which we should link here once it's filed).

vishkrish200 pushed a commit to vishkrish200/zulip that referenced this issue Oct 15, 2020
This preserves the alpha layer on GIF images that need to be resized
before being uploaded.  Two important changes occur here:

1. The new frame is a *copy* of the original image, which preserves the
   GIF info.
2. The disposal method of the original GIF is preserved.  This
   essentially determines what state each frame of the GIF starts from
   when it is drawn; see PIL's docs:
   https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#saving
   for more info.

This resolves some but not all of the test cases in zulip#16370.
GregoryDRowe pushed a commit to GregoryDRowe/zulip that referenced this issue Oct 20, 2020
Replaced ImageOps.fit by ImageOps.pad, in zerver/lib/upload.py, which
returns a sized and padded version of the image, expanded to fill the
requested aspect ratio and size.
Fixes part of zulip#16370.
GregoryDRowe pushed a commit to GregoryDRowe/zulip that referenced this issue Oct 20, 2020
This preserves the alpha layer on GIF images that need to be resized
before being uploaded.  Two important changes occur here:

1. The new frame is a *copy* of the original image, which preserves the
   GIF info.
2. The disposal method of the original GIF is preserved.  This
   essentially determines what state each frame of the GIF starts from
   when it is drawn; see PIL's docs:
   https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#saving
   for more info.

This resolves some but not all of the test cases in zulip#16370.
AstonBraham pushed a commit to AstonBraham/zulip that referenced this issue Oct 20, 2020
This preserves the alpha layer on GIF images that need to be resized
before being uploaded.  Two important changes occur here:

1. The new frame is a *copy* of the original image, which preserves the
   GIF info.
2. The disposal method of the original GIF is preserved.  This
   essentially determines what state each frame of the GIF starts from
   when it is drawn; see PIL's docs:
   https://pillow.readthedocs.io/en/stable/handbook/image-file-formats.html#saving
   for more info.

This resolves some but not all of the test cases in zulip#16370.
@gnprice
Copy link
Member

gnprice commented Mar 13, 2023

Reopening because the issue hasn't been fixed. Some recent discussion in chat.

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

No branches or pull requests

4 participants