feat(webgpu): add read() to p5.StorageBuffer with tests#8726
feat(webgpu): add read() to p5.StorageBuffer with tests#8726aashu2006 wants to merge 2 commits intoprocessing:dev-2.0from
Conversation
|
@davepagurek Hey, I’ve added the |
|
|
||
| // Inverse of _packStructArray reads packed buffer back into plain JS objects | ||
| // using the same schema layout - fields, stride and offsets | ||
| _unpackStructArray(floatView, schema) { |
There was a problem hiding this comment.
Do you think we can move this to the renderer next to _packStructArray for consistency, and defer to the renderer from this class?
There was a problem hiding this comment.
Yes, that makes sense. I will move _unpackStructArray next to _packStructArray in the renderer and call it from here for consistency.
|
|
||
| expect(result).to.be.an('array'); | ||
| expect(result.length).to.equal(2); | ||
| // Vector fields come back as plain arrays |
There was a problem hiding this comment.
ahh good catch. I guess we don't currently capture in the schema if these values came from arrays, vectors, or colors. I think we might want to record that so we can deserialize into the same format it came in as. Do you think something like that is feasible?
There was a problem hiding this comment.
yeah that’s a good point, right now we’re just returning plain data, but preserving the original types like vectors or colors would be useful.
I think it’s doable by extending the schema to store the original input type, but it might add some complexity. I can look into that as a follow-up issue if that sounds good.
There was a problem hiding this comment.
I think it might be worth exploring upfront, just because it changes the API a bit -- e.g. if we were to release just this then change the structure of return values, it could break users' code. (I don't think we're going to do a release in between PRs, but that's generally how I think about API changes -- we generally want to avoid, if we can, merging code with an API that we think we'll change.)
There was a problem hiding this comment.
If we think the return format might change, it’s better to settle that before merging. Would you prefer that we preserve original types like vectors or colors in this PR, or keep the current plain data approach and commit to that API for now?
|
this is looking good, and thanks for the detailed tests! |
|
@davepagurek I've pushed the changes moved |
|
@davepagurek Just wanted to follow up on the return format question, happy to proceed either way, just wanted to check what direction you'd prefer here |
2 participants
Resolves #8722
Changes:
I’ve added an async
.read()method top5.StorageBufferso we can read data back from GPU buffers into JavaScript.mapAsyncto copy data from GPU -> CPUFloat32Array_unpackStructArray()helper to rebuild struct data from the packed bufferreadFramebufferPixelsTests
I've added unit tests for:
.update()Some Notes
p5.Vectororp5.Color.read()introduces a GPU - CPU sync point, so frequent use maybe can impact the performancePR Checklist
npm run lintpasses