Hello, Sergey. > Is it possible in LWMouseInfoPeer that windowPeer == null? Yes, it's possible. Thank you for this catch, updated the webrev: http://cr.openjdk.java.net/~pchelko/9/8035568/webrev.02/
With best regards. Petr. On 17 июля 2014 г., at 16:28, Sergey Bylokhov <[email protected]> wrote: > Hi, Petr. > Is it possible in LWMouseInfoPeer that windowPeer == null? > > On 7/17/14 4:03 PM, Petr Pchelko wrote: >> Hello, >> >> Thank you for the review, the new version is here: >> http://cr.openjdk.java.net/~pchelko/9/8035568/webrev.01/ >> >> Mike wrote: >>> Has this change been tested on Retina? I don't see any effort to convert in >>> and out of the screen coordinate system. >> Yes, it works fine on Retina. Where should we convert the coords? >> CGEventGetLocation gives us coors in logical pixels and that's exactly what >> we need. Am I missing something? >> >> Anthony wrote: >>> 1. src/macosx/classes/sun/lwawt/LWMouseInfoPeer.java >>>> 55 // Most likely the cached window under cursor is correct and >>>> we do not need the native check. >>> Perhaps instead it would make sense to describe here when the first >>> condition may fail and the native check could actually become useful? >>> Otherwise the current comment doesn't add much value to understanding the >>> code. >> Done. I've removed the non-native check as it might be wrong. >> >>> . src/macosx/native/sun/awt/AWTWindow.m >>>> - AWT_ASSERT_APPKIT_THREAD; >>>> >>>> + [ThreadUtilities performOnMainThreadWaiting:YES block:^{ >>> This looks okay. But I'm wondering whether this could cause any dead locks >>> potentially? I'd suggest to run other tests that may involve (maybe >>> indirectly) calling the nativeGetTopmostPlatformWindowUnderMouse method >>> (grab/ungrab? focus? modal dialogs? tooltips/popups? maybe something else). >> Normally nativeGetTopmostPlatformWindowUnderMouse is called on the Appkit >> thread. It's called from EDT only in isWindowUnderMouse function, and it's >> not used too much in out code. >> >>> 3. src/macosx/native/sun/awt/CCursorManager.m >>>> - [ThreadUtilities performOnMainThreadWaiting:YES block:^(){ >>> Is it OK to call Core Graphics functions on a thread other than the main >>> thread? >> According to Apple, yes: "Quartz is thread safe on the whole, but individual >> Quartz objects are not. In general, you can operate on any object on any >> thread as long as you guarantee that no two threads are operating on the >> same object simultaneously. The easiest way to achieve this is to not share >> your objects between threads." >> >> With best regards. Petr. >> >> On 11 июля 2014 г., at 15:15, Anthony Petrov <[email protected]> >> wrote: >> >>> Hi Petr, >>> >>> The fix looks good to me overall. A few comments: >>> >>> 1. src/macosx/classes/sun/lwawt/LWMouseInfoPeer.java >>>> 55 // Most likely the cached window under cursor is correct and >>>> we do not need the native check. >>> Perhaps instead it would make sense to describe here when the first >>> condition may fail and the native check could actually become useful? >>> Otherwise the current comment doesn't add much value to understanding the >>> code. >>> >>> >>> 2. src/macosx/native/sun/awt/AWTWindow.m >>>> - AWT_ASSERT_APPKIT_THREAD; >>>> >>>> + [ThreadUtilities performOnMainThreadWaiting:YES block:^{ >>> This looks okay. But I'm wondering whether this could cause any dead locks >>> potentially? I'd suggest to run other tests that may involve (maybe >>> indirectly) calling the nativeGetTopmostPlatformWindowUnderMouse method >>> (grab/ungrab? focus? modal dialogs? tooltips/popups? maybe something else). >>> >>> >>> 3. src/macosx/native/sun/awt/CCursorManager.m >>>> - [ThreadUtilities performOnMainThreadWaiting:YES block:^(){ >>> Is it OK to call Core Graphics functions on a thread other than the main >>> thread? >>> >>> -- >>> best regards, >>> Anthony >>> >>> On 7/8/2014 2:19 PM, Petr Pchelko wrote: >>>> Hello, AWT Team. >>>> >>>> Please review the fix for the issue: >>>> https://bugs.openjdk.java.net/browse/JDK-8035568 >>>> The fix is available at: >>>> http://cr.openjdk.java.net/~pchelko/9/8035568/webrev.00/ >>>> >>>> We are using 2 different methods for getting a cursor position in Robot >>>> and in LWAWT. I've changed our implementation to use Carbon method. >>>> nativeGetCursorPosition is a very hot method and changing it's >>>> implementation makes it run 35 times faster. Also we will never deadlock >>>> on it any more. >>>> However, I needed to change the isWindowUnderMouse implementation. The >>>> problem's that LWWindowPeer.windowUnderCursor is updated on mouse events >>>> and generated mouse events, so sometimes it may be not updated when called >>>> from a component resize handler. Luckily we can test it using native code. >>>> isWindowUnderMouse is not a hot method at all, in real apps it's called >>>> very rarely (never called after a couple of hours of real IDE usage) so >>>> it's not a problem that it >>>> runs slower now. >>>> >>>> I've run all cursor/mouse tests. A couple of tests failed because they >>>> didn't have proper synchronization and we are too fast for them now. I've >>>> fixed it and open-sourced the tests. >>>> >>>> With best regards. Petr. >>>> > > > -- > Best regards, Sergey.
