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

Add WinUI #29

Open
Lakritzator opened this issue Jun 2, 2020 · 10 comments
Open

Add WinUI #29

Lakritzator opened this issue Jun 2, 2020 · 10 comments
Labels
enhancement New feature or request help wanted Extra attention is needed

Comments

@Lakritzator
Copy link
Member

I guess it should be possible to add WinUI support, with a sample project.

@Lakritzator Lakritzator added enhancement New feature or request help wanted Extra attention is needed labels Jun 2, 2020
@AliveDevil
Copy link
Contributor

Before tackling this part here (I do have a working hosted service running under WinUI 3 (0.8.4)) I'd need to know whether these shortcomings are relevant or should be worked around:

  • There is no ShutdownMode, last Window closes App-instance
    • For me this is a non-issue, as I just statically linked this to IHostApplicationLifetime.StopApplication
  • There is no way of getting away with not defining an Application-class (Xaml compiler is really angry then)
    • Cannot not use builder.UseApplication() would be configured with ConfigureWinUI<TApp>
  • Currently works with Packaged apps only, but can be enabled for unpackaged apps
    Install tools for Windows app development (Didn't tap into that one yet)
  • Window-Initialization (and IWpfShell/IWinUIShell) possibly (?) achievable, there is no MainWindow on Application
  • Implementation will rely on ActivatorUtilities, without IWinUIShell (IWpfShell equivalent)
  • There are no events to attach to (there is AppLifecycle, though I don't know whether that is reliably useful)

Anything I should look out for before implementing this for you here?

@Lakritzator
Copy link
Member Author

Thanks for taking a swing at this. 👍
I unfortunately didn't manage to have a detailed look at WinUI, so I'm a bit in a disadvantage.

  • For the shutdown mode, I'm not sure how the behavior of WinUI is, but as soon as someone needs this it can be looked at. I don't think it's a show stopper.
  • I need to see what you mean, you mean it's already a compile time issue?
  • Also not sure, but I guess for WinUI something needs to be installed before they work? Are you talking about MSIX packages? If WinUI support would be added to the repository, would a developer need to have a WinUI package installed to even use the WinUI project?
  • I kind off understand what you mean, but I need to see it. Can Application be extended?
  • I guess I also have to see it...
  • If there are no events, maybe we can extend something? I really didn't look into the app model, we will see.

My suggestion, as you already seem to have code, create a pull-request for a branch called "feature/winui" so I can see it. I'm not sure how much I can help, but it would be nice if you can make the behavior similar to the other projects.

@AliveDevil
Copy link
Contributor

I just added a draft PR for my initial implementation here.
The sample requires any version of Windows App SDK (>= 1.0) (or Project Reunion until <1.0). The implementation is a regular net5.0-windows10.0.19041.0 library.
I didn't branch off of the latest master version as I don't have .NET 6 installed, and would have been forced to install that.

  1. The default implementation is enough, I think regular desktop apps are OK with closing down the hosted services once the last window closed.
    In Configure() I just force-set IsLifetimeLinked to true to get this behavior.
    There is a possible timing issue here which could bring the application to stall due to the DispatcherQueue being stopped in the right moment, thus the TaskCompletionSource waits forever. Ideally there should be a timeout added for waiting. Your thoughts?
  2. Yes, it is a compile time issue by the App SDK, which I cannot circumvent, thus there would be no fallback - as there is WPF - for a generic Application-class instance
  3. Yes, the WinUI SDK needs to be installed in order for developers to use the samples, though if a developer already has the intention to use WinUI, then they should've installed that already.
    • That part got into "deployment"-stuff, though that wouldn't be an issue for this implementation.
  4. You need to derive from Application in an App.xaml-file, otherwise XAML compiler bails out.
    This means that there cannot be any implementation without App.xaml and App.xaml.cs including compiler-generated output here.
    I opted for requiring the user to specify which Application-type to use in the ConfigureWinUI-builder method. The developer can later with the IWinUIBuilder extensions add windows to the app, making the App-implementation boilerplate code only.
  5. I didn't find IWpfShell that useful … so I just got rid of it in the WinUI-implementation, windows are now created from the IWinUIBuilder.WindowTypes-list, copied into the IWinUIContext.WindowTypes list, and used in WinUIThread to create those types on demand.
    Maybe some validation, that thses windows are really constructible …
  6. There is no OnLaunch (or similar) to fill in some logic, and there is no exit-method to hook into.
    The only time we know that Application has been quit is when Application.Start (static method) exits. Thus using the default UiThread HandleAppShutdown implementation.

I'd help out as much as I can for supporting/extending the WinUI-part (migrating to App SDK 1.0, etc.).

@AliveDevil
Copy link
Contributor

AliveDevil commented Oct 2, 2021

So … I just played with it a bit and turns out handling Windows is much harder now, as everything is managed by WinUI/COM.
Going out of scope means several things, one being that resources can free themselves up, without anyone noticing - resulting in undefined behavior, usually memory access denied.

One solution: User cannot use UseWindow and has to use Application.OnLaunched with field reference, in order to workaround this.
Another solution: Keep first created windows all in a list, eventually react to Closed-events, and free them after that.

Tracking this here: microsoft/microsoft-ui-xaml#6020

@Lakritzator
Copy link
Member Author

I'm currently reacting to the comment above this one, about the WinUI/COM management.

I don't expect there will be a solution coming from Microsoft, as this is exactly the way things work when mixing managed & unmanaged. Compare it to the GDI Bitmap (System.Drawing.Bitmap) these are also interfaces into unmanaged resources, and will be cleaned when all references are gone via a finalizer. If you still use the Bitmap somewhere indirectly, e.g. in a button, it will cause issues.

The way you define a new window, in a local scope (var window = new Window), will exactly do that. And to be honest, it makes sense, as there is no reference to the window anymore, so it would be a memory leak to leave it hanging. A memory leak would be no good either and that is what actually happens with a lot of WinForms and other stuff, which is NOT collected, the amount of people who have issues with forgetting to dispose their resources like a Pen or a brush... I've had my share of disposing issues for Greenshot, which bite you in the face due to a "unknown" error.

So although I agree it's not nice behavior, I do not see any solution for it, besides making sure YOUR application keeps the references it needs. And maybe there is a way we can support making this a better experience via a library like this?

@AliveDevil
Copy link
Contributor

Ageed with the COM/Native-part, though I'd argue that WPF does work fine with fire-and-forget creation of Windows (though Application.Current keeps a list of open windows … so much for that).
And it looks like Microsoft is actually planning on this fire-and-forget approach for windows: microsoft/microsoft-ui-xaml#5202 (comment)
Unfortunately I'd need to remove multi-window support from the Hosting.WinUI-project as that doesn't look like it is supported: microsoft/microsoft-ui-xaml#5289 (or requires huge effort to pull-off, like spawning a thread per Window) until 1.0 is released.

So: Configure()-method would be turned down to
ConfigureWinUI<ApplicationType, AppWindowType>(), without further configuration options, and WinUIThread holding a reference to the AppWindowType-instance, until being closed. I'll get the PR updated.

For library-support: Eventually this would be a separate service IWindowService which could create and manage the lifetime of Windows.

@AliveDevil
Copy link
Contributor

Regarding the experimentation-branch: I'll mark the PR as ready for review once I finished migrating an app I have completely to WinUI with the Hosting extensions to ensure there are no major issues, other than those created by WinUI itself.

@Lakritzator
Copy link
Member Author

Yeah, absolutely no hurries. Take your time, I am so swamped with work that it doesn't matter if it takes another week.
But I am glad to see some code with WinUI, so I can experiment a bit with it, maybe it is a good substitute for Greenshot which is still WinForms.

Btw.
I recently had a try with Avalonia, which in fact is very nice and I was able to write something for a friend, and it works on Windows, OSX and Ubuntu.

@Lakritzator
Copy link
Member Author

@ChrisPulman II shine at the lack of WinUI knowledge, and with the merge of the WinUI code into master, it of course doesn't work.

This is when I start the sample project

System.TypeInitializationException
  HResult=0x80131534
  Message=The type initializer for 'WinRT.ActivationFactory`1' threw an exception.
  Source=Microsoft.WinUI
  StackTrace:
   at Microsoft.UI.Xaml.Application.Make___objRef_global__Microsoft_UI_Xaml_IApplicationStatics()
   at Microsoft.UI.Xaml.Application.Start(ApplicationInitializationCallback callback)
   at Dapplo.Microsoft.Extensions.Hosting.WinUI.Internals.WinUIThread.UiThreadStart() in D:\Projects\Dapplo.Microsoft.Extensions.Hosting\src\Dapplo.Microsoft.Extensions.Hosting.WinUI\Internals\WinUIThread.cs:line 34
   at Dapplo.Microsoft.Extensions.Hosting.UiThread.BaseUiThread`1.InternalUiThreadStart() in D:\Projects\Dapplo.Microsoft.Extensions.Hosting\src\Dapplo.Microsoft.Extensions.Hosting.UiThread\BaseUiThread.cs:line 67
   at System.Threading.Thread.StartCallback()

  This exception was originally thrown at this call stack:
    WinRT.BaseActivationFactory.BaseActivationFactory(string, string)
    WinRT.ActivationFactory<T>.ActivationFactory()

Inner Exception 1:
COMException: Class not registered (0x80040154 (REGDB_E_CLASSNOTREG))

And this is when the build on Azure runs:

023-04-02T19:22:25.6024517Z   Dapplo.Microsoft.Extensions.Hosting.ReactiveUI.Wpf -> D:\a\1\s\src\Dapplo.Microsoft.Extensions.Hosting.ReactiveUI.Wpf\bin\Release\net472\Dapplo.Microsoft.Extensions.Hosting.ReactiveUI.Wpf.dll
2023-04-02T19:22:29.7844383Z ##[error]C:\Users\VssAdministrator\.nuget\packages\microsoft.windowsappsdk\1.2.230313.1\buildTransitive\MrtCore.PriGen.targets(911,5): Error MSB4062: The "Microsoft.Build.Packaging.Pri.Tasks.ExpandPriContent" task could not be loaded from the assembly C:\hostedtoolcache\windows\dotnet\sdk\7.0.202\\Microsoft\VisualStudio\v17.0\AppxPackage\\Microsoft.Build.Packaging.Pri.Tasks.dll. Could not load file or assembly 'C:\hostedtoolcache\windows\dotnet\sdk\7.0.202\Microsoft\VisualStudio\v17.0\AppxPackage\Microsoft.Build.Packaging.Pri.Tasks.dll'. The system cannot find the path specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask.
2023-04-02T19:22:30.2956714Z C:\Users\VssAdministrator\.nuget\packages\microsoft.windowsappsdk\1.2.230313.1\buildTransitive\MrtCore.PriGen.targets(911,5): error MSB4062: The "Microsoft.Build.Packaging.Pri.Tasks.ExpandPriContent" task could not be loaded from the assembly C:\hostedtoolcache\windows\dotnet\sdk\7.0.202\\Microsoft\VisualStudio\v17.0\AppxPackage\\Microsoft.Build.Packaging.Pri.Tasks.dll. Could not load file or assembly 'C:\hostedtoolcache\windows\dotnet\sdk\7.0.202\Microsoft\VisualStudio\v17.0\AppxPackage\Microsoft.Build.Packaging.Pri.Tasks.dll'. The system cannot find the path specified. Confirm that the <UsingTask> declaration is correct, that the assembly and all its dependencies are available, and that the task contains a public class that implements Microsoft.Build.Framework.ITask. [D:\a\1\s\src\Dapplo.Microsoft.Extensions.Hosting.WinUI\Dapplo.Microsoft.Extensions.Hosting.WinUI.csproj]

Before I start from scratch, for which I do not have time right now, I rather disable the WinUI project for now.
But maybe you can take a look? (If things work in Saudi?)

@kevinchalet
Copy link

@ChrisPulman II shine at the lack of WinUI knowledge, and with the merge of the WinUI code into master, it of course doesn't work.

I just gave it a try and got the same error, which is likely caused by the fact the Dapplo.Hosting.Sample.WinUI sample is not a packaged WinUI app, so the Windows App SDK must be initialized manually (see https://learn.microsoft.com/en-us/windows/apps/windows-app-sdk/use-windows-app-sdk-run-time).

Updating Microsoft.WindowsAppSDK to the latest version and adding <WindowsPackageType>None</WindowsPackageType> seems to fix the issue (note: if you don't have the Windows app runtime installed, you'll be prompted to do so when launching the sample).

@Lakritzator any chance you could give it another try? 😃

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants