Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

New api spec : Window.AppWindow api #7605

Merged
merged 10 commits into from
Sep 15, 2022
Merged

Conversation

pratikone
Copy link
Contributor

Spec PR to introduce a new Window.Appwindow api

Description

This spec PR introduces a new api for accessing appwindow object from mux code. More details are in the spec document itself.

Motivation and Context

Mentioned in spec documented.

Screenshots (if appropriate):

Code examples included in spec document.

@ghost ghost added the needs-triage Issue needs to be triaged by the area owners label Aug 15, 2022
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
pratikone and others added 4 commits August 15, 2022 17:13
suggested description change of appwindow

Co-authored-by: MikeHillberg <[email protected]>
perf effect note

Co-authored-by: MikeHillberg <[email protected]>
Copy link
Contributor Author

@pratikone pratikone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

updated spec

Copy link
Contributor Author

@pratikone pratikone left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

changes to add note about helper function as not real api

@pratikone pratikone marked this pull request as ready for review August 18, 2022 22:24
@riverar
Copy link
Contributor

riverar commented Aug 19, 2022

I'm a bit confused. I think AppWindow was supposed to be a high-level abstraction of CoreWindow and HWND implementations. I find it odd that we're thinking about hanging AppWindow APIs off window implementations. Sounds like a hack for XAML. Something doesn't seem right. Did I misunderstood the proposal?

Perhaps we should zoom out and acknowledge UWP is on the way out and AppWindow appetite is low, having only one implementation (HWND) after all. Maybe AppWindow should instead pivot and just become a consistent set of inbox HWND helpers (e.g. various presenters for various scenarios) instead.

@Balkoth
Copy link

Balkoth commented Aug 19, 2022

To me it is much more confusing that i have to mess with 2 classes, Window and AppWindow, that essentially describe the same thing, a window. Why does it have to be this way and why can't you just make all the windowing functionality available at one site, preferably at the Window object?

@BreeceW
Copy link
Contributor

BreeceW commented Aug 19, 2022

Perhaps we should zoom out and acknowledge UWP is on the way out and AppWindow appetite is low, having only one implementation (HWND) after all. Maybe AppWindow should instead pivot and just become a consistent set of inbox HWND helpers (e.g. various presenters for various scenarios) instead.

I don’t see how this negates having an AppWindow property on Window. The way I see it, AppWindow is in some sense a collection of helpers for an HWND-style window, the only difference being that it manages itself to ensure consistent internal state (which is a great thing—win32 apps oftentimes do messy and inconsistent things to their windows unintentionally).

Since a Microsoft.UI.Xaml.Window is backed by an HWND, and AppWindow wraps the HWND, putting AppWindow as a property on Window seems to me the most logical and convenient place to put it. I would and I’m sure most developers would appreciate having one-line access to the convenient window helpers that AppWindow provides (like presenters but also the very useful title bar APIs, for example).

To me it is much more confusing that i have to mess with 2 classes, Window and AppWindow, that essentially describe the same thing, a window. Why does it have to be this way and why can't you just make all the windowing functionality available at one site, preferably at the Window object?

Since Windows App SDK is more than just WinUI, it makes sense to have these APIs be separate (e.g., you could take advantage of the AppWindow API from within a WPF app). The XAML Window arguably shouldn’t do much at all, since it’s basically just the content inside the HWND. But since HWND is not the most friendly API, the AppWindow APIs are conveniently there to manage the window for you.

@riverar
Copy link
Contributor

riverar commented Aug 19, 2022

I think we basically agree then that AppWindow is a collection of HWND helpers. So does that make sense to hang off the actual Window object? What if someone wants to develop a better set of helpers? Seems more appropriate to just have a helper object that devs can opt into using, e.g. WindowHelpers.FromHwnd(hwnd).EnterPictureInPictureMode();

it manages itself to ensure consistent internal state (which is a great thing—win32 apps oftentimes do messy and inconsistent things to their windows unintentionally).

