Infer storage buffer element type from the typed array passed in - #9083
Infer storage buffer element type from the typed array passed in#9083aashu2006 wants to merge 4 commits into
Conversation
davepagurek
left a comment
davepagurek
left a comment
There was a problem hiding this comment.
Thanks, this is looking good! Just added a bit of feedback around performance and docs.
| } else if (shader._storageBuffers) { | ||
| // The shader has been parsed, so we know what element type it | ||
| // declares for this buffer and can check it early | ||
| const parsedStorage = shader._storageBuffers.find( |
There was a problem hiding this comment.
Is this something we can turn into an object instead of an array so that we can look it up without looping over all the buffers? probably not a huge deal if you don't have too many of them, but it feels like something we should make a habit of optimizing since there may be many of these per frame
There was a problem hiding this comment.
Good call, I've added shader._storageBuffersByName alongside the array, built once at parse time, so setUniform does a keyed lookup instead of scanning. Kept the array too since a couple of other spots iterate it every frame.
| if (storageBuffer._schema !== null) return; | ||
|
|
||
| // atomic<u32> and friends store their underlying type | ||
| const elementType = parsedStorage.elementType.replace( |
There was a problem hiding this comment.
It looks like we'd also be calling this every time right? Is this something we can cache?
There was a problem hiding this comment.
Yup, it was. Cached in two places now: the WGSL -> typed array resolution happens once at parse time (expectedArrayType), and the buffer remembers which type it was last checked against, so repeat calls exit on a comparison. The warning still only fires once.
| * } | ||
| * ``` | ||
| * | ||
| * ```js example |
There was a problem hiding this comment.
This is a fairly advanced example so we should probably add some text above it explaining why you'd use it this way. The pitch is probably something like, while we're developing p5.strands, you might want to reach for features in wgsl that we haven't added yet, and here's how you might use atomics in compute shaders?
There was a problem hiding this comment.
added an intro along those lines - while p5.strands is still growing you may want WGSL features it doesn't cover yet, with atomics as the example since WGSL only allows them on u32/i32.
| * await createCanvas(100, 100, WEBGPU); | ||
| * | ||
| * data = createStorage(new Uint32Array([10, 20, 30, 40])); | ||
| * computeShader = baseComputeShader().modify({ |
There was a problem hiding this comment.
I think we can still pass an object into buildComputeShader(...) same as baseComputeShader().modify(...), so we can simplify a little bit
There was a problem hiding this comment.
nice, simplified to buildComputeShader({...})
| // Copy before unmapping because mapped memory becomes invalid after unmap | ||
| const rawCopy = new Float32Array(mappedRange.byteLength / 4); | ||
| rawCopy.set(new Float32Array(mappedRange)); | ||
| const ArrayType = this._arrayType; |
867ed7e to
e0a49c3
Compare
2 participants
Changes:
First piece of the compute shader work in #8820, which I'm picking up from @davepagurek.
Storage buffers currently assume every element is an f32. WGSL only supports atomics on u32 and i32, so a shader declaring
array<atomic<u32>>writes ints that come back to JS as meaningless floats. The atomic sketch in #8820has to reinterpret the bytes by hand because of this.
StorageBuffernow carries an_arrayType, inferred from the typed array passed tocreateStorage, andread()/update()use it instead of hardcodingFloat32Array. Plain JS arrays andcreateStorage(count)still giveFloat32Array, so existing sketches are unaffected.The storage regex also needed widening to match
array<atomic<u32>>, and it now keeps the element type it captures. That gets used for a friendly error when the JS typed array doesn't match what the shader declares, so aFloat32Arraybound to anarray<atomic<u32>>says so instead of silently reading back garbage. Checked fromsetUniformand from the bind group path, warning once per buffer so it doesn't spam a draw loop.Reference docs for
read(),update()andcreateStorage()were updated, including a second example showing the atomic u32 case. Note that@paramtypes are validated at runtime against globals, so these listFloat32Array|Uint32Array|Int32Arrayrather than an abstractTypedArray.Testing:
Verified in Chrome on WebGPU:
array<atomic<u32>>seeded with aUint32Arrayreads back as integers with no reinterpretingFloat32Arrayand plain arrays unchanged, includingupdate()Float32Array, no spurious warningsetUniforminsidedraw()warned exactly once across 300 framesUnit tests for the mismatch check are in
test/unit/webgpu-storage-element-type.js, kept outsidetest/unit/webgpu/so the
unit-testsproject picks them up. Happy to move them if there's a better home.Next from #8820 is
controlFlow: 'automatic' | 'manual'plus the workgroup builtins, which can be opened separately once this lands.PR Checklist
npm run lintpasses