<AWT Dev> [9] Review request for JDK-8145173 HiDPI splash screen support on Windows
Sergey Bylokhov
Sergey.Bylokhov at oracle.com
Wed Mar 23 12:27:07 UTC 2016
+1
On 23.03.16 13:48, Erik Joelsson wrote:
> 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
>>>>>
>>
>
--
Best regards, Sergey.
More information about the build-dev
mailing list