Repository navigation
Fix storage element type check and FES parameter data for createStorage/buildComputeShader - #9355
Conversation
Continuous ReleaseCDN linkPublished PackagesCommit hash: 4dc5a53 Previous deploymentsThis is an automated message. |
davepagurek
left a comment
There was a problem hiding this comment.
Thanks for catching this, the change looks good to me.
Just thinking through some next steps for this atomic system: right now we don't handle wgsl struct arrays that have atomics in them, right? And in struct data coming from js, we never get atomics because there's no js equivalent, so that would just be an issue for manually-written WGSL code? Possibly some next steps would eventually be to handle atomics inside structs, and think of a way to do all these things in js strands.
|
yeah that's right @davepagurek , struct arrays like array just get skipped by the check since the element type isn't f32/u32/i32, and struct data from js is always packed as floats so u would only hit this with handwritten wgsl that puts an atomic in a struct atomics inside structs + a strands way to do it seems a good next step. StorageList already generates an atomic length inside a struct, so there is kind of a pattern to build from there. let me know I will create an issue for the same. |
|
sure, we can make issues, although also no rush right now -- if it's a task you're interested in working on but don't have the bandwidth yet it's also ok to wait a bit until you do. we'll be busy with 2.4 things for a little bit anyway 🙂 (If it's something you want to work on now or open up to the community to work on we can do issues now too!) |
|
yes let’s wind up 2.4 properly first. I’ll start looking into atomics in structs after that. |
This is a follow up to #9083, I found this while testing v2.4.0-rc.2
The mismatch warning from #9083 never actually fired. #9086 widened the storage regex for StorageList, so it now grabs the whole type like
array<atomic<u32>>instead of justatomic<u32>, and the type lookup returned nothing. My old tests mocked the parsed object so they missed it.Fixed it by unwrapping
array<T>/array<T, N>to get the element type. once that worked, StorageLists in render shaders started hitting the check and crashed since they don't have_arrayType, which brokeinstances(storageList). Lists only hold floats or structs so the check just skips them now.Also
parameterData.jsonwas never regenerated after #9083, socreateStorage(new Uint32Array(...))gave a false FES warning. AndbuildComputeShader()only listed a function param, so the hooks object form from the reference logged a raw zod error. I added the missing overloads and updated just those 3 entriesI also added the "Storage element type check" suite that goes through the real shader parsing
PR Checklist
npm run lintpasses