Re: Review Request For 8149636: TextField flicker & over scroll when selection scrolls beyond the bounds of TextField.

2016-03-03 Thread Semyon Sadetsky

Hi Ambarish,

the first point is not valid. Please, ignore it.

--Semyon

On 3/1/2016 11:14 AM, Semyon Sadetsky wrote:

Hi Ambarish,

- The over scroll remains after the proposed fix.

- The bug is a regression of 7092551 and should be labeled 
correspondingly.


- An automatic test may and should be provided with the fix.

- The same issue existing in the TextArea should be fixed as well.

--Semyon


On 2/11/2016 12:23 PM, Ambarish Rapte wrote:


Hi,

Please review the fix for JDK9,

Bug: https://bugs.openjdk.java.net/browse/JDK-8149636

Webrev: 
http://cr.openjdk.java.net/~arapte/8149636/webrev.00/


Issue:

While scrolling with selection, when mouse is dragged 
outside the bounds of TextField,


a.TextField flickering is observed

b.TextField over scrolls on right. Text gets hidden on left side even 
when no scroll is needed.


Cause:

a.Flicker –
For simulating scroll, scroll bar information is used.
To get correct scroll info, scroll bar should be visible.
Showing and hiding of scroll bar causes the flickering.

b.Over Scroll –

The scrolling logic scrolled the textField in units of half the 
si.nPage. i.e. (si.inPage / 2) maximum till si.nMax,

Which causes the over scroll.

Fix:

ES_AUTOHSCROLL set for the TextField enables the 
automatic horizontal scrolling, so we need not add code for scrolling.


Only selection should be updated as the mouse moves.

Hence removed the code to related to SendMessage( 
WM_HSCROLL,…)  i.e. scrolling


Verification:

Manually Verified that textField editing / selection 
/ copy / paste works fine.


No Jtreg, JCK tests related to TextField fail due to 
this change.


Updated an existing test *ScrollSelectionTest.java 
*to test the scenarios mentioned in the bug.


Thanks,

Ambarish







Re: Review request for 6734341 : REGTEST fails: SelectionAutoscrollTest.html

2016-03-03 Thread Vikrant Agarwal
Hi Semyon,

Although the execution time of the previous version was shorter, but it was 
also failing many times, the extra execution time got introduced after repacing 
mouseMove() from java.awt.Robot with mouseMove() from 
test.java.awt.regtesthelpers.Util , this performes the mouse movement a little 
slower than the previous one, but also makes the test more stable, as the new 
version is passing on all the tested machines.

Best Regards,
Vikrant

-Original Message-
From: Semyon Sadetsky 
Sent: Thursday, March 03, 2016 5:53 PM
To: Vikrant Agarwal; Sergey Bylokhov; awt-dev@openjdk.java.net; Alexander 
Scherbatiy
Subject: Re: Review request for 6734341 : REGTEST fails: 
SelectionAutoscrollTest.html

Hi, Vikrant,

I ran the new test version on Windows. It's execution time is about 20 seconds 
while the previous version was instant.
Is that expected?

--Semyon



On 2/26/2016 3:53 PM, Vikrant Agarwal wrote:
> Hi Sergey,
>
> I have updated the test to a standalone test.
>
> Updated Webrev: 
> http://cr.openjdk.java.net/~ntv/vikrant/6734341/webrev.02/
>
> Bug: JDK-6734341 : REGTEST fails: SelectionAutoscrollTest.html
>
> Best Regards,
> Vikrant
>
> -Original Message-
> From: Sergey Bylokhov
> Sent: Thursday, February 25, 2016 2:03 AM
> To: Vikrant Agarwal; awt-dev@openjdk.java.net; Alexander Scherbatiy; 
> Semyon Sadetsky
> Subject: Re: Review request for 6734341 : REGTEST fails: 
> SelectionAutoscrollTest.html
>
> Hi, Vikrant.
> The @bug tag should be updated in the html file, because it have a @test tag. 
> But I suggest to remove the html file and change SelectionAutoscrollTest.java 
> to a standalone test(instead of Applet).
>
> On 24.02.16 11:25, Vikrant Agarwal wrote:
>> Hi All,
>>
>> Please review the fix for JDK9.
>>
>> Bug: https://bugs.openjdk.java.net/browse/JDK-6734341
>>
>> Webrev: http://cr.openjdk.java.net/~ntv/vikrant/6734341/webrev.00/
>>
>> Issue:  java/awt/TextArea/UsingWithMouse/SelectionAutoscrollTest  
>> test was always failing for mac and intermittently failing for other 
>> platforms.
>>
>> Fix: using mouseMove() from test.java.awt.regtesthelpers.Util insted 
>> of
>> mouseMove() from java.awt.Robot makes the test more stable and the 
>> test passes on all the tested machines including mac.
>>
>> Best Regards,
>>
>> Vikrant
>>
>
> --
> Best regards, Sergey.



