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

Consider adding strongly-typed Vtbl property to COM interface structs (if trimmable) #289

Closed
rickbrew opened this issue Dec 17, 2021 · 4 comments
Labels
proposal An issue that represents a proposed feature or change to the repo.

Comments

@rickbrew
Copy link
Contributor

In implementing my wrapper for IDWriteTextLayout, there are 12 methods that are easily made generic, which relies on passing in a function pointer from the vtbl. These are 6 getters and 6 setters for simple unmanaged value types.

Here's an example of how I'm currently implementing it. A strongly-typed Vtbl property on IDWriteTextLayout would remove the need for the casting in my Get____() and Set____() methods. As long as the property is trimmable, the Vtbl<TSelf> struct should still be trimmable ... right?

// This is repeated for GetFontWeight, GetFontStyle, GetFontStretch, GetUnderline, and GetStrikethrough
// Similar pattern for Set___() methods
public float GetFontSize(int currentPosition, out TextRange textRange)
{
    return GetProperty<float>(
        // This next line of code could be just: this.pDWriteTextLayout->Vtbl->GetFontSize1
        ((IDWriteTextLayout.Vtbl<IDWriteTextLayout>*)this.pDWriteTextLayout->lpVtbl)->GetFontSize1,
        currentPosition,
        out textRange);
}

private T GetProperty<T>(
    delegate* unmanaged<IDWriteTextLayout*, uint, T*, DWRITE_TEXT_RANGE*, int> pGetFunction,
    int currentPosition,
    out TextRange textRange)
    where T : unmanaged
{
    T value;
    DWRITE_TEXT_RANGE dwTextRange;
    HRESULT hr = pGetFunction(
        this.pDWriteTextLayout,
        (uint)currentPosition,
        &value,
        &dwTextRange);
    hr.ThrowOnError();

    textRange = Unsafe.As<DWRITE_TEXT_RANGE, TextRange>(ref dwTextRange);
    return value;
}

Implementation in TerraFX could be:

public unsafe struct IDWriteTextLayout
{
    ...
    public Vtbl<IDWriteTextLayout> Vtbl => ((Vtbl<IDWriteTextLayout>)this.lpvtbl);
    ...
}
@rickbrew rickbrew added proposal An issue that represents a proposed feature or change to the repo. untriaged An issue that has not been triaged by the repo maintainers. labels Dec 17, 2021
@rickbrew
Copy link
Contributor Author

Hmm, looks like my attempt to use generics here is not supported by the runtime, I get BadImageFormatException with "Bad element type in SizeOf" for the setters, or "Illegal or unimplemented ELEM_TYPE in signature" for the getters. As per dotnet/runtime#9136 this is just how it is right now.

Having the strongly-typed Vtbl property would probably be useful at some point, but not yet for me unfortunately :)

@rickbrew
Copy link
Contributor Author

rickbrew commented Dec 18, 2021

Alright I did find a use for this, in my wrappers for IWICComponentInfo and the derived interfaces (e.g. IWICBitmapCodecInfo). Strongly-typed vtbl would help shorten this code, although unfortunately because I can't go generic on the type, I still need the delegate cast (AFAIC -- it still gave BadImageFormatException if I tried generic).

There are many methods on these interfaces that are HRESULT GetFoo(uint cchBuffer, ushort* pszBuffer, uint* cchCopied). You call it once with (0, null, &count) to get the size, then again with the buffer to get the string.

public string Author
{
    get
    {
        if (this.author == null)
        {
            this.author = GetString(
                this.pWICComponentInfo,
                (delegate* unmanaged<void*, uint, ushort*, uint*, int>)
                    ((IWICComponentInfo.Vtbl<IWICComponentInfo>*)this.pWICComponentInfo->lpVtbl)->GetAuthor);
                    // ^^^ that could be simplified to `this.pWICComponentInfo->Vtbl->GetAuthor`
        }

        return this.author;
    }
}

// I couldn't spec this as `GetString<T>(T* pObject, ...) where T : unmanaged, IWICComponentInfo.Interface`, 
// I just get BadImageFormatException
protected static string GetString(void* pObject, delegate* unmanaged<void*, uint, ushort*, uint*, int> getStringFn)
{
    uint cch;
    HRESULT hr = getStringFn(
        pObject,
        0,
        null,
        &cch);
    hr.ThrowOnError();

    if (cch == 0)
    {
        return string.Empty;
    }
    else
    {
        return string.Create(
            (int)cch - 1,
            ((Ptr)pObject, (Ptr)getStringFn),
            static delegate (Span<char> dst, (Ptr pObject, Ptr getStringFn) e)
            {
                fixed (char* pDst = dst)
                {
                    uint cch2;
                    HRESULT hr = ((delegate* unmanaged<void*, uint, ushort*, uint*, int>)(void*)e.getStringFn)(
                        e.pObject.Get(),
                        (uint)(dst.Length + 1),
                        (ushort*)pDst,
                        &cch2);
                    hr.ThrowOnError();

                    if (cch2 != (uint)(dst.Length + 1))
                    {
                        throw new InternalErrorException();
                    }
                }
            });
    }
}

@tannergooding
Copy link
Member

This is non-trivial due to there already being a member named Vtbl (the vtbl struct) nested in the type.

There would need to be a different non-conflicting name available for use here. It ultimately would just be a short circuit for the user manually doing:

var lpVtbl = (IUnknown.Vtbl<IUnknown>*)pUnknown.lpVtbl;
lpVtbl->QueryInterface = &QueryInterface

@tannergooding tannergooding removed the untriaged An issue that has not been triaged by the repo maintainers. label Dec 23, 2021
@rickbrew
Copy link
Contributor Author

rickbrew commented Nov 5, 2022

We can close this, I found a reasonable solution a long time ago for this particular issue

@rickbrew rickbrew closed this as completed Nov 5, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
proposal An issue that represents a proposed feature or change to the repo.
Projects
None yet
Development

No branches or pull requests

2 participants