GIF transparency not applied correctly

[zewt]

The attached animated GIF image is decoded incorrectly (testing with master). This was exported by Photoshop and is decoded correctly by ffmpeg and other decoders.

The file is 3x1 pixels and three frames: XBX GBG XBX (X: transparent, G: green, B: blue).
Pillow decodes it as: XBX GXG XBX, leaving pixel 1x0 in the second frame transparent when it should be blue.

The issue seems to be that the transparent color index isn't handled correctly. It should leave the existing pixel in the buffer unchanged. Instead, the decoder overwrites the pixel with the transparent index, changing the blue pixel to transparent. There's a special case for dispose mode 1 in load_end, but it's a little odd. (The dispose mode only affects the next frame, but this is affecting the current frame, and it doesn't do anything with other dispose modes.)

I'll send a PR shortly to fix this. It adds two tests using this image: one for this issue, and one for an issue that came up while implementing this. (Reading img.n_pixels changed the image, since the image wasn't being reset when decoding frame 0 after rewinding. This didn't matter before, since the decoder overwrote the whole image, but it matters now.) The load_end special case is removed, since the compositing happens in the decoder. Background dispose clears to the transparency color if one is set, which matches other decoders.

If anyone's interested in trying to make the decoder work everywhere, it might be useful to check other projects for comprehensive GIF tests. There's probably one in Chromium.

[Artheau]

Another fix is described in #3665

[hugovk]

Another fix is described in #3665

PR #3708 has been merged which fixed #3665.

Is this issue still valid?

[zewt]

From the title ("GIF deltas from background colour"), it sounds unrelated. This is an issue with the core processing of GIF transparency, not background dispose.

(I haven't received any responses to this other than three variations of "can this be closed?" for 8 months, so I don't think anyone cares about this. I'd rather not spend a bunch of time refreshing my memory on this unless there's some actual interest in it beyond people looking for tickets to close.)

[radarhere]

Testing, I find that #5125 fixed the issue where the second frame middle pixel was not blue.

[radarhere]

Reading img.n_pixels changed the image, since the image wasn't being reset when decoding frame 0 after rewinding.

If this was the test_n_frames_invariant from your PR, #3434, I find that it passes even with Pillow 5.3.0, which was available at the time this issue was created.

If this was test_rewind from your PR, I find that it was also fixed by #5125