smourier / DirectN

Direct interop Code for .NET Framework, .NET Core and .NET 5+ : DXGI, WIC, DirectX 9 to 12, Direct2D, Direct Write, Direct Composition, Media Foundation, WASAPI, CodecAPI, GDI, Spatial Audio, DVD, Windows Media Player, UWP DXInterop, WinUI3, etc.
MIT License
311 stars 28 forks source link

GetFrameDirtyRects / GetFrameMoveRects have erroneous `out` on the buffer argument. #39

Closed gsuberland closed 1 year ago

gsuberland commented 1 year ago

The GetFrameDirtyRects and GetFrameMoveRects methods in IDXGIOutputDuplication are being auto-generated with out IntPtr arguments for the buffers:

https://github.com/smourier/DirectN/blob/1de2dd5af5be6ddef098a18571806c815203d396/DirectN/DirectN/Generated/IDXGIOutputDuplication.cs#L30-L34

The MSDN documentation is vague on this, but Microsoft's examples (e.g. the DuplicationManager example) make it clear that a pointer to a pre-allocated buffer is supposed to be passed in, rather than the functions passing a pointer to a buffer back out. As such, these should be IntPtr rather than out IntPtr.

The correct signature would be:

[PreserveSig]
HRESULT GetFrameDirtyRects(uint DirtyRectsBufferSize, IntPtr pDirtyRectsBuffer, out uint pDirtyRectsBufferSizeRequired);

[PreserveSig]
HRESULT GetFrameMoveRects(uint MoveRectsBufferSize, IntPtr pMoveRectBuffer, out uint pMoveRectsBufferSizeRequired);

Alternatively, it should be possible to do something like this to avoid having to use Marshal to do the alloc and structure copies:

[PreserveSig]
HRESULT GetFrameDirtyRects(uint DirtyRectsBufferSize, [In, Out] RECT[] pDirtyRectsBuffer, out uint pDirtyRectsBufferSizeRequired);

[PreserveSig]
HRESULT GetFrameMoveRects(uint MoveRectsBufferSize, [In, Out] RECT[] pMoveRectBuffer, out uint pMoveRectsBufferSizeRequired);

Although I've never done this with COM interop in particular, so it's possible there's some complexity I've overlooked in terms of array passing.

It could also be useful to add extension methods to automagically handle everything in a single call. Something along the lines of:

public static RECT[] GetFrameDirtyRects(this IDXGIOutputDuplication outputDuplication)
{
    // start with a reasonably sized buffer
    uint count = 4;
    uint size = (uint)(count * Marshal.SizeOf<RECT>());
    RECT[] rects = new RECT[count];
    // attempt to read dirty rects
    HRESULT hr = outputDuplication.GetFrameDirtyRects(size, rects, out size);
    // if we need more space, re-allocate the array based on the requested buffer size
    if (hr == HRESULTS.DXGI_ERROR_MORE_DATA)
    {
        count = (uint)(size / Marshal.SizeOf<RECT>());
        rects = new RECT[count];
        hr = outputDuplication.GetFrameDirtyRects(size, rects, out size);
    }
    // if there's still an error, return nothing (or maybe throw an exception?)
    if (!hr.IsSuccess)
    {
        return Array.Empty<RECT>();
    }
    // correct for fewer entries being returned than we asked for
    count = (uint)(size / Marshal.SizeOf<RECT>());
    return rects.Take((int)count).ToArray();
}

(plus the same kind of thing for GetFrameMoveRects)

smourier commented 1 year ago

Hi,

You're right. The real annotation we can see in dxgi1_2.h is this:

_Out_writes_bytes_to_(DirtyRectsBufferSize, *pDirtyRectsBufferSizeRequired) RECT *pDirtyRectsBuffer

Where _Out_writes is somewhat an _In_ from a caller perspective. This was not handled correctly by the generator, so I've fixed it and commited the changes. But it won't use a tagRect, it's still an IntPtr.

You can redeclare it and use your version, or write an extension method in the spirit of DirectN that should logically go in IDXGIOutputDuplicationExtensions.cs. I have written one but it's not commited since I can't test it right now:

public static tagRECT[] GetFrameDirtyRects(this IComObject<IDXGIOutputDuplication> output) => GetFrameDirtyRects(output?.Object);
public static tagRECT[] GetFrameDirtyRects(this IDXGIOutputDuplication duplication)
{
    if (duplication == null)
        throw new ArgumentNullException(nameof(duplication));

    var hr = duplication.GetFrameDirtyRects(0, IntPtr.Zero, out var size);
    if (size == 0)
    {
        hr.ThrowOnError(); // won't throw if no error
        return Array.Empty<tagRECT>();
    }

    var rects = new List<tagRECT>(); // tagRect is DirectN provided
    using (var mem = new ComMemory(size)) // this memory tool is DirectN provided
    {
        var rectSize = Marshal.SizeOf<tagRECT>();
        var curSize = 0;
        duplication.GetFrameDirtyRects(size, mem.Pointer, out size).ThrowOnError();
        do
        {
            rects.Add(Marshal.PtrToStructure<tagRECT>(mem.Pointer + curSize));
            curSize += rectSize;
        }
        while (curSize < size);
    }
    return rects.ToArray();
}
smourier commented 1 year ago

closed due to inactivity