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

Document implementing custom types with WinRT/COM interfaces mix #1283

Open
Sergio0694 opened this issue Dec 19, 2022 · 7 comments
Open

Document implementing custom types with WinRT/COM interfaces mix #1283

Sergio0694 opened this issue Dec 19, 2022 · 7 comments
Labels
documentation Improvements or additions to documentation enhancement New feature or request

Comments

@Sergio0694
Copy link
Member

Overview and context

I'm working with the Win2D team to add new APIs to allow developers to implement custom D2D effects that can interop with Win2D. This is a new feature which wasn't available before, as Win2D only supported built-in effects, and it's meant for advanced scenarios. That is, developers implementing these effects are meant to provide an implementation for the new COM interfaces that will be exposed by Win2D, whereas consumers of those custom effects should be able to use these effects just as easily as any other built-in Win2D effect. I'm working on then offering support for this through my library ComputeSharp, which specifically will provide two base types for developers to use to implement custom effects, and will take care of all the interop work for them.

For reference:

Note: the first PR is closed just because that was a public copy, the internal PR for microsoft/Win2D is now completed.

In a nutshell, developers implementing effects have to implement these two interfaces:

  • ICanvasImage, which is a WinRT interface
  • ICanvasImageInterop (privately, if they want to), which is a COM interface

On UWP, this is pretty straightforward:

  • Implement ICanvasImage from Win2D
  • Define ICanvasImageInterop as a [ComImport] interface, implement this too

Then everything works exactly as you'd expect: passing an instance of this object to Win2D would result in it receiving a pointer for the CCW which also supports QueryInterface for ICanvasImageInterop, which Win2D can then use as well. For developers needing to access the CCW manually, they can just use Marshal.GetIUnknownForObject on the managed instance and then manually QueryInterface for anything they need from it, and all the interfaces will be there. This is also extremely efficient on .NET Native due to MCG generating all marshalling stubs with blittable signatures for AOT.

The actual proposal

I'm opening this issue to ask:

  • Is there guidance on how can/should developers approach a situation like this in the new CsWinRT world? Is there a recommended approach on how to author a type that implements both a WinRT and a COM interface so that it can be seamlessly passed on a WinRT API like it can on UWP? I'd love to support WinUI 3 as well for all this new Win2D/ComputeSharp work, but I need to understand how I should adopt my public wrappers to enable this there as well.
  • Are any new APIs needed to support this scenario? If so, what would an exact API surface even look like?
  • In the context of trimming/AOT support with all the new ComWrappers support and upcoming source generators, should/could we also consider providing support for and/or documentation for this scenario that doesn't involve [ComImport] at all, but instead does all that's necessary strictly without triggering built-in COM support on any code paths?
  • Should we also consider writing some docs on this so that people in this situation can easily reference them?

cc. @manodasanW

Also cc. @jkoritzinsky @AaronRobinsonMSFT for the COM interop side from .NET

(and shout out to Jeremy for all his help gathering all this info to write this issue, and for explaining lots of this stuff 😄)

@Sergio0694 Sergio0694 added the enhancement New feature or request label Dec 19, 2022
@Sergio0694
Copy link
Member Author

Sergio0694 commented Dec 31, 2022

I've played around with this a bit (unsuccessfully), figured I'd leave some notes here for future reference 🙂

I've followed @jkoritzinsky's suggestion and tried to use ICustomQueryInterface on the managed type to handle the additional COM interface I wanted to add. In this example, I'm referencing Microsoft.Graphics.Win2D 1.0.24:

public sealed class MyInteropClass : ICanvasImage, ISomeComInterface, ICustomQueryInterface
{
    // ICanvasImage
    void IDisposable.Dispose() => throw new NotImplementedException();
    Rect ICanvasImage.GetBounds(ICanvasResourceCreator resourceCreator) => throw new NotImplementedException();
    Rect ICanvasImage.GetBounds(ICanvasResourceCreator resourceCreator, Matrix3x2 transform) => throw new NotImplementedException();

