d3d12: null-check the ComPtr produced by some creation functions

[nical]

Description

This PR makes the d3d12 a bit more defensive about the output of Create* functions

My understanding is that we shouldn't need to (although the d3d12 docs aren't very specific about that), but we have evidence that these functions sometimes leave the resource pointer set to null without returning an error.

Checklist

• Run cargo fmt.
• Run cargo clippy.
• Run cargo xtask test to run tests.

[nical]

https://bugzilla.mozilla.org/show_bug.cgi?id=1874478 appears to be another case of a null ComPtr produced without error.

Edit: Nevermind, the stack for this specific bug corresponds to a slightly older wgpu revision that does not check the hresult in FixedSizeHeap::new. The other linked stack trace does point to a revision where a texture is created with a null comptr after checking the hresult.

[teoxoy]

So, it seems that the docs of:

• CreateHeap
• CreateDescriptorHeap
• CreateQueryHeap
• CreateCommittedResource
• CreatePlacedResource

but not of:

• CreateFence
• CreateComputePipelineState
• CreateGraphicsPipelineState

mention something along the lines of:

ppvHeap can be NULL, to enable capability testing. When ppvHeap is NULL, no object will be created and S_FALSE will be returned when pDescriptorHeapDesc is valid.

I don't really understand what they mean by "capability testing" but if we want to treat S_FALSE as errors we could do so here:

wgpu/wgpu-hal/src/auxil/dxgi/result.rs

Lines 11 to 13 in 063e110

	if self >= 0 {
	return Ok(());
	}

Some relevant resources:
https://learn.microsoft.com/en-us/windows/win32/learnwin32/error-handling-in-com
https://learn.microsoft.com/en-us/windows/win32/direct3d12/d3d12-graphics-reference-returnvalues

[nical]

ppvHeap can be NULL, to enable capability testing. When ppvHeap is NULL, no object will be created and S_FALSE will be returned when pDescriptorHeapDesc is valid.

That's different. We are passing these functions a pointer to a ComPtr. What this part of the docs refers to is that if we pass a null pointer (instead of a pointer to a null pointer), then the API treats it as a dry run. Here we are passing a non-null pointer to a null ComPtr in which case they are meant to populate the ComPtr with some object in case of success.

[teoxoy]

Ahh, I see!

[teoxoy]

Looks good to me, we can consolidate the null check and the error check later.

[ErichDonGubler]

FTR: The hope I've had here is to eventually add some type state that we use instead of the current ComPtr (strawman: rename current ComPtr to NullableComPtr, make ComPtr only non-null). It's not a priority with a pragmatic fix like this in place, but it would make things less error-prone, for sure.

CC @jimblandy, @cwfitzgerald, @Wumpf.

[teoxoy]

My understanding is that we shouldn't need to (although the d3d12 docs aren't very specific about that), but we have evidence that these functions sometimes leave the resource pointer set to null without returning an error.

I think what was happening is that the code in our bindings was creating a texture via texture_from_raw with a null pointer. We were calling OpenSharedHandle and checking its HRESULT but not returning early. https://phabricator.services.mozilla.com/D213800 fixed this.