On Sat, 3 Jan 2026 04:31:15 GMT, Jeremy Wood <[email protected]> wrote:

>> test/jdk/sun/awt/image/png/PngImageDecoder8BitTest.java line 174:
>> 
>>> (failed to retrieve contents of file, check the PR for context)
>> Am I correct in assuming that both models end up using `PNGImageDecoder` 
>> under the covers? If so, won't `expected` and `actual` always match, even if 
>> there is a bug in `PNGImageDecoder`? I wonder if it would be better to keep 
>> the original `BufferedImage` around (the one we draw on), use it as 
>> `expected`, and compare it to the two model-generated images.
>
>> Am I correct in assuming that both models end up using PNGImageDecoder under 
>> the covers?
> 
> It'd make a lot of sense if they shared the same code, but no. It is my 
> understanding we have two separate decoders. ImageIO uses the 
> com.sun.imageio.plugins.png.PNGImageReader , and this PR modifies the 
> sun.awt.image.PNGImageDecoder.
> 
> The fact that the PNGImageDecoder is a little slower than the ImageIO classes 
> came to my attention because I was comparing the two decoding models, and the 
> older sun.awt classes [appear to be 
> faster](https://docs.google.com/spreadsheets/d/1SoiqnDPSVALb4xraq5hBIGAQLrOQTzA-XuVDohZbCJs/edit?usp=sharing)
>  than the newer ImageIO classes -- except for this one case. This PR will fix 
> this discrepancy.
> 
> (Separately: I've submitted a few other PRs that similarly focus on the older 
> sun.awt decoding classes. See 
> [JDK-8357034](https://github.com/openjdk/jdk/pull/25264) , 
> [JDK-8356320](https://github.com/openjdk/jdk/pull/25076) , 
> [JDK-8356137](https://github.com/openjdk/jdk/pull/25044) , 
> [JDK-8351913](https://github.com/openjdk/jdk/pull/24271) . They generally 
> compare the two models against each other for correctness. I also visually 
> inspected the results at the time to triple-check, but in all of those cases 
> ImageIO was already "doing the right thing". I guess I've made it my goal to 
> bring the older sun.awt decoding classes up-to-date.)
> 
>> I wonder if it would be better to keep the original BufferedImage around 
>> (the one we draw on), use it as expected, and compare it to the two 
>> model-generated images.
> 
> Sure. I updated this test so it avoids decoding with ImageIO.

I had no idea, thanks for clarifying!

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

PR Review Comment: https://git.openjdk.org/jdk/pull/29004#discussion_r2661180031

Reply via email to