<AWT Dev> [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

Alexandr Scherbatiy alexandr.scherbatiy at oracle.com
Tue Mar 22 14:44:55 UTC 2016


On 3/22/2016 12:43 AM, Rajeev Chamyal wrote:
> Hello All,
>
> Please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8145173/webrev.05/
>
> With VC2010  java.c ::ShowSplashScreen  fails with segmentation fault on  calling free on scaled_splash_name .
> This failure is due to different C runtime used by libjli and lib splashScreen.
> Fix: Allocating a max scaled image name length buffer in java.c and passing it to splashscreen_sys.c

    Just few small comments:
    - SplashGetScaledImageName can return a boolean value which 
indicates that high-resolution splash screen has been found
    - The getMaxScaledImageNamePostfixLen function can be added which 
just returns max length of the scaled image postfix.
      It allows to control this value on the java.desktop side rather to 
keep it in java.c file
    - systemScale.cpp copyright need to be updated to 2016

   Thanks,
   Alexandr.
>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Rajeev Chamyal
> Sent: 15 March 2016 18:26
> To: Alexander Scherbatiy; Sergey Bylokhov; awt-dev at openjdk.java.net; build-dev at openjdk.java.net
> Subject: RE: <AWT Dev> [9] Review request for JDK-8145173 HiDPI splash screen support on Windows
>
> Hello All,
>
> Please review the updated webrev.
> http://cr.openjdk.java.net/~rchamyal/8145173/webrev.04/
>
> Alexandr : I have build code with VS2013 and I didn't get any errors you mentioned.
> Still I have updated the code as suggested.
>
> Added build team for make file review. JPRT build with fix was successful.
>
> Regards,
> Rajeev Chamyal
>
> -----Original Message-----
> From: Alexander Scherbatiy
> Sent: 14 March 2016 17:28
> To: Rajeev Chamyal
> Cc: Sergey Bylokhov; awt-dev at openjdk.java.net
> Subject: Re: <AWT Dev> [9] Review request for JDK-8145173 HiDPI splash screen support on Windows
>
> On 3/14/2016 8:03 AM, Rajeev Chamyal wrote:
>> Hello Sergey,
>>
>> Could you please review the enhancement.
>>
>> I have raised a new enhancement for unifying the splash screen image
>> names across platforms.
>>
>> https://bugs.openjdk.java.net/browse/JDK-8151787
>>
>    575 float *scaleFactor)
>    576 {
>    577     *scaleFactor = 1.0;
>    578     float dpiScaleX = -1.0f;
>    579     float dpiScaleY = -1.0f;
>    580     GetScreenDpi(getPrimaryMonitor(), &dpiScaleX, &dpiScaleY);
>
> I have errors building the fix with VS2010:
> --------------------------------
> * For target
> support_native_java.desktop_libsplashscreen_splashscreen_sys.obj:
> splashscreen_sys.c
> jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(578)
> : error C2143: syntax error : missing ';' before 'type'
> jdk/src/java.desktop/windows/native/libsplashscreen/splashscreen_sys.c(579)
> : error C2143: syntax error : missing ';' before 'type'
> --------------------------------
>
> You need to move variables declaration to the beginning of a statement.
>
> Thanks,
> Alexandr.
>
>> Regards,
>>
>> Rajeev Chamyal
>>
>> *From:*Alexandr Scherbatiy
>> *Sent:* 10 March 2016 18:46
>> *To:* Rajeev Chamyal; awt-dev at openjdk.java.net; Sergey Bylokhov
>> *Subject:* Re: <AWT Dev> [9] Review request for JDK-8145173 HiDPI
>> splash screen support on Windows
>>
>> The fix looks good to me.
>>
>> Thanks,
>> Alexandr.
>>
>> On 3/10/2016 3:05 AM, Rajeev Chamyal wrote:
>>
>>      Hello Alexandr,
>>
>>      Thanks for the review. Below is the updated webrev as per review
>>      comments.
>>
>>      http://cr.openjdk.java.net/~rchamyal/8145173/webrev.03/
>>      <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.03/>
>>
>>      Regards,
>>
>>      Rajeev Chamyal
>>
>>      *From:*Alexandr Scherbatiy
>>      *Sent:* 10 March 2016 11:39
>>      *To:* Rajeev Chamyal; awt-dev at openjdk.java.net
>>      <mailto:awt-dev at openjdk.java.net>; Sergey Bylokhov
>>      *Subject:* Re: <AWT Dev> [9] Review request for JDK-8145173 HiDPI
>>      splash screen support on Windows
>>
>>      On 3/2/2016 9:50 PM, Rajeev Chamyal wrote:
>>
>>
>>          Hello All,
>>
>>          Please review the updated webrev.
>>
>>          Added a free call for duplicate file name in
>>          splashscreen_sys.c :: SplashGetScaledImageName
>>
>>          http://cr.openjdk.java.net/~rchamyal/8145173/webrev.02/
>>          <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.02/>
>>
>>
>>         awt_Win32GraphicsDevice.cpp
>>       656     dpiX = GetScreenDpi(GetMonitor());
>>       657     if (dpiX > 0) {
>>       658         dpiX = dpiX >= 96 ? dpiX / 96 : dpiX;
>>       659         SetScale(dpiX, dpiX);
>>
>>      The Windows HiDPI graphics support was designed to handle both DPI
>>      X and Y scales. The GetScreenDpi should return both values to be
>>      used in SetScale method.
>>
>>      systemScale.cpp
>>
>>         38     float scale = -2.0f;
>>
>>         39     if (scale == -2) {
>>
>>      Initially the scale variable was defined as static to avoid rereading the J2D_UISCALE test variable each time.
>>
>>      It is better to preserve the "// for debug purposes" comment also.
>>
>>        
>>
>>      MultiResolutionSplashTest.java
>>
>>      +   scaleFactor = (float)((SunGraphics2D) g).surfaceData.getDefaultScaleX();
>>
>>        
>>
>>      Now it is possible to get the the scale factor using
>> GraphicsEnvironment.getLocalGraphicsEnvironment().getDefaultScreenDevi
>> ce().getDefaultConfiguration().getDefaultTransform().getScaleX()
>>
>>               
>>
>>      Thanks,
>>       Alexandr.
>>
>>
>>
>>          Regards,
>>
>>          Rajeev Chamyal
>>
>>          *From:*Rajeev Chamyal
>>          *Sent:* 01 March 2016 15:45
>>          *To:* awt-dev at openjdk.java.net
>>          <mailto:awt-dev at openjdk.java.net>; Sergey Bylokhov; Alexander
>>          Scherbatiy
>>          *Subject:* RE: <AWT Dev> [9] Review request for JDK-8145173
>>          HiDPI splash screen support on Windows
>>
>>          Hello All,
>>
>>          Gentle reminder.
>>
>>          Please review the updated webrev.
>>
>>
>>          http://cr.openjdk.java.net/~rchamyal/8145173/webrev.01/
>>          <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.01/>
>>
>>          Regards,
>>
>>          Rajeev Chamyal
>>
>>          *From:*Rajeev Chamyal
>>          *Sent:* 16 February 2016 16:01
>>          *To:* awt-dev at openjdk.java.net
>>          <mailto:awt-dev at openjdk.java.net>; Sergey Bylokhov; Alexander
>>          Scherbatiy
>>          *Subject:* <AWT Dev> [9] Review request for JDK-8145173 HiDPI
>>          splash screen support on Windows
>>
>>          Hello All,
>>
>>          Could you please review the following fix.
>>
>>          Bug : https://bugs.openjdk.java.net/browse/JDK-8145173
>>
>>          Webrev :
>>          http://cr.openjdk.java.net/~rchamyal/8145173/webrev.00/
>>          <http://cr.openjdk.java.net/%7Erchamyal/8145173/webrev.00/>
>>
>>          This is an enhancement to support HiDPI splash screen on windows.
>>
>>          As a part of this enhancement implementation to
>>          splashscreen_sys.c::SplashGetScaledImageName method has been
>>          provided.
>>
>>          System dpi and scale factor are used to determine the scaled
>>          image name. Dpi value is read using GetDpiForMonitor API on
>>          Windows 8 and GetDesktopDpi API on Windows 7.
>>
>>          Scale factor is calculated from the dpi value.
>>
>>          The naming convention followed for scaled image name is as
>>          follows:
>>
>>          Refer :
>>          
>> https://msdn.microsoft.com/en-us/library/windows/apps/xaml/hh965325.as
>> px
>>
>>          Unscaled image name : image.ext
>>
>>          Scaled image name : image.scale-<dpi value>./ext/
>>
>>          Regards,
>>
>>          Rajeev Chamyal
>>



More information about the awt-dev mailing list