search

LukasBanana / LLGL

Low Level Graphics Library (LLGL) is a thin abstraction layer for the modern graphics APIs OpenGL, Direct3D, Vulkan, and Metal
BSD 3-Clause "New" or "Revised" License
2.1k stars 139 forks source link

[D3D12] SetPipelineState inside Secondary Command Buffers cause `ID3D12CommandList::Close()` to return E_INVALIDARG in parent command buffer. #99

Closed gdianaty closed 1 year ago

gdianaty commented 1 year ago

Hello.

In LLGL on D3D12, it appears that the usage of the call SetPipelineState(...) in a secondary command buffer on D3D12 causes the parent command buffer (the one that receives the secondary command buffer via the Execute(...) function) to fail with the error E_INVALIDARG.

Commenting out SetPipelineState but leaving all calls in place leads to expected functionality with or without the debug validation layer, but it may be possible that LLGL isn't submitting commands that require the PSO being set and I did not find that functionality during my time debugging this.

When turning on the debug validation layer, no errors are thrown prior to the throwing of E_INVALIDARG and everything functions as expected. When the debug validation layer is disabled, the bug occurs.

I have written a reproducible example of this inside the existing testing case for D3D12, that can be found here in my branch of LLGL: https://github.com/gdianaty/LLGL/commit/110d774bbbfecb127bb87d79114b45207ea28757

I am running the latest drivers available for my GPU (per NVidia Geforce Experience), and Windows 22H2 22621. I am personally using a NVidia GTX 3070 Ti, and I have been able to reproduce this bug on a NVidia GTX 3080.

Please let me know if any further information is required. Thank you for your time!

P.S. Happy spooky season! 🎃

LukasBanana commented 1 year ago

I can confirm the issue and thanks for the reproducible example. While I just finished some improvements to make LLGL's command buffer multi-threading capable, I haven't gotten back to the secondary command buffers in a while, but I'll take a look now.

gdianaty commented 1 year ago

Understood. I did see those edits - exciting stuff! Looking forward to what you have to say on this one.

LukasBanana commented 1 year ago

While not a perfect solution, 68f6214 should fix this. According to the D3D12 docs, SetDescriptorHeaps can be called on bundles, but they must match the caller's descriptor heap. I'll try to get a better solution by sharing/pooling the shader visible descriptor heaps, but this should work for now. Please let me know if this fixes the issue for you as well.

gdianaty commented 1 year ago

The commit you cited fixed the problem both in my test case and in my application, which are now both working as expected with and without the device debugger. Thank you for your help and hot-fix.

I honestly would have had no idea about those descriptor heap requirements. To my eye, it's highly curious that the validation layer seems to do something that fixes that behavior and doesn't throw some kind of warning.