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