Looks fine.
On 16/11/2018 02:16, Jayathirth D V wrote:
Hi Sergey,
As discussed offline I did more analysis on whether we can use common variable to determine number
of bands. Since we have "outputSampleSize.length - 1" and "inputBands + 1" kind
of things.
Actually scale array will be used on input data(ps[]), so we should use input
bands value to create and use scale array. Before JDK-6788458 there was no
difference between input bands and output bands so we didn't see any issue. But
after JDK-6788458 number of bands data is different between input and output
data for PNG_COLOR_RGB/GRAY having tRNS chunk.
So I have made change to use inputBands data for creation and use of scale
array. Ran complete imageio jtreg and JCK tests there are no failures.
Please find updated webrev for review:
http://cr.openjdk.java.net/~jdv/8211795/webrev.01/
Thanks,
Jay
-----Original Message-----
From: Jayathirth D V
Sent: Wednesday, November 14, 2018 1:09 PM
To: Sergey Bylokhov; 2d-dev
Subject: RE: [OpenJDK 2D-Dev] [12] RFR JDK-8211795:
ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458
Hi Sergey,
Thanks for the review.
As you pointed out, yes the AIOOB is happening for ps[b]. I have updated the
analysis in JBS bug also.
Basically the calculation of numBands is not proper because we take numBands
value from destination image. This destination image will have extra alpha
channel for Gray or RGB input data(ps[]) which will throw AIOOB.
So we need to update the logic of how we calculate numBands different for PNG
Gray/RGB image havng tRNS chunk. Fortunately, webrev.00 is actually doing this
job.
Regarding whether we need to change scale array logic : We expect first 3
channel to be RGB and first channel to be Gray for PNG_COLOR_RGB and
PNG_COLOR_GRAY respectively. So just updating numBands information will create
proper scale array. So there is no need to change the scale array logic.
History JDK-6788458 : Toolkit was able to show transparent color information
for RGB/Gray PNG image when it has tRNS chunk, but ImageIO didn't support it.
To use tRNS data and show transparent color in output image we needed to add
extra alpha channel for PNG RGB/Gray image with tRNS chunk. But fix present in
JDK-6788458 didn't handle the case where bitDepth adjustment is needed and we
are using band information from output image(having extra alpha channel) on
input image which has no alpha channel. Change in numBands logic for this bug
fixes that issue.
Thanks,
Jay
-----Original Message-----
From: Sergey Bylokhov
Sent: Wednesday, November 14, 2018 4:07 AM
To: Jayathirth D V; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [12] RFR JDK-8211795:
ArrayIndexOutOfBoundsException in PNGImageReader after JDK-6788458
Hi, Jay.
Can you please provide some more detail about this bug.
Root cause : In JDK-6788458 we are adding extra alpha channel for destination
whenever we have tRNS chunk. But the number of bands in bitDepth scale array
was not changed when we have tRNS chunk. This is causing
ArrayIndexOutOfBoundsException for scale array.
As far as I understand the AIOOB is occurred when we access ps[b] at line 1308 not when
we access the scale array, because the scale array is created as "scale = new
int[numBands][]". So maybe numBands should depends on the passRow? or the creation
of scale[][xxx] should be updated?
BTW this code uses +1/-1 in a lot of places already, and it is not always clear
why.
--
Best regards, Sergey.
