Hello Phil, Please review the updated webrev. http://cr.openjdk.java.net/~rchamyal/8151787/webrev.09/
Updates: Updated documentation in src/java.desktop/share/classes/java/awt/SplashScreen.java Review comments from Kumar are also updated in launcher.properties. Regards, Rajeev Chamyal -----Original Message----- From: Kumar Srinivasan Sent: 12 September 2016 20:46 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 -\ HiDPI and Non-HiDPI.Scaled filename convention should be\n\ +\ HiDPI and Non-HiDPI. Scaled filename convention should be\n\ +\ used for HiDPI images\n\ Space after . Approved, contingent on the above fix, I don't need to see another iteration. Thanks Kumar On 9/12/2016 5:25 AM, Rajeev Chamyal wrote: > 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.propertie >> s 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.de >> s 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.d >>> e 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.propert >>>> i 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.h >>>> t >>>> 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.prope >>>>>> r >>>>>> 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.prop >>>>>>> e >>>>>>> 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.prop >>>>>>> e >>>>>>> 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 >>>>>>>