RFR: 8357034: GifImageDecoder can produce wrong transparent pixels [v4]

[Jeremy Wood]

On Thu, 10 Jul 2025 09:18:47 GMT, Jayathirth D V <jdv at openjdk.org> wrote:

>> Jeremy Wood has updated the pull request incrementally with five additional commits since the last revision:
>> 
>>  - 8356320: Use new GifBuilder and remove ukraine-flag.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
>
> src/java.desktop/share/classes/sun/awt/image/GifImageDecoder.java line 401:
> 
>> 399:             if (isSimple) {
>> 400:                 for (int a = 0; a < saved_image.length; a++) {
>> 401:                     if ((saved_image[a] & 0xff) == trans_pixel) {
> 
> This check is scanning whole saved image(until 241 index in this scenario) for each scanline. So for each GIF frame we are running this loop height times.
> 
> We can maintain a common boolean variable and verify isSimple/not just before parseImage() call. This will make sure that we perform this check only once per GIF frame.
> 
> To verify, updated the code to move this check inside readImage() and just before parseImage() call and it works fine.

Good point. I [added the field](https://github.com/openjdk/jdk/pull/25264/commits/b8a8bf7d3d81372f686d48aec2a1733955ebd545) `isSimpleSavedImageComparison`. In addition to the unit test I also tested the gifs mentioned in the PR description.

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

PR Review Comment: https://git.openjdk.org/jdk/pull/25264#discussion_r2198870961