Re: [8] Review Request for JDK-8015477 Support single threaded AWT/FX mode.

Anthony Petrov Fri, 31 May 2013 03:04:21 -0700

Looks great now. Thanks!

--
best regards,
Anthony


On 05/31/2013 10:08 AM, Petr Pchelko wrote:

Hello, Anthony.

Oh, sorry, I've missed that comment. Updated the fix:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.02/

With best regards. Petr.

On May 30, 2013, at 6:26 PM, Anthony Petrov wrote:

Hi Petr,

Thanks for all the changes. Looks much better, indeed. However, there still 
seems to be an issue with using inAWT in LWCToolkit.m and with the ?: operator. 
Please see the quote below for details.

--
best regards,
Anthony

On 05/30/13 13:19, Petr Pchelko wrote:

Hello, Thank you for the reviews.

I have updated the fix, it is now available at:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.01/
And the fx part:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.rt.01/

I have updated comments and used AWTAccessor.

Anthony wrote:

Regarding the FX part, I'm a bit uncertain if it's a good idea to perform so 
many actions with undefined semantics. I mean, the SwingFXUtils.runOnFxThread() 
either executes the runnable here and now (blocking the calling code), or just 
schedules the execution for later time. Sometimes this indeed makes sense. But 
in some cases this may be the wrong thing to do because e.g. we may want to 
block always. I believe we should mention this explicitly in the spec for this 
method, so that people know what they do when they use this method. Anyway, as 
long as this code is experimental and is off by default I'm OK with it.

This method was added as an optimization so that we do not post a runnable if 
we are already on the needed thread. Currently it does not make any trouble as 
I have tested, but it is possible to remove it if we get any problems with it 
any time later. I have reduced it's visibility as it should only be used 
internally.

With best regards. Petr.

On May 29, 2013, at 6:54 PM, Anthony Petrov wrote:

Hi Petr,

src/macosx/classes/sun/lwawt/macosx/LWCToolkit.java

  88     /* If we operate in FX/AWT single threaded mode the nested loop should 
run in default mode */
  89     private static final boolean inAWT;


src/macosx/native/sun/awt/LWCToolkit.m

314         isRunning = [[NSRunLoop currentRunLoop] runMode:inAWT ? [JNFRunLoop 
javaRunLoopMode] : NSDefaultRunLoopMode


If I read the comment above right, then we should swap the modes in ?:, no? 
Also, since ':' has a special meaning in ObjC, I suggest to enclose the ?: 
expression in parentheses for better readability.

src/share/classes/sun/awt/FwDispatcher.java

   2  * Copyright (c) 2011, 2013, Oracle and/or its affiliates. All rights 
reserved.


Is it true that this code exists since 2011?

41  * @since 8.0


We use "1.8" to indicate that an API is available since JDK8.

Regarding the FX part, I'm a bit uncertain if it's a good idea to perform so 
many actions with undefined semantics. I mean, the SwingFXUtils.runOnFxThread() 
either executes the runnable here and now (blocking the calling code), or just 
schedules the execution for later time. Sometimes this indeed makes sense. But 
in some cases this may be the wrong thing to do because e.g. we may want to 
block always. I believe we should mention this explicitly in the spec for this 
method, so that people know what they do when they use this method. Anyway, as 
long as this code is experimental and is off by default I'm OK with it.

--
best regards,
Anthony

On 05/28/2013 01:28 PM, Petr Pchelko wrote:

Hello, AWT Team.

Please review the fix for the issue:
http://bugs.sun.com/view_bug.do?bug_id=8015477
The fix is available at:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.00/

The issue is just created, so it might not show up on bugs.sun.com

This fix adds a private API to support a single threaded mode for AWT/FX 
interop.
The main idea is to delegate some methods of an EventQueue to FX, so that AWT 
events were processed on FX thread.

Here's the webrev for FX part of changes, for a usage sample:
http://cr.openjdk.java.net/~pchelko/8015477/webrev.rt/

With best regards. Petr.

Re: [8] Review Request for JDK-8015477 Support single threaded AWT/FX mode.

Reply via email to