RFR: 8356137: GifImageDecode can produce opaque image when disposal method changes [v9]

[Jayathirth D V]

On Sun, 1 Jun 2025 06:43:44 GMT, Jeremy Wood <duke at openjdk.org> wrote:

>> This resolves a gif parsing bug where an unwanted opaque rectangle could appear under these conditions:
>> 
>> 1. The disposal method for frames is 1 (meaning "do not dispose", aka "DISPOSAL_SAVE")
>> 2. The transparent pixel is non-zero
>> 3. There's more than one such consecutive frame
>> 
>> Previously: the GifImageDecoder would leave the saved_image pixels as zero when they were supposed to be transparent. This works great if the transparent pixel index is zero, but it flood fills the background of your frame with the zeroeth color otherwise.
>> 
>> I wrote four PRs that share the GifComparison class in this PR. Once any of them clear code review the other PRs will be much simpler:
>> 
>> 1. [8357034](https://github.com/openjdk/jdk/pull/25264)
>> 2. [8356137](https://github.com/openjdk/jdk/pull/25044) (this one)
>> 3. [8356320](https://github.com/openjdk/jdk/pull/25076)
>> 4. [8351913](https://github.com/openjdk/jdk/pull/24271)
>
> Jeremy Wood has updated the pull request incrementally with four additional commits since the last revision:
> 
>  - 8356137: adding copyright
>  - 8356137: remove test file now that we use GifBuilder
>  - 8356137: Use new GifBuilder for 8356137 test case
>    
>    This is in response to:
>    https://github.com/openjdk/jdk/pull/25044#pullrequestreview-2871107750
>  - 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

> > I think this is better then current approach of having check multiple times.
> 
> OK. I switched back to this approach.
> 
> (The downside being: now we'll invoke Arrays.fill(..) for _all_ gifs with that disposal method. I assume (?) gifs with this architecture/problem are a small subset of that group; but it's impossible to quantify that hunch.)

Yes and also this will happen only when transparentPixelIndex > 0.

I have again verified updated test with updated code and change looks good to me. Only thing is change should be updated to follow 80 characters per line, i see at many places it is >100 characters and difficult to read.

I have given full CI test run and will update here after the results.

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

PR Comment: https://git.openjdk.org/jdk/pull/25044#issuecomment-3051978321