[11]JDK-8160767 [TEST_BUG] java/awt/Frame/MaximizedToIconified/MaximizedToIconified.java

[Prem Balakrishnan]

Hi All,

 

Please Review fix for JDK 11: 

 

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

 

Webrev: http://cr.openjdk.java.net/~pkbalakr/8160767/webrev.00/

 

Test fails due to synchronization issue, state changes are validated 
immediately after calling setExtendedState(). 

Adding delay after setExtendedState() call and validating the state changes 
solves this issue.

 

Regards,

Prem

Re: [11] Review Request: JDK-8196435 Regression automated Test 'java/awt/Mouse/GetMousePositionTest/GetMousePositionWithOverlay.java' fails

[Prem Balakrishnan]

Hi Sergey,

The existing waitForIdle() call at line 55 is called after calling setVisible() 
for both the frames (which I have removed in proposed fix). I have added 
waitForIdle() call after calling setVisible() for each frame(at line 103 and 
110 in proposed fix), which ensures frontFrame is rendered on top of backFrame.

Regards,
Prem

-Original Message-
From: Sergey Bylokhov 
Sent: Friday, March 02, 2018 3:19 AM
To: Prem Balakrishnan ; awt-dev@openjdk.java.net
Subject: Re:  [11] Review Request: JDK-8196435 Regression automated 
Test 'java/awt/Mouse/GetMousePositionTest/GetMousePositionWithOverlay.java' 
fails

Hi, Prem.
On 26/02/2018 21:20, Prem Balakrishnan wrote:
> At times backFrame is rendered on top of frontFrame, due to which mouse 
> position on backFrame is not NULL and test fails, solved this by adding 
> waitForIdle() calls appropriately. Furthermore, mousemove from frontFrame to 
> backFrame, caused the cursor to change to resize cursor and test fails, 
> solved this by setting setResizable() to false.

But the old version of the test also call Util.waitForIdle(null) after frames 
were shown at line 55.


-- 
Best regards, Sergey.

Re: RFR: JDK-8198844 Clean up GensrcX11Wrappers

[Erik Joelsson]

Hello,

In xlibtypes.txt comment, should it be sizes-64.txt?

Generating both 32 and 64 seems a bit outdated at this point. Surely 
this is a remnant of bundling 64 and 32 bit together on Solaris in the 
past? Perhaps someone in 2d can answer this? Would be nice to be able to 
clean up that part as well if possible.


Otherwise good.

/Erik

On 2018-02-28 12:12, Magnus Ihse Bursie wrote:
My hunt on technical debt continues. This time my aim has been on the 
sorry mess known as GensrcX11Wrappers.


I've disentangled it into two functions, one simple gensrc part that 
is actually run during the build, and which involves just a simple 
java tool and some pre-calculated data files, and a separate step for 
updating those pre-calculated data files. This step is now run using 
"make update-x11wrappers". It involves compiling a native binary, and 
running it on the target platform, so just as before, this assumes 
that you are not cross-compiling.


I'm not sure what role the "verification" step we had before ever 
played. For all the years we've been "verifying" this, we've detected 
no differences. In fact, as far as I understand, what we *really* 
would need to verify against, is the X11 libraries on the runtime 
system, i.e. where the users executes the JRE. But then again, we can 
assume that this matches, just as anyone compiling with header files 
on one place can assume that they can use the libraries elsewhere. The 
only reason, as I see it, to keep the generation in the makefiles at 
all, is to document how the files were generated, and to be ready for 
the need to update the files if the list of datatypes in xlibtypes.txt 
changes.


Bug: https://bugs.openjdk.java.net/browse/JDK-8198844
WebRev: 
http://cr.openjdk.java.net/~ihse/JDK-8198844-clean-up-GensrcX11Wrappers/webrev.01


/Magnus