I actually find AppWindow to be consistently unreliable release after release and generally no better than just doing the HWND work yourself. But that's just my take!

@BreeceW
Copy link
Contributor

BreeceW commented Aug 19, 2022

I think we basically agree then that AppWindow is a collection of HWND helpers. So does that make sense to hang off the actual Window object? What if someone wants to develop a better set of helpers? Seems more appropriate to just have a helper object that devs can opt into using, e.g. WindowHelpers.FromHwnd(hwnd).EnterPictureInPictureMode();

it manages itself to ensure consistent internal state (which is a great thing—win32 apps oftentimes do messy and inconsistent things to their windows unintentionally).

I actually find AppWindow to be consistently unreliable release after release and generally no better than just doing the HWND work yourself. But that's just my take!

In principle I don’t disagree. It is awkward to have Microsoft.UI.Windowing in any API in the Microsoft.UI.Xaml namespace at all. They don’t have anything to do with each other.

But say you wanted to move a window in WinUI 3. Today, you would do something like this:

var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(this);
SetWindowPos(hWnd, null, x, y, 0, 0, SWP_NOZORDER | SWP_NOSIZE | SWP_NOACTIVATE); // Clunky win32 API

or with the convenience of AppWindow,

var hWnd = WinRT.Interop.WindowNative.GetWindowHandle(this);
var windowId = Win32Interop.GetWindowIdFromWindow(hWnd);
var appWindow = AppWindow.GetFromWindowId(windowId);
appWindow.Move(...);

Or, as you said, use your own helpers.

Any one of those approaches requires significantly more developer effort than should be needed. You could just simplify and call it window.Move(...), but now you have a bespoke solution for just WinUI for no reason. And AppWindow still exists, so now you have redundant APIs.

AppWindow provides a lot of utility that win32 apps previously had to do themselves, as well as crucial basic functionality, so I think the priority should be to make it as easy as possible to access. In this PR, you would get
window.AppWindow.Move(...), which, once developers know about it, is not meaningfully harder than if Window itself had such methods.

You basically described the status quo for how getting AppWindow from a XAML Window works: get the HWND, get an AppWindow, and your hypothetical WindowHelpers.FromHwnd API might as well return an instance of AppWindow. I would like to see the HWND step cut out. Even though the abstraction is no longer needed per se, developers coming from things like WPF or UWP probably want something more straightforward, i.e., AppWindow. And, of course, the HWND is still available for those who want it using the APIs that exist today.

The issue here is how do you get an AppWindow from a Microsoft.UI.Xaml.Window, because today’s method is unacceptably clunky. You could do a static helper, something like WindowHelper.GetAppWindowFromWindow(Window window), but that is still clunky and also mixes the Windowing and Xaml namespaces anyway. The proposed Window.AppWindow property is the easiest way to expose common windowing properties and is the best option for discoverability.

And the way I understand AppWindow to work, none of its restrictions should apply until you start using the API, so there should be no concern over it interfering with your own helper methods if you wanted to use those over AppWindow’s.

Spec note: Mapping an hwnd to an AppWindow causes the hwnd to be subclassed, meaning that the timing of
when AppWindow.GetFromWindowId
is first called can potentially have a behavioral side effect. To ensure this is predictable, the AppWindow
for Xaml's hwnd will be created during the Xaml Window's construction. Note that this could potentially be
an observable behavior change from the behavior before the introduction of this API.

That said, this part of the spec is a little troubling, and it would be good to know what the side effects of this would be.

@Balkoth
Copy link

Balkoth commented Aug 19, 2022

I think the point about other UI-Frameworks is moot as they all support the missing functionality since long ago. So why make it more complicated for the current tech than necessary?

@MikeHillberg
Copy link
Contributor

As BreeceW says, the goal is to make it easier to get from a Window to an AppWindow, and to not copy all of the AppWindow API onto the Window API. (Note too that the UWP AppWindow is different than the WinAppSDK AppWindow.)

