<AWT Dev> [8] Request for review: 7193214 Consider simplifying CPlatformWindow.setResizable()

[Sergey Bylokhov]

I can file separate CR to ,move this logic to the java lvl, cache it if 
needed and convert native methods to the simple setters?

09.11.2012 15:00, Denis S. Fokin wrote:
> Hi Sergey,
>
> The fix looks fine. But I would slightly change the implementation of 
> the method.
>
> Instead of
>
> jint newBits = window.styleBits & ~mask | bits & mask;
>
> I would use
>
> Java_sun_lwawt_macosx_CPlatformWindow_nativeSetNSWindowStyleBits
> (JNIEnv *env, jclass clazz, jlong windowPtr, jint mask, jboolean value)
>
> // ....
> // ....
> // ....
>
> jint newBits = (value) ? (window.styleBits | mask) : (window.styleBits 
> & ~mask);
>
> Because, now we are trying to encode the boolean value in java and 
> decode it in the native code. It is the only purpose of the 'bits' 
> variable. It would be better to propagate the boolean value to the 
> native code.
>
> I am not sure about window.styleBits value. If it cannot be changed by 
> the system. I would cache it in java and do all the bitwise operations 
> on the java level.
>
>
> Thank you,
>    Denis.
>
> On 11/7/2012 8:03 PM, Sergey Bylokhov wrote:
>> Hi, Anthony.
>> Since this check is not necessary for the fix, I just drop it.
>> Updated webrev:
>> http://cr.openjdk.java.net/~serb/7193214/webrev.01/
>> GUI jck tests passed.
>>
>> 06.11.2012 19:34, Anthony Petrov wrote:
>>> Hi Sergey,
>>>
>>> Generally I'm fine with nativeSetNSWindowStyleBits updating the bits
>>> only when they've changed. Did you run AWT regression tests for
>>> top-level areas (both open and closed) to make sure there's no
>>> regressions associated with this change? Shouldn't we force
>>> reinstalling some style bits when e.g. a window gets hidden and then
>>> subsequently shown again?
>>>
>>> -- 
>>> best regards,
>>> Anthony
>>>
>>> On 11/6/2012 6:54 PM, Sergey Bylokhov wrote:
>>>>
>>>> Hi Everyone,
>>>> Please review the fix.
>>>> In the fix, unnecessary calls were removed from setResizable(), also
>>>> now we take into account zoom state in the setResizable() + plus
>>>> small cleanup;
>>>>
>>>> Bug: http://bugs.sun.com/bugdatabase/view_bug.do?bug_id=7193214
>>>> Webrev can be found at:
>>>> http://cr.openjdk.java.net/~serb/7193214/webrev.00
>>>>
>>>> -- 
>>>> Best regards, Sergey.
>>
>>
>


-- 
Best regards, Sergey.