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.
