GetSize and GetPixelSize methods on ID2D1RenderTarget need special ABI handling

[Shkyrockett]

Actual behavior

ID2D1RenderTarget.EndDraw does not return a needed HRESULT, and ID2D1RenderTarget.GetSize, and ID2D1RenderTarget.GetPixelSize throw System.AccessViolationException exceptions when accessed because they are also not returning HRESULTs.

Expected behavior

The EndDraw, GetSize, and GetPixelSize methods should return HRESULT in ID2D1RenderTarget and inherited interfaces, when using "allowMarshaling": true. Maybe these need to be marked as [PreserveSig]?

ID2D1RenderTarget.EndDraw should return an HRESULT so the result can be used to tell if the draw attempt failed and Direct2D needs to be reinitialized because of a display reset. See https://docs.microsoft.com/en-us/windows/win32/api/d2d1/nn-d2d1-id2d1rendertarget#remarks

Both ID2D1RenderTarget.GetSize, and ID2D1RenderTarget.GetPixelSize throw System.AccessViolationException exceptions when called using the current method signatures which return D2D_SIZE_F or D2D_SIZE_U respectively. Changing the methods to return HRESULT fixes the exception. Friendly overload extensions that do return D2D_SIZE_F or D2D_SIZE_U would be helpful for these methods.

Repro steps

1. NativeMethods.txt content:

ID2D1RenderTarget
S_OK
CreateDCW
ID2D1Factory7
D2D1CreateFactory

2. NativeMethods.json content (if present):

{
  "$schema": "https://aka.ms/CsWin32.schema.json",
  "allowMarshaling": true,
  "className": "PInvoke",
  "emitSingleFile": false,
  "public": true,
  "wideCharOnly": true
}

3. Any of your own code that should be shared?

Minimal repro:

using System.Runtime.InteropServices;
using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.Graphics.Direct2D;
using Windows.Win32.Graphics.Direct2D.Common;
using Windows.Win32.Graphics.Dxgi.Common;
using Windows.Win32.Graphics.Gdi;

unsafe
{
    var direct2dFactory = PInvoke.D2D1CreateFactory(D2D1_FACTORY_TYPE.D2D1_FACTORY_TYPE_SINGLE_THREADED, typeof(ID2D1Factory7).GUID, new D2D1_FACTORY_OPTIONS(), out var factory) switch { var h when h == HRESULT.S_OK => factory as ID2D1Factory7, _ => throw new Exception("Unspecified Error") };
    var dcRenderTarget = InitializeDirect2DDCRenderTarget(direct2dFactory!);

    var hdc = PInvoke.CreateDCW("DISPLAY", string.Empty, string.Empty, new()).DangerousGetHandle();
    bindDc(dcRenderTarget, new HDC(hdc), new());

    dcRenderTarget.BeginDraw();

    var pSize = dcRenderTarget.GetPixelSize(); // System.AccessViolationException exception here.
    var size = dcRenderTarget.GetSize(); // System.AccessViolationException exception here.

    dcRenderTarget.EndDraw(); // This ought to return an HRESULT. See https://docs.microsoft.com/en-us/windows/win32/api/d2d1/nn-d2d1-id2d1rendertarget#remarks

    PInvoke.DeleteDC(new(hdc));
}

static unsafe ID2D1DCRenderTarget InitializeDirect2DDCRenderTarget(ID2D1Factory7 direct2dFactory)
{
    direct2dFactory.CreateDCRenderTarget(
        new D2D1_RENDER_TARGET_PROPERTIES()
        {
            type = D2D1_RENDER_TARGET_TYPE.D2D1_RENDER_TARGET_TYPE_DEFAULT,
            pixelFormat = new D2D1_PIXEL_FORMAT()
            {
                format = DXGI_FORMAT.DXGI_FORMAT_B8G8R8A8_UNORM,
                alphaMode = D2D1_ALPHA_MODE.D2D1_ALPHA_MODE_IGNORE
            },
            dpiX = 0,
            dpiY = 0,
            usage = D2D1_RENDER_TARGET_USAGE.D2D1_RENDER_TARGET_USAGE_GDI_COMPATIBLE,
            minLevel = D2D1_FEATURE_LEVEL.D2D1_FEATURE_LEVEL_DEFAULT
        }, out var dcRenderTarget);
    Marshal.ReleaseComObject(direct2dFactory);
    return dcRenderTarget;
}

static unsafe void bindDc(ID2D1DCRenderTarget dcRenderTarget, HDC hdc, in RECT rect)
{
    fixed (RECT* r = &rect)
    {
        dcRenderTarget.BindDC(hdc, r);
    }
}

The code above should be rewritten to the following if the methods returned HRESULTS:

using System.Runtime.InteropServices;
using Windows.Win32;
using Windows.Win32.Foundation;
using Windows.Win32.Graphics.Direct2D;
using Windows.Win32.Graphics.Direct2D.Common;
using Windows.Win32.Graphics.Dxgi.Common;
using Windows.Win32.Graphics.Gdi;

