[OpenJDK 2D-Dev] RFR 6488522: PNG writer should permit setting compression level and iDAT chunk maximum size
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Thu Dec 10 12:44:18 UTC 2015
On 09/12/15 17:17, Laurent Bourgès wrote:
> Hi Sergey,
>
> Thanks for your comments, did you review the PNGImageWriter changes too ?
A few notes:
- Should PNGImageWriteParam.unsetCompression resets the
compressionType to compressionTypes[0]?
- Is it necessary to copy the checks in PNGImageWriteParam from the
parent? We can just call super() and expect that the parent class
validate current state.
> Yes, I could check that every png file is not zero-length or that better
> compression gives smaller file size !
It would be good, because right now it check not all changes in the fix.
> However, my changes rely on the public Deflater API that supports
> compression levels between 0 to 9.
>
> I suppose that this Deflater class is already covered by its own tests
> so I would prefer leaving the test as it is (no verification): if the
> Deflater raises any RuntimeException, then the PNGWriterCompressionTest
> will fail.
But this test is passed even for such fix:
---
a/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java
Thu Dec 10 14:21:44 2015 +0300
+++
b/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGImageWriter.java
Thu Dec 10 15:15:41 2015 +0300
@@ -280,6 +280,7 @@
super();
this.canWriteProgressive = true;
this.locale = locale;
+ this.canWriteCompressed = true;
}
}
> Why not but it is a different scope ?
>
> I wrote the test focused on testing my patch to validate all my changes
> and fix ASAP that very old bug (created in 2006) !
I understood, but to change the test and cover of existed/future plugins
should not require a big effort. It does not mean that you need to fix
some bugs(if any then create a new CR) in other plugins.
See for example this bug JDK-8144245 which was found in a new plugin
just because the test cover all of them. But this is up2you.
--
Best regards, Sergey.
More information about the 2d-dev
mailing list