[8] Review request for 7124513: [macosx] Support NSTexturedBackgroundWindowMask/different titlebar styles to create unified toolbar

[Anthony Petrov]

Thanks. The fix looks fine to me.

--
best regards,
Anthony

On 7/4/2012 2:51 PM, Sergey Bylokhov wrote:
> Hi, Anthony.
> Please check new version:
> http://cr.openjdk.java.net/~serb/7124513/webrev.01/
> 
> 04.07.2012 14:14, Anthony Petrov wrote:
>> On 7/4/2012 2:07 PM, Sergey Bylokhov wrote:
>>>> src/macosx/classes/sun/lwawt/macosx/CPlatformWindow.java
>>>>>          // If the target is a dialog, popup or tooltip we want it 
>>>>> to ignore the brushed metal look.
>>>>>          if (isPopup) {
>>>>> -            styleBits = SET(styleBits, TEXTURED, true);
>>>>> +            styleBits = SET(styleBits, TEXTURED, false);
>>>>
>>>> Given the change, does the comment above if() still apply to this code? 
>>> Well I just change the code according the comments. This code means 
>>> that we ignore this property if it was set.
>>> styleBits = SET(styleBits, TEXTURED, false);
>>
>> Thanks for clarifying that.
>>
>>
>>>> src/macosx/classes/sun/lwawt/LWWindowPeer.java
>>>>>  455     public final boolean isTranslucent() {
>>>>>  456         synchronized (getStateLock()) {
>>>>>  457             return !isOpaque || isShaped()|| isTextured();
>>>>
>>>> It is unclear why textured windows must be considered translucent. 
>>> Because textured window is a special case of translucent window. The 
>>> difference is only in nswindow background. So when we set texture 
>>> property our peer became fully translucent. It doesn't fill 
>>> background, create non opaque backbuffers and layer etc.
>>
>> Got it. Please add a comment to the code right above the line 457 with 
>> the explanation. Also, please insert a space in front of the last || 
>> operator for consistency.
>>
>> -- 
>> best regards,
>> Anthony
>>
>>>> Could you add a comment explaining that, or yet better, not do this 
>>>> change at all? After all, we can always insert explicit checks for 
>>>> isTextured() in places where it is really necessary. Otherwise I 
>>>> suggest to rename the isTranslucent() to something else, because 
>>>> textured doesn't really mean translucent.
>>>>
>>>> I'm not an expert in Swing/laf code, so I didn't review those parts.
>>>>
>>>> -- 
>>>> best regards,
>>>> Anthony
>>>>
>>>> On 7/2/2012 5:46 PM, Sergey Bylokhov wrote:
>>>>>
>>>>> Hi Everyone,
>>>>> Please review the fix.
>>>>> 1 When the property "apple.awt.brushMetalLook" is set we draw our 
>>>>> panels and toolbars with composite=SRc and alpha=0.
>>>>> 2 Changes in JViewport are needed, because copyArea does not handle 
>>>>> correctly content with alpha!=1.0.
>>>>>
>>>>>
>>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7124513
>>>>> Webrev can be found at: 
>>>>> http://cr.openjdk.java.net/~serb/7124513/webrev.00/
>>>>>
>>>>> Also note that this CR depends from CR 7124244.
>>>>>
>>>>> -- 
>>>>> Best regards, Sergey.
>>>>>
>>>
>>>
> 
>