Re: [9] RFR for 8150724: [TEST] HiDPI: create a test for multiresolution icons

2016-03-03 Thread Sergey Bylokhov

Hi, Alexander.
I suggest to add these lines to the test instead of replace the old 
lines. The test will pass after the fix for 8151060.


On 03.03.16 17:45, Alexander Stepanov wrote:

Could you please review the updated version of the test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.01

Line
  106 tabbedPane.addTab("", icon, p);
was replaced with
  109 tabbedPane.addTab("", p);
  110 tabbedPane.setTabComponentAt(0, new JLabel(icon));

- in such a case the test passes for OS X (as well as for Windows, Linux).
Probably these changes should be reverted after JDK-8151060 fix.

Thanks,
Alexander

On 3/2/2016 7:15 PM, Alexander Stepanov wrote:

Hello Sergey,

It fails because of
https://bugs.openjdk.java.net/browse/JDK-8151060

(plus we need here some tricky check for resolution by the analogy
with 8150258 because of JDK-8150844).

Thanks,
Alexander

On 2/26/2016 4:25 PM, Sergey Bylokhov wrote:

Hi, Alexander.
The test failed on osx 10.11 + retina. Is it expected?

On 26.02.16 15:53, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8150724/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8150724 ?

Just a single test added.

Thanks,
Alexander










--
Best regards, Sergey.


Re: Review-request for 8143227: Platform-Specific Desktop Features

2016-03-03 Thread Sergey Bylokhov
In general the fix looks fine to me. All other cases can be covered via 
separate CRs.


On 03.03.16 18:31, Alexander Zvegintsev wrote:

Hi,

CCC is now in final state.

For these raised questions I filed following issues:
https://bugs.openjdk.java.net/browse/JDK-8151184 [macosx] window does
not receive normal window events indicating the window is hidden
https://bugs.openjdk.java.net/browse/JDK-8151189 Possible
getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar

--
Thanks,
Alexander.

On 02/24/2016 07:27 AM, Philip Race wrote:

Hi,

API-wise this now looks like it could go to CCC.
I have not reviewed the implementation yet but that
should not stop the CCC from being submitted as final.

Perhaps the only concern I have is around AppHidden/Appreopened
where I am not convinced that it is the right thing to not send normal
window events indicating the window is hidden. What would the harm
be in sending such events so that apps that aren't able to use these
Mac-specific events because they pre-date these APIs don't get a
notification which seems to mean the same thing to me.
That could be an implementation detail or something we can
refine later in an targeted CCC if needed.

The AppContext issue is an implementation issue and we can discuss
it later but the reason other places do not check is it never used to
return null. Now it can. So do we really want to add more places
that we need to fix ? In other words I don't think the fix is going
to be to revert to it not returning null, so you are going to have
to deal with it directly here.

-phil.

On 2/21/16, 5:08 PM, Alexander Zvegintsev wrote:

Hi Phil,
please see updated the webrev here
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/
and comments inline

On 18.02.2016 0:10, Phil Race wrote:

Desktop.java looks fine now. Moving on to the other API classes

Top-level question :
Why are AppEvent and TaskBar in java.awt and not java.awt.desktop ?

Agreed on AppEvent, but Taskbar is placed in java.awt on purpose to
make it adjacent with Desktop and SystemTray.

And AppEvent seems to use a lot of nested classes.
If they were in java.awt.desktop then they could be top-level classes
without creating too much clutter. I don't think it matters that 'this
is how the com.apple APIs were done' - people need to recode anyway


AppEvent.java :
more wildcards ..
   28 import java.awt.desktop.*;

I notice the docs in this file (and perhaps more I have yet to see)
reference types without @code :-
eg :
  * Constructs a FilesEvent
* Constructs an OpenFilesEvent
..
  * Get the URI the app was asked to open
  146  * @return the URI

and more. Not essential I suppose but preferable.

   69 public List getFiles() {
   70 return files;
   71 }

Is there any need to return a copy for any of these ?

   95  * Gets the search term. The platform may additionally provide 
the search
   96  * term that was used to find the files. This is for example the 
case
   97  * on Mac OS X, when the files were opened using the Spotlight 
search
   98  * menu or a Finder search window.
   99  *
  100  * This is useful for highlighting the search term in the 
documents when
  101  * they are opened.
  102  * @return the search term used to find the files

Could we say "optionally" instead of "additionally"
and make clear that it may be "null"

a very long line :-

209  * Event sent when the application has become the foreground app, and 
when it has resigned being the foreground app.

And "resigned" is a voluntary term. If something else is placed into
the foreground the
this app may not get asked if it wants to allow that.
Maybe instead say "and when it is no longer the foreground app."


Done.

Also is there anything we can (or should) say about how some of
these events interact ?

eg, if a foreground app is hidden does only the appHidden listener
get called ?


Both listeners could be called, it's already mentioned here:

 * Called when the hidden app is shown again (but not
necessarily brought to
 * the foreground).
 *
 * @param e event
 */
public void appUnhidden(final AppHiddenEvent e);



BTW the term 'unhidden' makes sense if the app was previously
'hidden' but less so
if the app is being newly shown.


  /**
  252  * The system does not provide a reason for a session change.
  253  */
  254 @Native
  255 public static final int REASON_UNSPECIFIED = 0x0;
  256

I think we should use an enum here.


Also unless there is a reason I have not yet come across, as many as
possible
of these classes should be marked 'final'


Taskbar.java :

One meta-concern I have here is that it seems to have a very blurry
distinction
with existing API :
https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html
https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html

Aren't both of these APIs dealing with the same area ?


Re: Review-request for 8143227: Platform-Specific Desktop Features

2016-03-03 Thread Alexander Zvegintsev


I think that this fine is at is.
Regarding permissions I filed the JDK-8151190 issue:
https://bugs.openjdk.java.net/browse/JDK-8151190 Revise permissions for 
java.awt.Desktop and java.awt.Taskbar


--
Thanks,
Alexander.

On 02/24/2016 08:12 PM, Sergey Bylokhov wrote:
Small question about moveToTrash(). Should we describe that the user 
can delete folders as well? Also I am not sure about permissions which 
we check. For example the usual File.delete() calls "SM.checkDelete()" 
but we check SM.checkWrite() for "<>". Moreover 
checkFileValidation() checks the read permission for the file as well. 
At the end it is an interesting question: what permissions are needed 
to delete the file.


On 21.02.16 14:38, Alexander Zvegintsev wrote:

Hi Phil,
please see updated the webrev here
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/
and comments inline

On 18.02.2016 0:10, Phil Race wrote:

Desktop.java looks fine now. Moving on to the other API classes

Top-level question :
Why are AppEvent and TaskBar in java.awt and not java.awt.desktop ?

Agreed on AppEvent, but Taskbar is placed in java.awt on purpose to make
it adjacent with Desktop and SystemTray.

And AppEvent seems to use a lot of nested classes.
If they were in java.awt.desktop then they could be top-level classes
without creating too much clutter. I don't think it matters that 'this
is how the com.apple APIs were done' - people need to recode anyway


AppEvent.java :
more wildcards ..
   28 import java.awt.desktop.*;

I notice the docs in this file (and perhaps more I have yet to see)
reference types without @code :-
eg :
  * Constructs a FilesEvent
* Constructs an OpenFilesEvent
..
  * Get the URI the app was asked to open
  146  * @return the URI

and more. Not essential I suppose but preferable.

   69 public List getFiles() {
   70 return files;
   71 }

Is there any need to return a copy for any of these ?

   95  * Gets the search term. The platform may additionally 
