feat: Add API for fetching window Z-order #14909

BAndysc · 2024-03-11T00:27:54Z

What does the pull request do?

Adds new API to Window which returns window z-order. The top most window has the highest value.

✅ I have tested Windows, macOS and Linux (Ubuntu both on XWayland and XOrg)! ✅

What is the current behavior?

n/a

What is the updated/expected behavior with this PR?

n/a

How was the solution implemented (if it's not obvious)?

As described in #6309 (comment), it uses GetWindow on Windows, NSWindow.orderedIndex on macOS and XQueryTree on X. Note: on Windows and X by default the topmost window has the highest zorder value, on macOS it is reversed, thus on macOS the value has to be negated to mimic the same behaviour.

My questions:

macOS Api returns NSInteger = Int, which is 4 bytes on 32 bits systems and 8 bytes on 64 bit systems. Therefore I've decided to go with returning IntPtr on c# side. Let me know if instead I should just use long.

Checklist

Added unit tests (if possible)?
Added XML documentation to any related classes?
Consider submitting a PR to https://github.com/AvaloniaUI/avalonia-docs with user documentation

Breaking changes

n/a

Obsoletions / Deprecations

n/a

Fixed issues

Fixed #6309

(side note: this is required for proper Dock support wieslawsoltes/Dock#329)

BAndysc · 2024-03-11T00:59:54Z

Edit: Tested Windows (11), macOS 14.3.1, Linux (Ubuntu with XWayland and X) and everything seems to work.

avaloniaui-bot · 2024-03-11T01:15:48Z

You can test this PR using the following package version. 11.2.999-cibuild0046038-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

maxkatz6 · 2024-03-11T04:23:48Z

src/Avalonia.Native/WindowImpl.cs

+                // macOS returns z-order in reverse order - the topmost window has the lowest z-order
+                return new IntPtr(-_native.WindowZOrder.ToInt64());


nit: this negation should happen on native side, since technically Avalonia.Native project isn't supposed to be macOS specific.

Alright, changed

maxkatz6 · 2024-03-11T04:28:03Z

native/Avalonia.Native/src/OSX/WindowImpl.mm

+            return E_POINTER;
+        }
+
+        *zOrder = [Window orderedIndex];


This new prop can be used in

Avalonia/samples/IntegrationTestApp/ShowWindowTest.axaml.cs

Line 78 in 86bfc26

_orderTextBox!.Text = MacOSIntegration.GetOrderedIndex(this).ToString();

Changed the GetOrderedIndex to the new WindowZOrder, I guess those tests could be expanded to all desktop platforms now (though rather than checking the absolute z-order value, it should check its relation to the parent window)

avaloniaui-bot · 2024-03-11T05:22:56Z

You can test this PR using the following package version. 11.2.999-cibuild0046040-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

kekekeks · 2024-03-13T09:23:53Z

src/Avalonia.X11/X11Window.cs

@@ -1133,6 +1133,46 @@ public PixelPoint Position
            }
        }

+        public IntPtr? ZOrder


Enumerating all windows in the the entire desktop doesn't seem like a good idea.

Especially since that code goes deep into the window tree, so stuff like GTK buttons gets enumerated too.

Also, are you sure that stacking order even matches one from XQueryTree?

Doesn't seem to be the case, windows are stacked as Konsole>Nemo>GEdit, while XQueryTree returns Nemo-GEdit-Konsole

https://tronche.com/gui/x/xlib/window-information/XQueryTree.html

That's what the specification says:

The children are listed in current stacking order, from bottommost (first) to topmost (last).

And that seem to be the case:

Screen.Recording.2024-03-13.at.13.29.19.mov

Those nemo, gedit and Konsole on your screeenshot are something different than the windows themselves. Take a look that under Qt Selection Owner for konsole the order is correct and the windows have the correct title indicating those represent the actual windows. Maybe this is QT thing to comply with the specs?

