WUX/MUX prototype #1421
WUX/MUX prototype #1421
Conversation
…elected WUX vs MUX mode.
…t a runtime configuration WUX/MUX switch.
… of the consumption logic in the process)
… INPC and types that need the XAML engine initialized on the thread.
jkoritzinsky
changed the title
|
Follow-up notes from @Sergio0694 in the other PR: We'll need to react to #1395 and have a separate manifest for AOT that only specifies the "latest tested OS version" to allow the Xaml Islands APIs. Also, we should hook up a linker substitution file to hard-code the UiXamlMode switch when trimming or AOT compiling. |
|
Wanted to clarify the comment I made in #1418. The |
src/WinRT.Runtime/Projections/NotifyCollectionChangedEventHandler.cs
Outdated
Show resolved
Hide resolved
Given this is a single, well known case, perhaps we can special case it so that when |
Honestly, some manual type identity nonsense isn't the worst idea. I'll try that out in this PR and see what I can do. |
jkoritzinsky
force-pushed
the
users/jkoritzinsky/wux-mux
branch
from
6db6128
to
c873dd7
Compare
…elected WUX vs MUX mode. Implement the base TODO-WuxMux items that must be completed to support a runtime configuration WUX/MUX switch. Add WUX test projection and fix bug with ICommand special-casing. Inline MUX guids. Add CsWinRT SDK support Add tests for authoring components with WUX APIs (also validates some of the consumption logic in the process) Remove Debug.Breaks Change approach so WUX/MUX works for delegates Use Xaml Islands and a custom main to enable writing GTest tests with INPC and types that need the XAML engine initialized on the thread. Add AuthoringWuxComsumptionTest to the CI Change IIDOptimizer to fall back to GuidGenerator for WUX/MUX types. Fix IList ABI type to implement WUX and MUX Add breaks PR feedback and fix unit test failures. Fix configurations for new projects. Fix configuration for Windows.UI.Xaml projection
jkoritzinsky
force-pushed
the
users/jkoritzinsky/wux-mux
branch
from
c873dd7
to
21b99e5
Compare
| @@ -170,6 +174,13 @@ public SignaturePart GetSignatureParts(TypeReference type) | |||
| return new RuntimeClassSignature(type, GetSignatureParts(iface)); | |||
| } | |||
|
|
|||
| // For types projected from WUX or MUX into .NET, we'll need to do a runtime lookup for the IID. | |||
| // TODO-WuxMux: We can instead take an option in the IID optimizer to hard-code the lookup for WUX or MUX when specified, which would be more efficient for scenarios where this is possible. | |||
There was a problem hiding this comment.
I think is definitely worth doing, also because I don't think we'd want this new support for WUX projections to introduce a performance regression in these cases for exiting MUX consumers. It should be pay-for-play ideally, or in this case, just free. I'd assume that we'd always have this info anyway, since by the time the IID optimizer runs on a given application assembly, you'd always have the WUX/MUX flag set to either of the two options? 🤔
There was a problem hiding this comment.
I think we can do it, but since it's not needed for the MVP of this feature, I'd rather not fight MSBuild and the command line to get it working.
I don't remember if we run the IID optimizer on library assemblies too though, and I think you could build a library that works with both WUX and MUX if you're writing it in C# and it only uses the mapped WUX/MUX types.
| { "System.Collections.Generic.IDictionary`2", new MappedType("Windows.Foundation.Collections", "IMap`2", "Windows.Foundation.FoundationContract") }, | ||
| { "System.Collections.Generic.IReadOnlyList`1", new MappedType("Windows.Foundation.Collections", "IVectorView`1", "Windows.Foundation.FoundationContract") }, | ||
| { "System.Collections.Generic.IList`1", new MappedType("Windows.Foundation.Collections", "IVector`1", "Windows.Foundation.FoundationContract") }, | ||
| { "Windows.UI.Color", new MappedType("Windows.UI", "Color", "Windows.Foundation.UniversalApiContract", true, true) }, |
There was a problem hiding this comment.
We still have a separate TODO to eventually find a way to fix the conflicts around the Color type, right?
There was a problem hiding this comment.
Yes, but that applies to the authoring of the projection assembly itself, not the basic Wux/Mux switch (so I'd rather track it separately).
| } | ||
|
|
||
| [DynamicInterfaceCastableImplementation] | ||
| [EditorBrowsable(EditorBrowsableState.Never)] | ||
| [Guid("530155E1-28A5-5693-87CE-30724D95A06D")] | ||
| [Guid("530155E1-28A5-5693-87CE-30724D95A06D")] |
There was a problem hiding this comment.
Should we not remove the [Guid] attribute on all types with [WuxMuxProjectedType]?
There was a problem hiding this comment.
I'd rather leave it as otherwise we'd get an effectively-randomly-generated guid for Type.GUID instead of maintaining existing behavior.
|
|
||
| private static ReadOnlySpan<byte> GetIID() | ||
| => FeatureSwitches.IsWuxMode | ||
| ? new(new byte[] { 0xd5, 0x67, 0xb1, 0x28, 0x31, 0x1a, 0x5b, 0x46, 0x9b, 0x25, 0xd5, 0xc3, 0xae, 0x68, 0x6c, 0x40 }) |
There was a problem hiding this comment.
Need to leave the ROS<byte> cast in all of these, otherwise we allocate an array instead of using RVA fields. Leaving this comment here but applies to all such IID initialization paths introduced in this PR, for all WUX/MUX types.
…ux and it was causing failures in AOT scenarios to have them split)
jkoritzinsky
force-pushed
the
users/jkoritzinsky/wux-mux
branch
from
ca2abfa
to
24ed428
Compare
| public static IntPtr AbiToProjectionVftablePtr => INotifyCollectionChanged.Vftbl.AbiToProjectionVftablePtr; | ||
| } | ||
|
|
||
| public static global::System.Guid IID { get; } = GuidGenerator.GetWuxMuxIID(typeof(INotifyCollectionChanged).GetCustomAttribute<WuxMuxProjectedTypeAttribute>()); |
There was a problem hiding this comment.
Can we not hardcode this by switching on the flag here too, to avoid the attribute lookup?
| } | ||
|
|
||
| [DynamicInterfaceCastableImplementation] | ||
| [EditorBrowsable(EditorBrowsableState.Never)] | ||
| [Guid("530155E1-28A5-5693-87CE-30724D95A06D")] | ||
| [Guid("530155E1-28A5-5693-87CE-30724D95A06D")] | ||
| [WuxMuxProjectedType(wuxIID: "CF75D69C-F2F4-486B-B302-BB4C09BAEBFA", muxIID: "530155E1-28A5-5693-87CE-30724D95A06D")] |
There was a problem hiding this comment.
Do we actually need this, if we hardcode this above? Same for the vftbl type below.
| { | ||
| public static global::System.Guid IID { get; } = new Guid(new global::System.ReadOnlySpan<byte>(new byte[] { 0x01, 0x76, 0xB1, 0x90, 0x65, 0xB0, 0x6E, 0x58, 0x83, 0xD9, 0x9A, 0xDC, 0x3A, 0x69, 0x52, 0x84 })); | ||
| { | ||
| public static global::System.Guid IID { get; } = GuidGenerator.GetWuxMuxIID(typeof(INotifyPropertyChanged).GetCustomAttribute<WuxMuxProjectedTypeAttribute>()); |
There was a problem hiding this comment.
Not sure why the comment here was resolved, I'm still seeing the attribute lookup? Or do you just plan on addressing all of these later when you get around to rebasing this PR? I won't leave other comments about this for now 😄
| @@ -92,6 +97,7 @@ private static unsafe int Do_Abi_add_PropertyChanged_0(IntPtr thisPtr, IntPtr ha | |||
| *token = default; | |||
| try | |||
| { | |||
| global::System.Diagnostics.Debugger.Launch(); | |||
There was a problem hiding this comment.
This also was resolved but the leftover Debugger.Launch() call is still here? 😅
There was a problem hiding this comment.
I think I had some lost commits between rebases and different machines, sorry.
|
|
||
|
|
||
|
|
||
| [global::WinRT.ObjectReferenceWrapper(nameof(_obj))] |
There was a problem hiding this comment.
This attribute should only be used downlevel. On .NET, shouldn't we also implement IWinRTObject here?
| @@ -41,28 +42,32 @@ static unsafe NotifyCollectionChangedEventHandler() | |||
| var nativeVftbl = ComWrappersSupport.AllocateVtableMemory(typeof(NotifyCollectionChangedEventHandler), sizeof(global::WinRT.Interop.IDelegateVftbl)); | |||
| Marshal.StructureToPtr(AbiToProjectionVftable, nativeVftbl, false); | |||
| AbiToProjectionVftablePtr = nativeVftbl; | |||
|
|
|||
| IID = Projections.UiXamlModeSetting == Projections.UiXamlMode.WindowsUiXaml | |||
There was a problem hiding this comment.
Nit: this should also just be a switch with two RVA fields, rather than parsing at runtime
…inRT into wux-mux
…ions of the same namespace are emitted into multiple different projections, and only one should define the addition types)
|
What does WUX/MUX stand for please? |
|
Windows.UI.Xaml (System XAML) + Microsoft.UI.Xaml (WinUI 3)
|
Implement prototype WUX/MUX support for both consumption and authoring.
The remaining TODO-WuxMux items will provide a better UX or perf but are not required for basic functionality.
Replacement for #1418 that will get CI.