Incorrect indices for push constants in webgl/gles backend

[mistodon]

Description

Note: I have a possible fix for this bug: https://github.com/mistodon/wgpu/tree/gles_push_constant_fix - but I would like to clarify my understanding of the bug first.

There seems to be a bug in gles/command/set_push_constants where the indices into the actual push constant data are incorrect if the start_index is not zero. I believe this is because of a confusion in two different address spaces:

In the above image, I've highlighted in cyan variables that index into the command buffer's data_bytes. This is a separate space from the magenta which are the conceptual push constant indices. So if I push two sets of constants for two different draw calls with the same (magenta) start index - they will be stored at different (cyan) indices within data_bytes.

But we're wrongly combining them in the C::SetPushConstants command, because offset starts at start_offset, but the actual data is stored in range. When start_offset is zero, this doesn't matter - but with any other number, you end up with the wrong indices. For example in my app I was seeing:

panicked at 'range start index 448 out of range for slice of length 432', /.../wgpu/wgpu-hal/src/gles/queue.rs:1412:32

And with some janky logging:

Setting push constants
  start_offset = 0  data len in bytes = 80
  range = 0..80
    offset = 0  size = 64
      range.start + offset = 0
    offset = 64  size = 16
      range.start + offset = 64
Setting push constants
  start_offset = 80  data len in bytes = 64
  range = 80..144
    offset = 80  size = 16
      range.start + offset = 160
    offset = 96  size = 16
      range.start + offset = 176
    offset = 112  size = 16
      range.start + offset = 192
    offset = 128  size = 16
      range.start + offset = 208
Setting push constants
  start_offset = 0  data len in bytes = 80
  range = 144..224
    offset = 0  size = 64
      range.start + offset = 144
    offset = 64  size = 16
      range.start + offset = 208
Setting push constants
  start_offset = 80  data len in bytes = 64
  range = 224..288
    offset = 80  size = 16
      range.start + offset = 304
    offset = 96  size = 16
      range.start + offset = 320
    offset = 112  size = 16
      range.start + offset = 336
    offset = 128  size = 16
      range.start + offset = 352
Setting push constants
  start_offset = 0  data len in bytes = 80
  range = 288..368
    offset = 0  size = 64
      range.start + offset = 288
    offset = 64  size = 16
      range.start + offset = 352
Setting push constants
  start_offset = 80  data len in bytes = 64
  range = 368..432
    offset = 80  size = 16
      range.start + offset = 448

Repro steps
I modified the hello-triangle example in a branch to demonstrate the bug: https://github.com/mistodon/wgpu/tree/gles_push_constants_bug_example/examples/hello-triangle

To run in browser, from the hello-triangle dir:

RUSTFLAGS=--cfg=web_sys_unstable_apis cargo build --target wasm32-unknown-unknown
wasm-bindgen ../../target/wasm32-unknown-unknown/debug/hello-triangle.wasm --out-dir gen --web
http gen

Expected vs observed behavior
Expected behaviour is that push constants will not be out of range. Observed behaviour as described above.

Extra materials

Possible fix: https://github.com/mistodon/wgpu/tree/gles_push_constant_fix

Screenshot of above repro with the fix applied:

Platform

• macOS 12.6.2
• Firefox 109.0.1
• latest wgpu trunk (also occurs in 16.1)

[cwfitzgerald]

Woah, this is by far the most thorough issue I've seen! This is great!

We do actually have push constant layout tests in our test suite which have been failing on GL. It's https://github.com/gfx-rs/wgpu/blob/trunk/tests/tests/shader/struct_layout.rs#L216 - see if they help in debugging.

Everything makes sense to me!

[cwfitzgerald]

This was pushed through to fruition in #4607