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.
>>
