Hello Kumar, Thanks for the review. Please review the updated webrev.
http://cr.openjdk.java.net/~rchamyal/8151787/webrev.09/ Updates: 1) Updated launcher properties. 2) Corrected indentation in splashscreen_impl.c Regards, Rajeev Chamyal -----Original Message----- From: Kumar Srinivasan Sent: 09 September 2016 21:32 To: Rajeev Chamyal Cc: Philip Race; Alexander Scherbatiy; 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 Hi Rajeev, In launcher.properties do you need an additional line to say, one must use the filename conventions to specify resolution ? btw: I think you have formatting/indent issues in this file: libsplashscreen/splashscreen_impl.c Thanks Kumar > Hello Kumar,Phil, > > Please review the update updated webrev. > > http://cr.openjdk.java.net/~rchamyal/8151787/webrev.08/ > Updates in files : > src/java.base/share/classes/sun/launcher/resources/launcher.properties > src/java.desktop/share/classes/java/awt/SplashScreen.java > > Regards, > Rajeev Chamyal > > -----Original Message----- > From: Kumar Srinivasan > Sent: 02 September 2016 19:10 > To: Rajeev Chamyal > Cc: Alexander Scherbatiy; awt-dev@openjdk.java.net; Sergey Bylokhov; > Philip Race > Subject: Re: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 Unify > the HiDPI splash screen image naming convention > > > Hi, > >> Hello Kumar, >> >> I have further updated launcher.properties. >> http://cr.openjdk.java.net/~rchamyal/8151787/webrev.07/ >> >> For documentation update I have already raise a bug. >> https://bugs.openjdk.java.net/browse/JDK-8165009 > Ok. > > so what about ? > > http://hg.openjdk.java.net/jdk9/dev/jdk/file/1c28399f1b50/src/java.des > ktop/share/classes/java/awt/SplashScreen.java > > > This is the SplashScreen specification, is it not ? But there seems to be no > effort to clarify this specification ? > > Kumar > >> Regards, >> Rajeev Chamyal >> >> -----Original Message----- >> From: Kumar Srinivasan >> Sent: 01 September 2016 19:17 >> To: Rajeev Chamyal >> Cc: Alexander Scherbatiy; awt-dev@openjdk.java.net; Sergey Bylokhov; >> Philip Race >> Subject: Re: <AWT Dev> <Swing Dev>[9] Review Request JDK-8151787 >> Unify the HiDPI splash screen image naming convention >> >> >> Hello Rajeev, >> >> IMHO, this really belongs here: >> >> http://hg.openjdk.java.net/jdk9/dev/jdk/file/1c28399f1b50/src/java.de >> s ktop/share/classes/java/awt/SplashScreen.java >> >> and here: >> >> https://docs.oracle.com/javase/8/docs/technotes/tools/unix/java.html >> >> If you can reduce the words in the launcher, it would be good, also please >> make sure the lines do not exceed 80 chars ie. the output of java -help. >> >> >> Kumar >> >> >> >>> Hello Kumar, >>> >>> Can you please review the updated >>> src/java.base/share/classes/sun/launcher/resources/launcher.properti >>> e s http://cr.openjdk.java.net/~rchamyal/8151787/webrev.06/ >>> >>> Regards, >>> Rajeev Chamyal >>> >>> -----Original Message----- >>> From: Philip Race >>> Sent: 27 August 2016 03:10 >>> To: Rajeev Chamyal >>> Cc: Alexander Scherbatiy; 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 >>> >>> Seems fine now. >>> 1) Please do a CCC for this >>> 2) Please file a doc. bug so docs team can update the HTML man page >>> and also perhaps >>> https://docs.oracle.com/javase/tutorial/uiswing/misc/splashscreen.ht >>> m >>> l >>> >>> -phil >>> >>> On 8/25/16, 11:49 PM, Rajeev Chamyal wrote: >>>> Hello Alexandr, >>>> >>>> Please review the updated webrev. >>>> http://cr.openjdk.java.net/~rchamyal/8151787/webrev.05/ >>>> >>>> Regards, >>>> Rajeev Chamyal >>>> >>>> -----Original Message----- >>>> From: Alexandr Scherbatiy >>>> Sent: 25 August 2016 22:07 >>>> To: Rajeev Chamyal; Philip Race >>>> Cc: 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 8/22/2016 2:13 PM, Rajeev Chamyal wrote: >>>>> 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.proper >>>>> t >>>>> i >>>>> e s >>>>> src/java.desktop/macosx/native/libsplashscreen/splashscreen_sys.m >>>>> src/java.desktop/macosx/native/libsplashscreen/splashscreen_config. >>>>> h >>>> The findScaledImageName(...) method is only used in >>>> splashscreen_sys.m file. Is it possible to not declare it in >>>> splashscreen_config.h? >>>> >>>> Thanks, >>>> Alexandr. >>>> >>>>> 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.prope >>>>>> r >>>>>> t >>>>>> i >>>>>> e >>>>>> s >>>>>> >>>>>> 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.prope >>>>>> r >>>>>> t >>>>>> i >>>>>> e >>>>>> s >>>>>> >>>>>> 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. >>>>>> h >>>>>> t >>>>>> m >>>>>> l >>>>>> >>>>>> Thanks, >>>>>> Alexandr. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Thanks, >>>>>> Alexandr. >>>>>> >>>>>> >>>>>> >>>>>> >>>>>> Regards, >>>>>> >>>>>> Rajeev Chamyal >>>>>>