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

Fix issues with GIF transparency #3434

Closed
wants to merge 13 commits into from
Binary file added Tests/images/transparent_dispose.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
10 changes: 10 additions & 0 deletions Tests/test_file_gif.py
Original file line number Diff line number Diff line change
Expand Up @@ -331,6 +331,16 @@ def test_dispose_background():
pass


def test_transparent_dispose():
expected_colors = [(2, 1, 2), (0, 1, 0), (2, 1, 2)]
with Image.open("Tests/images/transparent_dispose.gif") as img:
for frame in range(3):
img.seek(frame)
for x in range(3):
color = img.getpixel((x, 0))
assert color == expected_colors[frame][x]


def test_dispose_previous():
with Image.open("Tests/images/dispose_prev.gif") as img:
try:
Expand Down
23 changes: 19 additions & 4 deletions src/PIL/GifImagePlugin.py
Original file line number Diff line number Diff line change
Expand Up @@ -262,16 +262,20 @@ def _seek(self, frame):
# do not dispose or none specified
self.dispose = None
elif self.disposal_method == 2:
# replace with background colour
# replace with transparency if there is one, otherwise
# background colour
Copy link
Member

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.

Copy link
Member

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.


# only dispose the extent in this frame
x0, y0, x1, y1 = self.dispose_extent
dispose_size = (x1 - x0, y1 - y0)

Image._decompression_bomb_check(dispose_size)
self.dispose = Image.core.fill(
"P", dispose_size, self.info.get("background", 0)
)

if frame_transparency is not None:
bg = frame_transparency
else:
bg = self.info.get("background", 0)
self.dispose = Image.core.fill("P", dispose_size, bg)
else:
# replace with previous contents
if self.im:
Expand Down Expand Up @@ -309,6 +313,17 @@ def _seek(self, frame):
if self.palette:
self.mode = "P"

def load_prepare(self):
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]))
Comment on lines +317 to +325
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
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.

Copy link
Member

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.


def tell(self):
return self.__frame

Expand Down