> On May 2, 2016, at 10:53, Rajeev Chamyal <rajeev.cham...@oracle.com> wrote: > > Hello Hendrik, > > Thanks for the review.
Rajeev, please understand that I just gave some comments/opinions—I do not have reviewer status. You might want to wait for a *real* review! > Please review the updated webrev. > > 1) I’d remove the space between OS name and colon—unless you line up the > colons to improve readability. > Removed the colons as suggested. I guess you mean the spaces. > > 2) The message also seems to imply that the “image.java-scale2x.ext” format > is only supported on Linux and and Mac, but not on Windows. Is that the case? > (haven’t looked at the impl.) > > “image.java-scale2x.ext” name format is supported on linux and mac only. On > mac and linux currently we are supporting scale factor of 2 only. > On windows scale factor ranging from 1 and 2(inclusive) is supported. So, > scale factor can be a floating point value e.g. for a screen with dpi value > of 120 the scale factor would be 1.25. I didn’t follow the discussion, so please don’t see this as anything else but a comment from the sideline. Really? We are unifying the naming and what works on OS X won’t work on Windows? What happened to WORA? Why not support image.java-scale2x.ext on Windows as well? (Please ignore this, if it has been discussed sufficiently.) Also, I’m not a native English speaker, but I’d spell “non scaled” with a hyphen, i.e. “non-scaled” or perhaps use “unscaled”. But I guess a native speaker should comment on that. Just my 2c. -hendrik > > -----Original Message----- > From: Hendrik Schreiber [mailto:h...@tagtraum.com] > Sent: 02 May 2016 13:55 > To: Rajeev Chamyal > Cc: Ambarish Rapte; Alexander Scherbatiy; awt-dev@openjdk.java.net; Sergey > Bylokhov; Avik Niyogi > Subject: Re: <AWT Dev> [9] Review request for JDK-8151787 Unify the HiDPI > splash screen image naming convention > > Hey, > > Just looked at the launcher.properties (out of curiosity). Here’s some > feedback. > >> 87 \ -splash:<imagepath>\n\ >> 88 \ show splash screen with specified image\n\ >> 89 \ HiDPI scaled image is also supported\n\ >> 90 \ For a non scaled image file image.ext below are the >> scaled image names\n\ >> 91 \ Windows : image.java-scale<dpi-value>.ext >> e.g.image.java-scale196.ext\n\ >> 92 \ Linux : image.java-scale2x.ext\n\ >> 93 \ Mac : im...@2x.ext also supports >> image.java-scale2x.ext\n\ > > I’d remove the space between OS name and colon—unless you line up the colons > to improve readability. > > The message also seems to imply that the “image.java-scale2x.ext” format is > only supported on Linux and and Mac, but not on Windows. Is that the case? > (haven’t looked at the impl.) > > If it's the case, I’d consider it a bug. > > Cheers, > > -hendrik > > > >> On May 2, 2016, at 10:07, Rajeev Chamyal <rajeev.cham...@oracle.com> wrote: >> >> Hello All, >> >> >> >> Please review the updated webrev. >> >> >> >> http://cr.openjdk.java.net/~rchamyal/8151787/webrev.03/ >> >> Added launcher.properties file to webrev. >> >> Updated the command line description for -splash:<imagepath> option. >> >> >> >> Regards, >> >> Rajeev Chamyal >> >> >> >> From: Ambarish Rapte >> Sent: 02 May 2016 12:01 >> To: Alexander Scherbatiy; Rajeev Chamyal; awt-dev@openjdk.java.net; >> Sergey Bylokhov >> Subject: RE: <AWT Dev> [9] Review request for JDK-8151787 Unify the >> HiDPI splash screen image naming convention >> >> >> >> Hi Rajeev, >> >> The fix looks good to me. >> >> >> >> Regards, >> >> Ambarish >> >> >> >> From: Alexander Scherbatiy >> Sent: Friday, April 29, 2016 5:18 PM >> To: Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov >> Subject: Re: <AWT Dev> [9] Review request for JDK-8151787 Unify the >> HiDPI splash screen image naming convention >> >> >> >> >> The fix looks good to me. >> >> Thanks, >> Alexandr. >> >> On 29/04/16 13:49, Rajeev Chamyal wrote: >> >> Hello Alexandr, >> >> >> >> Please review the updated fix. >> >> http://cr.openjdk.java.net/~rchamyal/8151787/webrev.02/ >> >> >> >> - Will the java-scale2x image be chosen if fileName2x is nil on the line 163? >> >> fileName2x cannot be nil SplashGetScaledImageName is called only if a >> not value is passed in filename and other parameters to new method >> findScaledImageName are constants. >> >> - Could you add the use case where both @2x and java-scale2x are provided >> and @2x is chosen in the test? >> >> I have updated the test as suggested. >> >> >> >> Regards, >> >> Rajeev Chamyal >> >> >> >> >> >> From: Alexandr Scherbatiy >> Sent: 28 April 2016 00:20 >> To: Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov >> Subject: Re: <AWT Dev> [9] Review request for JDK-8151787 Unify the >> HiDPI splash screen image naming convention >> >> >> >> On 4/27/2016 7:40 PM, Rajeev Chamyal wrote: >> >> >> Hello Alexandr, >> >> >> >> Please review the updated fix. >> >> http://cr.openjdk.java.net/~rchamyal/8151787/webrev.01/ >> >> >> 162 fileName2x = findScaledImageName(fileName, dotIndex, @"@2x"); >> 163 if(fileName2x != nil && ![[NSFileManager defaultManager] >> 164 fileExistsAtPath: fileName2x]) { >> 165 fileName2x = findScaledImageName(fileName, dotIndex, >> @".java-scale2x"); >> 166 } >> >> - Will the java-scale2x image be chosen if fileName2x is nil on the line 163? >> - Could you add the use case where both @2x and java-scale2x are provided >> and @2x is chosen in the test? >> >> Thanks, >> Alexandr. >> >> >> >> >> >> >> >> Regards, >> >> Rajeev Chamyal >> >> >> >> From: Alexandr Scherbatiy >> Sent: 26 April 2016 14:22 >> To: Rajeev Chamyal; awt-dev@openjdk.java.net; Sergey Bylokhov >> Subject: Re: <AWT Dev> [9] Review request for JDK-8151787 Unify the >> HiDPI splash screen image naming convention >> >> >> >> On 4/26/2016 11:13 AM, Rajeev Chamyal wrote: >> >> >> >> Hello All, >> >> >> >> Could you please review the following fix. >> >> Bug : https://bugs.openjdk.java.net/browse/JDK-8151787 >> >> Webrev : http://cr.openjdk.java.net/~rchamyal/8151787/webrev.00/ >> >> >> >> This is a small enhancement to support similar HiDPI splash image name >> convention on all platforms. >> >> >> >> Currently we have different naming convention for scaled different platforms. >> >> >> >> Image name : image.ext >> >> >> >> Scaled image names: >> >> Windows : image.scale-dpiValue.ext >> Linux : image.java-scale2x.ext >> MAC im...@2x.ext >> >> >> >> After the fix naming convention on Mac and Linux would be : >> >> Image name : image.ext >> >> Scaled image name : image.java-scale2x.ext >> >> Both name conventions @2x and java-scale2x should be supported on >> The more specific one @2x should be checked in the first place and the >> java-scale2x in the second. >> >> Thanks, >> Alexandr. >> >> >> >> >> >> >> Naming convention on windows : >> >> Image name : image.ext >> >> Scaled image name : image.java-scale<dpi value>.ext >> >> >> >> Regards, >> >> Rajeev Chamyal >> >> >> >> >> >> >> >> >> >
