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

[Alexander Scherbatiy]

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, &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

Re: [9] RFR for 8150643: [TEST] add test for JDK-8150176

[Alexander Stepanov]

Hello Sergey,

Thank you for the note;

Could you please review the fix for this issue
http://cr.openjdk.java.net/~avstepan/8150643/webrev.02/

If the changes are appropriate then I'll create a separate test bug and 
push them.


Thanks,
Alexander

On 2/26/2016 8:34 PM, Sergey Bylokhov wrote:
Just one additional note: When you created a MRI you set a nok image 
as a base image, which means that the size of the MRI will consider 
wrong(small). And this contradicts the description of the test:
"Please check if both of them have correct size", because 
setImageAutoSize is false by default. But it seems we have a bug 
because setImageAutoSize(true) does not help. I'll update the test in 
JDK-8150176.


On 26.02.16 19:09, Alexander Stepanov wrote:

Thanks! (will remove "for a few seconds").

Regards,
Alexander

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

Looks fine.
Note that the text "Two tray icons will appear for a few seconds
" should be updated also.

On 26.02.16 18:04, Alexander Stepanov wrote:

 > Is it possible to clear the tray when the test ended?

Yes, we can use the Applet's stop() method, please see
http://cr.openjdk.java.net/~avstepan/8150643/webrev.01/

Thanks,
Alexander

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

I think it is not a good idea to block the EDT, or to expect that the
user will be able to find icon in a 7 seconds, especially if he run
the test for the firts time. Is it possible to clear the tray when 
the

test ended?

On 26.02.16 16:39, Alexander Stepanov wrote:

Hello Sergey,

This is a time interval for the user to examine the icons (which
could,
e.g., go to the tray pool). So my feeling is that it is a more or 
less

suitable interval, but it could be shortened for a couple of
seconds, of
course.

Thanks,
Alexander

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

Is it really necessary to block the EDT for 7 seconds?

On 25.02.16 18:22, Alexander Stepanov wrote:
(sorry,  misprint,  [9], not [8]. the same for RFR 
8150258)


On 2/25/2016 5:50 PM, Alexander Stepanov wrote:

Hello,

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

Just a single test added (still failing).

Thanks,
Alexander

Re: [8] RFR for 8150258: [TEST] HiDPI: create a test for multiresolution menu items icons

[Alexander Stepanov]

Hello Sergey,

When running the following trivial test case (having Color set to 
"Generic RGB Profile")


import java.awt.*;

public class ColorTest extends Frame {

private void UI() {
setSize(100, 100);
setBackground(Color.BLUE);
setVisible(true);
}

public ColorTest() throws Exception {
EventQueue.invokeAndWait(this::UI);
}

public static void main(String[] args) throws Exception {
ColorTest test = new ColorTest();
Thread.sleep(1000);

Point pt = test.getLocationOnScreen();
System.out.println("color = " +
(new Robot()).getPixelColor(pt.x + 50, pt.y + 50));
test.dispose();
}
}

the expected output appears:

color = java.awt.Color[r=0,g=0,b=255]

At the same time the Color for the Icon is "[r=0,g=0,b=248]"

Could you please let me know if the test still fails for you when simply 
set "expected = C2X" on the ln 116 and comment out the 1st "@run"?


Not sure if it should be considered as a bug, or some additional 
settings are required, or the color check should be simply softened 
(some tolerance used).


Thanks,
Alexander


On 2/26/2016 6:13 PM, Alexander Stepanov wrote:

It seemingly does not help (but maybe I'm doing something wrong).

On 2/26/2016 5:45 PM, Sergey Bylokhov wrote:

On 26.02.16 17:27, Alexander Stepanov wrote:

It seems that for osx + retina

1. -Dsun.java2d.uiScale option is ignored for multires. image (in
contrast for HiDPI + Win. 8, for example). Should it be considered as a
bug? At a 1st glance, yes.
2. even in case of correct color robot's getPixelColor() returns for
retina a bit shifted color ((0, 0, 251) for blue instead of (0, 0, 
255))

- not sure if that could be considered as an issue, or the color check
should be simply softened.


Can you try to set Generic color RGB profile for the Display?



Thanks,
Alexander

P.S. I performed test runs on HiDPI Windows 8 and regular Ubuntu 14.04
Linux initially (which is incomplete)



On 2/26/2016 4:41 PM, Alexander Stepanov wrote:

Hello Sergey,

No, as well as for 8150724. I have to re-check, thank you.

Regards,
Alexander

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

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

On 25.02.16 17:51, Alexander Stepanov wrote:

Sorry, just a reminder.

Thanks,
Alexander

On 2/19/2016 7:17 PM, Alexander Stepanov wrote:

Hello,

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

Just a single test added.

Thanks,
Alexander

Re: [8] RFR for 8150258: [TEST] HiDPI: create a test for multiresolution menu items icons

[Alexander Stepanov]

The following version of the test passes for OS X + retina (as well as 
for Windows and Linux):

http://cr.openjdk.java.net/~avstepan/8150258/webrev.01/

not sure if "-Dsun.java2d.uiScale" option was intended for use with OS X 
(probably an enhancement bug should be created).


So for OS X now we have only single test run in fact.

Thanks,
Alexander

On 2/29/2016 5:12 PM, Alexander Stepanov wrote:

Hello Sergey,

When running the following trivial test case (having Color set to 
"Generic RGB Profile")


import java.awt.*;

public class ColorTest extends Frame {

private void UI() {
setSize(100, 100);
setBackground(Color.BLUE);
setVisible(true);
}

public ColorTest() throws Exception {
EventQueue.invokeAndWait(this::UI);
}

public static void main(String[] args) throws Exception {
ColorTest test = new ColorTest();
Thread.sleep(1000);

Point pt = test.getLocationOnScreen();
System.out.println("color = " +
(new Robot()).getPixelColor(pt.x + 50, pt.y + 50));
test.dispose();
}
}

the expected output appears:

color = java.awt.Color[r=0,g=0,b=255]

At the same time the Color for the Icon is "[r=0,g=0,b=248]"

Could you please let me know if the test still fails for you when 
simply set "expected = C2X" on the ln 116 and comment out the 1st "@run"?


Not sure if it should be considered as a bug, or some additional 
settings are required, or the color check should be simply softened 
(some tolerance used).


Thanks,
Alexander


On 2/26/2016 6:13 PM, Alexander Stepanov wrote:

It seemingly does not help (but maybe I'm doing something wrong).

On 2/26/2016 5:45 PM, Sergey Bylokhov wrote:

On 26.02.16 17:27, Alexander Stepanov wrote:

It seems that for osx + retina

1. -Dsun.java2d.uiScale option is ignored for multires. image (in
contrast for HiDPI + Win. 8, for example). Should it be considered 
as a

bug? At a 1st glance, yes.
2. even in case of correct color robot's getPixelColor() returns for
retina a bit shifted color ((0, 0, 251) for blue instead of (0, 0, 
255))

- not sure if that could be considered as an issue, or the color check
should be simply softened.


Can you try to set Generic color RGB profile for the Display?



Thanks,
Alexander

P.S. I performed test runs on HiDPI Windows 8 and regular Ubuntu 14.04
Linux initially (which is incomplete)



On 2/26/2016 4:41 PM, Alexander Stepanov wrote:

Hello Sergey,

No, as well as for 8150724. I have to re-check, thank you.

Regards,
Alexander

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

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

On 25.02.16 17:51, Alexander Stepanov wrote:

Sorry, just a reminder.

Thanks,
Alexander

On 2/19/2016 7:17 PM, Alexander Stepanov wrote:

Hello,

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

Just a single test added.

Thanks,
Alexander

[9] Review Request for 8150535: [TEST_BUG] fix @library for test/java/awt/TrayIcon/MouseMovedTest/MouseMovedTest.java

[Yuri Nesterenko]

Colleagues,

please review this test-only fix for some tray icon AWT tests
in 9-jigsaw repository.
CR is https://bugs.openjdk.java.net/browse/JDK-8150535
Webrev:
http://cr.openjdk.java.net/~yan/8150535/webrev.00

The fix should go to 9-jigsaw since there are extra lines
using Xpatch (comparing with regular jdk9 repository) in
the every file touched.

Note that the fix is a sort of workaround of a (possible)
jtreg issue reproducible i.e. on Linux: a @library of just "../"
doesn't work. It works on Windows.
We discussed it some time ago but I filed no bug. Should I?

Thank you,
-yan

Re: [9] Review Request: 8144164 [macosx] Test java/awt/Focus/MouseClickRequestFocusRaceTest/MouseClickRequestFocusRaceTest failed

[Yuri Nesterenko]

Hi Sergey,

fix looks OK to me.
We didn't see the original issue on Mac, did we?

Thanks,
-yan

On 02/28/2016 10:33 PM, Sergey Bylokhov wrote:

Hello,
Please review the small fix for jdk9.

The test tries to use Alt+F4 but it is meaningless on OSX.
- On OSX Ctrl+f4 (cycle between open windows in all applications) now is
used, to simulate the native focus lost.
- cleanup.

Bug: https://bugs.openjdk.java.net/browse/JDK-8144164
Webrev can be found at: http://cr.openjdk.java.net/~serb/8144164/webrev.01

Re: [8] RFR for 8150258: [TEST] HiDPI: create a test for multiresolution menu items icons

[Sergey Bylokhov]

On 29.02.16 18:45, Alexander Stepanov wrote:

not sure if "-Dsun.java2d.uiScale" option was intended for use with OS X
(probably an enhancement bug should be created).

So for OS X now we have only single test run in fact.


I do not remember that such request was filed, please file a separate 
enhancement.



--
Best regards, Sergey.

Re: [9] Review Request: 8144164 [macosx] Test java/awt/Focus/MouseClickRequestFocusRaceTest/MouseClickRequestFocusRaceTest failed

[Sergey Bylokhov]

On 29.02.16 18:49, Yuri Nesterenko wrote:

Hi Sergey,

fix looks OK to me.
We didn't see the original issue on Mac, did we?


It was fixed long time ago before osx-port in jdk6. So on OSX, I just 
tries to check something similar to what we do on other platforms 
instead of excluding the test for OSX.




Thanks,
-yan

On 02/28/2016 10:33 PM, Sergey Bylokhov wrote:

Hello,
Please review the small fix for jdk9.

The test tries to use Alt+F4 but it is meaningless on OSX.
- On OSX Ctrl+f4 (cycle between open windows in all applications) now is
used, to simulate the native focus lost.
- cleanup.

Bug: https://bugs.openjdk.java.net/browse/JDK-8144164
Webrev can be found at:
http://cr.openjdk.java.net/~serb/8144164/webrev.01






--
Best regards, Sergey.

Re: [8] RFR for 8150258: [TEST] HiDPI: create a test for multiresolution menu items icons

[Alexander Stepanov]

Please see JDK-8150844

On 2/29/2016 6:52 PM, Sergey Bylokhov wrote:

On 29.02.16 18:45, Alexander Stepanov wrote:

not sure if "-Dsun.java2d.uiScale" option was intended for use with OS X
(probably an enhancement bug should be created).

So for OS X now we have only single test run in fact.


I do not remember that such request was filed, please file a separate 
enhancement.

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

[Sergey Bylokhov]

The fix looks fine. Small notes about the test:
 - Probably it will be better to increase the height of the instruction 
to eliminate the vertical scroll?

 - The typo: 67 "Mouse press on te TextField text",

On 11.02.16 12:23, 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




--
Best regards, Sergey.

Re: Review Request for 6211389: Window.getMostRecentFocusOwner() returns null

[Semyon Sadetsky]

Hi Prem,

Component cannot become a global focus owner before it receives the 
focus gained event.

Did you try to run focus reg tests with your fix?

--Semyon

On 2/29/2016 10:03 AM, Prem Balakrishnan wrote:


Hi*,*

Please review fix for JDK9,

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

*Webrev:*http://cr.openjdk.java.net/~arapte/prem/6211389/webrev.00/ 



*Issue:*

Window.getMostRecentFocusOwner() returns null

*Cause:*

focusowner is NOT set when window gained Focused.

*Fix:*

focusowner is SET when window gained focus.

Fix is tested across all supported platforms(Windows, Linux and Mac).

Regards,
Prem