    // ISomeComInterface
    unsafe int ISomeComInterface.Foo() => throw new NotImplementedException();

    unsafe CustomQueryInterfaceResult ICustomQueryInterface.GetInterface(ref Guid iid, out IntPtr ppv)
    {
        if (iid == typeof(ISomeComInterface).GUID)
        {
            using ComPtr<IUnknown> unknown = default;

            unknown.Attach((IUnknown*)Marshal.GetIUnknownForObject(this));

            fixed (Guid* pGuid = &iid)
            fixed (IntPtr* pPpv = &ppv)
            {
                Marshal.ThrowExceptionForHR(unknown.Get()->QueryInterface(pGuid, (void**)pPpv));
            }
        }

        ppv = default;
        return CustomQueryInterfaceResult.NotHandled;
    }
}

[ComImport]
[Guid("8594FED7-C427-4D5D-AF16-213B1B02DB2B")]
[InterfaceType(ComInterfaceType.InterfaceIsIUnknown)]
internal unsafe interface ISomeComInterface
{
    [PreserveSig]
    int Foo();
}

You can see my type is implementing both ICanvasImage (a WinRT interface from the Win2D package) as well as ISomeComInterface, which is a plain COM interface. I'm then trying to implement ICustomQueryInterface to handle the case where the IID of this COM interface is requested. The goal is that QueryInterface for the COM interface works on the CCW produced by WinRT for any of the WinRT interfaces.

In other words, this should work::

MyInteropClass myInteropClass = new();

// Get ABI for ICanvasImage (this is what WinRT autogenerates)
IObjectReference objectReference = MarshalInterface<ICanvasImage>.CreateMarshaler(myInteropClass);
IntPtr thisPtr = MarshalInterface<ICanvasImage>.GetAbi(objectReference);

Guid guid = typeof(ICanvasImageInterop).GUID;
void* someComInterfacePtr;

HRESULT hresult = ((IUnknown*)thisPtr)->QueryInterface(&guid, &someComInterfacePtr);

// hresult is S_OK and the operation succeeds

The implementation above doesn't seem to work though:

  • If I don't implement ICustomQueryInterface, then the extra COM interface just isn't present in the CCW at all.
  • If I do implement ICustomQueryInterface like in the code above, then that QueryInterface call on the input IID will just cause the ICustomQueryInterface to recursively being invoked, so the whole thing just results in a stack overflow 🤦‍♂️

A couple thoughts:

  • Is there a way to make this work at all? Am I just implementing ICustomQueryInterface wrong maybe?
  • Like I mentioned in the first comment, even if this worked it'd still rely on runtime COM support. It'd be nice if it didn't.

@BenJKuhn BenJKuhn added the documentation Improvements or additions to documentation label Jan 10, 2023
@dotMorten
Copy link

dotMorten commented Jan 11, 2023

I second this. I was looking into implement IMediaExtension from C#, and so far haven't been able to get there. @Sergio0694's post above got me one step further, but I do see the same stack overflow issue.
I did notice the old Marshal.GetComInterfaceForObject has a second parameter where you can set CustomQueryInterfaceMode.Ignore that avoids the recursive loop and makes this work, but the new Marshal.FromManaged doesn't have that.

@kant2002
Copy link
Contributor

Can you guys have ThreadLocal<bool> which indicates that you inside your custom ICustomQueryInterface and then opt-out of returning your value. That's inefficient, but probably move you forward one bit.

@Sergio0694
Copy link
Member Author