provide the search
   96  * term that was used to find the files. This is for 
example the case
   97  * on Mac OS X, when the files were opened using the 
Spotlight search

   98  * menu or a Finder search window.
   99  *
  100  * This is useful for highlighting the search term in 
the documents when

  101  * they are opened.
  102  * @return the search term used to find the files

Could we say "optionally" instead of "additionally"
and make clear that it may be "null"

a very long line :-

209  * Event sent when the application has become the foreground 
app, and when it has resigned being the foreground app.


And "resigned" is a voluntary term. If something else is placed into
the foreground the
this app may not get asked if it wants to allow that.
Maybe instead say "and when it is no longer the foreground app."


Done.

Also is there anything we can (or should) say about how some of these
events interact ?

eg, if a foreground app is hidden does only the appHidden listener get
called ?


Both listeners could be called, it's already mentioned here:

 * Called when the hidden app is shown again (but not necessarily
brought to
 * the foreground).
 *
 * @param e event
 */
public void appUnhidden(final AppHiddenEvent e);



BTW the term 'unhidden' makes sense if the app was previously 'hidden'
but less so
if the app is being newly shown.


  /**
  252  * The system does not provide a reason for a session 
change.

  253  */
  254 @Native
  255 public static final int REASON_UNSPECIFIED = 0x0;
  256

I think we should use an enum here.


Also unless there is a reason I have not yet come across, as many as
possible
of these classes should be marked 'final'


Taskbar.java :

One meta-concern I have here is that it seems to have a very blurry
distinction
with existing API :
https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html
https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html

Aren't both of these APIs dealing with the same area ?

Should we really add this new class or tweak system tray as needed ?

On Windows they dealing in the same area with taskbar, but on Mac OS
SystemTray and Taskbar(Dock) are separated. So I prefer to left it as 
it is.


/**
  112  * Stops displaying the progress.
  113  */
  114 @Native
  115 public static final int STATE_OFF = 0x0;

Another case where an enum could be used.


   sun.awt.AppContext context = sun.awt.AppContext.getAppContext();

these days this sometimes can return null ..


There are many usages of getAppContext() without null check all over our
code, so I think that this should be fixed under a separate bug.

AppForegroundEvent :
* @param e the app became foreground notification.

I am not entirely sure what that means or even what it
should say, perhaps because  AppForegroundEvent does not
have any fields.
How about
"@param e an 

Re: Review-request for 8143227: Platform-Specific Desktop Features

2016-03-03 Thread Alexander Zvegintsev

Hi,

CCC is now in final state.

For these raised questions I filed following issues:
https://bugs.openjdk.java.net/browse/JDK-8151184 [macosx] window does 
not receive normal window events indicating the window is hidden
https://bugs.openjdk.java.net/browse/JDK-8151189 Possible 
getAppContext() NPE in java.awt.Desktop and java.awt.Taskbar


--
Thanks,
Alexander.

On 02/24/2016 07:27 AM, Philip Race wrote:

Hi,

API-wise this now looks like it could go to CCC.
I have not reviewed the implementation yet but that
should not stop the CCC from being submitted as final.

Perhaps the only concern I have is around AppHidden/Appreopened
where I am not convinced that it is the right thing to not send normal
window events indicating the window is hidden. What would the harm
be in sending such events so that apps that aren't able to use these
Mac-specific events because they pre-date these APIs don't get a
notification which seems to mean the same thing to me.
That could be an implementation detail or something we can
refine later in an targeted CCC if needed.

The AppContext issue is an implementation issue and we can discuss
it later but the reason other places do not check is it never used to
return null. Now it can. So do we really want to add more places
that we need to fix ? In other words I don't think the fix is going
to be to revert to it not returning null, so you are going to have
to deal with it directly here.

-phil.

On 2/21/16, 5:08 PM, Alexander Zvegintsev wrote:

Hi Phil,
please see updated the webrev here
http://cr.openjdk.java.net/~azvegint/jdk/9/8143227/08/
and comments inline

On 18.02.2016 0:10, Phil Race wrote:

Desktop.java looks fine now. Moving on to the other API classes

