[8] Request for review: 8009012 [macosx] DisplayChangedListener is not implemented in LWWindowPeer/CGraphicsEnvironment (original) (raw)

Sergey Bylokhov Sergey.Bylokhov at oracle.com
Fri Apr 19 03:49:55 PDT 2013

Thanks for review. I'll add additional comment.

On 4/19/13 2:39 PM, Anthony Petrov wrote:

Thanks for the explanation. Finally I understand why you need to use this pattern.

Since this is an exceptional use, could you please add comments (just above the "performOnMainThreadWaiting:[NSThread isMainThread]" lines) to indicate why we do it this way? This would resolve my concern. -- best regards, Anthony On 04/19/2013 02:26 PM, Sergey Bylokhov wrote: On 4/19/13 2:11 PM, Anthony Petrov wrote:

I understand that. However, having methods producing different immediate results depending on what thread they are called on doesn't seem like a good design. It would be best to define explicit threading semantics where possible. We already have it. We always call all setXX methods asynchronously, exception is only in this method where I need to change native texture size and layer's scale in one call on appkit, otherwise I'll get window's contents blinking, during screen-2-screen moving.

Since this task isn't directly related to 8009012 and may require a lot of code refactoring actually, I suggest to file a separate issue and evaluate it later, perhaps in a future release. -- best regards, Anthony On 04/18/2013 06:45 PM, Sergey Bylokhov wrote: On 4/18/13 6:13 PM, Anthony Petrov wrote: I see. But this still looks fishy. Could you file a separate issue to investigate the replaceSurfaceData() calls and see if we can redispatch them all to the AppKit thread, thus clarifying the contract for the nativeSetScale as well? But this leads to the situation which I try to eliminate - never call synchronously methods on a appkit, if possible.

The fix looks good otherwise. -- best regards, Anthony On 04/18/13 18:06, Sergey Bylokhov wrote: Hi, Anthony. Currently this method is called from the replaceSurfaceData, which usually is called on the appkit, but can be called on a other thread. So this change don't block other thread. If it was called on appkit it completes synchronously, or asynchronously on another thread.

On 4/18/13 5:54 PM, Anthony Petrov wrote: 219 [ThreadUtilities performOnMainThreadWaiting:[NSThread isMainThread] block:^(){

That's an interesting pattern, but it looks more like a workaround to me, because the contract defined by the nativeSetScale is unclear with this implementation. Where do we call this method from? Can we ensure all calls always occur on a specific thread? -- best regards, Anthony On 04/17/13 13:42, Sergey Bylokhov wrote: Hello, Please review the fix for jdk 8. displayChanged() was added to LWWindowPeer and CGraphicsDevice. Note that I move out unnecessary calls to the appkit thread(in CGLayer.nativeSetScale() and CGraphicsDevice.getX), because it can cause a deadlock, when called from the non-appkit thread.

Bug: http://bugs.sun.com/bugdatabase/viewbug.do?bugid=8009012 Webrev can be found at: http://cr.openjdk.java.net/~serb/8009012/webrev.00 -- Best regards, Sergey.

-- Best regards, Sergey.

More information about the awt-dev mailing list