-
-
Notifications
You must be signed in to change notification settings - Fork 2.2k
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
Fix issues with GIF transparency #3434
Conversation
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
3ca1889
to
2d72036
Compare
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This test is fixed by the next commits.
Allow the transparency index to be passed to the native decoder. If not -1, pixels with this index will be left at their previous value. This only adds the decoder support and isn't active yet.
Remove the special case for disposal_method == 1 and handle GIF transparency by telling the decoder the transparent index.
…ame 0. This ensures that transparent pixels in the first frame data clear to transparency and not to some previously decoded frame.
Background dispose should prefer the transparency color over the background color, if there is one. This matches other decoders and makes transparent_dispose.gif decode correctly.
_seek checked whether self.im is None, but if we've decoded a frame and then seeked back to 0, self.im will be set to the previously decoded frame. Instead, check if self.tile has data, which means _seek set up a tile to decode and it hasn't been decoded yet.
1c74072
to
4ff4770
Compare
self.dispose = Image.core.fill("P", self.size, | ||
self.info["background"]) | ||
# replace with transparency if there is one, otherwise | ||
# background colour |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Why potentially use the transparency? I don't see this in the spec.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've found https://legacy.imagemagick.org/Usage/anim_basics/#dispose
is cleared to transparency
...
There is some thinking that rather than clearing the overlaid area to the transparent color, this disposal should clear it to the 'background' color meta-data setting stored in the GIF animation
...
However all modern web browsers clear just the area that was last overlaid to transparency, as such this is now accepted practice, and what IM now follows.
I've cherry-picked two commits from here into #5333 |
I've removed the tests from here that pass without this PR. |
super(GifImageFile, self).load_prepare() | ||
|
||
if self.__frame == 0: | ||
# On the first frame, clear the buffer. Use the transparency index | ||
# if we have one, otherwise use the background index. | ||
default_color = self.info.get( | ||
"transparency", self.info.get("background", 0) | ||
) | ||
self.im.paste(default_color, (0, 0, self.size[0], self.size[1])) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
super(GifImageFile, self).load_prepare() | |
if self.__frame == 0: | |
# On the first frame, clear the buffer. Use the transparency index | |
# if we have one, otherwise use the background index. | |
default_color = self.info.get( | |
"transparency", self.info.get("background", 0) | |
) | |
self.im.paste(default_color, (0, 0, self.size[0], self.size[1])) | |
if not self.im: | |
# On the first frame, clear the buffer. Use the transparency index | |
# if we have one, otherwise use the background index. | |
default_color = self.info.get( | |
"transparency", self.info.get("background", 0) | |
) | |
self.im = Image.core.fill(self.mode, self.size, default_color) | |
super(GifImageFile, self).load_prepare() |
This saves the parent load_prepare
from creating an image only to have it overwritten here. Instead, this just creates the image and the parent does not.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I've created #5557 with this functionality, minus the background color fallback.
I've pulled the remaining parts of this into #5557 |
Fixes #3433.