The Xaml Window API in WinUI2 has the same approach; Window.CoreWindow gets you easily from a Window to a CoreWindow, but Window doesn't try to duplicate CoreWindow.

I am, though, expecting that with we'll add more APIs on Window that wrap the underlying AppWindow, more so than CoreWindow is wrapped, to avoid app code from commonly having to switch coordinate spaces. (Xaml APIs including Window are in relative view coordinates, AppWindow APIs are in screen coordinates.)

@MikeHillberg
Copy link
Contributor

@nnshah99 FYI

@Balkoth
Copy link

Balkoth commented Aug 21, 2022

I think you live in your own bubble way too hard. Ask a dev coming over from winforms/wpf if your strategy is intuitive in any way.

Create a new project there and you are presented with the designer and the mainwindow, where you can set its properties, like size, location, title, maximize, minimize, icon and so on...

Now create a new winui3 project and you are also presented with the mainwindow, but you can not do anything useful with it. Everything you want to do with it, you have to google for it, because it is obfuscated and abstracted away in ways that might be logical to you, but not the devs who want to use this.

specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
@ranjeshj ranjeshj added the team-Reach Issue for the Reach team label Aug 26, 2022
@roxk
Copy link

roxk commented Aug 27, 2022

I think you live in your own bubble way too hard. Ask a dev coming over from winforms/wpf if your strategy is intuitive in any way.

Not disagreeing current API can be improved, or more windowing API should be wrapped by XAMl's Window, but this line strikes me as missing the picture still. There are windows devs/platforms other than winforms/wpf. E.g. Flutter on windows could use the new WASDK's AppWindow without using XAML's Window.

This is not a sign of living in a bubble. This is good engineering.

Bottom line, I'd agree XAML Window should have more functionality out of the box, but complaining about "having two window classes" is missing the point of this separation of concern.

@Balkoth
Copy link

Balkoth commented Aug 27, 2022

I gave a concrete example, for the 2 most successful .NET UI frameworks on windows. If you can't see why it is the most crucial thing to ease the transition for them, to have this platform succeed and progress in any meaningful way, then there is nothing i can do for you.

@pratikone pratikone self-assigned this Sep 9, 2022
@pratikone pratikone removed the request for review from codendone September 9, 2022 00:47
@pratikone
Copy link
Contributor Author

@MikeHillberg @jeffstall since most of questions have been answered and resolved, any final comments you want to capture ?

specs/appwindow-spec.md Outdated Show resolved Hide resolved
specs/appwindow-spec.md Outdated Show resolved Hide resolved
@pratikone pratikone removed the needs-triage Issue needs to be triaged by the area owners label Sep 15, 2022
@pratikone
Copy link
Contributor Author

/azp run

@azure-pipelines
Copy link

Azure Pipelines successfully started running 1 pipeline(s).

@pratikone pratikone merged commit 5efa10b into main Sep 15, 2022
@pratikone pratikone deleted the user/praanan/appwindowspec branch September 15, 2022 20:00
for Xaml's hwnd will be created during the Xaml Window's construction. Note that this could potentially be
an observable behavior change from the behavior before the introduction of this API.

New subclassing order with this new feature change:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If I derive from Window and subclass myself as well would the position of the subclass change in the order?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

When you say "subclass" you're talking about the hwnd subclass (SetWindowSubclass)? You can change the order if you create the AppWindow ahead of the Window, whether deriving or not.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

With subclass I mean SetWindowLongPtr(hwnd, GWL_WNDPROC, newWndProc)

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The first time an AppWindow is created for an hwnd (AppWindow.GetFromWindowId) it will add its subclass. So any subclassing you do before/after that will be in that order. And also if you create the AppWindow before creating the Xaml Window, you'll cause it to be potentially earlier in the order.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
team-Reach Issue for the Reach team
Projects
None yet
Development

Successfully merging this pull request may close these issues.