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

[Alexander Scherbatiy]

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/ 
> <http://cr.openjdk.java.net/%7Erchamyal/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; 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
>

-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160229/528858ee/attachment-0001.html>