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

Do not load transparent pixels from subsequent GIF frames #5333

Merged
merged 3 commits into from Mar 31, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
Binary file added Tests/images/different_transparency.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Binary file added Tests/images/different_transparency_merged.gif
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
17 changes: 15 additions & 2 deletions Tests/test_file_gif.py
Expand Up @@ -468,12 +468,25 @@ def test_dispose2_background(tmp_path):
assert im.getpixel((0, 0)) == 0


def test_iss634():
def test_transparency_in_second_frame():
with Image.open("Tests/images/different_transparency.gif") as im:
assert im.info["transparency"] == 0

# Seek to the second frame
im.seek(im.tell() + 1)
assert im.info["transparency"] == 0

assert_image_equal_tofile(im, "Tests/images/different_transparency_merged.gif")


def test_no_transparency_in_second_frame():
with Image.open("Tests/images/iss634.gif") as img:
# Seek to the second frame
img.seek(img.tell() + 1)
assert "transparency" not in img.info

# All transparent pixels should be replaced with the color from the first frame
assert img.histogram()[img.info["transparency"]] == 0
assert img.histogram()[255] == 0


def test_duration(tmp_path):
Expand Down
4 changes: 2 additions & 2 deletions Tests/test_file_webp_animated.py
Expand Up @@ -45,12 +45,12 @@ def test_write_animation_L(tmp_path):
# Compare first and last frames to the original animated GIF
orig.load()
im.load()
assert_image_similar(im, orig.convert("RGBA"), 25.0)
assert_image_similar(im, orig.convert("RGBA"), 32.9)
orig.seek(orig.n_frames - 1)
im.seek(im.n_frames - 1)
orig.load()
im.load()
assert_image_similar(im, orig.convert("RGBA"), 25.0)
assert_image_similar(im, orig.convert("RGBA"), 32.9)


@pytest.mark.xfail(is_big_endian(), reason="Fails on big-endian")
Expand Down
41 changes: 20 additions & 21 deletions src/PIL/GifImagePlugin.py
Expand Up @@ -145,7 +145,6 @@ def _seek(self, frame):
self.dispose_extent = [0, 0, 0, 0] # x0, y0, x1, y1
self.__frame = -1
self.__fp.seek(self.__rewind)
self._prev_im = None
self.disposal_method = 0
else:
# ensure that the previous frame was loaded
Expand Down Expand Up @@ -174,6 +173,8 @@ def _seek(self, frame):
self.palette = copy(self.global_palette)

info = {}
frame_transparency = None
interlace = None
while True:

s = self.fp.read(1)
Expand All @@ -192,7 +193,7 @@ def _seek(self, frame):
#
flags = block[0]
if flags & 1:
info["transparency"] = block[3]
frame_transparency = block[3]
info["duration"] = i16(block, 1) * 10

# disposal method - find the value of bits 4 - 6
Expand Down Expand Up @@ -250,9 +251,6 @@ def _seek(self, frame):
# image data
bits = self.fp.read(1)[0]
self.__offset = self.fp.tell()
self.tile = [
("gif", (x0, y0, x1, y1), self.__offset, (bits, interlace))
]
break

else:
Expand All @@ -278,11 +276,26 @@ def _seek(self, frame):
except (AttributeError, KeyError):
pass

if not self.tile:
if interlace is not None:
transparency = -1
if frame_transparency is not None:
if frame == 0:
self.info["transparency"] = frame_transparency
else:
transparency = frame_transparency
self.tile = [
(
"gif",
(x0, y0, x1, y1),
self.__offset,
(bits, interlace, transparency),
)
]
else:
# self.__fp = None
raise EOFError

for k in ["transparency", "duration", "comment", "extension", "loop"]:
for k in ["duration", "comment", "extension", "loop"]:
if k in info:
self.info[k] = info[k]
elif k in self.info:
Expand All @@ -295,20 +308,6 @@ def _seek(self, frame):
def tell(self):
return self.__frame

def load_end(self):
ImageFile.ImageFile.load_end(self)

# if the disposal method is 'do not dispose', transparent
# pixels should show the content of the previous frame
if self._prev_im and self._prev_disposal_method == 1:
# we do this by pasting the updated area onto the previous
# frame which we then use as the current image content
updated = self._crop(self.im, self.dispose_extent)
self._prev_im.paste(updated, self.dispose_extent, updated.convert("RGBA"))
self.im = self._prev_im
self._prev_im = self.im.copy()
self._prev_disposal_method = self.disposal_method

def _close__fp(self):
try:
if self.__fp != self.fp:
Expand Down
4 changes: 3 additions & 1 deletion src/decode.c
Expand Up @@ -430,7 +430,8 @@ PyImaging_GifDecoderNew(PyObject *self, PyObject *args) {
char *mode;
int bits = 8;
int interlace = 0;
if (!PyArg_ParseTuple(args, "s|ii", &mode, &bits, &interlace)) {
int transparency = -1;
if (!PyArg_ParseTuple(args, "s|iii", &mode, &bits, &interlace, &transparency)) {
return NULL;
}

Expand All @@ -448,6 +449,7 @@ PyImaging_GifDecoderNew(PyObject *self, PyObject *args) {

((GIFDECODERSTATE *)decoder->state.context)->bits = bits;
((GIFDECODERSTATE *)decoder->state.context)->interlace = interlace;
((GIFDECODERSTATE *)decoder->state.context)->transparency = transparency;

return (PyObject *)decoder;
}
Expand Down
3 changes: 3 additions & 0 deletions src/libImaging/Gif.h
Expand Up @@ -30,6 +30,9 @@ typedef struct {
*/
int interlace;

/* The transparent palette index, or -1 for no transparency. */
int transparency;

/* PRIVATE CONTEXT (set by decoder) */

/* Interlace parameters */
Expand Down
38 changes: 21 additions & 17 deletions src/libImaging/GifDecode.c
Expand Up @@ -248,29 +248,33 @@ ImagingGifDecode(Imaging im, ImagingCodecState state, UINT8 *buffer, Py_ssize_t
/* To squeeze some extra pixels out of this loop, we test for
some common cases and handle them separately. */

/* FIXME: should we handle the transparency index in here??? */

if (i == 1) {
if (state->x < state->xsize - 1) {
/* Single pixel, not at the end of the line. */
*out++ = p[0];
state->x++;
/* If we have transparency, we need to use the regular loop. */
if (context->transparency == -1) {
if (i == 1) {
if (state->x < state->xsize - 1) {
/* Single pixel, not at the end of the line. */
*out++ = p[0];
state->x++;
continue;
}
} else if (state->x + i <= state->xsize) {
/* This string fits into current line. */
memcpy(out, p, i);
out += i;
state->x += i;
if (state->x == state->xsize) {
NEWLINE(state, context);
}
continue;
}
} else if (state->x + i <= state->xsize) {
/* This string fits into current line. */
memcpy(out, p, i);
out += i;
state->x += i;
if (state->x == state->xsize) {
NEWLINE(state, context);
}
continue;
}

/* No shortcut, copy pixel by pixel */
for (c = 0; c < i; c++) {
*out++ = p[c];
if (p[c] != context->transparency) {
*out = p[c];
}
out++;
if (++state->x >= state->xsize) {
NEWLINE(state, context);
}
Expand Down