<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