The fix looks ok. A separate Cr seems a good idea to me.
Thank you,
Denis.
09.11.2012, в 16:23, Sergey Bylokhov <[email protected]> написал(а):
> 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.
>
