Make ID3D12GraphicsCommandList::SetDescriptorHeaps take ManuallyDrop'd references

[damyanp]

Suggestion

IIRC, most D3D12 structs that hold references to other D3D objects take these via ManuallyDrop. eg:

}
#[repr(C)]
pub struct D3D12_BUFFER_BARRIER {
    pub SyncBefore: D3D12_BARRIER_SYNC,
    pub SyncAfter: D3D12_BARRIER_SYNC,
    pub AccessBefore: D3D12_BARRIER_ACCESS,
    pub AccessAfter: D3D12_BARRIER_ACCESS,
    pub pResource: ::std::mem::ManuallyDrop<::core::option::Option<ID3D12Resource>>,
    pub Offset: u64,
    pub Size: u64,
}

However, when a function such as SetDescriptorHeaps does use a struct, but just expects an array of pointers to objects, these aren't wrapped in ManuallyDrop:

pub unsafe fn SetDescriptorHeaps(
    &self,
    ppdescriptorheaps: &[::core::option::Option<ID3D12DescriptorHeap>],
)

As a result it's necessary to clone values passed in to this array:

command_list.SetDescriptorHeaps(&[Some(resources.srv_heap.clone())]);

Although this works, it does mean that there's an unnecessary AddRef/Release going on for each call to SetDescriptorHeaps. This is unfortunate as this is one of the high frequency methods that we expect D3D12 apps to call fairly often so the overhead of AddRef/Release could start to add up.

The solution that I think would fit well here would be if we could get SetDescriptorHeaps to be:

pub unsafe fn SetDescriptorHeaps(
    &self,
    ppdescriptorheaps: &[::std::mem::ManuallyDrop<::core::option::Option<ID3D12DescriptorHeap>>],
) {

Ideally, it would be:

pub unsafe fn SetDescriptorHeaps(
    &self,
    ppdescriptorheaps: &[::std::mem::ManuallyDrop<ID3D12DescriptorHeap>],
) {

But I suspect that we don't have any metadata to indicate that null values aren't valid in the array.

[kennykerr]

Yep, I've wanted to fix this but the usability of &[ManuallyDrop<Option<T>>] isn't ideal compared with C++. But I agree the extra reference bumps aren't great. Perhaps if there were some kind of IntoParam specialization for this as we do for non-array parameters.

[MarijnS95]

That's an interesting issue indeed. While &[] doesn't move/transfer ownership, the API indeed "requires" the caller to put multiple owned ID3D12DescriptorHeaps in a contiguous array which can only happen by incrementing the refcount.

The only alternative I'd like to mention, for single descriptor heaps, is the existence of std::slice::from_ref(&T) -> &[T], which turns a borrow of an object into a slice of length 1. Given there is no way to turn an Option<&T> into an &Option<T>, I see now that I ended up implementing that via a transmute in our own codebase though...