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@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@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@openjdk.java.net <mailto:awt-dev@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 im...@2x.ext <mailto:im...@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@openjdk.java.net >> <mailto:awt-dev@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 im...@2x.ext <mailto:im...@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.
