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

Phil Race philip.race at oracle.com
Wed Apr 11 16:52:50 UTC 2018


http://cr.openjdk.java.net/~jdv/6788458/webrev.07/src/java.desktop/share/classes/com/sun/imageio/plugins/png/PNGMetadata.java.udiff.html

There's an extra line added at the end which you can remove.

Why do the tests needs to create temp disk files ?
Can't we do this all with ByteArrayInput/OutputStream ?
Less worry about clean up there.

I note that this

286                 boolean tRNSTransparentPixelPresent =
1287                     theImage.getSampleModel().getNumBands() == inputBands + 1 &&
1288                     metadata.hasTransparentColor();

is evaluated for each row when I expect it only needs to be
evaluated once for the whole decode pass.
But the overhed is presumably low and it is next to the use so
I think it should be OK as is.

However the code that does the transparent per-pixel settings does seem
needlessly wasteful. These can be moved outside at least the entire
row decode :

final int[] temp = new int[inputBands + 1];

final int opaque = (bitDepth < 16) ? 255 : 65535;

Note that I added final to them ..

Also array accesses are (relatively) slow but since you need
an array to pass to setPixel and there's only two accesses I
don't see how we can improve on that here by assigning
the values from the ps array to local variables.

-phil.


On 04/09/2018 11:19 PM, Jayathirth D V wrote:
> HI Prahalad,
>
> Thanks for your inputs.
>
> Regarding-
> Few minor corrections to PNGImageReader- . Line: 1310
>      . This missed my eye in the last review.
>      . We could create int[] temp outside the for loop. I tried this with your changes & it works.
>      . It's an optimization so that we don't end up creating int[] array for every pixel in the row.
>
> Before sending webrev.06, I actually made similar changes of moving temp[] creation and/or calculating "opaque" to outside of for loop.
> But this will cause creation of temp[] and/ or calculation of "opaque" for each row in all the cases where there is no RGB/Gray & tRNS combination. So I reverted those change before sending webrev.06.
> I would like to keep all the calculation specific to RGB/Gray & tRNS combination in its own code flow.
>
> Other modifications are made.
> Please find updated webrev for review:
> http://cr.openjdk.java.net/~jdv/6788458/webrev.07/
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Prahalad Kumar Narayanan
> Sent: Monday, April 09, 2018 2:23 PM
> 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
>
> Hello Jay
>
> I looked into the latest code changes.
> The code changes are good.
>
> Few minor corrections to PNGImageReader- . Line: 1310
>      . This missed my eye in the last review.
>      . We could create int[] temp outside the for loop. I tried this with your changes & it works.
>      . It's an optimization so that we don't end up creating int[] array for every pixel in the row.
>
> . Line: 1320, 1329
>      . Align the opening { on the same line as the if statement.
>
> . Line: 1479
>      . Correct the indentation of the expression within if condition.
>      . A suggestion that could help:
>              if ((theImage.getSampleModel().getNumBands()
>                      == inputBandsForColorType[colorType] + 1)
>                      && metadata.hasTransparentColor()) {
>                  // code block
>                  checkReadParamBandSettings(param,
>                      ...
>              }
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Jayathirth D V
> Sent: Friday, April 06, 2018 5:19 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images
>
> Hi Prahalad,
>
> Thanks for your inputs.
>
> Regarding :
> File: PNGImageReader.java
> Line: 1329, 1342
>      . The else block for if (check for transparent pixel) is redundant across both PNG_COLOR_RGB and PNG_COLOR_GRAY.
>      . We could use Arrays.fill with opaque value and set the alpha to 0 if pixel is transparent.
>              int[] temp = new int[inputBands + 1];
>              int opaque = (bitDepth < 16) ? 255 : 65535;
>              Arrays.fill(temp, opaque);
>      . All subsequent operations checking for transparent color value and setting alpha to 0 would be required.
>
> I think we should not use Arrays.fill() at this place and assign values to all the channels. It is a per pixel operation and we would be writing data twice.
>
> Instead of using Arrays.fill(),  I thought of just assigning value to alpha channel:
>   int[] temp = new int[inputBands + 1];
>   temp[inputBands] = (bitDepth < 16) ? 255 : 65535;  if (metadata.tRNS_colorType == PNG_COLOR_RGB) {
>
> I think even doing this is not ideal because anyway for all the pixels we will be checking pixel data with tRNS data, and assign value in alpha channel. There is no need for us to assign some value and override it again later if a condition satisfies.
> So I am assigning opaque value to alpha channel at same place. But I have reduced the LOC by using ternary operator for determining the opaque value for alpha channel.
>
> All other changes are updated.
> Please find updated webrev for review:
> http://cr.openjdk.java.net/~jdv/6788458/webrev.06/
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Prahalad Kumar Narayanan
> Sent: Friday, April 06, 2018 1:42 PM
> 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
>
> Hello Jay
>
> Good day to you.
>
> I looked into the latest changes.
> The code changes are nearly complete. Just a few tweaks.
>
> File: PNGImageReader.java
> Line: 1280
> Rephrase from:
>                 /*
>                  * In case of colortype PNG_COLOR_RGB or PNG_COLOR_GRAY
>                  * if we have transparent pixel information from tRNS chunk
>                  * we need to consider that also and store proper information
>                  * in alpha channel.
>                  *
>                  * Also we create destination image with extra alpha channel
>                  * in getImageTypes() when we have tRNS chunk for colorType
>                  * PNG_COLOR_RGB or PNG_COLOR_GRAY.
>                  */
> Rephrase to:
>                 /*
>                  * For PNG images of color type PNG_COLOR_RGB or PNG_COLOR_GRAY
>                  * that contain a specific transparent color (given by tRNS
>                  * chunk), we compare the decoded pixel color with the color
>                  * given by tRNS chunk to set the alpha on the destination.
>                  */
>
> File: PNGImageReader.java
> Line: 1588
> Rephrase from:
>          /*
>           * In case of PNG_COLOR_RGB or PNG_COLOR_GRAY, if we
>           * have transparent pixel information in tRNS chunk
>           * we create destination image having alpha channel.
>           */
>
> Rephrase to:
>          /*
>           * For PNG images of color type PNG_COLOR_RGB or PNG_COLOR_GRAY that
>           * contain a specific transparent color (given by tRNS chunk), we add
>           * ImageTypeSpecifier(s) that support transparency to the list of
>           * supported image types.
>           */
>
> File: PNGImageReader.java
> Line(s): 1290, 1493, 1596, 1619
>      . The lines mentioned above check whether metadata has specific transparent color using-
>              metadata.tRNS_present &&
>              (metadata.tRNS_colorType == PNG_COLOR_RGB ||
>               metadata.tRNS_colorType == PNG_COLOR_GRAY)
>
>      . The above check is not only redundant but also metadata specific.
>      . We could move the code to a common method in PNGMetadata-
>          boolean hasTransparentColor() {
>              return tRNS_present
>                     && (tRNS_colorType == PNG_COLOR_RGB
>                         || tRNS_colorType == PNG_COLOR_GRAY);
>          }
>      . I understand 1596 and 1619 check for specific color type but they can be avoided with this method as well.
>      . As per the specification, tRNS values depend on the image's color type.
>      . This will reduce the complex check from-
>              if (theImage.getSampleModel().getNumBands() ==
>                      inputBandsForColorType[colorType] + 1 &&
>                  metadata.tRNS_present &&
>                  (metadata.tRNS_colorType == PNG_COLOR_RGB ||
>                   metadata.tRNS_colorType == PNG_COLOR_GRAY))
>              {
>        to-
>              if (metadata.hasTransparentColor()
>                  && (theImage.getSampleModel().getNumBands() ==
>                      inputBandsForColorType[colorType] + 1)) {
>
> File: PNGImageReader.java
> Line: 1329, 1342
>      . The else block for if (check for transparent pixel) is redundant across both PNG_COLOR_RGB and PNG_COLOR_GRAY.
>      . We could use Arrays.fill with opaque value and set the alpha to 0 if pixel is transparent.
>              int[] temp = new int[inputBands + 1];
>              int opaque = (bitDepth < 16) ? 255 : 65535;
>              Arrays.fill(temp, opaque);
>      . All subsequent operations checking for transparent color value and setting alpha to 0 would be required.
>
> File: PNGImageReader.java
> All modified Lines:
>      . The opening braces '{' can appear in the new line. Some engineers do follow this.
>      . Since the rest of the code aligns opening braces in the same line as the ' if ' statement we could follow the same for code readability.
>
> Test File: ReadPngGrayImageWithTRNSChunk.java and
>                     ReadPngRGBImageWithTRNSChunk.java
>      . The finally blocks should check whether the objects- ImageOutputStream and File, are not null.
>      . The call to directory(while is a File).delete() is not required in my view. Verify this once.
>      . Dispose the writer after the write operation is complete.
>
> Thank you
> Have a good day
>
> Prahalad N.
>
> -----Original Message-----
> From: Jayathirth D V
> Sent: Thursday, April 05, 2018 3:26 PM
> To: Prahalad Kumar Narayanan; 2d-dev
> Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images
>
> Hello Prahalad,
>
> Thank you for the inputs.
>
> I gave updated the code to not change ImageTypeSpecifier of passRow. Now while storing the pixel values into imRas we comparing the pixel RGB/Gray values to tRNS RGB/Gray values and storing appropriate value for alpha channel.
> I have added support to use tRNS_Gray value when IHDR color type is PNG_COLOR_GRAY - We are now creating 2 channel destination image whenever we see that tRNS chunk is present for PNG_COLOR_GRAY in getImageTypes().
> isTransparentPixelPresent() code is removed and we are using available tRNS properties.
>
> I have merged previous test cases. Now we have 2 test cases each verifying the code changes for RGB & Gray image for 8 & 16 bit depth values.
> Please find updated webrev for review:
> http://cr.openjdk.java.net/~jdv/6788458/webrev.05/
>
> Thanks,
> Jay
>
> -----Original Message-----
> From: Prahalad Kumar Narayanan
> Sent: Monday, April 02, 2018 12:03 PM
> To: Krishna Addepalli; Jayathirth D V; 2d-dev
> Subject: RE: [OpenJDK 2D-Dev] [11] RFR JDK-6788458: PNGImageReader ignores tRNS chunk while reading non-indexed RGB images
>
> Hello Jay
>
> Good day to you.
>
> I looked into the latest code changes.
> Please find my review observation & suggestions herewith.
>
> My understanding of problem statement:
> . As I understand, our PNGImageReader parses tRNS chunk information from metadata whenever ignoreMetadata is false or paletted image is decoded.
> . But doesn't use the transparency information contained in the chunk to return BufferedImages with alpha/ transparency.
>
> Appropriate Changes:
>
> File: PNGImageReader.java,
> Method: Iterator<ImageTypeSpecifier> getImageTypes
> Line: 1633
>      . This method is internally invoked while creating BufferedImage for destination at Line 1409:
>                  theImage = getDestination(param,
>                                        getImageTypes(0),
>                                        width,
>                                        height);
>      . The move to add ImageTypeSpecifier based on BufferedImage.TYPE_4BYTE_ABGR as the first element (index: 0) of the list is good.
>
> File: PNGImageReader.java
> Method: readMetadata
> Line: 731
>      . The if check for parsing tRNS chunk even when ignoreMetadata is set to true is good.
>      . The chunk's information will be needed.
>
> Other Observation & Suggestions:
>
> File: PNGImageReader.java,
> Method: decodePass
> Line: (1088 - 1112), (1277-1327)
>      . In the code chunks of listed line numbers, you have modified passRow's internal type- ImageTypeSpecifier, and thus its manipulation logic as well.
>      . The changes are not required in my view. Reasons are-
>          . passRow corresponds to the data from input image source.
>          . imgRas corresponds to the destination buffered image.
>          . To return a BufferedImage with alpha/ transparency, it would suffice to update imgRas with alpha component at Line 1371.
>                  imRas.setPixel(dstX, dstY, ps); // if ps contains alpha, it would suffice the need.
>          . However, the proposed changes modify how we interpret the source through passRow.
>          . To set alpha component on imgRas, we would require comparison of color components present in passRow with that of tRNS chunk.
>          . But, passRow 's internal type- ImageTypeSpecifier need not be changed. This way most of the complex code changes can be avoided.
>
> File: PNGImageReader.java,
> Method: isTransparentPixelPresent
> Line: 1545
>      . The logic of this method considers both input image source and destination bands to decide whether transparency is available.
>      . In my view, The method should consider only the alpha in input image source and not the destination.
>      . Here are code points where the method is invoked
>             a) 1089 : To create a writable raster- passRow. passRow corresponds to the data of image source.
>             b) 1279 : To update the passRow's databuffer. passRow corresponds to the data of image source.
>             c) 1512 : To pass appropriate band length to checkParamBandSettings. (Hardcoded value: 4)
>             d) 1632 & 1648 : To update ArrayList<ImageTypeSpecifier> l based on presence of tRNS in image source.
>      . Each of the locations except (c) pertain to image source and not the destination.
>      . One possible solution would be to discard this method and use the existing flag tRNS_present at (c).
>
>      . Besides, The proposed changes don't address images with gray scale with alpha in tRNS chunk.
>          . Your first email reads- "PNG_COLOR_GRAY image with tRNS chunk(which is very rare)"
>          . Just curious to know if there 's any basis for this observation ?
>      . If we consider suggestions on decodePass method, I believe, we could support setting alpha values for grayscale images as well.
>
> Line: 1555
>      . Please use tRNS_colorType together with tRNS_present flag.
>      
> File: PNGImageReader.java,
> Method: readMetadata
> Line: 701
>      Rephrase From:
>           * Optimization: We can skip reading metadata if ignoreMetadata
>           * flag is set and colorType is not PNG_COLOR_PALETTE. But we need
>           * to parse only the tRNS chunk even in the case where IHDR colortype
>           * is not PNG_COLOR_PALETTE, because we need tRNS chunk transparent
>           * pixel information for PNG_COLOR_RGB while storing the pixel data
>           * in decodePass().
>      To:
>           * Optimization: We can skip reading metadata if ignoreMetadata
>           * flag is set and colorType is not PNG_COLOR_PALETTE. However,
>           * we parse tRNS chunk to retrieve the transparent color from the
>           * metadata irrespective of the colorType. Doing so, helps
>           * PNGImageReader to appropriately identify and set transparent
>           * pixels in the decoded image.
>
> File: PNGMetadata.java
> Line: 271
>      . Reset the tRNS_colorType to -1 in the reset() method.
>      . This will not concern if tRNS_colorType is used in conjunction with tRNS_present flag.
>      . However, the new method isTransparentPixelAvailable() uses tRNS_colorType directly.
>      . When the same ImageReader is used to read multiple PNG images, this could pose a problem.
>      
> Thank you
> Have a good day
>
> Prahalad N.
>
>
> ----- Original Message -----
> From: Krishna Addepalli
> Sent: Monday, April 02, 2018 11:40 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
>
> Hmmm, thanks for the clarification, but this raises one more question: Now that you are initializing the colorType to an invalid value, do you need to check appropriately in all the places where the color is being used? Otherwise, it might lead to undefined behavior.
> Also, I would like you to either add a comment at the point of initialization or better yet, define another static constant of "UNDEFINED", and then set the tRNS_colorType to this value, so that the code reads correct naturally without any comments.
>
> Thanks,
> Krishna
>
> From: Jayathirth D V
> Sent: Monday, April 2, 2018 11:33 AM
> To: Krishna Addepalli <krishna.addepalli at oracle.com>; 2d-dev <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
>
> Hi Krishna,
>
> The constant values for different color types is :
>      static final int PNG_COLOR_GRAY = 0;
>      static final int PNG_COLOR_RGB = 2;
>      static final int PNG_COLOR_PALETTE = 3;
>      static final int PNG_COLOR_GRAY_ALPHA = 4;
>      static final int PNG_COLOR_RGB_ALPHA = 6;
>
> Since tRNS_colorType is used to hold one of the above mentioned values if we don't initialize it to some unused value(here we are using -1) we will be representing PNG_COLOR_GRAY by default.
> By default the value will be -1 after the change and after we parse tRNS chunk it will contain appropriate value. The initialization of tRNS_colorType change can be considered as newly added check.
>
> Thanks,
> Jay
>
> From: Krishna Addepalli
> Sent: Monday, April 02, 2018 9:56 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,
>
> The changes look fine. However, I have one more question: Why did you have to explicitly specify the initial value to "tRNS_colorType" in PNGMetadata.java? How is it affecting your implementation?
>
> Thanks,
> Krishna
>
> From: Jayathirth D V
> Sent: Wednesday, March 28, 2018 1:43 PM
> To: Krishna Addepalli <krishna.addepalli at oracle.com>; 2d-dev <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
>
> 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 <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



More information about the 2d-dev mailing list