On Fri, 16 May 2025 10:26:47 GMT, Jeremy Wood <[email protected]> wrote:
> If there are two consecutive frames that use DISPOSAL_SAVE, but the > transparent pixel index changed: we might accidentally send the wrong data to > the ImageConsumer. > > We already had logic that submits info "the hard way" (see comment in code); > this PR just makes sure we trigger that block. > > There are a cluster of four related PRs that share the GifComparison class in > this PR. > > 1. [8357034](https://github.com/openjdk/jdk/pull/25264) (this one) > 2. ~~[8356137](https://github.com/openjdk/jdk/pull/25044)~~ (integrated) > 3. [8356320](https://github.com/openjdk/jdk/pull/25076) > 4. [8351913](https://github.com/openjdk/jdk/pull/24271) > > This bug can be observed reading these gif animations: > > https://giphy.com/gifs/fomoduck-duck-fomo-ZUlzR40oGACqNmy0q8 > https://media4.giphy.com/media/Zbf4JQzcDhzeraQvk9/giphy.gif > https://giphy.com/gifs/computer-working-cat-LHZyixOnHwDDy > https://giphy.com/gifs/90s-fgfgif-r4BmI9xUaDuKJQdd3l > > ### Describing This Bug > > The steps/explanation that lead to this failure are a little complicated. > I'll try to summarize the original bug here. > > All three frames in the unit test's GIF use disposal code doNotDispose, > meaning the frames are layered one on top of the previous. > > The first frame produces 3 circles from left to right using these colors: > 0xff0000 (red) 0xffff00 (yellow) 0x00ff00 (green) > > The background is 0x00ffff (cyan). It is the transparent pixel, so the first > frame is: > > <img width="200" height="100" alt="frame_0" > src="https://github.com/user-attachments/assets/8bb53530-fc2b-4a8b-b3a6-5c58782ea5ab"; > /> > > Now we start processing the second frame. The pixels are the same, but the > color model has changed so the zeroeth color (red) is the transparent index. > > When the GifImageDecoder layers the 2nd frame on top of the 1st frame, it > notices that `model.equals(saved_model)` is `false`. This means it enter a > block of code that refers to "the hard way" of passing new pixel information > to the ImageConsumers. > > The 2nd frame, after being painted on the 1st frame, looks like: > <img width="200" height="100" alt="frame_1" > src="https://github.com/user-attachments/assets/0c8ca55b-e25d-418e-bb95-2e6f9e427c08"; > /> > > The 3rd frame is exactly the same as the 2nd. But here's where the bug is: > now `model.equals(saved_model)` is `true`, so GifImageDecoder tries to pass > the pixel information the easy way. The result is: > > <img width="200" height="100" alt="frame_2_awt" > src="https://github.com/user-attachments/assets/835dd918-f1bc-459e-9967-295fd01fa61c"; > /> > > Because it'... This pull request has now been integrated. Changeset: acc8a76d Author: Jeremy Wood <[email protected]> Committer: Phil Race <[email protected]> URL: https://git.openjdk.org/jdk/commit/acc8a76db2314211dd29a5b84c5bbe73d9055c76 Stats: 147 lines in 5 files changed: 112 ins; 4 del; 31 mod 8357034: GifImageDecoder can produce wrong transparent pixels Reviewed-by: jdv, prr ------------- PR: https://git.openjdk.org/jdk/pull/25264
