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 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 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
