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

Denis S. Fokin denis.fokin at oracle.com
Fri Nov 9 03:00:24 PST 2012


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.
>
>




More information about the awt-dev mailing list