<AWT Dev> [9] Review request for JDK-8151787 Unify the HiDPI splash screen image naming convention

Mon May 2 08:54:56 UTC 2016

Updated Webrev link: 

http://cr.openjdk.java.net/~rchamyal/8151787/webrev.04/

Regards,
Rajeev Chamyal

-----Original Message-----
From: Rajeev Chamyal 
Sent: 02 May 2016 14:24
To: Hendrik Schreiber
Cc: awt-dev at openjdk.java.net; Avik Niyogi
Subject: Re: <AWT Dev> [9] Review request for JDK-8151787 Unify the HiDPI splash screen image naming convention

Hello Hendrik,

Thanks for the 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.

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.

Regards,
Rajeev Chamyal

-----Original Message-----
From: Hendrik Schreiber [mailto:hs at tagtraum.com]
Sent: 02 May 2016 13:55
To: Rajeev Chamyal
Cc: Ambarish Rapte; Alexander Scherbatiy; awt-dev at 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 : image at 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.chamyal at 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 at 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 at 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 at 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 at 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 image at 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
> 
>  
> 
>  
> 
>  
> 
>  
> 