microsoft / WindowsAppSDK

The Windows App SDK empowers all Windows desktop apps with modern Windows UI, APIs, and platform features, including back-compat support, shipped via NuGet.
https://docs.microsoft.com/windows/apps/windows-app-sdk/
MIT License
3.8k stars 320 forks source link

AppWindow::GetFromWindowId locks Window Styles #2049

Open tom-huntington opened 2 years ago

tom-huntington commented 2 years ago

Describe the bug

In project reunion, we where able to change window styles with SetWindowLongPtrA.

But with the Windows App SDK, with the new AppWindowPresenter update, it seems like you have taken this away, forcing us to use the new (more limited) windowing APIs.

Very unhappy. Need Freedom

Steps to reproduce the bug

auto print_styles = [=]() 
{
    auto styles = GetWindowLongPtrA(m_hwnd, GWL_STYLE);

    std::stringstream ss;
    ss << styles << std::endl;
    for (unsigned i = 63; i < 64; i--)
        ss << (((1ull << i) & styles) != 0);
     ss << std::endl;
     OutputDebugStringA(ss.str().c_str());
};
print_styles();
SetWindowLongPtrA(m_hwnd, GWL_STYLE, 0);
print_styles();
riverar commented 2 years ago

@tom-huntington Can you describe the problem and expected results here? What version of WindowsAppSDK? WinUI? Windows? Basically, I'm asking for you to complete the bug reporting form properly.

riverar commented 2 years ago

Here's a sample C++ project that uses your code above + Windows App SDK 1.0, on Windows 11 22543. Seems to work great. was_issue_2049.zip

So we'll need a bit more info to figure out what's going on. (Is your HWND valid? How did you retrieve it?)

tom-huntington commented 2 years ago

Thanks @riverar, it only happens in my main project.

Should really should check in a Blank App before I waste your time.

riverar commented 2 years ago

Feel free to ping this thread if you get stuck. Maybe check for sneaky calls to GetActiveWindow or GetWindow.

tom-huntington commented 2 years ago

I was right. Although this only occurs after calling AppWindow::GetFromWindowId.

Here's a minimal reproducible example:

https://github.com/tom-huntington/CantChangeWindowStylesBug

Notably you can zero out one bit but not the others, so it seems that some of the styles get locked.

I don't know if I would really call this a bug though, just the new behaviour. Feel free to close.

riverar commented 2 years ago

Solid repro @tom-huntington 👍

Before calling AppWindow::GetFromWindowId no_call

After calling AppWindow::GetFromWindowId after_call

Appears there's indeed some unexpected behavior here.

@rkarman Is this something you're tracking? cc: @andrewleader

JaiganeshKumaran commented 2 years ago

Maybe this is because of subclassing?

rkarman commented 2 years ago

We are looking into this in the feature team. For some of the Presenters this is expected behavior (as in, once you apply say a FullScreen presenter we will try to block certain style changes), but for the default/overlapped presenter I did not expect this to happen. I'll get back here as soon as we have an update.

rkarman commented 2 years ago

Here's finally an update, I know it's been a while, but we had some interesting problems and architectural discussions to work through to see if we could make a change to the behavior here without a breaking change to the actual APIs. Spoiler alert: We did! 😄

What is described below is the fix coming for the Stable release of WinAppSDK 1.1. We are also looking into servicing it for 1.0.x as well (it's a contained and small enough change to do so).

Why did this happen? The reason this happens is because we have a Presenter applied by default to any AppWindow so that developers can easily get to the features that are tied to the Presenter (such as minimize/maximize, checking if there's a title bar/border present, etc.). This default Presenter applied to all AppWindows is the OverlappedPresenter.

Now, each Presenter has a set of restrictions it applies to the window in order for it to control the experience of that window so that "others" don't change it into a state it cannot properly represent, is orthogonal to the experience it is designed to create, or where it risks of being out of sync for properties/states.

One way we make sure we are in control of the experience is that we (try to as far extent as possible) reject any changes to styles for the window where we have a corresponding property/method for controlling that state in our API surface. We can then make sure that any change to those styles only come from "us" and that the AppWindow surface is in sync with the actual state of the underlying HWND, not changed by "outside forces", thus creating a consistent UX for the Presenter as well as the end-user.

But we never intended for windows that weren't created from our APIs to put these guardrails in place. We want you to be able to gradually adopt the AppWindow API surface if you stared elsewhere. This is where the bug crept in. By applying the same Presenter to an AppWindow created by the AppWindow::Create API as to one created by the AppWindow::GetFromWindowId we inadvertently put too many restrictions on the window, making a gradual adoption of the API less feasible. Again, this was not our intention and I apologize for not seeing this happening earlier.

So how do we fix this? Well, we put the guardrails in place gradually as you adopt the APIs of course!

What does this mean for you as a consumer of these APIs? It means that if you use AppWindow.GetFromWindowId to get hold of an Appwindow, the default Presenter will still be there, it will still be an OverlappedPresenter with all the features you'd expect from that, but it will have no restrictions in terms of changes to window styles coming from other APIs.

If, at a later time, you decide to start using the features of the OverlappedPresenter that is applied to your AppWindow to control window styles (setting say the IsMaximizable property to false) we will start enforcing changes to that particular style to only be allowed from our API surface. There is no going back once you've done this, we will not lighten the restriction if you "unset" the property. Once you've used it from our API surface you enter into an agreement that you won't go and try to pull the rug from under us. And if you do? We will happily refuse to fall over, and the rug will be put back in its proper place. 😃

If, on the other hand, you start from our API surface by calling AppWindow::Create to get your AppWindow, the default Presenter will have all the guardrails in place from the start. I.e., by creating the window from our API surface, you have entered into an agreement to use our API surface for any changes going forward. We will not let you change these styles from elsewhere. If you change window styles outside of what our API can control, or styles that does not impact the experience we are trying to maintain in the applied Presenter, you're still allowed to do so using non-AppWindow APIs.

As part of the next documentation update, we will add the window styles that are being controlled/enforced by each Presenter.

I hope this helps better understand the Presenter pattern we have, as well as solve the problem identified in this issue and provide you with a reasonable migration path from user32 to AppWindow APIs.