Skip to content

Fix #8756: Allow computed array indexing on non-storage vectors #8768

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
davepagurek merged 3 commits into from
May 12, 2026
Merged

Fix #8756: Allow computed array indexing on non-storage vectors #8768

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
547c52a
- Allowing get() method
SOUMITRO-SAHA May 4, 2026
e6d4b33
- Replace generic Error with FES.userError for better error handling.
SOUMITRO-SAHA May 11, 2026
57c5dea
Merge branch 'dev-2.0' into fix-8756-array-indexing
davepagurek May 12, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 4 additions & 7 deletions src/strands/strands_node.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -166,18 +166,15 @@ export class StrandsNode {
}

get(index) {
// Validate baseType is 'storage'
const nodeData = getNodeDataFromID(this.strandsContext.dag, this.id);
if (nodeData.baseType !== 'storage') {
throw new Error('get() can only be used on storage buffers');
}

// For struct storage, return a proxy with per-field getters/setters
if (this._schema) {
// Validate baseType is 'storage'
// For struct storage buffers, return a proxy with per-field getters/setters
if (nodeData.baseType === 'storage' && this._schema) {
return createStructArrayElementProxy(this.strandsContext, this, index, this._schema);
}

// Create array access node: buffer.get(index) -> buffer[index]
// Create array access node for storage and non-storage (vector) access
const { id, dimension } = arrayAccessNode(
this.strandsContext,
this,
Expand Down
8 changes: 8 additions & 0 deletions src/strands/strands_transpiler.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -566,6 +566,14 @@ const ASTCallbacks = {
if (ancestors.some(a => nodeIsUniform(a) || nodeIsUniformCallbackFn(a, state.uniformCallbackNames))) {
return;
}

if (node.elements.length < 2 || node.elements.length > 4) {
FES.userError(
'type error',
`Array literals in shader functions are transpiled to vectors and must have 2-4 elements (got ${node.elements.length}).`
);
}

const original = JSON.parse(JSON.stringify(node));
node.type = 'CallExpression';
node.callee = {
Expand Down
6 changes: 6 additions & 0 deletions src/webgl/strands_glslBackend.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -330,6 +330,12 @@ export const glslBackend = {
const parentExpr = this.generateExpression(generationContext, dag, parentID);
return `${parentExpr}.${node.swizzle}`;
}
if (node.opCode === OpCode.Binary.ARRAY_ACCESS) {
const [bufferID, indexID] = node.dependsOn;

@davepagurek davepagurek May 10, 2026

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Do you think we could add a test for the WebGL functionality too, in addition to the WebGPU tests?

const bufferExpr = this.generateExpression(generationContext, dag, bufferID);
const indexExpr = this.generateExpression(generationContext, dag, indexID);
return `${bufferExpr}[${indexExpr}]`;
}
if (node.dependsOn.length === 2) {
const [lID, rID] = node.dependsOn;
const left = this.generateExpression(generationContext, dag, lID);
Expand Down
101 changes: 101 additions & 0 deletions test/unit/webgl/p5.Shader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -679,6 +679,107 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca
assert.approximately(pixelColor[2], 153, 5);
});

suite('array indexing on non-storage vectors (#8756)', () => {
afterEach(() => {
mockUserError.mockClear();
});

test('indexing into array returned from helper function works in WebGL', () => {
myp5.createCanvas(5, 5, myp5.WEBGL);
const myShader = myp5.baseMaterialShader().modify(() => {
const brightness = myp5.uniformFloat();
function getArr() {
return [1, 2];
}
const arr = getArr();
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0] * brightness, arr[1], 0, 1];
return inputs;
});
}, { myp5 });
expect(() => {
myp5.shader(myShader);
myp5.plane(myp5.width, myp5.height);
}).not.toThrowError();
});

test('inline literal indexing [1, 2][0] works in WebGL', () => {
myp5.createCanvas(5, 5, myp5.WEBGL);
const myShader = myp5.baseMaterialShader().modify(() => {
const brightness = myp5.uniformFloat();
myp5.getPixelInputs(inputs => {
inputs.color = [[1, 2][0] * brightness, 0, 0, 1];
return inputs;
});
}, { myp5 });
expect(() => {
myp5.shader(myShader);
myp5.plane(myp5.width, myp5.height);
}).not.toThrowError();
});

test('array literal with 1 element throws descriptive error in WebGL', () => {
myp5.createCanvas(5, 5, myp5.WEBGL);
expect(() => {
myp5.baseMaterialShader().modify(() => {
const arr = [1];
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0], 0, 0, 1];
return inputs;
});
}, { myp5 });
}).toThrowError('and must have 2-4 elements (got 1)');
});

