<AWT Dev> [8] Request for review: 8009012 [macosx] DisplayChangedListener is not implemented in LWWindowPeer/CGraphicsEnvironment

[Sergey Bylokhov]

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/view_bug.do?bug_id=8009012
>>>>>>>> Webrev can be found at:
>>>>>>>> http://cr.openjdk.java.net/~serb/8009012/webrev.00
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> Best regards, Sergey.
>>>>>>>>
>>>>>>
>>>>>>
>>>>
>>>>
>>
>>


-- 
Best regards, Sergey.