<AWT Dev> [9] Review Request: 7124365 [macosx] setMaximizedBounds() should be implemented

[Anthony Petrov]

On 4/16/2014 4:14 PM, Petr Pchelko wrote:
>>> 125 @synthesize preFullScreenLevel;
>>> 126 @synthesize javaMaximizedBounds = _javaMaximizedBounds;
>>
>> The style of instantiation of the javaMaximizedBounds property differs from a bunch of other properties.
>> Is there any particular reason for that? Do we need to change other @synthesize directives above this one as well?
>
> The new style is kind of better, because do not need to declare the instance cariable - it's created for you.
> With an old pattern you had to declare it in the .h file. I can replace this with and old style for consistency.

I'm fine with either style as long as our code uses it everywhere 
consistently.


>>> 1033         if (y >= 0) {
>>
>> The specification of Frame.setMaximizedBounds() allows a developer to provide some of the fixed parameters only (e.g. the width and the x coordinate),
>> leaving others set to their default, system-provided values. So the condition you're checking here seems incorrect.
> According to the setMaximizedBounds docs the user needs to set Integer.MAX_VALUE to leave a particular coordinate default.
> In LWWindowPeer I replace the Integer.MAX_VALUE with -1, because the MAX_VALUE is not that reliable in native environment.
> Cocoa has NSUIntegerMax constant, but it is be different in 32bit/64bit, NSRect coords is stored in a float and using huge values
> there could lead to unpredictable bugs because of rounding. This condition is added because we do not want to flip the y coord
> if it's -1 (which means not set).

Right. Thanks for the clarification.


>>> 255     self.javaMaximizedBounds = NSMakeRect(-1, -1, -1, -1);
>>
>> In a multi-screen environment, could -1 indicate a valid value for a coordinate that user code might actually want to use?
>  From Apple's documentation the return rectangle is clipped to fit the current screen of the window. So no, it can not.
> Even if the user will set negative values the will be clipped anyway and it will not work, so I think we need to explicitly
> threat all negatives as "Not Set". I'll update the fix.

I don't believe so. Try configuring a secondary screen on the left from 
the main screen. The secondary screen will have negative coordinates 
then, and you could position you window at, say, (-200, 100), which is 
perfectly valid, no? The same is true for (-1, 100) then, too.

--
best regards,
Anthony

>
>
> On 16.04.2014, at 15:56, Anthony Petrov <anthony.petrov at oracle.com> wrote:
>
>> Hi Petr,
>>
>> src/macosx/native/sun/awt/AWTWindow.m
>>> 125 @synthesize preFullScreenLevel;
>>> 126 @synthesize javaMaximizedBounds = _javaMaximizedBounds;
>>
>> The style of instantiation of the javaMaximizedBounds property differs from a bunch of other properties. Is there any particular reason for that? Do we need to change other @synthesize directives above this one as well?
>>
>>
>>> 255     self.javaMaximizedBounds = NSMakeRect(-1, -1, -1, -1);
>>
>> In a multi-screen environment, could -1 indicate a valid value for a coordinate that user code might actually want to use?
>>
>>
>>> 1033         if (y >= 0) {
>>
>> The specification of Frame.setMaximizedBounds() allows a developer to provide some of the fixed parameters only (e.g. the width and the x coordinate), leaving others set to their default, system-provided values. So the condition you're checking here seems incorrect.
>>
>> --
>> best regards,
>> Anthony
>>
>> On 4/15/2014 4:33 PM, Petr Pchelko wrote:
>>> Hello, AWT Team.
>>>
>>> Please review the fix for the issue:
>>> https://bugs.openjdk.java.net/browse/JDK-7124365
>>> The fix is available at:
>>> http://cr.openjdk.java.net/~pchelko/9/7124365/webrev/
>>>
>>> The implementation is pretty straightforward.
>>> In native -1 indicates that this bound is not set. Using Integer.MAX_VALUE as in Frame’s javadoc isn’t convenient in native code.
>>> We do not need to worry about bounds that do not fit the screen - Cocoa handles it for us.
>>> But we need to handle the maximum/minimum sizes manually, because these are applied before the willUseStandardFrame callback is called.
>>> The regression test for this exists, it’s being moved from SQE right now. I’ll modify it to support Mac as soon as it’s pushed.
>>>
>>> Thank you.
>>> With best regards. Petr.
>>>
>