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

[Jayathirth D V]

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

 

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

[Jayathirth D V]

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

 

Re: [OpenJDK 2D-Dev] [11] Upgrade to Marlin renderer 0.9.1

[Laurent Bourgès]

Hi Phil,

Thank you a lot for your review and your time on this long & complex review.

2018-03-26 22:50 GMT+02:00 Philip Race :

> Hi Laurent,
>
> It took me I at least 5 tries to get all the way through this.
>
>
I agree it was a complex patch (clipping) with many other small
improvements, sorry.


> I don't have any substantial comments, just a few clean up questions.
>
>
> What is this in Curve.java + DCurve.java ?
>
> Even if derivatives are totally useless for lines, I removed the condition
to reset these values anyway.

> +if (false) {+dax = 0.0d;+day = 0.0d;+
> dbx = 0.0d;+dby = 0.0d;+}
>
> In Renderer.java, should the commented line be removed ?
>
> +&& ((Math.abs(ddx) + Math.abs(ddy) * _SCALE_DY) <= 
> _INC_BND+// && (Math.abs(ddx + dddx) + Math.abs(ddy + 
> dddy) * _SCALE_DY) <= _INC_BND
>
> A similar pattern occurs a little later in the same file.
>
> +//|| (Math.abs(ddx + dddx) + Math.abs(ddy + dddy) * 
> _SCALE_DY) >= _DEC_BND
>
> Fixed (removed lines)


> +static final float LEN_TH = 
> MarlinProperties.getSubdividerMinLength();
>
> You really meant to name it LEN_TH and not LENGTH ?
>
> That was deliberate, I wanted to shorten LENGTH_THRESHOLD => LEN_TH, not
LENGTH.

There are a few TODO's I see but they seem to be more about later
clean up or optimisation
> so are probably all OK.
>
> I added few new TODO that I hope to fix later (nothing critical).

> So I am OK to push, and if you clean up any of the above first I don't need 
> to see a new webrev.
>
> Thank you again,

Laurent


>
> On 3/23/18, 2:03 PM, Laurent Bourgès wrote:
>
> Hi,
>
> Sorry to insist but I would like to get feedback on this Marlin patch soon
> before going forward on tile-size tuning in java2d accelerated pipelines.
>
> Laurent
>
> 2018-03-21 22:56 GMT+01:00 Laurent Bourgès :
>
>> Hi,
>>
>> Here is the updated webrev:
>> http://cr.openjdk.java.net/~lbourges/marlin/marlin-091.1/
>>
>> Changes in MarlinProperties only:
>> - getTileSize_Log2() & getTileWidth_Log2(); 32x32 tiles ie default = 5
>> (log2)
>>
>> I hope it is good for now as tile settings are the same as in jdk9/10.
>>
>> Regards,
>> Laurent
>>
>>
>> 2018-03-21 21:44 GMT+01:00 Laurent Bourgès :
>>
>>> Sergey,
>>>
>>> Le mer. 14 mars 2018 à 17:14, Sergey Bylokhov <
>>> sergey.bylok...@oracle.com> a écrit :
>>>
 On 13/03/2018 17:04, Sergey Bylokhov wrote:

> I have started to look to this review, will run some closed tests and
> send a feedback soon.
>

 No issues found so far, +1.
>>>
>>>
>>> Thanks for your vote.
>>> I need another approval I suppose ?
>>>
>>> I will prepare another review asap reverting only tile size changes as
>>> using large tiles has performance drop on d3d & ogl that needs more work.
>>> It can be done later in follow-up issues.
>>>
>>> Phil do you agree the proposed plan ?
>>>
>>> Laurent
>>>
>>
>>
>

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Thomas Stüfe]

Hi Phil,

On Tue, Mar 27, 2018 at 6:44 PM, Phil Race  wrote:

> As I said I prefer the make file change, since this is a change to an
> upstream library.
> I've looked at jpeg-9c and it still looks identical to what we have in 6b,
> so this code
> seems to have stood the test of time. I'm also unclear why the compiler
> would
> complain about that and not the one a few lines later
>
>
>  819   while (bits[i] == 0)  /* find largest codelength still in use 
> */
>  820 i--;
>
> A push to jchuff.c will get blown away if/when we upgrade.
> A tool-chain specific fix in the makefile with an appropriate comment is more 
> targeted.
>
>
> -phil.
>
>
I dislike switching off the compiler warning, since it may be actually
useful. Also I cannot exclude the possibility (I stared at the code some
time already) that the warning is actually meaningful and that we actually
should fix this underflow. Can you?

However, I see your arguments, and since you object to this patch, I revoke
it. Unfortunately I am out of time. I think for the time being we will
continue building with --disable-warnings-as-errors on zlinux.

Thanks, Thomas



>
> On 03/27/2018 05:44 AM, Thomas Stüfe wrote:
>
> Hi all,
>
> just a friendly reminder. I would like to push this tiny fix because
> tripping over this on our linux s390x builds is annoying (yes, we can
> maintain patch queues, but this is a constant error source).
>
> I will wait for 24 more hours until a reaction. If no serious objections
> are forcoming, I want to push it (tier1 tests ran thru, and me and
> Christoph langer are both Reviewers).
>
> Thanks! Thomas
>
> On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe 
> wrote:
>
>> Hi all,
>>
>> may I please have another review for this really trivial change. It fixes
>> a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this.
>>
>> bug: https://bugs.openjdk.java.net/browse/JDK-8200052
>> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/8200052-
>> fix-warning-in-jchuff.c/webrev.00/webrev/
>>
>> This was contributed by Adam Farley at IBM.
>>
>> I already reviewed this. I also test-built on zlinux and it works.
>>
>> Thanks, Thomas
>>
>
>
>

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Phil Race]

As I said I prefer the make file change, since this is a change to an 
upstream library.
I've looked at jpeg-9c and it still looks identical to what we have in 
6b, so this code
seems to have stood the test of time. I'm also unclear why the compiler 
would

complain about that and not the one a few lines later

 819   while (bits[i] == 0)  /* find largest codelength still in use */
 820 i--;

A push to jchuff.c will get blown away if/when we upgrade.
A tool-chain specific fix in the makefile with an appropriate comment is more 
targeted.


-phil.

On 03/27/2018 05:44 AM, Thomas Stüfe wrote:

Hi all,

just a friendly reminder. I would like to push this tiny fix because 
tripping over this on our linux s390x builds is annoying (yes, we can 
maintain patch queues, but this is a constant error source).


I will wait for 24 more hours until a reaction. If no serious 
objections are forcoming, I want to push it (tier1 tests ran thru, and 
me and Christoph langer are both Reviewers).


Thanks! Thomas

On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe > wrote:


Hi all,

may I please have another review for this really trivial change.
It fixes a gcc warning on s390 and ppc. Also, it is probably a
good idea to fix this.

bug: https://bugs.openjdk.java.net/browse/JDK-8200052

webrev:

http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/



This was contributed by Adam Farley at IBM.

I already reviewed this. I also test-built on zlinux and it works.

Thanks, Thomas

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

[Jayathirth D V]

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

 

Re: [OpenJDK 2D-Dev] RFR: Bug Pending: Build fails to compile jchuff.c

[Thomas Stüfe]

 Hi all,

Last week I openend JDK-8200052 and posted it to 2d-dev for RFR.

Me included we have two reviewers and the tier1 tests ran through. Are
there really any serious objections against pushing this tiny fix? It would
make life for us (working on zLinux) easier.

I will wait for 24 more hours until a reaction. If no serious objections
are forthcoming, I will push it.

Thanks! Thomas


On Thu, Mar 22, 2018 at 10:57 AM, Adam Farley8 
wrote:

> Hi Guys,
>
> I've provided a gcc-specific fix in the makefile to prevent the warning.
>
> -- Awt2dLibraries.gmk:471 --
> DISABLED_WARNINGS_gcc := *array-bounds* clobbered implicit-fallthrough
> shift-negative-value, \
>
> I've also provided an underflow fix in the .c file to fix the problem
> *causing* the warning.
>
> -- jchuff.c:808 --
> while (*(*bits[j] == 0)* && (j > 0)*)
>
> Either will work fine.
>
> Note: After determining that it affects multiple gcc versions, and that
> the logic to make a makefile do a
> compare (the shell business) on the gcc version seemed hacky to me, I
> considered the best solution
> to be one of the two simple fixes outlined above. This seemed to be
> acceptable to people in the
> community, yet we're still having trouble getting this fix through.
>
> I'm not sure why.
>
> Best Regards
>
> Adam Farley
>
>
> > Hi Phil!
> >
> >
> > thanks for pointing out the history, I was not aware of that.
> >
> >
> > I looked at that huffman coding and tried to determine whether the
> underflow may happen in real life scenarios. I could at least not exclude
> that possibility. I looked thru the mailing list threads - did someone
> analyse and conclude for sure this was just a pointless compiler warning?
> >
> >
> > I would prefer the pragmatic solution (and IMHO also safer one) of
> fixing this underflow in the proposed fashion. I had opened a bug report
> earlier today. However, if someone already spent brain cycles on it and a
> patch - in whatever form - is forthcoming, I do not want to butt in. In
> that case I will close this bug again.
> >
> >
> > I would just like to see this fixed this because it affects us at SAP
> too.
> >
> >
> > Kind Regards, Thomas
> >
> >
> > On Wed, Mar 21, 2018 at 6:56 PM, Phil Race 
> wrote:
> >
> > I prefer the makefile fix, since we don't by policy, make changes to the
> imported libraries.
> >
> > On Jan 23rd [1] I expressed such a tool-chain specific makefile fix
> would be fine by me.
> >
> > Toolchain specific means ideally it would look like what Magnus wrote [2]
> >
> > Although you said GC 5.4.0 would need to be included in the logic.
> >
> > If it can be shown to affect current / future versions of gcc then it
> could be unqualified.
> >
> > I think we've just been waiting for a webrev since then ..
> >
> > -phil.
> >
> > [1] http://mail.openjdk.java.net/pipermail/2d-dev/2018-January/
> 008855.html
> > [2] http://mail.openjdk.java.net/pipermail/build-dev/2018-
> January/020695.html
> >
> >
> >
> > On 03/21/2018 09:53 AM, Adam Farley8 wrote:
> >
> > :)
> >
> > > Hi Adam,
> > >
> > > no problem. I'll open a bug and if necessary find a second reviewer.
> Thanks for fixing, maybe I can stop building with warnings disabled on our
> s390 machines now.
> > >
> > > ..Thomas
> > >
> > > > On Wed, Mar 21, 2018 at 5:10 PM, Andrew Leonard <
> andrew_m_leon...@uk.ibm.com> wrote:
> > > > Hi Thomas,
> > > > I'm a "contributor", but not a "committer", so not on that list,
> didn't even know that
> > > > list existed! I was sort of assuming since it was a trivial change,
> and the request was
> > > > for a review, i'd chip in...!
> > > > Thanks
> > > > Andrew
> > >
> > > > Andrew Leonard
> > > > Java Runtimes Development
> > > > IBM Hursley
> > > > IBM United Kingdom Ltd
> > > > Phone internal: 245913, external: 01962 815913
> > > > internet email: andrew_m_leon...@uk.ibm.com
> > >
> >
>
> Unless stated otherwise above:
> IBM United Kingdom Limited - Registered in England and Wales with number
> 741598.
> Registered office: PO Box 41, North Harbour, Portsmouth, Hampshire PO6 3AU
>

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Thomas Stüfe]

Hi all,

just a friendly reminder. I would like to push this tiny fix because
tripping over this on our linux s390x builds is annoying (yes, we can
maintain patch queues, but this is a constant error source).

I will wait for 24 more hours until a reaction. If no serious objections
are forcoming, I want to push it (tier1 tests ran thru, and me and
Christoph langer are both Reviewers).

Thanks! Thomas

On Wed, Mar 21, 2018 at 6:20 PM, Thomas Stüfe 
wrote:

> Hi all,
>
> may I please have another review for this really trivial change. It fixes
> a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this.
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8200052
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8200052-fix-warning-in-jchuff.c/webrev.00/webrev/
>
> This was contributed by Adam Farley at IBM.
>
> I already reviewed this. I also test-built on zlinux and it works.
>
> Thanks, Thomas
>

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Thomas Stüfe]

Thank you Christoph!

On Tue, Mar 27, 2018 at 2:30 PM, Langer, Christoph  wrote:

> Hi Thomas,
>
>
>
> to me this fix looks good.
>
>
>
> Best regards
>
> Christoph
>
>
>
> *From:* 2d-dev [mailto:2d-dev-boun...@openjdk.java.net] *On Behalf Of *Thomas
> Stüfe
> *Sent:* Mittwoch, 21. März 2018 18:20
> *To:* 2d-dev <2d-dev@openjdk.java.net>
> *Cc:* Andrew Leonard ; Adam Farley8 <
> adam.far...@uk.ibm.com>
> *Subject:* [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile
> warning in jchuff.c
>
>
>
> Hi all,
>
>
>
> may I please have another review for this really trivial change. It fixes
> a gcc warning on s390 and ppc. Also, it is probably a good idea to fix this.
>
>
>
> bug: https://bugs.openjdk.java.net/browse/JDK-8200052
>
> webrev: http://cr.openjdk.java.net/~stuefe/webrevs/
> 8200052-fix-warning-in-jchuff.c/webrev.00/webrev/
>
>
>
> This was contributed by Adam Farley at IBM.
>
>
>
> I already reviewed this. I also test-built on zlinux and it works.
>
>
>
> Thanks, Thomas
>

Re: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning in jchuff.c

[Langer, Christoph]

Hi Thomas,

to me this fix looks good.

Best regards
Christoph

From: 2d-dev [mailto:2d-dev-boun...@openjdk.java.net] On Behalf Of Thomas Stüfe
Sent: Mittwoch, 21. März 2018 18:20
To: 2d-dev <2d-dev@openjdk.java.net>
Cc: Andrew Leonard ; Adam Farley8 

Subject: [OpenJDK 2D-Dev] RFR(xxxs): 8200052: libjavajpeg: Fix compile warning 
in jchuff.c

Hi all,

may I please have another review for this really trivial change. It fixes a gcc 
warning on s390 and ppc. Also, it is probably a good idea to fix this.

bug: https://bugs.openjdk.java.net/browse/JDK-8200052
webrev: 
http://cr.openjdk.java.net/~stuefe/webrevs/8200052-fix-warning-in-jchuff.c/webrev.00/webrev/

This was contributed by Adam Farley at IBM.

I already reviewed this. I also test-built on zlinux and it works.

Thanks, Thomas

Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values

[Prahalad Kumar Narayanan]

Hello Phil, Sergey and Jay

Thank you for your time in review.

I've incorporated your review suggestions.
In the updated code, ImageWriter gets disposed appropriately.

Kindly review the updated code at your convenience.
Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.01/

Thank you once again
Have a good day

Prahalad N.


-Original Message-
From: Philip Race 
Sent: Tuesday, March 27, 2018 2:40 AM
To: Sergey Bylokhov
Cc: Prahalad Kumar Narayanan; 2d-dev
Subject: Re: [OpenJDK 2D-Dev] [11] RFR: [JDK-4954348]: 
JPGWriter.getNumThumbnailsSupported does not return -1 when passing null values



On 3/23/18, 11:56 AM, Sergey Bylokhov wrote:
> Hi, Prahalad.
> A few small comments about the test:
>  - Is it possible to test all installed ImageWriterSpi? It seems that 
> the test itself is not a  JPEG plugin specific?

None of the other plugins support thumbnails so they will all return 0 
regardless.
So I think that aspect of the test is OK


>  - You will need to dispose the jpgWriter even in case of exception.

Probably should fix that.

Everything else seems fine.

-phil.
>
> On 23/03/2018 01:10, Prahalad Kumar Narayanan wrote:
>> Kindly review the changes at your convenience
>> Link: http://cr.openjdk.java.net/~pnarayanan/4954348/webrev.00
>>
>> Thank you
>> Have a good day
>>
>> Prahalad N.
>>
>
>