But I do agree maybe traversing the from root is not perfect. In that case, I guess in order to get correct z-order within the app, I have to traverse up the tree from each window, find a common parent, then find the index within the parent.

@kekekeks Updated the implementation to traverse only those windows which are required to compute the relative (within the app) z-order.

And that seem to be the case

Might be WM-specific behavior. I'm running KWin 5.20.5 with XOrg 1.20.11, no wayland.

avaloniaui-bot · 2024-03-13T14:33:55Z

You can test this PR using the following package version. 11.2.999-cibuild0046157-beta. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

MrJul · 2024-03-15T11:53:51Z

System vs app windows

On the public API side, I'd definitely use long instead of IntPtr here.
IntPtr leaks too much about the implementations and isn't a type normally used in such a high level API.

More generally, I'm a bit concerned about the API usage here.
The current implementation has an "almost opaque" ZOrder, in the sense that it depends on what the system returns.

Imagine an user having a two windows app and wondering why the first window has ZOrder=2 and the second one ZOrder=6, simply because there are 3 other unrelated apps in between on the desktop.
Is it a useful piece of information? It could be, but there's no way to know what windows 3 to 5 are, so ultimately the user can't do anything with it.
Shouldn't that be hidden away, with ZOrder only taking the current application's windows into account?

Performance

I'm also a bit concerned about potential performance problems. The property iterates through most windows on Win32 and X11: that seems a bit heavy weight, shouldn't that be a method instead to indicate that accessing the z-order isn't "free"?

A common usage of this new property is probably lifetime.Windows.OrderBy(w => w.ZOrder). That would iterate through most windows on the desktop N times (with N being the number of windows in the app), and that's only because OrderBy fetches the key only once. Use a custom comparer, and that iteration count can now explode depending on the sort algorithm.

Stability

Continuing with ZOrder being a property and sorting, is ZOrder guaranteed to stable during such a sort?
Quoting the remarks for Win32 GetWindow():

The EnumChildWindows function is more reliable than calling GetWindow in a loop. An application that calls GetWindow to perform this task risks being caught in an infinite loop or referencing a handle to a window that has been destroyed.

Makes me think it isn't. It doesn't seem stable even for a single loop.

Shouldn't we provide something like GetOrderedWindows() instead and use the appropriate underlying API?

maxkatz6 · 2024-03-15T18:49:47Z

See #6479 as well, another issue that can be closed after this PR.
It has proposal API of:

Window GetTopWindow(Window w1, Window w2) // likely still would involve enumeration of all windows on each call
void SetTopWindow(Window w) // no activation, which might not be possible on all platforms

kekekeks · 2024-03-16T09:59:37Z

A common usage of this new property is probably lifetime.Windows.OrderBy(w => w.ZOrder)

The API should probably accept multiple windows at once and maybe just sort those over ZOrder, since what platform backends are returning isn't stable and not really useful as a raw value.

…that are not required to compute z-order

This reverts commit 691541a.

BAndysc · 2024-03-28T01:35:35Z

@MrJul @kekekeks @maxkatz6 I have redesigned the PR to expose a single method:

IClassicDesktopStyleApplicationLifetime::SortWindowsByZOrder(Window[] windows)

which sorts the array by windows z order, bottom to top.

I have also changed Windows API to use EnumChildWindows, which unfortunately still need to traverse through all windows, but at least this is a single API call and also it will no longer traverse once it finds all windows (in my tests it does reduce the number of callback calls a lot - apparently there are a lot of hidden windows in Windows).

Also Linux X11 version now only calls XQueryTree n + | path to root | times.

Unfortunately Windows and Linux need to allocate some additional memory, I don't have an idea how to reduce the allocations.

kekekeks · 2024-03-28T07:39:49Z

Note that lifetime is an entirely optional concept and no non-lifetime related APIs should be put there

BAndysc · 2024-03-28T09:00:49Z

Note that lifetime is an entirely optional concept and no non-lifetime related APIs should be put there

