Review request for 7124554: [macosx] JWindow does ignore setAlwaysOnTop property

Anthony Petrov anthony.petrov at oracle.com
Tue Jan 24 06:39:07 PST 2012 

Hi Mike,

Thanks for proposing an alternative solution for this issue. I agree I 
find it more correct. Please take a look at the new fix at:

http://cr.openjdk.java.net/~anthony/x-6-alwaysOnTop.2/

Note that I decided to eliminate the changes from the 
CWrapper.NSWindow.addChildWindow() since the CWrapper is supposed to be 
a transparent proxy for Cocoa API w/o adding some logic to the wrapped 
methods. So instead I'm introducing CWrapper.NSWindow.setLevel() and 
using it where appropriate in the CPlatformWindow.

--
best regards,
Anthony

On 1/18/2012 3:36 AM, Mike Swingler wrote:
> On Jan 17, 2012, at 11:07 AM, Anthony Petrov wrote:
> 
>> Hi Mike,
>>
>> On 1/17/2012 8:52 PM, Mike Swingler wrote:
>>> On Jan 17, 2012, at 3:54 AM, Anthony Petrov wrote:
>>>> On 1/16/2012 8:24 PM, Mike Swingler wrote:
>>>>> I still don't understand why the list of hardcoded window levels needs to be included. Can't you just compare the window levels by their NSInteger numeric values?
>>>> There are two reasons for not using the numeric values directly, or comparing them directly:
>>>>
>>>> 1. Cocoa specification doesn't define the numeric values for the NS*WindowLevel constants. They may (theoretically) change in the future. As long as they aren't specified, we don't want to rely on their current values.
>>> Why do you have to know the values at all? Couldn't you just reset the child window level if it's different after the parent/child operation? After I re-read what you are trying to accomplish, even numeric comparison seems inappropriate.
>> I've already answered this question to Leonid on Jan 12, but I'll copy my reply here for your convenience:
>>
>> If the parent window is an always-on-top window, its level is NSFloatingWindowLevel. Suppose a child window being added to it hasn't been assigned any level explicitly, so its default level is NSNormalWindowLevel.
>>
>> Now, when we call -addChildWindow:, we really want to update the level
>> of the child window so that both the parent and the child share the same
>> window level. The if check ensures that we don't reset the level of the
>> child window back to normal in this case.
> 
> I think what you really want to do here is check if the parent and the child are Always-On-Top, and then reset the window level of the child only when the child is AOT and the parent is not. -[NSWindow addChildWindow:order:] forces the child to the same level as the parent already.
> 
> I'd avoid trying to infer if there is any "higher" vs. "lower" relationship between the parent based on the window level as seen from AppKit using that table.
> 
>>>> 2. There's a reference to the Quartz spec that defines the kCG*WindowLevel constants as a enum. The NS*WindowLevel macros are defined using these constants. However, if you take a look at the order of the constants in that enum [1] (and therefore their relative numerical order), you may notice that they don't exactly reflect the relative visual z-order between the levels as defined in the Cocoa spec [2]. E.g. kCGModalPanelWindowLevelKey is greater than kCGDockWindowLevelKey, while NSDockWindowLevel is defined being visually above the NSModalPanelWindowLevel. As such, a direct numeric comparison of window levels produces incorrect results.
>>> Have you tried setting something at the "dock" level? That level may have been true for the NextStep dock, but not the Mac. And BTW: the NSDockWindowLevel is deprecated. Don't use it.
>> Yes, this is a good tip. I've removed the NSDockWindowLevel from the array. An updated fix is available at:
>>
>> http://cr.openjdk.java.net/~anthony/x-6-alwaysOnTop.1/
> 
> Also, NSSubmenuWindowLevel and NSTornOffMenuWindowLevel are synonyms for each other, and end up being the same value. The documentation for NSWindow [2] says "The constant NSTornOffMenuWindowLevel is preferable to its synonym, NSSubmenuWindowLevel.", but again, I'd recommend just removing the table and higher/lower comparison.
> 
>>>> The array of hardcoded window levels allows us to convert between window levels ordered arbitrarily and integer values ordered according to their visual z-order. This allows us to compare windows level with respect to their relative z-order as specified by Cocoa documentation.
>>> These constants are for assignment based on intention, not simple comparison of stacking level. They have behavioral meaning beyond just simple visual level ordering. Windows above the normal window level will disappear in Exposé/Mission Control, and certain levels will float in all Spaces. Other levels exclude the window from Cmd-~ keyboard window cycling, or showing up in the Dock window menu. Please, please, please test the side effects of these changes before just popping windows into different levels.
>> Could you please clarify what exact "different levels" you're talking about here? AWT currently uses only two window levels: NSNormalWindowLevel is used for regular windows, and NSFloatingWindowLevel for always-on-top windows (see AWTWindow.m). This code is already in the repository, and tests for always-on-top windows do work OK.
> 
> The AWT will need to use more levels, and it's not clear how parent/child relationships need to affect these window levels. NSFloatingWindowLevel is used for palettes (like non-modal parented dialogs with small title bars), which do not participate in Exposé/Mission Control. NSModalPanelWindowLevel is what app-modal dialogs (like Open/Save) live at, which you might want "always on top" windows to be on top of too. Obviously popup menus should be at NSPopUpMenuWindowLevel. Another complication we encountered in Java SE 6 is full-screen exclusive windows, which still expect popups to be visible, so we had to push them above the NSScreenSaverWindowLevel as a gross hacky workaround.
> 
> In your testing have you moved these windows between spaces? Can you separate them between spaces? Do the windows show as visible in Mission Control? When you right-click on the Dock icon, do they show in the window list? Should they? When you press Cmd-~, do they participate in the window cycle? These are the manual tests we do every time we futz with something like "window level" or "child windows" in Java SE 6, which can't easily be automated.
> 
>> So back to the fix we're discussing, in reality the oldChildLevel in the CWrapper.addChildWindow() implementation can be either of these two levels, but not anything else. I guess we could simplify the fix and only include these two levels in the ORDERED_LEVELS{} array. I suppose Lubomir just wanted to provide a more generic and complete implementation that won't break if we ever start using any other levels in the future (or e.g. if OS temporarily puts our window on some other level). This is certainly unlikely, of course, however, I see absolutely no problems with an implementation of compareWindowLevels() that can work with all window levels officially supported by Cocoa, not just the two that we use currently.
> 
> In the future, I agree, the existing child window level will have to be more than two values, but either it should be allowed to inherit from it's parent, or it should be reset to the original value. This is really a decision to be made based on the Java model state, and not an observed property in the AppKit model which is being changed behind your back and compared against a table. Right now the only decision hooks on if the AOT state of the parent and child, which is already present in the styleBits of the window.
> 
> See the IS, SET, and MASK macro usage in AWTWindow.m for more info. You could make a simple accessor to report if ALWAYS_ON_TOP is set on both the parent and the child.
> 
>>> The safest thing to do (if it works) is just to simply re-assign the window level after adding the child to the parent, but you will have to recursively descend into the child windows of the child, because setting the window level sets the level of all the children too (and changes their collection/spaces/cycle behavior). This may not be an issue if you never change a child window's parent.
>> I've clarified above why simple reassignment won't work well.
> 
> Got it. I think I've straightened out my thinking on this too, and paged back years of hacks and corner cases I would have preferred to have forgotten...
> 
> Cheers,
> Mike Swingler
> Apple Inc.
> 
>> --
>> best regards,
>> Anthony
>>
>>> Regards,
>>> Mike Swingler
>>> Apple Inc.
>>>> Does this clarify the need for the array of window levels? Do you have any other concerns regarding the fix?
>>>>
>>>> [1] http://developer.apple.com/library/mac/documentation/GraphicsImaging/Reference/Quartz_Services_Ref/Reference/reference.html#//apple_ref/c/tdef/CGWindowLevelKey
>>>>
>>>> [2] http://developer.apple.com/library/mac/documentation/Cocoa/Reference/ApplicationKit/Classes/NSWindow_Class/Reference/Reference.html#//apple_ref/doc/constant_group/Window_Levels
>>>>
>>>> --
>>>> best regards,
>>>> Anthony
>>>>
>>>>
>>>>> Regards,
>>>>> Mike Swingler
>>>>> Apple Inc.
>>>>> On Jan 14, 2012, at 7:40 PM, Mike Swingler wrote:
>>>>>> In the course of preparing to contribute our current window stacking algorithm, I realized that there was a pair of internal SPI calls it was making. In the next revision we ship of the JavaRuntimeSupport.framework we can provide cover methods of these SPI, however that is of little benefit to the OpenJDK port in the immediate here-and-now.
>>>>>>
>>>>>> For now, using -addChildWindow: will be sufficient, however, we can provide an alternate implementation that uses a new pair of category methods on NSWindow if -respondsToSelector: shows that they are available:
>>>>>> - (void)javaAddToOrderingGroup:(NSWindow *)ownedWindow;
>>>>>> - (void)javaRemoveFromOrderingGroup:(NSWindow *)ownedWindow;
>>>>>>
>>>>>> These functions will allow the window server to move each child window with respect to it's parent's level, but without being added to it's movement group.
>>>>>>
>>>>>> Once customers get an updated JavaNativeFoundation.framework by either Java update or OS update, the implementation will switch over dynamically at runtime.
>>>>>>
>>>>>> How does this sound for a plan?
>>>>>>
>>>>>> Regards,
>>>>>> Mike Swingler
>>>>>> Apple Inc.
>>>>>>
>>>>>> On Jan 12, 2012, at 2:13 PM, steve.x.northover at oracle.com <mailto:steve.x.northover at oracle.com> wrote:
>>>>>>
>>>>>>> We fought this one in SWT and ended up going with the puppy route.
>>>>>>>
>>>>>>> Steve
>>>>>>>
>>>>>>> On 12/01/2012 2:57 PM, Anthony Petrov wrote:
>>>>>>>> Hi Mike,
>>>>>>>>
>>>>>>>> I recall we've discussed this issue with you back in 2010. Unfortunately, I wasn't able to implement anything like this that would work reliably back then (and I tried hard, really), that's why we ended up with -addChildWindow:. Note that JCK is very happy with this implementation, and so are we, I presume. As long as child windows receive their respective MOVE events, it seems that everything is all right. Besides, such behavior is very Mac-friendly, making Java apps behave like native apps.
>>>>>>>>
>>>>>>>> I realize that for some developers this behavior may be inconvenient. But then again, why not listen to MOVE events on the parent frame and compensate for its movement repositioning all its children? ( :) yeah, yeah, I know, sounds weird, but... as Steve says, you gotta do what you gotta do... I mean, there's a workaround at least!)
>>>>>>>>
>>>>>>>> So, if you or anyone else wish to contribute an alternative implementation for owned windows, that would be greatly appreciated. Otherwise I'm afraid we have to stick with using -addChildWindow: for now since we simply don't have a better solution.
>>>>>>>>
>>>>>>>> -- 
>>>>>>>> best regards,
>>>>>>>> Anthony
>>>>>>>>
>>>>>>>> On 1/12/2012 11:17 PM, Mike Swingler wrote:
>>>>>>>>> Also, you should not use -addChildWindow:, because you also get added to the movement group of the parent (moving the parent moves all it's children). This is *highly* undesirable behavior for Java's general use (and you can see it in Eclipse when the find window follows around the IDE window like a puppy).
>>>>>>>>>
>>>>>>>>> In the Java SE 6 AWT we manually restack every Java window in native with -orderFront: every time a Java window changes it's level to correctly handle it's children and the relationships between them. This actually works out ok, since all the changes happen at once, and when the next turn of the event loop happens, the new stacking order only has one new window moving to the background and one new window becoming key/main.
>>>>>>>>>
>>>>>>>>> Regards,
>>>>>>>>> Mike Swingler
>>>>>>>>> Apple Inc.
>>>>>>>>> On Jan 12, 2012, at 6:34 AM, Leonid Romanov wrote:
>>>>>>>>>
>>>>>>>>>> Bah, I was wrong. The value of NS*WindowLevel is really funky, it was a wrong suggestion to rely on it. Sorry for wasting your time.
>>>>>>>>>>
>>>>>>>>>>
>>>>>>>>>> On 12.01.2012, at 17:52, Anthony Petrov wrote:
>>>>>>>>>>
>>>>>>>>>>> The values of the NS*WindowLevel macros are not a part of the contract for Cocoa API. Therefore, we shouldn't rely on their current numerical values. The names, however, and their relative z-order are clearly specified in the documentation, and as such we may use them.
>>>>>>>>>>>
>>>>>>>>>>> -- 
>>>>>>>>>>> best regards,
>>>>>>>>>>> Anthony
>>>>>>>>>>>
>>>>>>>>>>> On 01/12/12 17:44, Leonid Romanov wrote:
>>>>>>>>>>>> I see… It looks like the higher window level is, the higher is the value of corresponding NS*WindowLevel macro.
>>>>>>>>>>>> Wouldn't it be better to implement compareWindowLevels() by simply subtracting one value from another?
>>>>>>>>>>>>
>>>>>>>>>>>> On 12.01.2012, at 17:06, Anthony Petrov wrote:
>>>>>>>>>>>>
>>>>>>>>>>>>> Hi Leonid,
>>>>>>>>>>>>>
>>>>>>>>>>>>> Thanks for taking a look at the fix.
>>>>>>>>>>>>>
>>>>>>>>>>>>> The if check is needed for the following case. If the parent window is an always-on-top window, its level is NSFloatingWindowLevel. Suppose a child window being added to it hasn't been assigned any level explicitly, so its default level is NSNormalWindowLevel.
>>>>>>>>>>>>>
>>>>>>>>>>>>> Now, when we call -addChildWindow:, we really want to update the level of the child window so that both the parent and the child share the same window level. The if check ensures that we don't reset the level of the child window back to normal in this case.
>>>>>>>>>>>>>
>>>>>>>>>>>>> -- 
>>>>>>>>>>>>> best regards,
>>>>>>>>>>>>> Anthony
>>>>>>>>>>>>>
>>>>>>>>>>>>> On 01/11/12 21:48, Leonid Romanov wrote:
>>>>>>>>>>>>>> Just wondering: what would happen if you simply set oldChildlevel without that "if" check?
>>>>>>>>>>>>>>
>>>>>>>>>>>>>> On 11.01.2012, at 19:22, Anthony Petrov wrote:
>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Hello,
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Please review a fix for http://bugs.sun.com/view_bug.do?bug_id=7124554 at:
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> http://cr.openjdk.java.net/~anthony/x-6-alwaysOnTop.0/
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> Lubomir Nerad (CC'ed) should be credited for the fix itself. I'm just going to integrate it into the repository.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> A JWindow object is always a child window with either an explicit parent, or a shared invisible owner frame. Therefore, we always call NSWindow -addChildWindow: when showing a JWindow object. The root cause of the issue is that the -addChildWindow: resets the level of the child window to match that of the parent window. With this fix we restore the level back to its original value after the -addChildWindow: call, and as such preserve the always-on-top state of the child window.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> I've verified the fix with a test app attached to the original issue at http://java.net/jira/browse/MACOSX_PORT-158 , and it works fine.
>>>>>>>>>>>>>>>
>>>>>>>>>>>>>>> -- 
>>>>>>>>>>>>>>> best regards,
>>>>>>>>>>>>>>> Anthony
>

More information about the macosx-port-dev mailing list