RFR: 8356320: GifImageDecoder can produce wrong image when GCE changes transparent pixel index [v3]

Jeremy Wood duke at openjdk.org
Mon Aug 4 17:02:00 UTC 2025


On Fri, 11 Jul 2025 23:01:24 GMT, Jeremy Wood <duke at openjdk.org> wrote:

>> When a gif frame's disposal code is DISPOSAL_BGCOLOR and we're maintaining a saved image: we have to flood-fill the frame's bounds. And if the transparent pixel index changes across frames: we could sometimes flood fill the wrong value, resulting in an opaque rectangle in the output image.
>> 
>> 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)
>> 2. ~~[8356137](https://github.com/openjdk/jdk/pull/25044)~~ (integrated)
>> 3. [8356320](https://github.com/openjdk/jdk/pull/25076) (this one)
>> 4. [8351913](https://github.com/openjdk/jdk/pull/24271)
>> 
>> The test in this PR required [8356137's PR](https://github.com/openjdk/jdk/pull/25044) to pass.
>> 
>> This bug can be observed reading this gif animation:
>> https://tenor.com/view/fnf-free-download-fnf-free-download-fnf-fd-cyan-gif-15334412416964826224
>
> Jeremy Wood has updated the pull request with a new target base due to a merge or a rebase. The pull request now contains 31 commits:
> 
>  - Merge branch 'master' into JDK-8356320
>    
>    # Conflicts:
>    #	test/jdk/sun/awt/image/gif/GifBuilder.java
>    #	test/jdk/sun/awt/image/gif/GifComparison.java
>  - Merge branch 'master' into JDK-8356320
>  - 8356320: Use new GifBuilder and remove leo.gif
>    
>    This is an extension of work for this PR:
>    https://github.com/openjdk/jdk/pull/25044#pullrequestreview-2871107750
>  - 8356137: adding copyright
>  - 8356137: Adding GifBuilder to dynamically create test file
>    
>    This can be used by multiple gif tests in this directory.
>    
>    This is in response to:
>    https://github.com/openjdk/jdk/pull/25044#pullrequestreview-2871107750
>  - 8356137: trivial javadoc update
>  - 8356137: only inspect last frame of gif
>    
>    This makes the main() method much less useful, so I removed it too. (I originally used this class to explore a folder of hundreds of gifs to look for discrepancies. But the discrepancies were rarely only on the last frame.)
>    
>    This is in response to:
>    https://github.com/openjdk/jdk/pull/25044#discussion_r2109298270
>  - 8356137: Adding copyright to GifComparison
>  - 8356320: trivial whitespace and comment changes
>  - Merge branch 'master' into JDK-8356320
>  - ... and 21 more: https://git.openjdk.org/jdk/compare/a86dd56d...3deeb497

Regarding the image dumping logic: that will (mostly) automatically appear once we merge https://github.com/openjdk/jdk/pull/25264 (specifically https://github.com/openjdk/jdk/pull/25264/commits/b418a02d1d8854d58aa8da9805c69e1ddd640502 ). I'll remove `rfr` for now until that ticket is merged.

-------------

PR Comment: https://git.openjdk.org/jdk/pull/25076#issuecomment-3151582438


More information about the client-libs-dev mailing list