[OpenJDK 2D-Dev] [11] RFR JDK-5109146: PNGMetadata Background color initialization from standard metadata is incomplete
Phil Race
philip.race at oracle.com
Mon May 14 16:49:20 UTC 2018
+1 from me.
-phil.
On 05/09/2018 05:43 AM, Jayathirth D V wrote:
>
> Hi Krishna,
>
> Thanks for the review.
>
> 1.I see that the fix is to update the bKGD_colorType for Background
> color child. However, don’t you need to do the same thing for paletted
> images as well?
>
> /We are not updating bKGD_colorType, we are adding missing
> bKGD_colorType when user wants to specify background color. In
> standard metadata if user wants to provide color palette and with
> corresponding background color index then he should specify palette
> using Chroma->Palette node and background index using Chroma->
> //BackgroundIndex node./
>
> 1.As for the test case, I’m not sure how ImageReaders and ImageWriters
> work.
>
> I see that you query them from ImageIO, so the question is, do you
> need to get a new ImageReader/ImageWriter for each image read/write
> operation?
>
> If so, then, there is a potential problem when you call
> VerifyNativeRGBValuesFromBKGDChunk() and
> VerifyStandardRGBValuesFromBKGDChunk() in succession, since each in
> turn calls writeAndReadMetaData() and the writer is disposed.
>
> On the other hand, if querying for the reader/writer for once is
> enough, then you can initialize the image reader/writer in the static
> block itself, and then run the whole test.
>
> /We don’t need multiple instances of writer/reader.I have changed the
> code to create ImageReader & Writer instance only once. Also
> PNGImageWriter/Reader doesn’t override dispose() method present in
> super class which is just empty method, that is the reason why we
> didn’t get NPE when writer was getting disposed and used again
> previously. I have still kept writer & reader dispose() calls at the
> end in test case which will be used if we plan to override these
> methods in future./
>
> Also I noticed that test case was not failing even without fix. This
> was happening because of usage of common metadata between
> VerifyNativeRGBValuesFromBKGDChunk() &
> VerifyStandardRGBValuesFromBKGDChunk(). I have updated the test case
> to create independent metadata variables for
> VerifyNativeRGBValuesFromBKGDChunk() &
> VerifyStandardRGBValuesFromBKGDChunk().
>
> Please find updated webrev for review:
>
> http://cr.openjdk.java.net/~jdv/5109146/webrev.02/
> <http://cr.openjdk.java.net/%7Ejdv/5109146/webrev.02/>
>
> Thanks,
>
> Jay
>
> *From:* Krishna Addepalli
> *Sent:* Wednesday, May 09, 2018 11:51 AM
> *To:* 2d-dev
> *Subject:* Re: [OpenJDK 2D-Dev] [11] RFR JDK-5109146: PNGMetadata
> Background color initialization from standard metadata is incomplete
>
> Hi Jay,
>
> Here are my observations/questions:
>
> 1.I see that the fix is to update the bKGD_colorType for Background
> color child. However, don’t you need to do the same thing for paletted
> images as well?
>
> 2.As for the test case, I’m not sure how ImageReaders and ImageWriters
> work.
>
> I see that you query them from ImageIO, so the question is, do you
> need to get a new ImageReader/ImageWriter for each image read/write
> operation?
>
> If so, then, there is a potential problem when you call
> VerifyNativeRGBValuesFromBKGDChunk() and
> VerifyStandardRGBValuesFromBKGDChunk() in succession, since each in
> turn calls writeAndReadMetaData() and the writer is disposed.
>
> On the other hand, if querying for the reader/writer for once is
> enough, then you can initialize the image reader/writer in the static
> block itself, and then run the whole test.
>
> Thanks,
>
> Krishna
>
> *From:* Jayathirth D V
> *Sent:* Wednesday, April 18, 2018 3:34 PM
> *To:* 2d-dev
> *Subject:* RE: [OpenJDK 2D-Dev] [11] RFR JDK-5109146: PNGMetadata
> Background color initialization from standard metadata is incomplete
>
> Hello All,
>
> Since changes related to JDK-6574555 is pushed.
>
> Please find new webrev which captures test case changes over JDK-6574555.
>
> http://cr.openjdk.java.net/~jdv/5109146/webrev.01/
> <http://cr.openjdk.java.net/%7Ejdv/5109146/webrev.01/>
>
> Thanks,
>
> Jay
>
> *From:* Jayathirth D V
> *Sent:* Tuesday, April 17, 2018 3:34 PM
> *To:* 2d-dev
> *Subject:* [OpenJDK 2D-Dev] [11] RFR JDK-5109146: PNGMetadata
> Background color initialization from standard metadata is incomplete
>
> Hello All,
>
> Please review the following fix in JDK11 :
>
> Bug : https://bugs.openjdk.java.net/browse/JDK-5109146
>
> Webrev : http://cr.openjdk.java.net/~jdv/5109146/webrev.00/
> <http://cr.openjdk.java.net/%7Ejdv/5109146/webrev.00/>
>
> _Issue:_ PNGMetadata.mergeStandardTree() function doesn’t set proper
> bKGD colortype.
>
> _Solution:_ When bKGD R, G, B values are unique we need to store
> appropriate bKGD colortype in metadata.
>
> _Note_ : Test case which is present as part of review in JDK-6574555
> is only updated to handle regression scenario for this bug also.
> Regression scenario between this bug and JDK-6574555 differ only in
> what type(native/standard) metadata we use to merge bKGD RGB values.
>
> Thanks,
>
> Jay
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180514/e142f439/attachment.html>
More information about the 2d-dev
mailing list