<AWT Dev> [9] Review request for JDK-8145174 HiDPI splash screen support on Linux
Alexandr Scherbatiy
alexandr.scherbatiy at oracle.com
Thu Mar 10 05:27:20 UTC 2016
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/~rchamyal/8145174/webrev.03/
> <http://cr.openjdk.java.net/%7Erchamyal/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/
> <http://cr.openjdk.java.net/%7Erchamyal/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/
> <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
> <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
>
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.openjdk.java.net/pipermail/awt-dev/attachments/20160309/5da1bf69/attachment-0001.html>
More information about the awt-dev
mailing list