@dotMorten to clarify, that attempt was more of a "let's see if this works to at least get unblocked in the meantime", but it was never meant to be a long term solution even if it had worked, as it has some obvious issues (it relies on built-in COM support, so it's immediately a complete non starter on NativeAOT). Ideally we'd want proper, documented support for this that's also trimmable and AOT friendly. The question is just: is this doable and just undocumented, or do we need new APIs to enable this. So let's see what CsWinRT folks say once they have time to chime in here 🙂

"Can you guys have ThreadLocal<bool> which indicates that you inside your custom ICustomQueryInterface and then opt-out of returning your value. That's inefficient, but probably move you forward one bit."

@kant2002 That might potentially work, but it's definitely a hack and not something I'd want to ship to production 😅

@Sergio0694
Copy link
Member Author

Ran another (failed) experiment, leaving it here for future reference. I followed @jkoritzinsky's advice of trying to replicate what CsWinRT is doing for some manual projections (eg. IDisposable), and I came up with this test. Consider this interface:

namespace ABI.MyApis
{
    [WindowsRuntimeHelperType(typeof(MyInterface))]
    [Guid("670E4C85-AD86-4932-8805-186B626D13E5")]
    internal unsafe interface MyInterface
    {
        [Guid("670E4C85-AD86-4932-8805-186B626D13E5")]
        public struct Vftbl
        {
            public IUnknownVftbl IUnknownVftbl;
            public delegate* unmanaged<IntPtr, int> FooPtr;

            public static readonly Vftbl AbiToProjectionVftable;
            public static readonly IntPtr AbiToProjectionVftablePtr;


            static unsafe Vftbl()
            {
                AbiToProjectionVftable = new Vftbl
                {
                    IUnknownVftbl = IUnknownVftbl.AbiToProjectionVftbl,
                    FooPtr = (delegate* unmanaged<IntPtr, int>)&Foo
                };

                void** vftbl = (void**)ComWrappersSupport.AllocateVtableMemory(typeof(Vftbl), sizeof(void*) * 4);

                *(Vftbl*)vftbl = AbiToProjectionVftable;

                AbiToProjectionVftablePtr = (IntPtr)vftbl;
            }


            [UnmanagedCallersOnly]
            private static unsafe int Foo(IntPtr thisPtr)
            {
                try
                {
                    global::WinRT.ComWrappersSupport.FindObject<MyInterface>(thisPtr).Foo();
                }
                catch (global::System.Exception __exception__)
                {
                    global::WinRT.ExceptionHelpers.SetErrorInfo(__exception__);

                    return global::WinRT.ExceptionHelpers.GetHRForException(__exception__);
                }

                return 0;
            }
        }

        void Foo();
    }
}

And this C# type:

internal class MyClass : ICanvasImage, MyInterface
{
    // MyInterface
    public void Foo() => Console.WriteLine("FOO called");

    // ICanvasImage
    public void Dispose() => throw new NotImplementedException();
    public Rect GetBounds(ICanvasResourceCreator resourceCreator) => throw new NotImplementedException();
    public Rect GetBounds(ICanvasResourceCreator resourceCreator, Matrix3x2 transform) => throw new NotImplementedException();
}

Here I'm implementing both ICanvasImage (just as an example of some WinRT interface), and MyInterface. Then:

MyClass myClass = new();

// Get ABI for ICanvasImage
IObjectReference objectReference = MarshalInterface<ICanvasImage>.CreateMarshaler(myClass);
IntPtr thisPtr = MarshalInterface<ICanvasImage>.GetAbi(objectReference);

Guid guid = typeof(MyInterface).GUID;
IntPtr someComInterfacePtr;

// Try to QI on the IUnknown for ICanvasImage, for MyInterface.
// This just returns E_NOINTERFACE ❌
var hresult = (*(IUnknownVftbl**)thisPtr)->QueryInterface(thisPtr, &guid, &someComInterfacePtr);

IntPtr myInterfacePtr = MarshalInterface<MyInterface>.GetAbi(objectReference);

// This just straight up AVs ❌
_ = (*(MyInterface.Vftbl**)myInterfacePtr)->FooPtr(test);

So essentially:

  • QueryInterface on the CCW still doesn't work for MyInterface
  • Somehow MarshalInterface<MyInterface>.GetAbi does return a pointer, but calling Foo on it AVs
  • I put a breakpoint in MyInterface.Vftbl's static constructor, this is just never hit at all

I think this should be somewhat on the right direction, but I just couldn't for the love of me get it to work 😅

@Sergio0694
Copy link
Member Author

Sergio0694 commented Feb 19, 2023

Alright, I think I finally figured this out! 🎉
I was missing the [WindowsRuntimeType] attribute, which seems to be necessary for some reason. This is a bit weird given that the interface in question is not a WinRT type at all (it's just a COM type), but it seems CsWinRT needs the attribute to work at all.

This is an example of a minimal COM interface:

namespace ABI.MyApp;

[Guid("571CC6A0-8160-488E-9CE7-71C60F968711")]
[WindowsRuntimeType]
[WindowsRuntimeHelperType(typeof(IMyInterface))]
public interface IMyInterface
{
    [Guid("571CC6A0-8160-488E-9CE7-71C60F968711")]
    public struct Vftbl
    {
        public static readonly IntPtr AbiToProjectionVftablePtr = InitVtbl();

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

            lpVtbl->IUnknownVftbl = IUnknownVftbl.AbiToProjectionVftbl;
            lpVtbl->Foo = &FooFromAbi;

            return (IntPtr)lpVtbl;
        }

        internal IUnknownVftbl IUnknownVftbl;
        internal delegate* unmanaged[Stdcall]<IntPtr, int> Foo;

        [UnmanagedCallersOnly(CallConvs = new[] { typeof(CallConvStdcall) })]
        private static int FooFromAbi(IntPtr thisPtr)
        {
            try
            {
                return ComWrappersSupport.FindObject<IMyInterface>(thisPtr).Foo();
            }
            catch (Exception e)
            {
                return Marshal.GetHRForException(e);
            }
        }
    }

    int Foo();
}

A couple notes:

  • The ABI type and helper type here are the same interface. You can also split this into two if you prefer.
  • In this case I'm only supporting internal implementations, as the interface isn't public for me. If you do need to also support this for getting RCWs from arbitrary native objects, you'll also need to add DIMs stubs for the interface methods.

@manodasanW a few notes and feedbacks I gathered from this:

  • We definitely need proper docs about all of this
  • Why is [WindowsRuntimeType] needed if the type isn't a WinRT type? It seems a bit unintuitive.
  • I really dislike how the whole thing is just duck-typed, mostly for two reasons:
    • It makes it extremely difficult to wire up (as you have no compile time validation or support whatsoever)
    • The whole thing is not really trimming-friendly. It's "safe" thanks to annotations, yes, but:
      1. Too much stuff is preserved
      2. The reflection metadata is preserved, not just the actual methods/types
      3. While it does work, using reflection still isn't exactly the fastest approach ever

Are there any plans to eventually ditching this reflection-based approach entirely (at least for scenarios where dynamic loading isn't needed), and instead use something relying either on static abstracts or some combination of an interface + RuntimeHelpers.GetUninitializedObject to create dummy instances to use as stubs for the various stub methods?

The latter has the advantage that also works all the way down on .NET Standard 2.0 and doesn't have viral constraints, and instead allows you to dynamically check whether a type supports the interface and then statically invoke its members. We're using it extensively in the Microsoft Store too to achieve trimming safe solutions for somewhat similar scenarios (runtime discovered "static" interface methods on generic type arguments that support opt-in and are not mandatory constraints).

Other than that, really happy I could finally get this to work, once properly setup it does work great! 😄

@AaronRobinsonMSFT
Copy link
Member

which seems to be necessary for some reason

I did quick search in this repo for uses of that attribute and it seems rather core. Something that seems interesting is the following:

if (Projections.IsTypeWindowsRuntimeType(iface))
{
var ifaceAbiType = iface.FindHelperType();
Guid iid = GuidGenerator.GetIID(ifaceAbiType);
entries.Add(new ComInterfaceEntry
{
IID = iid,
Vtable = (IntPtr)ifaceAbiType.GetAbiToProjectionVftblPtr()
});
if (!hasCustomIMarshalInterface && iid == ABI.WinRT.Interop.IMarshal.IID)
{
hasCustomIMarshalInterface = true;
}
}

Perhaps @jkoritzinsky @manodasanW or @Scottj1s can provide commentary on this particular code path. Not saying it is related, but it seems like a relevent code path.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

5 participants