Top-level question :
Why are AppEvent and TaskBar in java.awt and not java.awt.desktop ?
Agreed on AppEvent, but Taskbar is placed in java.awt on purpose to 
make it adjacent with Desktop and SystemTray.

And AppEvent seems to use a lot of nested classes.
If they were in java.awt.desktop then they could be top-level classes
without creating too much clutter. I don't think it matters that 'this
is how the com.apple APIs were done' - people need to recode anyway


AppEvent.java :
more wildcards ..
   28 import java.awt.desktop.*;

I notice the docs in this file (and perhaps more I have yet to see)
reference types without @code :-
eg :
  * Constructs a FilesEvent
* Constructs an OpenFilesEvent
..
  * Get the URI the app was asked to open
  146  * @return the URI

and more. Not essential I suppose but preferable.

   69 public List getFiles() {
   70 return files;
   71 }

Is there any need to return a copy for any of these ?

   95  * Gets the search term. The platform may additionally provide 
the search
   96  * term that was used to find the files. This is for example the 
case
   97  * on Mac OS X, when the files were opened using the Spotlight 
search
   98  * menu or a Finder search window.
   99  *
  100  * This is useful for highlighting the search term in the 
documents when
  101  * they are opened.
  102  * @return the search term used to find the files

Could we say "optionally" instead of "additionally"
and make clear that it may be "null"

a very long line :-

209  * Event sent when the application has become the foreground app, and 
when it has resigned being the foreground app.

And "resigned" is a voluntary term. If something else is placed into 
the foreground the

this app may not get asked if it wants to allow that.
Maybe instead say "and when it is no longer the foreground app."


Done.
Also is there anything we can (or should) say about how some of 
these events interact ?


eg, if a foreground app is hidden does only the appHidden listener 
get called ?



Both listeners could be called, it's already mentioned here:
 * Called when the hidden app is shown again (but not 
necessarily brought to

 * the foreground).
 *
 * @param e event
 */
public void appUnhidden(final AppHiddenEvent e);


BTW the term 'unhidden' makes sense if the app was previously 
'hidden' but less so

if the app is being newly shown.


  /**
  252  * The system does not provide a reason for a session change.
  253  */
  254 @Native
  255 public static final int REASON_UNSPECIFIED = 0x0;
  256

I think we should use an enum here.


Also unless there is a reason I have not yet come across, as many as 
possible

of these classes should be marked 'final'


Taskbar.java :

One meta-concern I have here is that it seems to have a very blurry 
distinction

with existing API :
https://docs.oracle.com/javase/8/docs/api/java/awt/SystemTray.html
https://docs.oracle.com/javase/8/docs/api/java/awt/TrayIcon.html

Aren't both of these APIs dealing with the same area ?

Should we really add this new class or tweak system tray as needed ?
On Windows they dealing in the same area with taskbar, 

Re: [9] RFR for 8150724: [TEST] HiDPI: create a test for multiresolution icons

2016-03-03 Thread Alexander Stepanov

Could you please review the updated version of the test?
http://cr.openjdk.java.net/~avstepan/8150724/webrev.01

Line
 106 tabbedPane.addTab("", icon, p);
was replaced with
 109 tabbedPane.addTab("", p);
 110 tabbedPane.setTabComponentAt(0, new JLabel(icon));

- in such a case the test passes for OS X (as well as for Windows, Linux).
Probably these changes should be reverted after JDK-8151060 fix.

Thanks,
Alexander

On 3/2/2016 7:15 PM, Alexander Stepanov wrote:

Hello Sergey,

It fails because of
https://bugs.openjdk.java.net/browse/JDK-8151060

(plus we need here some tricky check for resolution by the analogy 
with 8150258 because of JDK-8150844).


Thanks,
Alexander

On 2/26/2016 4:25 PM, Sergey Bylokhov wrote:

Hi, Alexander.
The test failed on osx 10.11 + retina. Is it expected?

On 26.02.16 15:53, Alexander Stepanov wrote:

Hello,

Could you please review the following fix
http://cr.openjdk.java.net/~avstepan/8150724/webrev.00/
for
https://bugs.openjdk.java.net/browse/JDK-8150724 ?

Just a single test added.

Thanks,
Alexander









