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

[Krishna Addepalli]

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 <2d-dev@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