d3d12: Problem with functions that return structures by value

[michal-z]

Hi, first of all, nice project!

The original d3d12.h that comes with Windows SDK has wrong C prototypes for functions that return structures. Fixed headers are released by Microsoft here: https://github.com/microsoft/DirectX-Headers/blob/main/include/directx/d3d12.h

Consider ID3D12Heap::GetDesc method, it should be defined like this:

        DECLSPEC_XFGVIRT(ID3D12Heap, GetDesc)
        D3D12_HEAP_DESC *( STDMETHODCALLTYPE *GetDesc )( 
            ID3D12Heap * This,
            D3D12_HEAP_DESC * RetVal);

The proper C prototype takes a pointer to RetVal as a second parameter and also returns the same pointer. This applies to all functions that return structure by value.

Currently zigwin32 defines those functions incorrectly which will cause a crash.

The proper zig declaration should be:

            GetDesc: fn (*T, *D3D12_HEAP_DESC) callconv(WINAPI) *D3D12_HEAP_DESC,

In my project, I use (it works fine I tested it):

pub const ID3D12Heap = extern struct {
    const Self = @This();
    v: *const extern struct {
        unknown: IUnknown.VTable(Self),
        object: ID3D12Object.VTable(Self),
        devchild: ID3D12DeviceChild.VTable(Self),
        pageable: ID3D12Pageable.VTable(Self),
        heap: VTable(Self),
    },
    usingnamespace IUnknown.Methods(Self);
    usingnamespace ID3D12Object.Methods(Self);
    usingnamespace ID3D12DeviceChild.Methods(Self);
    usingnamespace ID3D12Pageable.Methods(Self);
    usingnamespace Methods(Self);

    fn Methods(comptime T: type) type {
        return extern struct {
            pub inline fn GetDesc(self: *T) D3D12_HEAP_DESC {
                var desc: D3D12_HEAP_DESC = undefined;
                self.v.heap.GetDesc(self, &desc);
                return desc;
            }
        };
    }

    fn VTable(comptime T: type) type {
        return extern struct {
            GetDesc: fn (*T, *D3D12_HEAP_DESC) callconv(WINAPI) *D3D12_HEAP_DESC,
        };
    }
};

[marler8997]

Thanks for finding this problem and taking the time to report it. I've created a corresponding issue in the win32metadata repository here: microsoft/win32metadata#636

The metadata repo is usually pretty good about fixing issues like this. I expect they'll probably fix it within a couple weeks or so.

[coeuvre]

Thanks for the nice project! I'm just wondering what's the next step here given that the upstream is probably not going to fix the metadata?

[marler8997]

Re-reading the thread, it looks like we've got 2 options here. The first would be to translate all COM method definitions, which use the virtual function ABI, to their equivalent stdcall ABI when we generate the bindings for Zig. The second option would be to add a new calling convention in Zig, maybe named thiscall. The advantage of the second option is it will be cleaner to call the methods themselves since you won't have the return value as both a return value and a reference parameter.

We could try implementing the first option, and then implement the second option at a later date at which point zigwin32 could then leverage it. I think the path forward here would be to implement option 1 and go from there.

[lassade]

Any updates/fixes on this?