Add automatic (possibly optional) dispatching to the DispatcherQueue
instance for the current UI thread in the _BindingsTracking
type and to the necessary internal WinUI paths (eg. to deal with collection changed UI updates), specifically in the PropertyChanged
event handler and the CollectionChanged
. This would completely reverse the chain of responsability for proper thread dispatching from the viewmodel side to the UI side, and make the whole thing much more resilient and flexible.
A similar concept would apply to other events handled by code within WinUI.
The core idea is this: it should be responsability of an event _handler_ to dispatch to whatever synchronization context it needs, _not_ to the object _raising_ the event. Specifically because if you have multiple listeners on different threads, the latter simply will never work anyway. And in general, objects raising events shouldn't need to account for who's listening to them.
Note: what follows is just the details for the
PropertyChanged
event specifically, not the others. I'll describe this one since it's more "easily" addressed by looking at thex:Bind
codegen, whereas the others would likely need some tweaks in the code within WinUI dealing with those events. The idea in general is still the same though: WinUI as a framework should automatically deal with dispatching when handling notification events tied to the UI.
Handling the synchronization context for the PropertyChanged
event in viewmodels has always been a major pain point when working with MVVM. There have been a variety of different solutions proposed, but each with their shortcomings, namely:
PropertyChanged
event. Again this has two issues: for one it's not really flexible as it wouldn't work in case the same observable object is being displayed in different ways across more than a single window (where two different UI threads would be used), and the second issue is that it would inject purely platform dependent code back into a more abstract layer, i.e. the viewmodel.INotifyPropertyChanged
interface, plus again I'd argue that conceptually the UI thread dispatching should only be relegated to the actual platform dependent code. From the point of view of a .NET Standard component, individual threads should (generally) not matter much.The proposed solution is to completely flip this over and add a simple extension to the codegen for x:Bind
(pinging @MikeHillberg about this) to allow for a more general solution to this issue, built right into the framework itself.
The advantages here would be multiple, as mentioned above as well:
Consider this simple viewmodel (not implemented, it doesn't matter here):
public class MainPageViewModel : INotifyPropertyChanged
{
public event PropertyChangedEventHandler? PropertyChanged;
public string Text { get; set; }
}
And this XAML snippet:
<Page
x:Class="XBindSample.MainPage"
xmlns="http://schemas.microsoft.com/winfx/2006/xaml/presentation"
xmlns:x="http://schemas.microsoft.com/winfx/2006/xaml"
xmlns:local="using:XBindSample">
<Page.DataContext>
<local:MainPageViewModel x:Name="ViewModel"/>
</Page.DataContext>
<Grid>
<TextBlock Text="{x:Bind ViewModel.Text, Mode=OneWay}"/>
</Grid>
</Page>
If we build this and go to inspect the generated MainPage.g.cs
file, we'll find a number of classes with various responsabilities - from updating individual UI controls to tracking the bindings, etc. In particular, we're interested in this one:
[global::System.CodeDom.Compiler.GeneratedCodeAttribute("Microsoft.Windows.UI.Xaml.Build.Tasks"," 10.0.19041.1")]
[global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
private class MainPage_obj1_BindingsTracking
{
private global::System.WeakReference<MainPage_obj1_Bindings> weakRefToBindingObj;
public MainPage_obj1_BindingsTracking(MainPage_obj1_Bindings obj)
{
weakRefToBindingObj = new global::System.WeakReference<MainPage_obj1_Bindings>(obj);
}
public MainPage_obj1_Bindings TryGetBindingObject()
{
return null; // Removed for brevity
}
public void ReleaseAllListeners()
{
// Removed for brevity
}
public void PropertyChanged_ViewModel(object sender, global::System.ComponentModel.PropertyChangedEventArgs e)
{
MainPage_obj1_Bindings bindings = TryGetBindingObject();
if (bindings != null)
{
string propName = e.PropertyName;
global::XBindSynchronizationContextSample.MainPageViewModel obj = sender as global::XBindSynchronizationContextSample.MainPageViewModel;
if (global::System.String.IsNullOrEmpty(propName))
{
if (obj != null)
{
bindings.Update_ViewModel_Text(obj.Text, DATA_CHANGED);
}
}
else
{
switch (propName)
{
case "Text":
{
if (obj != null)
{
bindings.Update_ViewModel_Text(obj.Text, DATA_CHANGED);
}
break;
}
default:
break;
}
}
}
}
private global::XBindSynchronizationContextSample.MainPageViewModel cache_ViewModel = null;
public void UpdateChildListeners_ViewModel(global::XBindSynchronizationContextSample.MainPageViewModel obj)
{
if (obj != cache_ViewModel)
{
if (cache_ViewModel != null)
{
((global::System.ComponentModel.INotifyPropertyChanged)cache_ViewModel).PropertyChanged -= PropertyChanged_ViewModel;
cache_ViewModel = null;
}
if (obj != null)
{
cache_ViewModel = obj;
((global::System.ComponentModel.INotifyPropertyChanged)obj).PropertyChanged += PropertyChanged_ViewModel;
}
}
}
}
This is the type that actually subscribes to the PropertyChanged
event in our viewmodel and goes about updating the UI components. This is the only part that should need to care about the "UI thread", and this is the only part that should include the code to automatically deal with this for the user, both because it'd make all the rest of the code much simpler, and also because injecting the change here would be pretty efficient and with a small code change required.
In particular, the issue is when the PropertyChanged_ViewModel
handler is invoked, since that could be done from an other thread if the PropertyChanged
event was raised on another thread. To fix this, I propose the following:
private global::System.WeakReference<MainPage_obj1_Bindings> weakRefToBindingObj;
private readonly DispatcherQueue dispatcherQueue;
public MainPage_obj1_BindingsTracking(MainPage_obj1_Bindings obj)
{
weakRefToBindingObj = new global::System.WeakReference<MainPage_obj1_Bindings>(obj);
dispatcherQueue = DispatcherQueue.GetForCurrentThread();
}
public void PropertyChanged_ViewModel(object obj, global::System.ComponentModel.PropertyChangedEventArgs e)
{
if (dispatcherQueue.HasThreadAccess)
{
PropertyChanged_ViewModel_OnDispatcherQueue(obj, e);
}
else
{
PropertyChanged_ViewModel_Dispatch(obj, e);
}
}
[MethodImpl(MethodImplOptions.NoInlining)]
private void PropertyChanged_ViewModel_Dispatch(Object obj, global::System.ComponentModel.PropertyChangedEventArgs e)
{
_ = dispatcherQueue.TryEnqueue(() => PropertyChanged_ViewModel_OnDispatcherQueue(obj, e));
}
private void PropertyChanged_ViewModel_OnDispatcherQueue(object sender, global::System.ComponentModel.PropertyChangedEventArgs e)
{
MainPage_obj1_Bindings bindings = TryGetBindingObject();
if (bindings != null)
{
string propName = e.PropertyName;
global::XBindSynchronizationContextSample.MainPageViewModel obj = sender as global::XBindSynchronizationContextSample.MainPageViewModel;
if (global::System.String.IsNullOrEmpty(propName))
{
if (obj != null)
{
bindings.Update_ViewModel_Text(obj.Text, DATA_CHANGED);
}
}
else
{
switch (propName)
{
case "Text":
{
if (obj != null)
{
bindings.Update_ViewModel_Text(obj.Text, DATA_CHANGED);
}
break;
}
default:
break;
}
}
}
}
Here's the git diff:
+using Windows.System;
+
namespace XBindSynchronizationContextSample
{
partial class MainPage :
@@ -170,11 +172,13 @@ namespace XBindSynchronizationContextSample
[global::System.Diagnostics.DebuggerNonUserCodeAttribute()]
private class MainPage_obj1_BindingsTracking
{
+ private readonly DispatcherQueue dispatcherQueue;
public MainPage_obj1_BindingsTracking(MainPage_obj1_Bindings obj)
{
weakRefToBindingObj = new global::System.WeakReference<MainPage_obj1_Bindings>(obj);
+ dispatcherQueue = DispatcherQueue.GetForCurrentThread();
}
public MainPage_obj1_Bindings TryGetBindingObject()
@@ -198,6 +202,17 @@ namespace XBindSynchronizationContextSample
}
public void PropertyChanged_ViewModel(object sender, global::System.ComponentModel.PropertyChangedEventArgs e)
+ {
+ if (dispatcherQueue.HasThreadAccess)
+ {
+ PropertyChanged_ViewModel_OnDispatcherQueue(sender, e);
+ }
+ else
+ {
+ PropertyChanged_ViewModel_Dispatch(obj, e);
+ }
+ }
+
+ [MethodImpl(MethodImplOptions.NoInlining)]
+ private void PropertyChanged_ViewModel_Dispatch(Object obj, global::System.ComponentModel.PropertyChangedEventArgs e)
+ {
+ _ = dispatcherQueue.TryEnqueue(() => PropertyChanged_ViewModel_OnDispatcherQueue(obj, e));
+ }
+
+ public void PropertyChanged_ViewModel_OnDispatcherQueue(object sender, global::System.ComponentModel.PropertyChangedEventArgs e)
Now, this is just a proof of concept, but hopefully it illustrates the concept well enough 😄
| Capability | Priority |
| :---------- | :------- |
| Update the x:Bind
codegen to handle the UI thread dispatch | Must |
| Update internal handlers to handle UI thread dispatch (eg. CollectionChanged
) | Must |
| Introduce changes that would break existing codebases | Won't |
| Introduce overhead in current codebases or when the dispatch is not needed | Won't |
@marb2000 FYI
In my scenario, I have a web socket client that is shared across multiple windows. I see two options for displaying content I get from that connection: I could keep redundant instances of my models in the page associated with the window and update them on the page’s dispatcher through events fired by web socket messages, or I could share all the models throughout the windows. I would rather do the second, as it would be much simpler and keep everything consolidated. But here lies the problem—I want to bind my UI to those models.
Here’s what happens:
That was expected (but unwelcome). I don’t want my web socket class to have a concept of a UI thread, though. But what if I run the change on the Dispatcher of the page, anyway?
Dispatcher.RunAsync
Great. The UI has been updated. But now I open another window, and as soon as the web socket receives a new message, my app will crash. What happened?
Dispatcher.RunAsync
None of this is ideal (and I have no solution at all for binding to the same object from different windows), and I very much support changing the generated code to run property changes on the correct thread without having to manually call anything. That would solve my two problems: the improper separation of concerns (my web socket class should not care whether there is a UI thread or not) and the inability to update a property bound to UI on different threads.
For consistency I would expect same behaviour with old-style Binding as well.
Thank you for chiming in @BreeceW! Yes that's exactly one of the reasons why having this functionality be built-in would be great, and why handling the "UI thread" on the backend side (like some MVVM libraries do) is not only impractical and breaking the framework-agnostic architecture, but also still not a proper solutions if you have in fact more than one UI thread 👍
Just spotted a minor codegen issue in my original proposal, updating it now 😄
This part:
if (dispatcherQueue.HasThreadAccess)
{
PropertyChanged_ViewModel_OnDispatcherQueue(sender, e);
}
else
{
_ = dispatcherQueue.TryEnqueue(() => PropertyChanged_ViewModel_OnDispatcherQueue(sender, e));
}
Actually causes the C# compiler to always allocate the closure class used in the second branch, even when we don't actually need it at all. This would introduce unnecessary allocations even when the dispatch is not actually done, see:
<>c__DisplayClass1_0 <>c__DisplayClass1_ = new <>c__DisplayClass1_0();
<>c__DisplayClass1_.<>4__this = this;
<>c__DisplayClass1_.obj = obj;
<>c__DisplayClass1_.e = e;
if (dispatcherQueue.HasThreadAccess)
{
PropertyChanged_ViewModel_OnDispatcherQueue(<>c__DisplayClass1_.obj, <>c__DisplayClass1_.e);
}
else
{
dispatcherQueue.TryEnqueue(new Action(<>c__DisplayClass1_.<PropertyChanged_ViewModel>b__0));
}
The simple solution is just to move the dispatch in a separate, non-inlined method, like so:
public void PropertyChanged_ViewModel(object obj, global::System.ComponentModel.PropertyChangedEventArgs e)
{
if (dispatcherQueue.HasThreadAccess) PropertyChanged_ViewModel_OnDispatcherQueue(obj, e);
else PropertyChanged_ViewModel_Dispatch(obj, e);
}
[MethodImpl(MethodImplOptions.NoInlining)]
private void PropertyChanged_ViewModel_Dispatch(Object obj, global::System.ComponentModel.PropertyChangedEventArgs e)
{
_ = dispatcherQueue.TryEnqueue(() => PropertyChanged_ViewModel_OnDispatcherQueue(obj, e));
}
Which results in the correct codegen here:
public void PropertyChanged_ViewModel(object obj, PropertyChangedEventArgs e)
{
if (dispatcherQueue.HasThreadAccess)
{
PropertyChanged_ViewModel_OnDispatcherQueue(obj, e);
}
else
{
PropertyChanged_ViewModel_Dispatch(obj, e);
}
}
[MethodImpl(MethodImplOptions.NoInlining)]
private void PropertyChanged_ViewModel_Dispatch(object obj, PropertyChangedEventArgs e)
{
<>c__DisplayClass2_0 <>c__DisplayClass2_ = new <>c__DisplayClass2_0();
<>c__DisplayClass2_.<>4__this = this;
<>c__DisplayClass2_.obj = obj;
<>c__DisplayClass2_.e = e;
dispatcherQueue.TryEnqueue(new Action(<>c__DisplayClass2_.<PropertyChanged_ViewModel_Dispatch>b__0));
}
Where the overhead/allocation is _only_ paid if we do need to dispatch (which is expected anyway).
In all cases where we're already on the right thread, the performance would be just like before 🚀
Thank you for this @Sergio0694. We would be able to look into this more post WinUI 3.0. I will leave this open for now.
Most helpful comment
In my scenario, I have a web socket client that is shared across multiple windows. I see two options for displaying content I get from that connection: I could keep redundant instances of my models in the page associated with the window and update them on the page’s dispatcher through events fired by web socket messages, or I could share all the models throughout the windows. I would rather do the second, as it would be much simpler and keep everything consolidated. But here lies the problem—I want to bind my UI to those models.
Here’s what happens:
That was expected (but unwelcome). I don’t want my web socket class to have a concept of a UI thread, though. But what if I run the change on the Dispatcher of the page, anyway?
Dispatcher.RunAsync
Great. The UI has been updated. But now I open another window, and as soon as the web socket receives a new message, my app will crash. What happened?
Dispatcher.RunAsync
None of this is ideal (and I have no solution at all for binding to the same object from different windows), and I very much support changing the generated code to run property changes on the correct thread without having to manually call anything. That would solve my two problems: the improper separation of concerns (my web socket class should not care whether there is a UI thread or not) and the inability to update a property bound to UI on different threads.