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@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@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/~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.
