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

Erik Joelsson erik.joelsson at oracle.com
Wed Mar 23 10:48:01 UTC 2016


Build changes are still good.

/Erik

On 2016-03-23 11:46, Alexander Scherbatiy wrote:
> On 23/03/16 13:56, Rajeev Chamyal wrote:
>> Hello Alexandr,
>>
>> I have updated the webrev as per review comments.
>>
>> http://cr.openjdk.java.net/~rchamyal/8145173/webrev.06/
>
>   The fix looks good to me.
>
>   Thanks,
>   Alexandr.
>>
>> Regards,
>> Rajeev Chamyal
>>
>> -----Original Message-----
>> From: Alexandr Scherbatiy
>> Sent: 22 March 2016 20:15
>> To: Rajeev Chamyal; 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
>>
>> 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().getDefaultScreenDev
>>>> i
>>>> 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.a
>>>> s
>>>> px
>>>>
>>>>           Unscaled image name : image.ext
>>>>
>>>>           Scaled image name : image.scale-<dpi value>./ext/
>>>>
>>>>           Regards,
>>>>
>>>>           Rajeev Chamyal
>>>>
>




More information about the build-dev mailing list