Re: [11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component

[Dmitry Markov]

Hi Anton,

The fix looks good, but I wonder if we also shall check results of 
isDisplayable() and isVisible() before showing touch keyboard. My assumption is 
the following: we cannot transfer focus to non-displayable or invisible 
components, so it is unnecessary to display touch keyboard for such components 
too.

Thanks,
Dmitry  

> On 1 Mar 2018, at 16:30, anton.litvi...@oracle.com wrote:
> 
> Hello,
> 
> Could you please review the following fix for the bug.
> 
> Bug: https://bugs.openjdk.java.net/browse/JDK-8198605
> Webrev: http://cr.openjdk.java.net/~alitvinov/8198605/jdk11/webrev.00
> 
> The fix extends a number of the required conditions in the method 
> "sun.awt.windows.WToolkit.showOrHideTouchKeyboard(Component, AWTEvent)", 
> which should be satisfied for showing of the touch keyboard, by checking if 
> the component is focusable or not.
> 
> Thank you,
> Anton

Re: [11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component

[Sergey Bylokhov]

Hi, Anton.
Looks fine.

ps: maybe we can create some rfe to enable Touch keyboard even if the 
system has some keyboards for the test purpose?


On 01/03/2018 08:30, anton.litvi...@oracle.com wrote:

Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198605
Webrev: http://cr.openjdk.java.net/~alitvinov/8198605/jdk11/webrev.00

The fix extends a number of the required conditions in the method 
"sun.awt.windows.WToolkit.showOrHideTouchKeyboard(Component, AWTEvent)", 
which should be satisfied for showing of the touch keyboard, by checking 
if the component is focusable or not.


Thank you,
Anton



--
Best regards, Sergey.

Re: [11] Review Request: JDK-8196435 Regression automated Test 'java/awt/Mouse/GetMousePositionTest/GetMousePositionWithOverlay.java' fails

[Sergey Bylokhov]

Hi, Prem.
On 26/02/2018 21:20, Prem Balakrishnan wrote:

At times backFrame is rendered on top of frontFrame, due to which mouse 
position on backFrame is not NULL and test fails, solved this by adding 
waitForIdle() calls appropriately. Furthermore, mousemove from frontFrame to 
backFrame, caused the cursor to change to resize cursor and test fails, solved 
this by setting setResizable() to false.


But the old version of the test also call Util.waitForIdle(null) after 
frames were shown at line 55.



--
Best regards, Sergey.

Re: [11] JDK-8196017: java/awt/Mouse/GetMousePositionTest/GetMousePositionWithPopup.java fails

[Sergey Bylokhov]

On 01/03/2018 02:41, Shashidhara Veerabhadraiah wrote:

Sergey, we have a floating point dpi scale(dpi value/96) and we use it to scale 
down and up to convert from user space to device space and vice versa. This 
code is present at awt_Win32GraphicsDevice.cpp. In this code if you see we use 
clipRound() with ceil() function. Since there float to int involved with ceil 
operation may cause + or - 1 pixel variation in it's value I think. Please 
correct me if I am wrong here.


The similar code is executed when we set the size of the frame/window, 
we convert size in the user space to device space, and we do the 
opposite when we request the size of the frame. Same with robot API, 
when we move the mouse to some point and request the location of cursor.

We should try to implement mouse events in the similar way.


Since there is an issue with windows 10 build 1703 and also the original fix is 
mac specific, I think my test fix to keep it specific to mac is the right 
decision to make. Please correct me if I am wrong here.

This test is not a mac specific, there is no any platforms specific code.


--
Best regards, Sergey.

Re: [11] JDK-8195738: scroll poistion in ScrollPane is reset after calling validate()

[shashidhara . veerabhadraiah]

Thank you Semyon for the review.


On 01/03/18 11:16 PM, semyon.sadet...@oracle.com wrote:


On 3/1/18 9:37 AM, semyon.sadet...@oracle.com wrote:


On 2/28/18 8:09 PM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, It is not getting decreased for the case of 
SCROLLBARS_NEVER. The Boolean flags “needsHorz” and “needsVert” are 
false for the never case and true for the other cases. We decrease 
by the size of the scroll bars only for the true cases, so in the 
false case we will use the entire width and height(SM_CXEDGE and 
SM_CYEDGE).


Can you clarify this a bit more? I see the next lines executed 
without any conditions


215 parentWidth -= (horzBorder * 2);
216 parentHeight -= (vertBorder * 2);

I was not right this is just borders.

Looks good to me.

--Semyon


--Semyon


Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Thursday, March 1, 2018 12:39 AM
*To:* Shashidhara Veerabhadraiah 
*Cc:* awt-dev@openjdk.java.net
*Subject:* Re:  [11] JDK-8195738: scroll poistion in 
ScrollPane is reset after calling validate()


Hi Sashi,

The parent with and height shouldn't be decreased by the size of the 
scrollbars in case of SCROLLBARS_NEVER.


--Semyon

On 2/27/2018 12:45 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your help. I did some debugging from that
perspective and could find the problem and the solution. Here is
the new webrev, please review it:

http://cr.openjdk.java.net/~sveerabhadra/8195738/webrev.01/


The problem was that in the case of SCROLLBARS_NEVER, scroll bar
info was set using setScrollInfo() with SIF_RANGE with ranges
being (0, 0). This forced the windows to pick a default value
and hence reset it to that value every time. Now the logic is
modified in a very similar fashion as the other scroll bar
display modes but without displaying it using the new win api
ShowScrollBar(). The scroll bar info is supplied with a proper
range per the corresponding base component but without the
scroll bar pane size(as we don’t display it). This should
suffice a good range for the scroll bars to operate and correct
itself if there is any out of range value is supplied.

Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Tuesday, February 27, 2018 1:22 AM
*To:* shashidhara veerabhadraiah


*Cc:* awt-dev@openjdk.java.net 
*Subject:* Re:  [11] JDK-8195738: scroll poistion in
ScrollPane is reset after calling validate()

The bug was failed against Windows platform. So, I'd try to find
the root cause why when the scroll is notified with the scrolled
component size the scroll position is set to the wrong value.

--Semyon

On 02/26/2018 09:51 AM, shashidhara veerabhadraiah wrote:

I think I agree Semyon. Thanks for your detailed explanation.

So how do we approach on this? I see a different behaviour
on other platforms. Shouldn’t we have a common behaviour
across platforms? I agree with your analysis and hence
should we change the behaviour on other platforms so that
all of them will have the same behaviour?

Thanks and regards,

Shashi

On 26-Feb-2018, at 10:17 PM, Semyon Sadetsky
mailto:semyon.sadet...@oracle.com>> wrote:

On 02/24/2018 05:27 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your review and these
questions help us to reach the right requirements.

Without this change, there is different behavior on
windows compared to Mac/Linux platforms. Hence this
change is required to make the behavior same across
the platforms. Since the scroll pane control is not
displayed for the SCROLLBARS_NEVER case and if not
in the setScrollPosition() then how can the user can
move to a different position? Since we used to
update/recalculate the scroll pane geometry caused
changes to move to a different position other than
the setScrollPosition()!! This causes
SCROLLBARS_NEVER mode as unusable as the user is
unable to control the scrolling movement because
neither he can set one setScrollPosition() nor the
control is displayed to explicitly set the movement.
Hope this answers your question.

You need to think once again about two facts I brought
in my previous message.

1. User still may scroll using mouse wheel even when
scrollbars are not visible.

2. Visibility of scrollbars doesn't affect the
requirement to notify the scroll about the scrolled
compone

Re: [11] JDK-8195738: scroll poistion in ScrollPane is reset after calling validate()

[semyon . sadetsky]

On 3/1/18 9:37 AM, semyon.sadet...@oracle.com wrote:


On 2/28/18 8:09 PM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, It is not getting decreased for the case of 
SCROLLBARS_NEVER. The Boolean flags “needsHorz” and “needsVert” are 
false for the never case and true for the other cases. We decrease by 
the size of the scroll bars only for the true cases, so in the false 
case we will use the entire width and height(SM_CXEDGE and SM_CYEDGE).


Can you clarify this a bit more? I see the next lines executed without 
any conditions


215 parentWidth -= (horzBorder * 2);
216 parentHeight -= (vertBorder * 2);

I was not right this is just borders.

Looks good to me.

--Semyon


--Semyon


Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Thursday, March 1, 2018 12:39 AM
*To:* Shashidhara Veerabhadraiah 
*Cc:* awt-dev@openjdk.java.net
*Subject:* Re:  [11] JDK-8195738: scroll poistion in 
ScrollPane is reset after calling validate()


Hi Sashi,

The parent with and height shouldn't be decreased by the size of the 
scrollbars in case of SCROLLBARS_NEVER.


--Semyon

On 2/27/2018 12:45 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your help. I did some debugging from that
perspective and could find the problem and the solution. Here is
the new webrev, please review it:

http://cr.openjdk.java.net/~sveerabhadra/8195738/webrev.01/


The problem was that in the case of SCROLLBARS_NEVER, scroll bar
info was set using setScrollInfo() with SIF_RANGE with ranges
being (0, 0). This forced the windows to pick a default value and
hence reset it to that value every time. Now the logic is
modified in a very similar fashion as the other scroll bar
display modes but without displaying it using the new win api
ShowScrollBar(). The scroll bar info is supplied with a proper
range per the corresponding base component but without the scroll
bar pane size(as we don’t display it). This should suffice a good
range for the scroll bars to operate and correct itself if there
is any out of range value is supplied.

Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Tuesday, February 27, 2018 1:22 AM
*To:* shashidhara veerabhadraiah


*Cc:* awt-dev@openjdk.java.net 
*Subject:* Re:  [11] JDK-8195738: scroll poistion in
ScrollPane is reset after calling validate()

The bug was failed against Windows platform. So, I'd try to find
the root cause why when the scroll is notified with the scrolled
component size the scroll position is set to the wrong value.

--Semyon

On 02/26/2018 09:51 AM, shashidhara veerabhadraiah wrote:

I think I agree Semyon. Thanks for your detailed explanation.

So how do we approach on this? I see a different behaviour on
other platforms. Shouldn’t we have a common behaviour across
platforms? I agree with your analysis and hence should we
change the behaviour on other platforms so that all of them
will have the same behaviour?

Thanks and regards,

Shashi

On 26-Feb-2018, at 10:17 PM, Semyon Sadetsky
mailto:semyon.sadet...@oracle.com>> wrote:

On 02/24/2018 05:27 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your review and these questions
help us to reach the right requirements.

Without this change, there is different behavior on
windows compared to Mac/Linux platforms. Hence this
change is required to make the behavior same across
the platforms. Since the scroll pane control is not
displayed for the SCROLLBARS_NEVER case and if not in
the setScrollPosition() then how can the user can
move to a different position? Since we used to
update/recalculate the scroll pane geometry caused
changes to move to a different position other than
the setScrollPosition()!! This causes
SCROLLBARS_NEVER mode as unusable as the user is
unable to control the scrolling movement because
neither he can set one setScrollPosition() nor the
control is displayed to explicitly set the movement.
Hope this answers your question.

You need to think once again about two facts I brought in
my previous message.

1. User still may scroll using mouse wheel even when
scrollbars are not visible.

2. Visibility of scrollbars doesn't affect the
requirement to notify the scroll about the scrolled
component size changes. If the scrolled component was
500x500 in size and its position in

Re: [11] JDK-8195738: scroll poistion in ScrollPane is reset after calling validate()

[semyon . sadetsky]

On 2/28/18 8:09 PM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, It is not getting decreased for the case of 
SCROLLBARS_NEVER. The Boolean flags “needsHorz” and “needsVert” are 
false for the never case and true for the other cases. We decrease by 
the size of the scroll bars only for the true cases, so in the false 
case we will use the entire width and height(SM_CXEDGE and SM_CYEDGE).


Can you clarify this a bit more? I see the next lines executed without 
any conditions


215 parentWidth -= (horzBorder * 2);
216 parentHeight -= (vertBorder * 2);

--Semyon


Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Thursday, March 1, 2018 12:39 AM
*To:* Shashidhara Veerabhadraiah 
*Cc:* awt-dev@openjdk.java.net
*Subject:* Re:  [11] JDK-8195738: scroll poistion in 
ScrollPane is reset after calling validate()


Hi Sashi,

The parent with and height shouldn't be decreased by the size of the 
scrollbars in case of SCROLLBARS_NEVER.


--Semyon

On 2/27/2018 12:45 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your help. I did some debugging from that
perspective and could find the problem and the solution. Here is
the new webrev, please review it:

http://cr.openjdk.java.net/~sveerabhadra/8195738/webrev.01/


The problem was that in the case of SCROLLBARS_NEVER, scroll bar
info was set using setScrollInfo() with SIF_RANGE with ranges
being (0, 0). This forced the windows to pick a default value and
hence reset it to that value every time. Now the logic is modified
in a very similar fashion as the other scroll bar display modes
but without displaying it using the new win api ShowScrollBar().
The scroll bar info is supplied with a proper range per the
corresponding base component but without the scroll bar pane
size(as we don’t display it). This should suffice a good range for
the scroll bars to operate and correct itself if there is any out
of range value is supplied.

Thanks and regards,
Shashi

*From:*Semyon Sadetsky
*Sent:* Tuesday, February 27, 2018 1:22 AM
*To:* shashidhara veerabhadraiah


*Cc:* awt-dev@openjdk.java.net 
*Subject:* Re:  [11] JDK-8195738: scroll poistion in
ScrollPane is reset after calling validate()

The bug was failed against Windows platform. So, I'd try to find
the root cause why when the scroll is notified with the scrolled
component size the scroll position is set to the wrong value.

--Semyon

On 02/26/2018 09:51 AM, shashidhara veerabhadraiah wrote:

I think I agree Semyon. Thanks for your detailed explanation.

So how do we approach on this? I see a different behaviour on
other platforms. Shouldn’t we have a common behaviour across
platforms? I agree with your analysis and hence should we
change the behaviour on other platforms so that all of them
will have the same behaviour?

Thanks and regards,

Shashi

On 26-Feb-2018, at 10:17 PM, Semyon Sadetsky
mailto:semyon.sadet...@oracle.com>> wrote:

On 02/24/2018 05:27 AM, Shashidhara Veerabhadraiah wrote:

Hi Semyon, Thanks for your review and these questions
help us to reach the right requirements.

Without this change, there is different behavior on
windows compared to Mac/Linux platforms. Hence this
change is required to make the behavior same across
the platforms. Since the scroll pane control is not
displayed for the SCROLLBARS_NEVER case and if not in
the setScrollPosition() then how can the user can move
to a different position? Since we used to
update/recalculate the scroll pane geometry caused
changes to move to a different position other than the
setScrollPosition()!! This causes SCROLLBARS_NEVER
mode as unusable as the user is unable to control the
scrolling movement because neither he can set one
setScrollPosition() nor the control is displayed to
explicitly set the movement. Hope this answers your
question.

You need to think once again about two facts I brought in
my previous message.

1. User still may scroll using mouse wheel even when
scrollbars are not visible.

2. Visibility of scrollbars doesn't affect the requirement
to notify the scroll about the scrolled component size
changes. If the scrolled component was 500x500 in size and
its position in the 200x200 viewport was (300,300) after
the size of the component is changed to 300x300 it may not
   

[11] Review request for 8198605: Touch keyboard is shown for a non-focusable text component

[anton . litvinov]

Hello,

Could you please review the following fix for the bug.

Bug: https://bugs.openjdk.java.net/browse/JDK-8198605
Webrev: http://cr.openjdk.java.net/~alitvinov/8198605/jdk11/webrev.00

The fix extends a number of the required conditions in the method 
"sun.awt.windows.WToolkit.showOrHideTouchKeyboard(Component, AWTEvent)", 
which should be satisfied for showing of the touch keyboard, by checking 
if the component is focusable or not.


Thank you,
Anton

Re: [11] JDK-8196017: java/awt/Mouse/GetMousePositionTest/GetMousePositionWithPopup.java fails

[Shashidhara Veerabhadraiah]

Hi All, I have verified this test by running on Krishna's machine which has 
windows 10 build 1703 and found that the test is falsely passing there as well. 
So we can safely say that Windows 10 build 1703 as a "fall creators" build.

Sergey, we have a floating point dpi scale(dpi value/96) and we use it to scale 
down and up to convert from user space to device space and vice versa. This 
code is present at awt_Win32GraphicsDevice.cpp. In this code if you see we use 
clipRound() with ceil() function. Since there float to int involved with ceil 
operation may cause + or - 1 pixel variation in it's value I think. Please 
correct me if I am wrong here.

Since there is an issue with windows 10 build 1703 and also the original fix is 
mac specific, I think my test fix to keep it specific to mac is the right 
decision to make. Please correct me if I am wrong here.

Thanks and regards,
Shashi

-Original Message-
From: Sergey Bylokhov 
Sent: Thursday, March 1, 2018 2:23 AM
To: Shashidhara Veerabhadraiah ; Krishna 
Addepalli ; awt-dev@openjdk.java.net
Subject: Re:  [11] JDK-8196017: 
java/awt/Mouse/GetMousePositionTest/GetMousePositionWithPopup.java fails

On 27/02/2018 07:51, Shashidhara Veerabhadraiah wrote:
> Threshold is added because I think it is quite possible especially with 
> hidpi(fractional) since convert back and forth from user space to device 
> coordinates which typically results in plus or minus 1 pixel 
> correction(because of division or multiplication). The bug link contains the 
> call stack where it is reported as (29, 29) as the location and hence the 
> test fails.

If we have an incorrect conversion to/from device space somewhere, then we 
should check the reason of this bug instead of adding some threshold to the 
test.



--
Best regards, Sergey.