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

[Sergey Bylokhov]

In general it looks fine to me. I will try to patch my local ws and run 
some sanity test.

On 03.03.16 8:51, 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/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/~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/~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.