cg-tuwien / Math2Model

Build the Shadertoy of Parametric Modeling
4 stars 1 forks source link

Fix multiple race conditions #22

Closed stefnotch closed 6 months ago

stefnotch commented 6 months ago

StorageBarrier

Today we found out that storageBarrier in WGSL did not do what we expected. https://stackoverflow.com/questions/72035548/what-does-storagebarrier-in-webgpu-actually-do

storageBarrier only synchronises within a single workgroup. It does not synchronise between the workgroups of a single dispatch.

We had code like this

  storageBarrier(); // only waits for the current workgroup. :(
  if(global_id.x == 0u && global_id.y == 0u && global_id.z == 0u) {
    dispatch_next.x = ceil_div(atomicLoad(&patches_to_buffer.patches_length), WORKGROUP_SIZE);
  }

and fixed it by no longer letting the threads race, and instead letting all threads do the work and using an atomic for synchronisation

// everyone does this
atomicMax(&dispatch_next.x, ceil_div(final_patches_length, WORKGROUP_SIZE));

An alternative fix would have been writing a compute shader which runs a single line of code. dispatch_next.x = ceil_div(atomicLoad(&patches_to_buffer.patches_length), WORKGROUP_SIZE);

Babylon.js bypassing

When one calls a perfectly normal looking function

computePatchesShader.value[0].setUniformBuffer(
    "input_buffer",
    patchesInputBuffer.value
  );

then Babylon.js will internally keep track of it, and of course not record it as a command. We then proceeded to write code like

loop {
// Immediately change the uniform
// and do weird internal stuff
computePatchesShader.value.setUniformBuffer(
    "patches_from_buffer",
    patchesBuffer.value[0]
  );

// record a command
// this gets submitted at a later date
// so it'll only see the last *computePatchesShader.value.setUniformBuffer* call. aaah
computePatchesShader.value.dispatchIndirect(indirectComputeBuffer.value[1]);
}

Use copyBufferToBuffer

We're using copyBufferToBuffer instead of .update. The difference is that copyBufferToBuffer is a command that gets recorded, and properly executed at the proper time. .update happens instantly.

So

// Will happen in the future, whenever Babylon.js feels like it
computePatchesShader.value.dispatchIndirect(indirectComputeBuffer.value[1]);

// Happens now
computePatchesShader.value.update(some new value)

// Babylon.js is getting around calling `dispatchIndirect`. And now sees the update that happened afterwards in the code.
// *cries*