On Sun, 1 Jun 2025 07:13:14 GMT, Jeremy Wood <d...@openjdk.org> 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. >> >> 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) (this one) >> 2. [8356137](https://github.com/openjdk/jdk/pull/25044) >> 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 > > 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. ------------- PR Review Comment: https://git.openjdk.org/jdk/pull/25264#discussion_r2197097522
