fix: fallback to UPLOAD heapType for devices without GPU_UPLOAD support

[dreamsyntax]

I was running into this issue for a project when running D3D12 on an Intel iGPU (13th gen i9, UHD Graphics, Driver 32.0.101.7084 - on Win 11 23H2 ENT/BUS), where upon parsing a vertex declaration I would get
CreateResource failed with error code 0x80004001., specifically breaking on

    static void setObjectName(ID3D12Object *object, const std::string &name) {
        const std::wstring wideCharName = Utf8ToUtf16(name);
        object->SetName(wideCharName.c_str());
    }

Line 773 of plume_d3d12.cpp

With the proposed change I am able to use D3D12 for both my Nvidia dGPU and Intel iGPU

[DarioSamo]

It makes sense to me but I think it needs to be changed how the fix is implemented. In particular, I'd change this instead (

plume/plume_d3d12.cpp

Lines 468 to 482 in 519e325

	static D3D12_HEAP_TYPE toD3D12(RenderHeapType type) {
	switch (type) {
	case RenderHeapType::DEFAULT:
	return D3D12_HEAP_TYPE_DEFAULT;
	case RenderHeapType::UPLOAD:
	return D3D12_HEAP_TYPE_UPLOAD;
	case RenderHeapType::READBACK:
	return D3D12_HEAP_TYPE_READBACK;
	case RenderHeapType::GPU_UPLOAD:
	return D3D12_HEAP_TYPE_GPU_UPLOAD;
	default:
	assert(false && "Unknown heap type.");
	return D3D12_HEAP_TYPE_DEFAULT;
	}
	}

) to take the boolean for gpuUploadHeapFallback and return the regular upload heap type if the boolean is true on the case of GPU UPLOAD.

[DarioSamo]

The other bit that seems odd to me is the check for customUploadPool && customUploadPool->d3d. If it has indeed failed to create for some reason, we should just avoid the fallback path altogether and set the capability to false for GPU Uploads so the user can take the correct paths for them. That would happen during initialization instead.

Were you running into the upload pool being empty?

[dreamsyntax]

Were you running into the upload pool being empty?

This was an oversight on my part from testing early on, removed.

To take the boolean for gpuUploadHeapFallback and return the regular upload heap type if the boolean is true on the case of GPU UPLOAD.

        D3D12MA::ALLOCATION_DESC allocationDesc = {};
        allocationDesc.Flags = desc.committed ? D3D12MA::ALLOCATION_FLAG_COMMITTED : D3D12MA::ALLOCATION_FLAG_NONE;
        allocationDesc.HeapType = toD3D12(desc.heapType, device->gpuUploadHeapFallback);
        allocationDesc.CustomPool = (pool != nullptr) ? pool->d3d : nullptr;

I wasn't sure how to get gpuUploadHeapFallback for line 2763, so used the member pointer. I did a test forcing to false, where it breaks, but using this works, so presumably it is being set elsewhere? If it should be done a different way let me know.

[blueskythlikesclouds]

This is not right. GPU_UPLOAD supports both read/write operations whereas UPLOAD is readonly. This is why a fallback custom pool with UPLOAD properties is created in the first place.

Sounds like you are trying to use GPU upload heaps without checking for whether they are actually supported first, which is not intended: https://github.com/renderbag/plume/blob/main/plume_render_interface_types.h#L1807

[dreamsyntax]

Sounds like you are trying to use GPU upload heaps without checking for whether they are actually supported first[...]

Thanks, did some more digging based on this and even tested in UnleashedRecomp (forcing d3d12).

As it turns out my device is reporting 'true' for gpuUpload (when this is NOT the case). I know in the case of your project you redirect to Vulkan as Intel iGPU is a mess to deal with DX12.

Even if UMA was checked for explicitly, using HeapType::DEFAULT will result in the same mapping error unless HeapType::UPLOAD is used instead.

-   return g_capabilities.gpuUploadHeap ? RenderHeapType::GPU_UPLOAD : RenderHeapType::DEFAULT;
+   return g_capabilities.gpuUploadHeap && !g_capabilities.uma ? RenderHeapType::GPU_UPLOAD : RenderHeapType::UPLOAD;

https://github.com/hedge-dev/UnleashedRecomp/blob/5e8695a157ce9d2a783944d63439cc8c76a38fc2/UnleashedRecomp/gpu/video.cpp#L3157

[dreamsyntax]

Looks like I misunderstood the DEFAULT heap https://learn.microsoft.com/en-us/windows/win32/api/d3d12/ne-d3d12-d3d12_heap_type, there's no redirection with GPU_UPLOAD involved, so not clear why DEFAULT would have the same issue VS UPLOAD.

[dreamsyntax]

Verified I can get the same crash in the UnleashedRecomp project with DEFAULT on the title screen instead of UPLOAD

Also can re-produce on MarathonRecomp project (immediately post installer)

[dreamsyntax]

As a sanity check, also tested on Intel drivers 31.0.101.5187 - the behavior is the same.

Regardless its safe to say this is a Intel or D3D12 bug not a plume bug.
Well, actually seems its intentional by Plume

plume/plume_d3d12.cpp

Line 3767 in 519e325

// Pretend GPU Upload heaps are supported if UMA is supported, as

So...

Projects using plume will have to deal with this if they care, with the proposal here that fixes it for UnleashedRecomp

-   return g_capabilities.gpuUploadHeap ? RenderHeapType::GPU_UPLOAD : RenderHeapType::DEFAULT;
+   return g_capabilities.gpuUploadHeap && !g_capabilities.uma ? RenderHeapType::GPU_UPLOAD : RenderHeapType::UPLOAD;

Closing.