Re: Review request for 6734341 : REGTEST fails: SelectionAutoscrollTest.html

2016-03-03 Thread Semyon Sadetsky

Hi, Vikrant,

I ran the new test version on Windows. It's execution time is about 20 seconds 
while the previous version was instant.
Is that expected?

--Semyon



On 2/26/2016 3:53 PM, Vikrant Agarwal wrote:

Hi Sergey,

I have updated the test to a standalone test.

Updated Webrev: http://cr.openjdk.java.net/~ntv/vikrant/6734341/webrev.02/

Bug: JDK-6734341 : REGTEST fails: SelectionAutoscrollTest.html

Best Regards,
Vikrant

-Original Message-
From: Sergey Bylokhov
Sent: Thursday, February 25, 2016 2:03 AM
To: Vikrant Agarwal; awt-dev@openjdk.java.net; Alexander Scherbatiy; Semyon 
Sadetsky
Subject: Re: Review request for 6734341 : REGTEST fails: 
SelectionAutoscrollTest.html

Hi, Vikrant.
The @bug tag should be updated in the html file, because it have a @test tag. 
But I suggest to remove the html file and change SelectionAutoscrollTest.java 
to a standalone test(instead of Applet).

On 24.02.16 11:25, Vikrant Agarwal wrote:

Hi All,

Please review the fix for JDK9.

Bug: https://bugs.openjdk.java.net/browse/JDK-6734341

Webrev: http://cr.openjdk.java.net/~ntv/vikrant/6734341/webrev.00/

Issue:  java/awt/TextArea/UsingWithMouse/SelectionAutoscrollTest  test
was always failing for mac and intermittently failing for other platforms.

Fix: using mouseMove() from test.java.awt.regtesthelpers.Util insted
of
mouseMove() from java.awt.Robot makes the test more stable and the
test passes on all the tested machines including mac.

Best Regards,

Vikrant



--
Best regards, Sergey.




Re: [9] Review request for JDK-8145174 HiDPI splash screen support on Linux

2016-03-03 Thread Sergey Bylokhov
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:  [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 ; Sergey
Bylokhov
*Subject:* Re:  [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  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
; Sergey Bylokhov
*Subject:* Re:  [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/


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  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, _factor);
 DoSplashInit();
 DoSplashSetScaleFactor(scale_factor);
 DoSplashLoadFile(scaled_splash_name);
 --
   To 

Re: [9] Review request for JDK-8145173 HiDPI splash screen support on Windows

2016-03-03 Thread Sergey Bylokhov
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:50, 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/8145173/webrev.02/

Regards,

Rajeev Chamyal

*From:*Rajeev Chamyal
*Sent:* 01 March 2016 15:45
*To:* awt-dev@openjdk.java.net; Sergey Bylokhov; Alexander Scherbatiy
*Subject:* RE:  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

Hello All,

Gentle reminder.

Please review the updated webrev.


http://cr.openjdk.java.net/~rchamyal/8145173/webrev.01/

Regards,

Rajeev Chamyal

*From:*Rajeev Chamyal
*Sent:* 16 February 2016 16:01
*To:* awt-dev@openjdk.java.net ; Sergey
Bylokhov; Alexander Scherbatiy
*Subject:*  [9] Review request for JDK-8145173 HiDPI splash
screen support on Windows

Hello All,

Could you please review the following fix.

Bug : https://bugs.openjdk.java.net/browse/JDK-8145173

Webrev : http://cr.openjdk.java.net/~rchamyal/8145173/webrev.00/

This is an enhancement to support HiDPI splash screen on windows.

As a part of this enhancement implementation to
splashscreen_sys.c::SplashGetScaledImageName method has been provided.

System dpi and scale factor are used to determine the scaled image name.
Dpi value is read using GetDpiForMonitor API on Windows 8 and
GetDesktopDpi API on Windows 7.

Scale factor is calculated from the dpi value.

The naming convention followed for scaled image name is as follows:

Refer :
https://msdn.microsoft.com/en-us/library/windows/apps/xaml/hh965325.aspx

Unscaled image name : image.ext

Scaled image name : image.scale-./ext/

Regards,

Rajeev Chamyal




--
Best regards, Sergey.