Where could I put it then to access os specific things? I could just put it in Window class, but it might be a bit weird that Window class has a method that in fact has nothing to do with the instance you call it on. But maybe it is a minor inconvenience?

kekekeks · 2024-03-28T09:17:46Z

A static method on the Window would be fine, I think.

A few other points:

we probably want to have that API for WindowBase, so it would support popups as well
the platform-specific implementation should be queried from the first window from the list, rather than through the locator

BAndysc · 2024-03-28T10:43:10Z

A static method on the Window would be fine, I think.

A few other points:

we probably want to have that API for WindowBase, so it would support popups as well

the platform-specific implementation should be queried from the first window from the list, rather than through the locator

IWindowingPlatform is not easily accessible from Window now. To make it accessible, I would have to pass this during creating an instance of window implementation:

Avalonia/src/Windows/Avalonia.Win32/Win32Platform.cs

Lines 213 to 215 in 0b72cdb

    
           public IWindowImpl CreateWindow() 
        
           { 
        
               return new WindowImpl();

Avalonia/src/Avalonia.Native/AvaloniaNativePlatform.cs

Lines 182 to 185 in 0b72cdb

    
           public IWindowImpl CreateWindow() 
        
           { 
        
               return new WindowImpl(_factory, _options); 
        
           }

Avalonia/src/Headless/Avalonia.Headless/AvaloniaHeadlessPlatform.cs

Lines 59 to 63 in 0b72cdb

    
           public IWindowImpl CreateWindow() => new HeadlessWindowImpl(false, _frameBufferFormat); 
        
           public IWindowImpl CreateEmbeddableWindow() => throw new PlatformNotSupportedException(); 
        
           public IPopupImpl CreatePopup() => new HeadlessWindowImpl(true, _frameBufferFormat);

Avalonia/src/Avalonia.DesignerSupport/Remote/PreviewerWindowingPlatform.cs

Lines 23 to 44 in 0b72cdb

    
           public IWindowImpl CreateWindow() => new WindowStub(); 
        
           public IWindowImpl CreateEmbeddableWindow() 
        
           { 
        
               if (s_lastWindow != null) 
        
               { 
        
                   s_lastWindowTransport.Dispose(); 
        
                   try 
        
                   { 
        
                       s_lastWindow.Dispose(); 
        
                   } 
        
                   catch 
        
                   { 
        
                       //Ignore 
        
                   } 
        
               } 
        
               s_lastWindow = 
        
                   new PreviewerWindowImpl(s_lastWindowTransport = new DetachableTransportConnection(s_transport)); 
        
               foreach (var pf in PreFlightMessages) 
        
                   s_lastWindowTransport.FireOnMessage(s_lastWindowTransport, pf); 
        
               return s_lastWindow; 
        
           }

Is that ok? Alternatively, I could move all the logic from IWindowingPlatform to IWindowImpl. Which one do you prefer?

kekekeks · 2024-03-28T16:27:59Z

IWindowingPlatform is not easily accessible from Window now. To make it accessible, I would have to pass this during creating an instance of window implementation:

Just move the comparer method to IWindowBaseImpl.

maxkatz6 · 2024-03-30T08:21:32Z

src/Headless/Avalonia.Headless/HeadlessWindowImpl.cs

@@ -413,5 +413,7 @@ public void SetFrameThemeVariant(PlatformThemeVariant themeVariant)
        {

        }
+
+        public void GetWindowsZOrder(Span<Window> windows, Span<long> zOrder) => throw new NotSupportedException();


Headless platform should try to mimic real platform as much as possible, and not throw exceptions.
For example, we can keep static counter and increment it in HeadlessWindow.Activate(), saving locally per window. And use incremented index in GetWindowsZOrder. A simple solution, which can be then used in tests by somebody.

maxkatz6 · 2024-03-30T08:23:44Z

src/Avalonia.DesignerSupport/Remote/PreviewerWindowImpl.cs

@@ -153,5 +153,7 @@ public void SetExtendClientAreaChromeHints(ExtendClientAreaChromeHints hints)
        public void SetExtendClientAreaTitleBarHeightHint(double titleBarHeight)
        {            
        }
+
+        public void GetWindowsZOrder(Span<Window> windows, Span<long> zOrder) => throw new NotSupportedException();


I am wondering, if we should use headless windowing subsystem in our previewer code. Unrelated to this PR.

maxkatz6 · 2024-03-30T08:26:09Z

So, do we keep this API in optional IClassicDesktopStyleApplicationLifetime? If so, API diffs should be updated, run nuke command in the repository root, and commit updated files. It will fix the build error. Since IClassicDesktopStyleApplicationLifetime interface was changed.

BAndysc · 2024-03-30T08:50:57Z

So, do we keep this API in optional IClassicDesktopStyleApplicationLifetime? If so, API diffs should be updated, run nuke command in the repository root, and commit updated files. It will fix the build error. Since IClassicDesktopStyleApplicationLifetime interface was changed.

Sorry, I forgot to move it to the Window class as a static method. Updated

avaloniaui-bot · 2024-03-30T09:33:32Z

You can test this PR using the following package version. 11.2.999-cibuild0046837-alpha. (feed url: https://nuget-feed-all.avaloniaui.net/v3/index.json) [PRBUILDID]

maxkatz6 · 2024-03-30T19:33:31Z

Needs to be double checked by @grokys as well.

* feat: Add API for fetching window Z-order * Addressed PR comments * Improve X11Window::ZOrder implementation to avoid traversing windows that are not required to compute z-order * Move zOrder API from Window to IWindowingPlatform * Revert "Addressed PR comments" This reverts commit 691541a. * Rename * Missing methods * Move GetWindowsZOrder from IWindowingPlatform to IWindowImpl * Cleanup * Move SortWindowsByZOrder to Window class as a static method * Implement zOrder for HeadlessWindowImpl --------- Co-authored-by: Max Katz <[email protected]>

BAndysc force-pushed the widow_zorder branch from bd9a23a to 8a2ced3 Compare March 11, 2024 00:30

maxkatz6 reviewed Mar 11, 2024

View reviewed changes

kekekeks reviewed Mar 13, 2024

View reviewed changes

maxkatz6 added the backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch label Mar 15, 2024

BAndysc added 5 commits March 27, 2024 23:33

feat: Add API for fetching window Z-order

f0c1437

Addressed PR comments

691541a

Improve X11Window::ZOrder implementation to avoid traversing windows …

6f5cbc0

…that are not required to compute z-order

Move zOrder API from Window to IWindowingPlatform

f618fc3

Revert "Addressed PR comments"

a5bd606

This reverts commit 691541a.

BAndysc force-pushed the widow_zorder branch from 9affdad to a5bd606 Compare March 28, 2024 01:26

Rename

571b676

Missing methods

0b3492d

BAndysc added 2 commits March 29, 2024 14:42

Move GetWindowsZOrder from IWindowingPlatform to IWindowImpl

88be66f

Cleanup

d2a72bc

maxkatz6 reviewed Mar 30, 2024

View reviewed changes

Move SortWindowsByZOrder to Window class as a static method

91e6755

Implement zOrder for HeadlessWindowImpl

3c53331

maxkatz6 approved these changes Mar 30, 2024

View reviewed changes

Merge branch 'master' into widow_zorder

584054f

maxkatz6 added the feature label Apr 20, 2024

maxkatz6 merged commit 7a7ab8e into AvaloniaUI:master Apr 20, 2024
2 of 10 checks passed

maxkatz6 added backported-11.1.x and removed backport-candidate-11.1.x Consider this PR for backporting to 11.1 branch labels Apr 20, 2024

BAndysc mentioned this pull request Jul 22, 2024

Sort dock control parent windows by their z-order to correctly determine potential window to dock to wieslawsoltes/Dock#346

Merged

racerxdl mentioned this pull request Jul 31, 2024

IWindowImpl has an INTERNAL member #16553

Closed

BAndysc mentioned this pull request Aug 2, 2024

Remove internal access modifier from IWindowImpl.GetWindowsZOrder #16562

Merged

grokys mentioned this pull request Aug 7, 2024

Need a way to control and check the Z-Order of multiple Avalonia UI windows within the same application #6479

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

feat: Add API for fetching window Z-order #14909

feat: Add API for fetching window Z-order #14909

BAndysc commented Mar 11, 2024 •

edited

Loading

BAndysc commented Mar 11, 2024

avaloniaui-bot commented Mar 11, 2024

maxkatz6 Mar 11, 2024

BAndysc Mar 11, 2024

maxkatz6 Mar 11, 2024

BAndysc Mar 11, 2024

avaloniaui-bot commented Mar 11, 2024

kekekeks Mar 13, 2024

kekekeks Mar 13, 2024

kekekeks Mar 13, 2024

BAndysc Mar 13, 2024 •

edited

Loading

BAndysc Mar 13, 2024

kekekeks Mar 15, 2024

avaloniaui-bot commented Mar 13, 2024

MrJul commented Mar 15, 2024 •

edited

Loading

maxkatz6 commented Mar 15, 2024

kekekeks commented Mar 16, 2024

BAndysc commented Mar 28, 2024 •

edited

Loading

kekekeks commented Mar 28, 2024

BAndysc commented Mar 28, 2024

kekekeks commented Mar 28, 2024

BAndysc commented Mar 28, 2024

kekekeks commented Mar 28, 2024

maxkatz6 Mar 30, 2024

maxkatz6 Mar 30, 2024 •

edited

Loading

maxkatz6 commented Mar 30, 2024

BAndysc commented Mar 30, 2024

avaloniaui-bot commented Mar 30, 2024

maxkatz6 commented Mar 30, 2024

		// macOS returns z-order in reverse order - the topmost window has the lowest z-order
		return new IntPtr(-_native.WindowZOrder.ToInt64());

feat: Add API for fetching window Z-order #14909

feat: Add API for fetching window Z-order #14909

Conversation

BAndysc commented Mar 11, 2024 • edited Loading

What does the pull request do?

✅ I have tested Windows, macOS and Linux (Ubuntu both on XWayland and XOrg)! ✅

What is the current behavior?

What is the updated/expected behavior with this PR?

How was the solution implemented (if it's not obvious)?

My questions:

Checklist

Breaking changes

Obsoletions / Deprecations

Fixed issues

BAndysc commented Mar 11, 2024

avaloniaui-bot commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaloniaui-bot commented Mar 11, 2024

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

BAndysc Mar 13, 2024 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

avaloniaui-bot commented Mar 13, 2024

MrJul commented Mar 15, 2024 • edited Loading

System vs app windows

Performance

Stability

maxkatz6 commented Mar 15, 2024

kekekeks commented Mar 16, 2024

BAndysc commented Mar 28, 2024 • edited Loading

kekekeks commented Mar 28, 2024

BAndysc commented Mar 28, 2024

kekekeks commented Mar 28, 2024

BAndysc commented Mar 28, 2024

kekekeks commented Mar 28, 2024

Choose a reason for hiding this comment

maxkatz6 Mar 30, 2024 • edited Loading

Choose a reason for hiding this comment

maxkatz6 commented Mar 30, 2024

BAndysc commented Mar 30, 2024

avaloniaui-bot commented Mar 30, 2024

maxkatz6 commented Mar 30, 2024

BAndysc commented Mar 11, 2024 •

edited

Loading

BAndysc Mar 13, 2024 •

edited

Loading

MrJul commented Mar 15, 2024 •

edited

Loading

BAndysc commented Mar 28, 2024 •

edited

Loading

maxkatz6 Mar 30, 2024 •

edited

Loading