Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Mon, 3 Jun 2024 06:07:37 GMT, Prasanta Sadhukhan
wrote:
>> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:
>>
>>> 1029: (p.x >= (frame.origin.x + contentRect.size.width -
>>> 3)) ||
>>> 1030: (fabs(frame.origin.x - p.x) < 3) ||
>>> 1031: (fabs(frame.origin.y - p.y) < 3)) {
>>
>> Does `fabs` use float? It makes code more compact but it may be imprecise
>> and less performant than integer arithmetics.
>
> Yes..https://man7.org/linux/man-pages/man3/fabs.3.html and [CGFLoat
> ](https://developer.apple.com/documentation/corefoundation/cgfloat?language=objc)
> can be double/float and fabs can accomodate both, in this case it uses
> double...We have tested on 3 different systems without any issue so would
> keep it for now until we find any adverse effect and also it is better to
> accommodate double in hi-dpi environment which is the case in OSX
I see, the coordinates are floats, then it's perfectly fine. I assumed the
coordinates were integers.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1625831262
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 14:49:45 GMT, Alexey Ivanov wrote:
>> Prasanta Sadhukhan has updated the pull request incrementally with one
>> additional commit since the last revision:
>>
>> Non-client aread edge area modified, test format
>
> src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:
>
>> 1029: (p.x >= (frame.origin.x + contentRect.size.width - 3))
>> ||
>> 1030: (fabs(frame.origin.x - p.x) < 3) ||
>> 1031: (fabs(frame.origin.y - p.y) < 3)) {
>
> Does `fabs` use float? It makes code more compact but it may be imprecise and
> less performant than integer arithmetics.
Yes..https://man7.org/linux/man-pages/man3/fabs.3.html and [CGFLoat
](https://developer.apple.com/documentation/corefoundation/cgfloat?language=objc)
can be double/float and fabs can accomodate both, in this case it uses
double...We have tested on 3 different systems without any issue so would keep
it for now until we find any adverse effect..
> test/jdk/javax/swing/JMenu/TestUngrab.java line 101:
>
>> 99: robot.delay(1000);
>> 100: System.out.println("isPopupMenuVisible " +
>> menu.isPopupMenuVisible());
>> 101: if (menu.isPopupMenuVisible()) {
>
> `menu.isPopupMenuVisible()` should also be accessed on EDT.
Yes...done..
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1623826880
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1623825295
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 12:24:42 GMT, Prasanta Sadhukhan wrote: >> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is >> resized from the lower right corner, then the Menu / JPopupmenu stays open >> unlike in native osx apps like Notes, Mail etc.. >> >> This is because when LMouseButton is pressed on non-client area, the window >> should get a UngrabEvent for it to close/cancel the popupmenu (as is done in >> [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) >> but it was seen that the mac AWTWindow code only recognizes the title-bar >> as the non-client area so >> [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) >> is not called. >> Fix is made to recognize the edges of the window as non-client area > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Non-client aread edge area modified, test format Marked as reviewed by abdelhak-za...@github.com (no known OpenJDK username). - PR Review: https://git.openjdk.org/jdk/pull/19474#pullrequestreview-2092280069
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 12:24:42 GMT, Prasanta Sadhukhan
wrote:
>> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is
>> resized from the lower right corner, then the Menu / JPopupmenu stays open
>> unlike in native osx apps like Notes, Mail etc..
>>
>> This is because when LMouseButton is pressed on non-client area, the window
>> should get a UngrabEvent for it to close/cancel the popupmenu (as is done in
>> [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620))
>> but it was seen that the mac AWTWindow code only recognizes the title-bar
>> as the non-client area so
>> [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804)
>> is not called.
>> Fix is made to recognize the edges of the window as non-client area
>
> Prasanta Sadhukhan has updated the pull request incrementally with one
> additional commit since the last revision:
>
> Non-client aread edge area modified, test format
src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1031:
> 1029: (p.x >= (frame.origin.x + contentRect.size.width - 3))
> ||
> 1030: (fabs(frame.origin.x - p.x) < 3) ||
> 1031: (fabs(frame.origin.y - p.y) < 3)) {
Does `fabs` use float? It makes code more compact but it may be imprecise and
less performant than integer arithmetics.
test/jdk/javax/swing/JMenu/TestUngrab.java line 82:
> 80: robot.waitForIdle();
> 81: robot.delay(1000);
> 82: SwingUtilities.invokeAndWait(() -> {
Suggestion:
robot.delay(1000);
SwingUtilities.invokeAndWait(() -> {
Adding blank lines between sections of code could improve its readability.
test/jdk/javax/swing/JMenu/TestUngrab.java line 101:
> 99: robot.delay(1000);
> 100: System.out.println("isPopupMenuVisible " +
> menu.isPopupMenuVisible());
> 101: if (menu.isPopupMenuVisible()) {
`menu.isPopupMenuVisible()` should also be accessed on EDT.
-
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622540182
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622542519
PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622533420
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 12:24:42 GMT, Prasanta Sadhukhan wrote: >> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is >> resized from the lower right corner, then the Menu / JPopupmenu stays open >> unlike in native osx apps like Notes, Mail etc.. >> >> This is because when LMouseButton is pressed on non-client area, the window >> should get a UngrabEvent for it to close/cancel the popupmenu (as is done in >> [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) >> but it was seen that the mac AWTWindow code only recognizes the title-bar >> as the non-client area so >> [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) >> is not called. >> Fix is made to recognize the edges of the window as non-client area > > Prasanta Sadhukhan has updated the pull request incrementally with one > additional commit since the last revision: > > Non-client aread edge area modified, test format Marked as reviewed by azvegint (Reviewer). - PR Review: https://git.openjdk.org/jdk/pull/19474#pullrequestreview-2090723235
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 10:51:29 GMT, Abhishek Kumar wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Non-client aread edge area modified, test format > > test/jdk/javax/swing/JMenu/TestUngrab.java line 81: > >> 79: robot.delay(1000); >> 80: Point point = menu.getLocationOnScreen(); >> 81: Dimension dim = menu.getSize(); > > Should it be accessed on EDT? Yes, updated - PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622344582
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
On Fri, 31 May 2024 09:55:37 GMT, Alexander Zvegintsev wrote: >> Prasanta Sadhukhan has updated the pull request incrementally with one >> additional commit since the last revision: >> >> Non-client aread edge area modified, test format > > src/java.desktop/macosx/native/libawt_lwawt/awt/AWTWindow.m line 1029: > >> 1027: // Also, non-client area includes the edges at left, right >> and botton of frame >> 1028: if ((p.y >= (frame.origin.y + contentRect.size.height)) || >> 1029: (p.x >= (frame.origin.x + contentRect.size.width)) || > > Suggestion: > > (p.x >= (frame.origin.x + contentRect.size.width - 3)) || > > > From my testing there is a 3 pixel dead zone at the right window border, when > the resize cursor is displayed, but `deliverNCMouseDown` is not called. > > Please check this on your system as well. Yes, there was a dead zone..Thanks for pointing it out...Updated.. - PR Review Comment: https://git.openjdk.org/jdk/pull/19474#discussion_r1622344303
Re: RFR: 8325435: [macos] Menu or JPopupMenu not closed when main window is resized [v2]
> Issue is in macosx, when a JMenu or JPopupmenu is opened and then window is > resized from the lower right corner, then the Menu / JPopupmenu stays open > unlike in native osx apps like Notes, Mail etc.. > > This is because when LMouseButton is pressed on non-client area, the window > should get a UngrabEvent for it to close/cancel the popupmenu (as is done in > [windows](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/windows/native/libawt/windows/awt_Frame.cpp#L618-L620)) > but it was seen that the mac AWTWindow code only recognizes the title-bar as > the non-client area so > [notifyNCMouseDown](https://github.com/openjdk/jdk/blob/f608918df3f887277845db383cf07b0863bba615/src/java.desktop/macosx/classes/sun/lwawt/LWWindowPeer.java#L797-L804) > is not called. > Fix is made to recognize the edges of the window as non-client area Prasanta Sadhukhan has updated the pull request incrementally with one additional commit since the last revision: Non-client aread edge area modified, test format - Changes: - all: https://git.openjdk.org/jdk/pull/19474/files - new: https://git.openjdk.org/jdk/pull/19474/files/4b22432b..d2f9256b Webrevs: - full: https://webrevs.openjdk.org/?repo=jdk&pr=19474&range=01 - incr: https://webrevs.openjdk.org/?repo=jdk&pr=19474&range=00-01 Stats: 17 lines in 2 files changed: 8 ins; 0 del; 9 mod Patch: https://git.openjdk.org/jdk/pull/19474.diff Fetch: git fetch https://git.openjdk.org/jdk.git pull/19474/head:pull/19474 PR: https://git.openjdk.org/jdk/pull/19474