test('array literal with 5 elements throws descriptive error in WebGL', () => {
myp5.createCanvas(5, 5, myp5.WEBGL);
expect(() => {
myp5.baseMaterialShader().modify(() => {
const arr = [1, 2, 3, 4, 5];
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0], 0, 0, 1];
return inputs;
});
}, { myp5 });
}).toThrowError('and must have 2-4 elements (got 5)');
});

test('valid array lengths 2, 3, 4 work in WebGL', () => {
myp5.createCanvas(5, 5, myp5.WEBGL);
expect(() => {
const s2 = myp5.baseMaterialShader().modify(() => {
const arr = [1, 2];
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0], 0, 0, 1];
return inputs;
});
}, { myp5 });
myp5.shader(s2);
myp5.plane(myp5.width, myp5.height);

const s3 = myp5.baseMaterialShader().modify(() => {
const arr = [1, 2, 3];
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0], 0, 0, 1];
return inputs;
});
}, { myp5 });
myp5.shader(s3);
myp5.plane(myp5.width, myp5.height);

const s4 = myp5.baseMaterialShader().modify(() => {
const arr = [1, 2, 3, 4];
myp5.getPixelInputs(inputs => {
inputs.color = [arr[0], 0, 0, 1];
return inputs;
});
}, { myp5 });
myp5.shader(s4);
myp5.plane(myp5.width, myp5.height);
}).not.toThrowError();
});
});

suite('if statement conditionals', () => {
test('handle simple if statement with true condition', () => {
myp5.createCanvas(50, 50, myp5.WEBGL);
Expand Down
93 changes: 93 additions & 0 deletions test/unit/webgpu/p5.Shader.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1269,5 +1269,98 @@ suite('WebGPU p5.Shader', function() {
}).not.toThrow();
});
});

suite('array indexing on non-storage vectors (#8756)', () => {
test('indexing into array returned from helper function does not throw', async () => {
await myp5.createCanvas(5, 5, myp5.WEBGPU);
const storage = myp5.createStorage(new Float32Array(4));

const computeShader = myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
function getArray() {
return [1, 2];
}
const arr = getArray();
data[myp5.index.x] = arr[0] + arr[1] + myp5.index.x;
}, { myp5 });

computeShader.setUniform('data', storage);

expect(() => {
myp5.compute(computeShader, 4);
}).not.toThrow();
});

test('inline literal indexing [1, 2][0] works end-to-end', async () => {
await myp5.createCanvas(5, 5, myp5.WEBGPU);
const storage = myp5.createStorage(new Float32Array(4));

const computeShader = myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
data[myp5.index.x] = [1, 2][0];
}, { myp5 });

computeShader.setUniform('data', storage);

expect(() => {
myp5.compute(computeShader, 4);
}).not.toThrow();
});

test('array literal with 1 element throws descriptive error', async () => {
await myp5.createCanvas(5, 5, myp5.WEBGPU);

expect(() => {
myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
const arr = [1];
data[myp5.index.x] = arr[0];
}, { myp5 });
}).toThrow('and must have 2-4 elements (got 1)');
});

test('array literal with 5 elements throws descriptive error', async () => {
await myp5.createCanvas(5, 5, myp5.WEBGPU);

expect(() => {
myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
const arr = [1, 2, 3, 4, 5];
data[myp5.index.x] = arr[0];
}, { myp5 });
}).toThrow('and must have 2-4 elements (got 5)');
});

test('valid array lengths 2, 3, 4 do not throw', async () => {
await myp5.createCanvas(5, 5, myp5.WEBGPU);
const storage = myp5.createStorage(new Float32Array(4));

expect(() => {
const s2 = myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
const arr = [1, 2];
data[myp5.index.x] = arr[0];
}, { myp5 });
s2.setUniform('data', storage);
myp5.compute(s2, 4);

const s3 = myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
const arr = [1, 2, 3];
data[myp5.index.x] = arr[0];
}, { myp5 });
s3.setUniform('data', storage);
myp5.compute(s3, 4);

const s4 = myp5.buildComputeShader(() => {
const data = myp5.uniformStorage();
const arr = [1, 2, 3, 4];
data[myp5.index.x] = arr[0];
}, { myp5 });
s4.setUniform('data', storage);
myp5.compute(s4, 4);
}).not.toThrow();
});
});
});
});
Loading