Fix #8756: Allow computed array indexing on non-storage vectors#8768
Fix #8756: Allow computed array indexing on non-storage vectors#8768SOUMITRO-SAHA wants to merge 2 commits intoprocessing:dev-2.0from
Conversation
- Enable array indexing for storage and vector buffers - Added validation for array literals in shaders (2-4 elements only)
davepagurek
left a comment
davepagurek
left a comment
There was a problem hiding this comment.
Thanks for working on this, the code is looking good!
| } | ||
|
|
||
| if (node.elements.length < 2 || node.elements.length > 4) { | ||
| throw new Error( |
There was a problem hiding this comment.
Maybe we can use this helper for consistency?
p5.js/src/strands/strands_FES.js
Line 6 in d3bb21c
| return `${parentExpr}.${node.swizzle}`; | ||
| } | ||
| if (node.opCode === OpCode.Binary.ARRAY_ACCESS) { | ||
| const [bufferID, indexID] = node.dependsOn; |
There was a problem hiding this comment.
Do you think we could add a test for the WebGL functionality too, in addition to the WebGPU tests?
|
Thanks for the review, @davepagurek Great suggestions — I'll address both: 1. 2. WebGL tests — Absolutely. I'll push the updates shortly. |
- Add comprehensive tests for WebGL.
|
Hi @davepagurek , I've pushed the changes addressing both of your review comments |
2 participants
Resolves #8756
Changes:
This PR fixes the bug where
get()could only be used on storage buffers inp5.strands, preventing array indexing (arr[0]) on non-storage vectors like those returned from helper functions.Root Cause:
The transpiler converted ALL computed member access (
arr[0]) to.get()calls, but theget()method instrands_node.jshad a guard that only allowed it for storage buffers, throwing "get() can only be used on storage buffers" for non-storage vectors.src/strands/strands_node.jssrc/strands/strands_transpiler.jssrc/webgl/strands_glslBackend.js(AddedARRAY_ACCESShandler, missing from GLSL backend - flagged by @Nixxx19 ).test/unit/webgpu/p5.Shader.jsScreenshots of the change:
PR Checklist
npm run lintpasses