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.



Reply via email to