Hello All, Can I get one more review please.
Thanks, Jay -----Original Message----- From: Sergey Bylokhov Sent: Thursday, October 15, 2015 6:05 PM To: Jayathirth D V; 2d-dev@openjdk.java.net; Philip Race Subject: Re: <OpenJDK 2D-Dev> Review request for JDK-7182758: BMPMetadata returns invalid PhysicalPixelSpacing The fix looks fine. The test can be improved a little bit to simplify the int->Integer boxing, but it is not necessary for now. Thanks. On 15.10.15 13:17, Jayathirth D V wrote: > Hi Sergey, > > I thought you suggested to check for tighter "true" condition instead of > "false" condition. > > I have made changes to map horizontalNodeValue and verticalNodeValue to > expected values. Please find updated Webrev link: > > http://cr.openjdk.java.net/~rchamyal/jay/7182758/webrev.04/ > > Please review. > > Thanks, > Jay > > -----Original Message----- > From: Sergey Bylokhov > Sent: Wednesday, October 14, 2015 7:34 PM > To: Jayathirth D V; 2d-dev@openjdk.java.net; Philip Race > Subject: Re: <OpenJDK 2D-Dev> Review request for JDK-7182758: > BMPMetadata returns invalid PhysicalPixelSpacing > > Hi, Jay. >> I suggest to check correct/expected result in the test instead of previous >> incorrect zero. > > Here, I suggested to check that the resulted horizontalNodeValue and > verticalNodeValue are equal to some expected value. Because if it bigger than > zero does not mean it is correct(note to use Float.compare(f1, f2) instead of > "=="). > >> >> Previously I forgot to mention, that it will be good to name the test by >> some useful name instead of some bug number(see examples in >> javax/imageio/plugins). >> >> On 13.10.15 11:12, Jayathirth D V wrote: >>> Hello All, >>> >>> Removed Trailing whitespace present in code. >>> >>> Please find updated webrev link: >>> >>> http://cr.openjdk.java.net/~rchamyal/jay/7182758/webrev.02/ >>> >>> Thanks, >>> >>> Jay >>> >>> *From:* Jayathirth D V >>> *Sent:* Monday, October 12, 2015 2:15 PM >>> *To:* 2d-dev@openjdk.java.net; Philip Race; Sergey Bylokhov >>> *Subject:* [OpenJDK 2D-Dev] <OpenJDK 2D-Dev> Review request for >>> JDK-7182758: BMPMetadata returns invalid PhysicalPixelSpacing >>> >>> Hello All, >>> >>> Made small change on how we need to represent floating point >>> constant in >>> Java(1000.0 -> 1000.0F). >>> >>> Please find updated Webrev link: >>> >>> Webrev : http://cr.openjdk.java.net/~rchamyal/jay/7182758/webrev.01/ >>> >>> Please review. >>> >>> Thanks, >>> >>> Jay >>> >>> *From:* Jayathirth D V >>> *Sent:* Thursday, October 08, 2015 2:20 PM >>> *To:* 2d-dev@openjdk.java.net <mailto:2d-dev@openjdk.java.net>; >>> Philip Race; Sergey Bylokhov >>> *Subject:* [OpenJDK 2D-Dev] <OpenJDK 2D-Dev> Review request for >>> JDK-7182758: BMPMetadata returns invalid PhysicalPixelSpacing >>> >>> Hello All, >>> >>> Please review following fix in jdk9: >>> >>> Bug : https://bugs.openjdk.java.net/browse/JDK-7182758 >>> >>> Webrev : http://cr.openjdk.java.net/~rchamyal/jay/7182758/webrev.00/ >>> >>> Bug : BMPMetadata returns invalid PhysicalPixelSpacing >>> >>> Root cause : Whenever XPixelsPerMter or YPixelsPerMeter is more than >>> value 1 in BMP header. Horizontal & Vertical Physical pixel spacing >>> were returned as zero. >>> >>> In getStandardDimensionNode() method of >>> BMPMetadata.java we are dividing 1 by XPixelsPerMter/ YPixelsPerMter. >>> When >>> >>> XPixelsPerMter/ YPixelsPerMter is more >>> than 1. Resulted value is stored without decimal part, which resulted in >>> zero. >>> >>> Solution : Made changes to how Horizontal & Vertical Physical pixel >>> spacing is calculated so that decimal value is not truncated. >>> >>> Thanks, >>> >>> Jay >>> >> >> >> -- >> Best regards, Sergey. >> > > > -- > Best regards, Sergey. > -- Best regards, Sergey.
