microsoft / CsWinRT

C# language projection for the Windows Runtime
MIT License
556 stars 107 forks source link

Proposal: APIs to easily get the ABI for an activation factory #1325

Closed Sergio0694 closed 3 months ago

Sergio0694 commented 1 year ago

Proposal: API to easily get the ABI for an activation factory

Summary

Currently, there is no easy way to get the ABI for a given activation factory with CsWinRT. This is a relatively common thing to need, as there are times where the activation factory also implements some interop interface that exposes some additional functionality. For instance, Win2D does this, where the activation factory for CanvasDevice also implements the ICanvasFactoryNative interface defined in the public header (see https://github.com/microsoft/Win2D/issues/910) which then exposes several lowlevel APIs (source here).

To get the ABI for an activation factory with CsWinRT today, since ActivationFactory<T> is internal, you need to declare a dummy interface for the activation factory, call SomeType.As<Interface>, and then marshal that interface to get the ABI. This is not intuitive at all, unnecessarily verbose (as you need to declare one dummy interface) and also not that fast, since you need to marshal that whole managed wrapper for nothing. The proposal is about adding first-class support for just getting the ABI of some activation factory.

namespace WinRT;

public static class MarshalInspectable<T>
{
    public static ObjectReferenceValue CreateActivationFactoryMarshaler2();
    public static ObjectReferenceValue CreateActivationFactoryMarshaler2(Guid iid);
    public static IntPtr GetActivationFactoryAbi();
    public static IntPtr GetActivationFactoryAbi(Guid iid);
}

Use case example

Compare what you currently have to do to get the ABI for ICanvasFactoryNative:

[Guid("695C440D-04B3-4EDD-BFD9-63E51E9F7202")]
[WindowsRuntimeType]
[WindowsRuntimeHelperType(typeof(Interface))]
public interface Interface
{
    [Guid("695C440D-04B3-4EDD-BFD9-63E51E9F7202")]
    public readonly struct Vftbl
    {
        public static readonly IntPtr AbiToProjectionVftablePtr = IUnknownVftbl.AbiToProjectionVftblPtr;
    }
}

ICanvasFactoryNative.Interface canvasDeviceActivationFactory = CanvasDevice.As<ICanvasFactoryNative.Interface>();

using ComPtr<ICanvasFactoryNative> factoryNative = default;

factoryNative.Attach((ICanvasFactoryNative*)MarshalInspectable<ICanvasFactoryNative.Interface>.FromManaged(canvasDeviceActivationFactory));

Compared to how simple it'd be with the new APIs:

using ComPtr<ICanvasFactoryNative> factoryNative = default;

factoryNativeUnknown.Attach((IUnknown*)MarshalInspectable<CanvasDevice>.GetActivationFactoryAbi(__uuidof<ICanvasFactoryNative>()));

Rationale

jkoritzinsky commented 1 year ago

I think a cleaner alternative design would be to expose ActivationFactory<T> or something similar to it instead of exposing members on the MarshalInspectable type (as that type is meant to only be used to marshal objects).

Sergio0694 commented 1 year ago

More than happy to update the proposal with a different API surface if you think it'd be best! 😄 Feel free to write down a draft as well if you want and I'll include it as alternative designs.

manodasanW commented 1 year ago

If the goal here is to be able to call functions for interop interfaces implemented on activation factories, I would say we should make it easier than having someone write out all the vftbl logic. Someone should be able to write out the interop interface in C# and then put an attribute like WinRT.Interop.InteropInterface or something similar and then CsWinRT could use like a source generator to generate all the respective vftbl code and implementation for calling functions in the interface.

Then a developer consuming an interop function would just need to do CanvasDevice.As<ICanvasFactory>().function() after defining the interface with the attribute. We today do support a similar model using the built-in COM support if you put the ComImport attribute on the interface but that probably isn't trimming / AOT friendly so we should probably have our own attribute supported by source generation. Thoughts?

jkoritzinsky commented 1 year ago

I think if you're looking to be trimming/AOT friendly, then you should look at interoperating with the new GeneratedComInterfaceAttribute in .NET 8 Preview 4 and use the existing ComImport attribute downlevel (which is AOT-compatible in UWP-like scenarios) instead of making a whole new source generator. That won't provide downlevel AOT-compatibility in .NET 6, but it would handle all future versions of .NET and significantly reduce the cost.

Sergio0694 commented 1 year ago

"Someone should be able to write out the interop interface in C# and then put an attribute like WinRT.Interop.InteropInterface or something similar and then CsWinRT could use like a source generator to generate all the respective vftbl code and implementation for calling functions in the interface."

Leaving the source generator part aside for a minute, since that's not available yet, isn't this already doable today? Eg:

var activationFactoryNative = CanvasDevice.As<ICanvasFactoryNative>();

[Guid("695C440D-04B3-4EDD-BFD9-63E51E9F7202")]
[WindowsRuntimeType]
[WindowsRuntimeHelperType(typeof(ICanvasFactoryNative))]
public interface ICanvasFactoryNative
{
    [Guid("695C440D-04B3-4EDD-BFD9-63E51E9F7202")]
    public unsafe struct Vftbl
    {
        public static readonly IntPtr AbiToProjectionVftablePtr = InitVtbl();

        private static IntPtr InitVtbl()
        {
            Vftbl* lpVtbl = (Vftbl*)ComWrappersSupport.AllocateVtableMemory(typeof(Vftbl), sizeof(Vftbl));

            lpVtbl->IUnknownVftbl = IInspectable.Vftbl.AbiToProjectionVftable;

            return (IntPtr)lpVtbl;
        }

        private IInspectable.Vftbl IUnknownVftbl;
    }
}

That is, [WindowsRuntimeHelperType] lets you do basically that. Of course, if you also want to call methods on this from the RCW then you need to manually add all those stubs, so a generator would certainly be nice, but in general it seems the support for this is already in place. I suppose this would cover the CreateActivationFactoryMarshaler2 proposed APIs.

One point that isn't covered though is GetActivationFactoryAbi. That is, it's kinda annoying (and verbose and error prone) to have to declare one dummy interface just so you can get that RCW and then get the ABI from it. Would it make sense to expose a way to directly get the IUnknown* value for the activation factory of a given type (possibly through a QI to a target Guid)? If this is not something that you think is worth adding, no problems with that, just figured I'd ask 🙂

"I think if you're looking to be trimming/AOT friendly, then you should look at interoperating with the new GeneratedComInterfaceAttribute"

This sounds great, and just relying on those would seem the best path forwards. Should we open a different issue to track what changes and/or new APIs would be needed from either .NET or CsWinRT to properly bridge that gap between the two?

Sergio0694 commented 12 months ago

@jlaanstra do you think this could fit into the work you're doing around activation factories? The current pattern is very clunky and quite inefficient, as you need to declare your own dummy interface and vtable, and then do TheType.As<MyInterface>(), which not just creates a bunch of unnecessary managed wrappers in doing so, but also I think triggers some dynamic interface castable fallback path, as I'm seeing this path which I think is taken?

https://github.com/microsoft/CsWinRT/blob/71169932920500310921b4c3fadba1e5a99c6138/src/WinRT.Runtime/ObjectReference.cs#L164

Especially if you don't even need a managed reference at all (which is the case if you need to get access to some interop interface), virtually all of this is just unnecessary work and lots of overhead for nothing. Could we maybe add an API like:

namespace WinRT;

public static class ActivationFactory
{
    public static IntPtr GetAbi(string typeName);
}

Then you could just do this:

using ComPtr<IUnknown> factoryUnknown = default;

factoryUnknown.Attach((IUnknown*)ActivationFactory.GetAbi("Microsoft.Graphics.Canvas.CanvasDevice"));

using ComPtr<ICanvasFactoryNative> factoryNative = default;

factoryUnknown.CopyTo(factoryNative.GetAddressOf());

And you're good to go, no extra interfaces needed, no unnecessary RCWs being allocated or anything like that. Thoughts? 🙂

jlaanstra commented 12 months ago

@jlaanstra do you think this could fit into the work you're doing around activation factories? The current pattern is very clunky and quite inefficient, as you need to declare your own dummy interface and vtable, and then do TheType.As<MyInterface>(), which not just creates a bunch of unnecessary managed wrappers in doing so, but also I think triggers some dynamic interface castable fallback path, as I'm seeing this path which I think is taken?

https://github.com/microsoft/CsWinRT/blob/71169932920500310921b4c3fadba1e5a99c6138/src/WinRT.Runtime/ObjectReference.cs#L164

Especially if you don't even need a managed reference at all (which is the case if you need to get access to some interop interface), virtually all of this is just unnecessary work and lots of overhead for nothing. Could we maybe add an API like:

namespace WinRT;

public static class ActivationFactory
{
    public static IntPtr GetAbi(string typeName);
}

Then you could just do this:

using ComPtr<IUnknown> factoryUnknown = default;

factoryUnknown.Attach((IUnknown*)ActivationFactory.GetAbi("Microsoft.Graphics.Canvas.CanvasDevice"));

using ComPtr<ICanvasFactoryNative> factoryNative = default;

factoryUnknown.CopyTo(factoryNative.GetAddressOf());

And you're good to go, no extra interfaces needed, no unnecessary RCWs being allocated or anything like that. Thoughts? 🙂

Let me see how this could fit in.

Sergio0694 commented 12 months ago

Thank you! Also on a related note: could this be another argument in favor of removing all of the As<T>() generated methods on projection types entirely? If it is the case that they are so inefficient and also cause code to go through the slow reflection based dynamic interface castable path, it seems bad to make such functionality so easily accessible and visible, and not clearly indicating that it has such overhead and that it relies on stuff that's also not really trim/AOT-friendly?

FYI @manodasanW

Sergio0694 commented 10 months ago

Leaving a note here too so I don't forget. Just copy pasting from Teams basically. I was looking at the new APIs and thinking this might be doable once #1390 is merged? Basically I'm thinking, once ActivationFactory is public, one could just do this.

OLD CODE

IMyInterface factory = TheType.As<IMyInterface>();

*nativeType = (NativeType*)MarshalInterface<IMyInterface>.FromManaged(factory);

NEW CODE

using IObjectReference factory = ActivationFactory.Get("My.Type");

factory.TryAs(NativeTypeIID, out *nativeType).Assert();

For context, I need this here. I think this would work and resolve the issue, right? 🤔

Sergio0694 commented 9 months ago

For context, the functionality was added in #1390 🎉

using IObjectReference factory = ActivationFactory.Get("My.Type", NativeTypeIID);

IntPtr factoryNative = factory.GetRef();

We can close this once we merge the staging branch into master.