8356320: GifImageDecoder can produce wrong image when GCE changes transparent pixel index #25076
Conversation
Merge openjdk/jdk into mickleness/jdk
Merge openjdk/jdk
Merge from openjdk/jdk
Updating mickleness/jdk from openjdk/jdk
updating to openjdk/jdk
In this ticket the problem was: `tpix` was the transparent pixel index of the current `model`. However `decoder.saved_image` uses its own IndexColorModel (see `decoder.saved_model`). In a simple gif these two models are the same, but in this case the gif has some frames that change the transparent pixel index between frames. We were filling the wrong tpix value, resulting in an opaque rectangle.
|
👋 Welcome back mickleness! A progress list of the required criteria for merging this PR into |
|
❗ This change is not yet ready to be integrated. |
|
@mickleness The following label will be automatically applied to this pull request:
When this pull request is ready to be reviewed, an "RFR" email will be sent to the corresponding mailing list. If you would like to change these labels, use the /label pull request command. |
Webrevs
|
This comment was marked as outdated.
This comment was marked as outdated.
|
|
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: openjdk#25044 (comment)
This can be used by multiple gif tests in this directory. This is in response to: openjdk#25044 (review)
This is an extension of work for this PR: openjdk#25044 (review)
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.
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:
This PR will fail without 8356137's PR; I'm removing the RFR label from this PR until that PR is merged.
Progress
Issue
Reviewing
Using
gitCheckout this PR locally:
$ git fetch https://git.openjdk.org/jdk.git pull/25076/head:pull/25076$ git checkout pull/25076Update a local copy of the PR:
$ git checkout pull/25076$ git pull https://git.openjdk.org/jdk.git pull/25076/headUsing Skara CLI tools
Checkout this PR locally:
$ git pr checkout 25076View PR using the GUI difftool:
$ git pr show -t 25076Using diff file
Download this PR as a diff file:
https://git.openjdk.org/jdk/pull/25076.diff
Using Webrev
Link to Webrev Comment