[OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images

Jayathirth D V jayathirth.d.v at oracle.com
Wed Mar 28 08:13:23 UTC 2018


Hi Krishna,

 

Thanks for providing the inputs.

 

1.       I suggest to rename considerTransparentPixel as isAlphaPresent.

As discussed I have added new name as isTransparentPixelPresent()

 

2.       You can refactor the function body as below:

      Return ((destinationBands == null ||

            destinationBands.length == 4) &&

             metadata.tRNS_colorType == PNG_COLOR_RGB);

 

                Changes are made.

 

3.       From line 1088 through 1113, I see some repeated calculations like bytesPerRow etc. Why can't we reuse the previously defined variables? Apart from destBands, all the other calculations look similar and repeated.

 

Previous to this change all the values like inputsBands, bytesPerRow, eltsPerRow were same between the decoded output and destination image.
Now because we are changing only the destination image due to the presence of transparent pixel, we need both these set of values. inputsBands, bytesPerRow, eltsPerRow  will be used  for decoded output and destBands, destEltsPerRow for destination image. Its better if don't mingle code flow or variables between these two code paths.

 

4.       When you are copying the pixels to a writable raster, at line 1273, you could reduce the repetition of the lines, by just having one statement inside if as below:

for (int i = 0; i < passWidth; i++) {

                             byteData[destidx] = curr[srcidx];

                             byteData[destidx + 1] = curr[srcidx + 1];

                             byteData[destidx + 2] = curr[srcidx + 2];

                        if (curr[srcidx] == (byte)metadata.tRNS_red &&

                             curr[srcidx + 1] == (byte)metadata.tRNS_green &&

                             curr[srcidx + 2] == (byte)metadata.tRNS_blue)

                         {

                             byteData[destidx + 3] = (byte)0;

                        } else {

                            byteData[destidx + 3] = (byte)255;

                         }

                         srcidx += 3;

                         destidx += 4;

                     }

Same thing can be done for the loop that executes for 16bit pixel data.

 

Changes are made.

 

 

Please find updated webrev for review :

http://cr.openjdk.java.net/~jdv/6788458/webrev.03/ 

 

Thanks,

Jay

 

From: Krishna Addepalli 
Sent: Wednesday, March 28, 2018 11:52 AM
To: Jayathirth D V; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images

 

Hi Jay,

 

I have some points as below:

1.       I suggest to rename considerTransparentPixel as isAlphaPresent.

2.       You can refactor the function body as below:

      Return ((destinationBands == null ||

            destinationBands.length == 4) &&

             metadata.tRNS_colorType == PNG_COLOR_RGB);

3.       From line 1088 through 1113, I see some repeated calculations like bytesPerRow etc. Why can't we reuse the previously defined variables? Apart from destBands, all the other calculations look similar and repeated.

4.       When you are copying the pixels to a writable raster, at line 1273, you could reduce the repetition of the lines, by just having one statement inside if as below:

for (int i = 0; i < passWidth; i++) {

                             byteData[destidx] = curr[srcidx];

                             byteData[destidx + 1] = curr[srcidx + 1];

                             byteData[destidx + 2] = curr[srcidx + 2];

                        if (curr[srcidx] == (byte)metadata.tRNS_red &&

                             curr[srcidx + 1] == (byte)metadata.tRNS_green &&

                             curr[srcidx + 2] == (byte)metadata.tRNS_blue)

                         {

                             byteData[destidx + 3] = (byte)0;

                        } else {

                            byteData[destidx + 3] = (byte)255;

                         }

                         srcidx += 3;

                         destidx += 4;

                     }

Same thing can be done for the loop that executes for 16bit pixel data.

 

 

I haven't yet checked the test cases.

 

Thanks,

Krishna

 

 

From: Jayathirth D V 
Sent: Wednesday, March 28, 2018 9:52 AM
To: 2d-dev <HYPERLINK "mailto:2d-dev at openjdk.java.net"2d-dev at openjdk.java.net>
Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images

 

Hello All,

 

I have enhanced both the test case to verify pixels which not only match tRNS transparent pixel data but also for them which doesn't match tRNS transparent pixel data.

 

Please find updated webrev for review:

http://cr.openjdk.java.net/~jdv/6788458/webrev.02/ 

 

Thanks,

Jay

 

From: Jayathirth D V 
Sent: Wednesday, March 28, 2018 8:28 AM
To: 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images

 

Hello All,

 

I just realized that the test case Read16BitPNGWithTRNSChunk.java is creating (50, 50) image(which was done while debugging the issue), which is not needed as we need single pixel to verify the fix as it is done in Read8BitPNGWithTRNSChunk.java. I have updated Read16BitPNGWithTRNSChunk.java to reflect this small change.

 

Please find updated webrev for review:

http://cr.openjdk.java.net/~jdv/6788458/webrev.01/ 

 

Thanks,

Jay

 

From: Jayathirth D V 
Sent: Tuesday, March 27, 2018 6:51 PM
To: 2d-dev
Subject: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images

 

Hello All,

 

Please review the following solution in JDK11 :

 

Bug : https://bugs.openjdk.java.net/browse/JDK-6788458 

Webrev : http://cr.openjdk.java.net/~jdv/6788458/webrev.00/ 

 

Issue: When we try to read non-indexed RGB PNG image having transparent pixel information in tRNS chunk, ImageIO.PNGImageReader ignores the transparent pixel information. If we use Toolkit.getDefaultToolkit().createImage() it doesn't ignore the transparent pixel information.

 

Root cause:  In ImageIO.PNGImageReader() we store tRNS chunk values in readMetadata() -> parse_tRNS_chunk (), but we never use the tRNS R, G, B value to compare with decoded image information. Also in ImageIO.PNGImageReader() for IHDR colortype RGB we use only 3 channel destination BufferedImage. So even if we use the tRNS chunk value we need destination BufferedImage of 4 channels to represent transparency.

 

Solution: While assigning destination image in PNGImageReader.getImageTypes() if we know that PNG image is of type RGB and has tRNS chunk then we need to assign a destination image having 4 channels. After that in decodePass() we need to create 4 channel destination raster and while we store decoded image information into the destination BufferedImage we need to compare decoded R, G, B values with tRNS R, G, B values and store appropriate alpha channel value.

 

Also we use 4 channel destination image in case of RGB image only when tRNS chunk is present and ImageReadParam.destinationBands is null or ImageReadParam.destinationBands is equal to 4.

 

One more change is that we have optimization in PNGImageReader.readMetadata() where if ignoreMetadata is true and IHDR colortype is not of type PNG_COLOR_PALETTE, we ignore all the chunk data and just try to find first IDAT chunk. But if we need tRNS chunk values in case of IHDR colortype PNG_COLOR_RGB we need to parse tNRS chunk also. So in readMetadata() also appropriate changes are made.

 

Note : Initially the enhancement request was present only for 8 bit RGB PNG images but after making more modifications realized that we can add support for 16 bit RGB image also by making similar incremental changes. The tRNS chunk value is still ignored for Gray PNG image. If we need to support PNG_COLOR_GRAY image with tRNS chunk(which is very rare) we can take that up in a separate bug as it needs different set of changes.

 

Thanks,

Jay

 
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/2d-dev/attachments/20180328/98b302e6/attachment-0001.html>


More information about the 2d-dev mailing list