unsafe
{
    var direct2dFactory = PInvoke.D2D1CreateFactory(D2D1_FACTORY_TYPE.D2D1_FACTORY_TYPE_SINGLE_THREADED, typeof(ID2D1Factory7).GUID, new D2D1_FACTORY_OPTIONS(), out var factory) switch { var h when h == HRESULT.S_OK => factory as ID2D1Factory7, _ => throw new Exception("Unspecified Error") };
    var dcRenderTarget = InitializeDirect2DDCRenderTarget(direct2dFactory!);

    var hdc = PInvoke.CreateDCW("DISPLAY", string.Empty, string.Empty, new()).DangerousGetHandle();
    bindDc(dcRenderTarget, new HDC(hdc), new());

    dcRenderTarget.BeginDraw();

    _ = dcRenderTarget.GetPixelSize(out var pSize);
    _ = dcRenderTarget.GetSize(out var size);

    var result = dcRenderTarget.EndDraw();
    if (result == HRESULT.D2DERR_RECREATE_TARGET)
    {
        // Recreate resources if they have become invalid. See https://docs.microsoft.com/en-us/windows/win32/api/d2d1/nn-d2d1-id2d1rendertarget#remarks
        dcRenderTarget = InitializeDirect2DDCRenderTarget(direct2dFactory);
    }

    PInvoke.DeleteDC(new(hdc));
}

static unsafe ID2D1DCRenderTarget InitializeDirect2DDCRenderTarget(ID2D1Factory7 direct2dFactory)
{
    direct2dFactory.CreateDCRenderTarget(
        new D2D1_RENDER_TARGET_PROPERTIES()
        {
            type = D2D1_RENDER_TARGET_TYPE.D2D1_RENDER_TARGET_TYPE_DEFAULT,
            pixelFormat = new D2D1_PIXEL_FORMAT()
            {
                format = DXGI_FORMAT.DXGI_FORMAT_B8G8R8A8_UNORM,
                alphaMode = D2D1_ALPHA_MODE.D2D1_ALPHA_MODE_IGNORE
            },
            dpiX = 0,
            dpiY = 0,
            usage = D2D1_RENDER_TARGET_USAGE.D2D1_RENDER_TARGET_USAGE_GDI_COMPATIBLE,
            minLevel = D2D1_FEATURE_LEVEL.D2D1_FEATURE_LEVEL_DEFAULT
        }, out var dcRenderTarget);
    Marshal.ReleaseComObject(direct2dFactory);
    return dcRenderTarget;
}

static unsafe void bindDc(ID2D1DCRenderTarget dcRenderTarget, HDC hdc, in RECT rect)
{
    fixed (RECT* r = &rect)
    {
        dcRenderTarget.BindDC(hdc, r);
    }
}

Context

• CsWin32 version: 0.2.63-beta
• Win32Metadata version: 35.0.14-preview
• Target Framework: net6.0-windows
• LangVersion: 10.0

[AArnott]

Thank you for the excellent report. The metadata for this interface appears to present several methods inconsistently. I'm going to transfer this issue to the metadata repo for resolution, since CsWin32 is just generating the methods as described there.

[kennykerr]

These methods all work correctly in Rust, so I can confirm the metadata is correct. Note that various Direct2D methods don't return HRESULT values. The GetSize and GetPixelSize methods return structs directly (depending on calling convention), which requires special handling for historical reasons. This was previously discussed here: microsoft/win32metadata#636

[AArnott]

These methods all work correctly in Rust, so I can confirm the metadata is correct.

I don't think that necessarily follows. Our projections interpret the metadata differently, so it's possible one works while a bug remains in the metadata. But I'm just exploring here and trying to understand.

Thanks for the link to the other issue. I'd personally say the metadata is incorrect, since we can't take the declaration as given, and there is no annotation in the metadata to tell us that projections need to do something special here. It's just that Rust has already figured out this 'bug' in the metadata and worked around it, and CsWin32 hasn't.
It's particularly ironic because the metadata is literally telling us to preserve the signature, when in fact the signature won't do the right thing.

EndDraw I see isn't defective... it's just a request like microsoft/win32metadata#1295 because it returns a failing HRESULT in success paths so avoiding the exception would be nice. So this is really about the GetSize and GetPixelSize methods.

[kennykerr]

Sure, you could argue the metadata could be improved. I'm just pointing out that it works today so we shouldn't blindly "fix" these methods. Another case of this: microsoft/win32metadata#1295 (comment)

[AArnott]

FYI we'll track EndDraw as part of microsoft/win32metadata#1315.

[Shkyrockett]

I suggested maybe these need to be marked as [PreserveSig], because if I am understanding the code and comments correctly for CsWin32; it looks like CsWin32 does some rewriting of methods in interfaces if the method is not marked as [PreserveSig], and only has out, or zero parameters: https://github.com/microsoft/CsWin32/blob/main/src/Microsoft.Windows.CsWin32/Generator.cs#L3653

These would fall into those specifications.

[AArnott]

I'll move this back to CsWin32 and track fixing our generator.

[AArnott]

Note to self: our fix in CsWin32 will require that we apply similar transformations that the rust projection applies, as described here:
microsoft/win32metadata#636 (comment)
microsoft/win32metadata#636 (comment)

To avoid user confusion though, we may want to adjust our friendly overloads to look like the documented signature (in effect, reversing the transformation).

[AArnott]

@Shkyrockett Can you confirm that once fixed, you're expecting pixel sizes of 0 to come out of these to get size methods? When I manually apply the prescribed fix, I'm getting 0,0 from both methods.

[Shkyrockett]

@AArnott Yes. The GDI object is not getting initiated with dimensions, so the sizes should be 0,0 here.

If more complete solutions would help, I have been playing around with this in Direct2DCanvasPlayground and AutoGenDirectWritePlayground. The Overrides folders contain the files overriding the CSWin32 generation.

[AArnott]

Duplicate of #167