gfx-rs / wgpu

A cross-platform, safe, pure-Rust graphics API.
https://wgpu.rs
Apache License 2.0
11.49k stars 856 forks source link

draw_indexed_indirect on DX12 doesn't handle non-zero base_instance #2471

Open FredrikNoren opened 2 years ago

FredrikNoren commented 2 years ago

Description DrawIndexedIndirect.base_instance is always 0 in the shader when using draw_indexed_indirect on DX12, regardless of the value provided. This works on Vulcan and Metal. Setting the INDIRECT_FIRST_INSTANCE flag doesn't help.

Platform wgpu 0.12.0, Windows DX12

(Filed this issue by request from @kvark from a discussion in the matrix chat)

kvark commented 2 years ago

Doesn't look like Dawn has this implemented either - https://bugs.chromium.org/p/dawn/issues/detail?id=548&q=548&can=2

kvark commented 2 years ago

I concluded the research here. Good example code can be found in d3d functional spec. Unfortunately, we can't just re-use the start instance you have in the draw structures. The indirect command signature requires this value to be placed separately in a draw structure... So the only way around for us is to copy your draw structure into one we temporarily create, so that we can also copy the start instance.

This was the bad news. I definitely didn't want to involve a copy of indirect arguments here... A bit of good news is that we have to copy it for safety anyway, see #2431. So there is no overhead on top of the safety precautions we wanted to do. But there is overhead if you don't need safety, yikes.

jimblandy commented 6 months ago

Dawn has fixed that bug.

teoxoy commented 1 month ago

We should remember to also remove DownlevelFlags::VERTEX_AND_INSTANCE_INDEX_RESPECTS_RESPECTIVE_FIRST_VALUE_IN_INDIRECT_DRAW (added recently in https://github.com/gfx-rs/wgpu/pull/4723) once this is resolved.

mikialex commented 1 month ago

Dx12 non-zero base_instance actually works but the result is just not exposed in the shader. Before this issue get fixed, one way to workaround is to manually generate(by compute shader) a base_instance buffer and bind it as an instance vertex buffer.