Hi Alexander,

[ I'm also adding Neil who's taking over the launcher code ]

1. There's a few code formatting issues that should be fixed. For instance:
src/share/bin/java.c
1846             if(scaled_splash_name){
1853         if(scaled_splash_name){

src/windows/native/sun/awt/splashscreen/splashscreen_sys.c
 571     *scaleFactor=1;

In all the above lines required spaces are missing. I believe there's a number of other occurrences of the same issue in your patch. Please check it carefully and fix the formatting on all lines.

2. Webrev shows 236 changed lines for java_awt_SplashScreen.c. I suspect you've changed the EOL characters because you've edited your code on MS Windows. Please configure your editor to use LF characters for line ends and revert the unnecessary changes (note that jcheck won't let you push CR-LF line endings anyway).

3. splashscreen_sys.m
 135     if (1 < screenScaleFactor) {

For consistency and clarity, I'd suggest to rewrite the condition as

        if (screenScaleFactor > 1) {

--
best regards,
Anthony

On 6/17/2014 6:20 PM, Petr Pchelko wrote:
Hello, Alexander.

2. In java.c:1859 DoSplashSetFileJarName sets the name of the file which was 
used for a splash. It can be
retrieved via public API SplashScreen.getImageURL. Is it correct to always set 
the file_name disregards the real
name we've used?

    Yes. The original splash screen image name and size are provided for the 
developer even the high resolution splash screen is shown.
Thank you for the clarification.

The updated version looks fine to me.

With best regards. Petr.

On 17 июня 2014 г., at 18:16, Alexander Scherbatiy 
<alexandr.scherba...@oracle.com> wrote:


Hello,

Could you review the updated fix:
  http://cr.openjdk.java.net/~alexsch/8043869/webrev.01/


On 6/17/2014 4:22 PM, Petr Pchelko wrote:
Hello, Alexander.

CCed Kumar as I believe he's the owner of the launcher code.

1. In splashscreen_sys.m you autorelease the fileName. But I do not see where 
the autoreleasepool is set up.
Wouldn't it be better to set up an autoreleasepool in this method explicitly?
     NSAutoreleasePool is added.
2. In java.c:1859 DoSplashSetFileJarName sets the name of the file which was 
used for a splash. It can be
retrieved via public API SplashScreen.getImageURL. Is it correct to always set 
the file_name disregards the real
name we've used?

    Yes. The original splash screen image name and size are provided for the 
developer even the high resolution splash screen is shown.

    Thanks,
    Alexandr.
Thank you.
With best regards. Petr.

On 17 июня 2014 г., at 15:36, Alexander Scherbatiy 
<alexandr.scherba...@oracle.com> wrote:

Hello,

Could you review the fix:
  bug: https://bugs.openjdk.java.net/browse/JDK-8043869
  webrev: http://cr.openjdk.java.net/~alexsch/8043869/webrev.00

  The scaleFactor field is added to the Splash struct.
  The SplahsScreen class scales the graphics and the size according to the 
scale factor.
  The native system tries to load high resolution splash image on HiDPI display.

Thanks,
Alexandr.



Reply via email to