<AWT Dev> [9] Review request for JDK-8145174 HiDPI splash screen support on Linux
Rajeev Chamyal
rajeev.chamyal at oracle.com
Thu Mar 10 10:36:37 UTC 2016
Hello Sergey,
Thanks for the review.
HiDpi is not supported for Solaris in jdk. I will raise a new CR for naming convention shortly.
Regards,
Rajeev Chamyal
-----Original Message-----
From: Sergey Bylokhov
Sent: 10 March 2016 15:49
To: Alexandr Scherbatiy; Rajeev Chamyal; awt-dev at openjdk.java.net
Subject: Re: <AWT Dev> [9] Review request for JDK-8145174 HiDPI splash screen support on Linux
Looks fine.
One of the last question: do we support hidpi on Solaris in jdk code?
Do we have a separate CR to unify/specify all our .ext naming conversion?(@2, java-scale2x, etc).
On 10.03.16 8:27, Alexandr Scherbatiy wrote:
> The fix looks good to me.
>
> Thanks,
> Alexandr.
>
> On 3/2/2016 9:51 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/%7Erchamyal/8145174/webrev.03/>http://cr.openjdk.java.net/~rchamyal/8145174/webrev.03/
>>
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>> *From:*Rajeev Chamyal
>> *Sent:* 01 March 2016 13:33
>> *To:* Alexander Scherbatiy
>> *Cc:* awt-dev at openjdk.java.net; Sergey Bylokhov
>> *Subject:* RE: <AWT Dev> [9] Review request for JDK-8145174 HiDPI
>> splash screen support on Linux
>>
>> Hello Alexandr,
>>
>> Thanks for the review. I have updated code as per review comments.
>>
>> <http://cr.openjdk.java.net/%7Erchamyal/8145174/webrev.02/>http://cr.openjdk.java.net/~rchamyal/8145174/webrev.02/
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>> *From:*Alexander Scherbatiy
>> *Sent:* 29 February 2016 14:24
>> *To:* Rajeev Chamyal
>> *Cc:* awt-dev at openjdk.java.net <mailto:awt-dev at openjdk.java.net>;
>> Sergey Bylokhov
>> *Subject:* Re: <AWT Dev> [9] Review request for JDK-8145174 HiDPI
>> splash screen support on Linux
>>
>> On 2/23/2016 12:41 PM, Rajeev Chamyal wrote:
>>
>> Hello Alexandr,
>>
>> Thanks for the review.
>>
>> I have updated the webrev as per review comments.
>>
>> Webrev :
>> <http://cr.openjdk.java.net/%7Erchamyal/8145174/webrev.01/>http://cr.openjdk.java.net/~rchamyal/8145174/webrev.01/
>>
>>
>> - splashscreen_sys.c
>> Is it possible to specify the substring to copy in the snprintf
>> using "%.*s" format to avoid copying of the file name to
>> fileNameWithoutExt buffer?
>> The returned error codes like in the snprintf should be properly
>> handled.
>>
>> - systemScale.c
>> The J2D_UISCALE property has been added for the testing purposes.
>> It is better to include it into the getNativeScaleFactor method to use
>> for splash screens too.
>>
>> - the copyright in the test need to be updated to 2016.
>>
>> - It may be useful to have the same name convention for
>> high-resolution splash screen on all platforms.
>> It allows to use only one image.java-scale2x.ext file instead
>> to have image at 2x.ext <mailto:image at 2x.ext> on Mac OS X and
>> name.scale-200.ext on Windows.
>> For windows we can have scale factor as float value so it
>> would be difficult to identify which image name to be displayed.
>>
>> I see. It can be an enhancement to support fractional scales too.
>> For example image.java-scale150%.ext and image.java-scale144dpi.ext
>> for scale factor 1.5.
>>
>> Thanks,
>> Alexandr.
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>> *From:*Alexander Scherbatiy
>> *Sent:* 18 February 2016 02:55
>> *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-8145174 HiDPI
>> splash screen support on Linux
>>
>> On 12/02/16 16:21, Rajeev Chamyal wrote:
>>
>> Hello All,
>>
>> Could you please review the following fix.
>>
>> Bug : https://bugs.openjdk.java.net/browse/JDK-8145174
>>
>> Webrev :
>> http://cr.openjdk.java.net/~rchamyal/8145174/webrev.00/
>> <http://cr.openjdk.java.net/%7Erchamyal/8145174/webrev.00/>
>>
>> This is an enhancement to support HiDPI splash screen on Linux.
>>
>> As a part of this enhancement implementation to
>> splashscreen_sys.c::SplashGetScaledImageName method has been
>> provided based on the GDK_SCALE environment variable set on
>> unix/linux system.
>>
>> The new implementation checks for GDK_SCALE set on system and
>> returns the scaled image name, if GDK_SCALE=2 otherwise NULL.
>>
>> The naming convention followed for scaled image is as follows:
>>
>> Unscaled image name : image.ext
>>
>> Scaled image name : image.java-scale2x.ext
>>
>>
>> - It may be useful to have the same name convention for
>> high-resolution splash screen on all platforms.
>> It allows to use only one image.java-scale2x.ext file instead
>> to have image at 2x.ext <mailto:image at 2x.ext> on Mac OS X and
>> name.scale-200.ext on Windows.
>> Could you create an enhancement on it and send it to the review?
>>
>> The automated jtreg test for this is currently failing due to
>> issues in robot.getPixelColor it is returning wrong pixel
>> color for GDK_SCALE=2.
>>
>> Also fixed issues in following files.
>>
>> 1)splashscreen_impl.c::SplashInit() was resetting the
>> scaleFactor to 1.
>>
>> - SplashSetScaleFactor should not be called from the
>> SplashGetScaledImageName method because SplashInit has not been
>> called yet.
>> - The problem with setting the scale factor in SplashInit is
>> that it is not clear is the high-resolution splash screen image
>> provided or not. If the the high-resolution splash screen is not
>> provided the scale factor should be set to 1.
>> - The java.c uses the following sequence for the splash screen
>> initialization:
>> --------------
>> scaled_splash_name = DoSplashGetScaledImageName(
>> jar_name, file_name, &scale_factor);
>> DoSplashInit();
>> DoSplashSetScaleFactor(scale_factor);
>> DoSplashLoadFile(scaled_splash_name);
>> --------------
>> To make the SplashSetScaleFactor method work it should also be
>> added to the make/mapfiles/libsplashscreen/mapfile-vers file.
>>
>> - There are two codes which detect the scale factor. One is in
>> the splash screen (getNativeScaleFactor() method)
>> and another in the AWT
>> (src/java.desktop/unix/native/libawt_xawt/awt/awt_GraphicsEnv.c file).
>> Is it possible to move it one code that it will be used both
>> from splash screen and from AWT?
>>
>> Thanks,
>> Alexandr.
>>
>> 2)SplashScreen.java:: getBounds fixed the typo.
>>
>> Regards,
>>
>> Rajeev Chamyal
>>
>
--
Best regards, Sergey.
More information about the awt-dev
mailing list