On Thu, 10 Jul 2025 09:18:47 GMT, Jayathirth D V <j...@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
