brendan-duncan / webgpu_inspector

Inspection debugger for WebGPU
MIT License
167 stars 5 forks source link

Order of dynamic offsets in setBindGroup call might be wrong. #22

Open floooh opened 3 days ago

floooh commented 3 days ago

First, thanks for the awesome tool, it's helping me a lot for debugging the WebGPU backend in sokol_gfx.h :)

I think I stumbled over an issue though, which at first I thought was an implementation bug in the WebGPU browser backends, but which turned out to just be a weird part of the WebGPU spec: when calling setBindGroup() with dynamic offsets, the dynamic offsets must not be in the order of BindGroupLayout entries, but in the order of the binding member of the BindGroupLayout entries - most samples with dynamic offsets have the bindings sorted in ascending order in the BindGroupLayout entries, that's why that specific detail doesn't matter for such samples.

Please see this discussion on the WebGPU Matrix channel:

https://matrix.to/#/!MFogdGJfnZLrDmgkBN:matrix.org/$mRD2UVOcFEeIFds8zEjTncpx3s-csmG83treukcSJ2w?via=matrix.org&via=mozilla.org&via=matrix.nrp-nautilus.io

I wrote a 'minimal reproducer' (which in the end isn't a reproducer for a WebGPU bug, because that behaves as expected):

https://floooh.github.io/sokol-repros/js/wgpu-dyn-offsets.html

The sample is written under the assumption that there's a WebGPU bug, and the triangle should actually be red instead of teal (but the teal color is actually the right behaviour).

When you look at a WebGPU Inspector frame capture, it shows 'misleading' uniform data for dynamic offsets in the draw call - which looks like you had the same wrong assumption as me about the order of dynamic offsets in the setBindGroup call :)

The capture shows the value for the shader uniform variable red as 1.5, 0.0, 0.0, 0.0 and the value for the shader variable grey as 0.5, 0.5, 0.5, 0.0. But if that would be correct, the triangle would show up in red color.

In reality, the uniform values are swapped (because the dynamic offset order in the setBindGroup call is [ red, grey ] - which would be in 'bind group entry order', but the dynamic offset order actually needs to be [ grey, red ] - because the expected order is by @binding, and not by the order of entries in the BindGroupLayout (e.g. the shader variable red has @binding(1) and the shader variable grey has @binding(0)).

Phew, I hope that makes sense, I certainly needed a while to wrap my head around that behaviour (I still think that WebGPU is very counterintuitive in that one detail).

brendan-duncan commented 3 days ago

I see, I have it mapping dynamic offsets in sequential order (offset = dynamicOffsets[dynamicOffsetIndex++]) but in this case, the BGL has them out of order, and it needs to map to the binding index of the entry. Fun. I could probably just sort the BGL entries first. I'll get it fixed up.

brendan-duncan commented 3 days ago

I pushed an update to git, but I need to do more testing to make sure I didn't break anything. With the change, "red" is getting the "gray" data now because WebGPU's dynamicOffset mapping being based on binding value and not the actual order in the BGL. image