Hello Phil, Please review the updated webrev. http://cr.openjdk.java.net/~rchamyal/8151787/webrev.04/
Regards, Rajeev Chamyal -----Original Message----- From: Rajeev Chamyal Sent: 22 August 2016 16:44 To: Philip Race Cc: awt-dev@openjdk.java.net; Sergey Bylokhov; Alexander Scherbatiy Subject: RE: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 Unify the HiDPI splash screen image naming convention Hello Phil, Thanks for the review, Please review updated webrev. http://cr.openjdk.java.net/~rchamyal/8151787/webrev.04/ Updated files: src/java.base/share/classes/sun/launcher/resources/launcher.properties src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m src/java.desktop/macosx/native/libsplashscreen/splashscreen_config.h Regards, Rajeev Chamyal -----Original Message----- From: Phil Race Sent: 20 August 2016 01:47 To: Rajeev Chamyal Cc: awt-dev@openjdk.java.net; Sergey Bylokhov; Alexander Scherbatiy Subject: Re: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 Unify the HiDPI splash screen image naming convention I recall that in order to be consistent we concluded that @200pct and @300pct needed to be supported in addition to the @2x and @3x syntax. -phil. On 8/19/2016 3:41 AM, Rajeev Chamyal wrote: > > Hello Phil, > > Please review the updated webrev. > > http://cr.openjdk.java.net/~rchamyal/8151787/webrev.03/ > <http://cr.openjdk.java.net/%7Erchamyal/8151787/webrev.03/> > > Updated file > src/java.base/share/classes/sun/launcher/resources/launcher.properties > > Added all other supported name extensions. > > Regards, > > Rajeev Chamyal > > *From:*Philip Race > *Sent:* 19 August 2016 04:48 > *To:* Rajeev Chamyal > *Cc:* awt-dev@openjdk.java.net; Sergey Bylokhov; Alexander Scherbatiy > *Subject:* Re: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 > Unify the HiDPI splash screen image naming convention > > Better, although it still does not document the supported set of scale > name extensions that we discussed/proposed off-line. > > -phil. > > On 8/18/16, 5:39 AM, Rajeev Chamyal wrote: > > Hello Phil, > > Thanks for the review. > > Please review the updated webrev. > > http://cr.openjdk.java.net/~rchamyal/8151787/webrev.02/ > <http://cr.openjdk.java.net/%7Erchamyal/8151787/webrev.02/> > > Updated file > > src/java.base/share/classes/sun/launcher/resources/launcher.properties > > Regards, > > Rajeev Chamyal > > *From:*Phil Race > *Sent:* 16 August 2016 22:28 > *To:* Alexandr Scherbatiy > *Cc:* Rajeev Chamyal; awt-dev@openjdk.java.net > <mailto:awt-dev@openjdk.java.net>; Sergey Bylokhov > *Subject:* Re: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 > Unify the HiDPI splash screen image naming convention > > On 08/16/2016 09:41 AM, Alexandr Scherbatiy wrote: > > > The fix looks good to me. > > It would be better if a native speaker look at the > documentation change in the launcher.properties file. > > > That documentation seems to cover only *some* of the extensions we > discussed. > It ought to cite all of them if it does so at all. How else are > people supposed > to know what they can use ? Where else are you documenting it? > Perhaps the launcher "man" page should be updated too > find . -name java.1 > ./src/linux/doc/man/java.1 > ./src/linux/doc/man/ja/java.1 > ./src/bsd/doc/man/java.1 > ./src/bsd/doc/man/ja/java.1 > ./src/solaris/doc/sun/man/man1/java.1 > ./src/solaris/doc/sun/man/man1/ja/java.1 > > .. although I think there is also some HTML version maintained by > the pubs/docs team > that is not in OpenJDK - the above does not include Windows or Mac. > I don't know offhand what is recommended these days. We'll have to > find someone > who does more with the launcher to help point to where to do the > documentation. > > And the doc does not really explain what is going on here. Reading > that I might > think I am supposed to pass -splash:im...@2x.ext if I want a > hi-dpi image > and that is not the idea at all, is it ? > The idea is you would still specify -splash:image.ext and the > *implementation* > will look for the most appropriate scaled image for the current > desktop. > > I think we should also have a CCC cover this (somehow). > > -phil. > > > > > Thanks, > Alexandr. > > On 8/16/2016 8:26 AM, Rajeev Chamyal wrote: > > Hello Alexandr, > > Please review the updated webrev. > > http://cr.openjdk.java.net/~rchamyal/8151787/webrev.01/ > > <http://cr.openjdk.java.net/%7Erchamyal/8151787/webrev.01/> > > Updates : > > 1)Updated the consition as suggested if(*scaleFactor - > (int)*scaleFactor < 0.000001) > > 2)Includes the changes of > > src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c > > 3)+ //map the splash co-ordinates as per system scale > + splash->x /= splash->scaleFactor; > + splash->y /= splash->scaleFactor; > > > > This change is required only for windows and linux. As we > use absolute system resolution for centring the splash on > screen on these. > > i.e. if system resolution is 1920 X 1080(i.e. unscaled > resolution) on windows and linux we use this for centring > the splash on screen. For mac scaled resolution is used > directly. > > Regards, > > Rajeev Chamyal > > *From:*Alexander Scherbatiy > *Sent:* 11 August 2016 14:44 > *To:* Rajeev Chamyal; awt-dev@openjdk.java.net > <mailto:awt-dev@openjdk.java.net>; Philip Race; Sergey > Bylokhov > *Subject:* Re: <AWT Dev> <Swing Dev>[9] Review Request > JDK-8151787 Unify the HiDPI splash screen image naming > convention > > On 10/08/16 19:24, Alexandr Scherbatiy wrote: > > > > On 8/9/2016 11:18 AM, Rajeev Chamyal wrote: > > Hello All, > > Please review the following webrev. > > Bug: > https://bugs.openjdk.java.net/browse/JDK-8151787 > > Webrev: > > http://cr.openjdk.java.net/~rchamyal/8151787/webrev.00/ > > <http://cr.openjdk.java.net/%7Erchamyal/8151787/webrev.00/> > > > Issue: Currently different naming conventions are > used for Hidpi image on different platforms. > > With this change the names will be unified across > all platforms. > > For a unscaled image image.ext following naming > convention will be followed. > > Unscaled name: image.ext > > Supported Scaled Names: > > If screen scale is integer number e.g. 2: > im...@2x.ext <mailto:im...@2x.ext> > > If screen scale is float value like 1.25: > im...@125pct.ext <mailto:im...@125pct.ext> > > > The fix should be reviewed on the awt-dev alias. > > + if(*scaleFactor - (int)*scaleFactor < 0.000001) > > Should there be so high precision there? Could only > percent values be compared like > if ((*scaleFactor *100) != ((int)(*scaleFactor)) * > 100) > > > + //map the splash co-ordinates as per system scale > + splash->x /= splash->scaleFactor; > + splash->y /= splash->scaleFactor; > > It looks like the splash coordinates and sizes are > rescaled in different places. Is it possible to do > that in the same place? May be in > java_awt_SplashScreen.c file getBounds() function? > > > src/java.desktop/unix/native/libsplashscreen/splashscreen_sys.c > *scaleFactor = getNativeScaleFactor(); > > Could you also include the change which requires to add > some default output screen name to the > getNativeScaleFactor() function on Linux. There is the > discussion about that: > > http://mail.openjdk.java.net/pipermail/awt-dev/2016-August/011766.html > > Thanks, > Alexandr. > > > > > Thanks, > Alexandr. > > > > > Regards, > > Rajeev Chamyal >