From 04d1123074b8c474caa0854b07261dcf52f87c94 Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Sun, 24 May 2026 01:25:43 +0530 Subject: [PATCH 1/3] Add friendly FES error for dimension mismatch on shared variable assignment (#8812) --- src/strands/ir_builders.js | 6 +++- src/strands/strands_FES.js | 7 ++++ src/strands/strands_api.js | 8 +++++ src/strands/strands_node.js | 15 ++++++++ test/unit/webgl/p5.Shader.js | 65 +++++++++++++++++++++++++++++++---- test/unit/webgpu/p5.Shader.js | 42 ++++++++++++++++++++++ 6 files changed, 135 insertions(+), 8 deletions(-) diff --git a/src/strands/ir_builders.js b/src/strands/ir_builders.js index 465572bebb..4ae69d90f4 100644 --- a/src/strands/ir_builders.js +++ b/src/strands/ir_builders.js @@ -572,7 +572,11 @@ export function swizzleTrap(id, dimension, strandsContext, onRebind) { scalars.push(createStrandsNode(id, dimension, strandsContext)); } } else { - FES.userError('type error', `Swizzle assignment: RHS vector does not match LHS vector (need ${chars.length}, got ${value.dimension}).`); + FES.dimensionMismatchError( + chars.length, + value.dimension, + `${target._originalIdentifier || 'value'}.${property}` + ); } } else if (Array.isArray(value)) { const flat = value.flat(Infinity); diff --git a/src/strands/strands_FES.js b/src/strands/strands_FES.js index 3af0aca90b..245ea4a302 100644 --- a/src/strands/strands_FES.js +++ b/src/strands/strands_FES.js @@ -6,4 +6,11 @@ export function internalError(errorMessage) { export function userError(errorType, errorMessage) { const prefixedMessage = `[p5.strands ${errorType}]: ${errorMessage}`; throw new Error(prefixedMessage); +} + +export function dimensionMismatchError(declaredDim,actualDim,varName){ + userError( + 'dimension mismatch', + `Cannot assign a value of dimension ${actualDim} to \`${varName}\`, which expects dimension ${declaredDim}.` + ); } \ No newline at end of file diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index 8675dc69b6..6fd60a72ca 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -671,6 +671,14 @@ function createHookArguments(strandsContext, parameters){ return createStrandsNode(propNode.id, propNode.dimension, strandsContext, onRebind); }, set(val) { + + if(val?.isStrandsNode&&val.dimension!==propertyType.dataType.dimension){ + FES.dimensionMismatchError( + propertyType.dataType.dimension, + val.dimension, + `${param.name}.${propertyType.name}` + ); + } const oldDependsOn = dag.dependsOn[structNode.id]; const newDependsOn = [...oldDependsOn]; let newValueID; diff --git a/src/strands/strands_node.js b/src/strands/strands_node.js index f7638855bc..1644465780 100644 --- a/src/strands/strands_node.js +++ b/src/strands/strands_node.js @@ -2,6 +2,7 @@ import { swizzleTrap, primitiveConstructorNode, variableNode, arrayAccessNode, a import { BaseType, NodeType, OpCode } from './ir_types'; import { getNodeDataFromID, createNodeData, getOrCreateNode } from './ir_dag'; import { recordInBasicBlock } from './ir_cfg'; +import { dimensionMismatchError } from './strands_FES'; export class StrandsNode { constructor(id, dimension, strandsContext) { this.id = id; @@ -56,6 +57,13 @@ export class StrandsNode { // For varying variables, we need both assignment generation AND a way to reference by identifier if (this._originalIdentifier) { + if(value?.isStrandsNode && value.dimension!==this._originalDimension){ + dimensionMismatchError( + this._originalDimension, + value.dimension, + this._originalIdentifier + ); + } // Create a variable node for the target (the varying variable) const { id: targetVarID } = variableNode( this.strandsContext, @@ -108,6 +116,13 @@ export class StrandsNode { // For varying variables, create swizzle assignment if (this._originalIdentifier) { + if(value?.isStrandsNode && value.dimension!==swizzlePattern.length){ + dimensionMismatchError( + swizzlePattern.length, + value.dimension, + `${this._originalIdentifier}.${swizzlePattern}` + ); + } // Create a variable node for the target with swizzle const { id: targetVarID } = variableNode( this.strandsContext, diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 786540cafa..c38eaa5c53 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -1,15 +1,24 @@ import p5 from '../../../src/app.js'; -import { vi } from 'vitest'; +import { beforeEach, vi } from 'vitest'; const mockUserError = vi.fn(); -vi.mock('../../../src/strands/strands_FES', () => ({ - userError: (...args) => { +vi.mock('../../../src/strands/strands_FES', () => { + const userError = (...args) => { mockUserError(...args); const prefixedMessage = `[p5.strands ${args[0]}]: ${args[1]}`; throw new Error(prefixedMessage); - }, - internalError: (msg) => { throw new Error(`[p5.strands internal error]: ${msg}`); } -})); + }; + return { + userError, + internalError: (msg) => { throw new Error(`[p5.strands internal error]: ${msg}`); }, + dimensionMismatchError: (declaredDim, actualDim, varName) => { + userError( + 'dimension mismatch', + `Cannot assign a value of dimension ${actualDim} to \`${varName}\`, which expects dimension ${declaredDim}.` + ); + }, + }; +}); suite('p5.Shader', function() { var myp5; @@ -2608,6 +2617,48 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.approximately(pixelColor[1], 0, 5); assert.approximately(pixelColor[2], 0, 5); }); + + test('reports a friendly error when assigning a scalar to a sharedVec3 (bridge)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGL); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let worldPosX = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + worldPosX = inputs.position.x; // scalar → vec3 mismatch + return inputs; + }); + },{myp5}); + }).toThrow(/dimension mismatch/); +}); + +test('reports a friendly error on dimension mismatch via swizzle write (bridgeSwizzle)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGL); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec.xy = inputs.position; // vec3 → 2-component swizzle mismatch + return inputs; + }); + },{myp5}); + }).toThrow(/dimension mismatch/); +}); + +test('does not error when shared variable assignment dimensions match', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGL); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position; // vec3 → vec3, OK + return inputs; + }); + },{myp5}); + }).not.toThrow(); +}); }); suite('p5.strands error messages', () => { @@ -2625,7 +2676,7 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.include(err.message, '// noprotect'); }; - afterEach(() => { + beforeEach(() => { mockUserError.mockClear(); }); diff --git a/test/unit/webgpu/p5.Shader.js b/test/unit/webgpu/p5.Shader.js index eb9bb79990..5f60b5db65 100644 --- a/test/unit/webgpu/p5.Shader.js +++ b/test/unit/webgpu/p5.Shader.js @@ -1361,6 +1361,48 @@ suite('WebGPU p5.Shader', function() { myp5.compute(s4, 4); }).not.toThrow(); }); + + test('reports a friendly error when assigning a scalar to a sharedVec3 (bridge)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let worldPosX = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + worldPosX = inputs.position.x; // scalar → vec3 mismatch + return inputs; + }); + },{myp5}); + }).toThrow(/dimension mismatch/); +}); + +test('reports a friendly error on dimension mismatch via swizzle write (bridgeSwizzle)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec.xy = inputs.position; // vec3 → 2-component swizzle mismatch + return inputs; + }); + },{myp5}); + }).toThrow(/dimension mismatch/); +}); + +test('does not error when shared variable assignment dimensions match', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position; // vec3 → vec3, OK + return inputs; + }); + },{myp5}); + }).not.toThrow(); +}); }); }); }); From 19462adfb0882c34d3f608d1ebbd081b33f27e82 Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Tue, 9 Jun 2026 02:24:43 +0530 Subject: [PATCH 2/3] Validate value dimension on shared variable assignment, allowing scalar broadcast --- src/strands/strands_api.js | 11 ++++++----- src/strands/strands_node.js | 10 ++++++---- test/unit/webgl/p5.Shader.js | 18 ++++++++++++++++-- test/unit/webgpu/p5.Shader.js | 18 ++++++++++++++++-- 4 files changed, 44 insertions(+), 13 deletions(-) diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index 7096c672b3..055db72b83 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -686,14 +686,15 @@ function createHookArguments(strandsContext, parameters){ return createStrandsNode(propNode.id, propNode.dimension, strandsContext, onRebind); }, set(val) { - - if(val?.isStrandsNode&&val.dimension!==propertyType.dataType.dimension){ + const valDim = val?.isStrandsNode?val.dimension:(Array.isArray(val)?val.length:1); + if(valDim!==propertyType.dataType.dimension&&valDim!==1){ FES.dimensionMismatchError( - propertyType.dataType.dimension, - val.dimension, - `${param.name}.${propertyType.name}` + propertyType.dataType.dimension, + valDim, + `${param.name}.${propertyType.name}` ); } + const oldDependsOn = dag.dependsOn[structNode.id]; const newDependsOn = [...oldDependsOn]; let newValueID; diff --git a/src/strands/strands_node.js b/src/strands/strands_node.js index 1644465780..d6102dbe42 100644 --- a/src/strands/strands_node.js +++ b/src/strands/strands_node.js @@ -57,10 +57,11 @@ export class StrandsNode { // For varying variables, we need both assignment generation AND a way to reference by identifier if (this._originalIdentifier) { - if(value?.isStrandsNode && value.dimension!==this._originalDimension){ + const valueDim = value?.isStrandsNode?value.dimension:(Array.isArray(value)?value.length:1); + if(valueDim!==this._originalDimension&&valueDim!==1){ dimensionMismatchError( this._originalDimension, - value.dimension, + valueDim, this._originalIdentifier ); } @@ -116,10 +117,11 @@ export class StrandsNode { // For varying variables, create swizzle assignment if (this._originalIdentifier) { - if(value?.isStrandsNode && value.dimension!==swizzlePattern.length){ + const valueDim = value?.isStrandsNode?value.dimension:(Array.isArray(value)?value.length:1); + if(valueDim!==swizzlePattern.length&&valueDim!==1){ dimensionMismatchError( swizzlePattern.length, - value.dimension, + valueDim, `${this._originalIdentifier}.${swizzlePattern}` ); } diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 04068b41d9..7a1297263c 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -2641,14 +2641,28 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.approximately(pixelColor[2], 0, 5); }); - test('reports a friendly error when assigning a scalar to a sharedVec3 (bridge)', async () => { + test('allows scalar broadcast when assigning a scalar to a sharedVec3 (bridge)', async () => { await myp5.createCanvas(5, 5, myp5.WEBGL); expect(() => { myp5.baseMaterialShader().modify(() => { let worldPosX = myp5.sharedVec3(); myp5.getWorldInputs(inputs => { - worldPosX = inputs.position.x; // scalar → vec3 mismatch + worldPosX = inputs.position.x; // scalar → vec3, valid broadcast + return inputs; + }); + },{myp5}); + }).not.toThrow(); +}); + +test('reports a friendly error when assigning a vec2 to a sharedVec3 (bridge)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGL); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position.xy; // vec2 → vec3 mismatch return inputs; }); },{myp5}); diff --git a/test/unit/webgpu/p5.Shader.js b/test/unit/webgpu/p5.Shader.js index 5f60b5db65..5b368ac319 100644 --- a/test/unit/webgpu/p5.Shader.js +++ b/test/unit/webgpu/p5.Shader.js @@ -1362,14 +1362,28 @@ suite('WebGPU p5.Shader', function() { }).not.toThrow(); }); - test('reports a friendly error when assigning a scalar to a sharedVec3 (bridge)', async () => { + test('allows scalar broadcast when assigning a scalar to a sharedVec3 (bridge)', async () => { await myp5.createCanvas(5, 5, myp5.WEBGPU); expect(() => { myp5.baseMaterialShader().modify(() => { let worldPosX = myp5.sharedVec3(); myp5.getWorldInputs(inputs => { - worldPosX = inputs.position.x; // scalar → vec3 mismatch + worldPosX = inputs.position.x; // scalar → vec3, valid broadcast + return inputs; + }); + },{myp5}); + }).not.toThrow(); +}); + +test('reports a friendly error when assigning a vec2 to a sharedVec3 (bridge)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position.xy; // vec2 → vec3 mismatch return inputs; }); },{myp5}); From 70ada893233322b6ef872924d6a1a0860b91054b Mon Sep 17 00:00:00 2001 From: harshiltewari2004 Date: Tue, 9 Jun 2026 13:09:31 +0530 Subject: [PATCH 3/3] Format dimension check ternary and fix webgpu test indentation per review --- src/strands/strands_api.js | 6 ++- src/strands/strands_node.js | 12 +++-- test/unit/webgpu/p5.Shader.js | 88 +++++++++++++++++------------------ 3 files changed, 56 insertions(+), 50 deletions(-) diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index 055db72b83..2b6e6eba2b 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -686,8 +686,10 @@ function createHookArguments(strandsContext, parameters){ return createStrandsNode(propNode.id, propNode.dimension, strandsContext, onRebind); }, set(val) { - const valDim = val?.isStrandsNode?val.dimension:(Array.isArray(val)?val.length:1); - if(valDim!==propertyType.dataType.dimension&&valDim!==1){ + const valDim = val?.isStrandsNode + ? val.dimension + : (Array.isArray(val) ? val.length : 1); + if( valDim !== propertyType.dataType.dimension && valDim !== 1){ FES.dimensionMismatchError( propertyType.dataType.dimension, valDim, diff --git a/src/strands/strands_node.js b/src/strands/strands_node.js index d6102dbe42..3973406659 100644 --- a/src/strands/strands_node.js +++ b/src/strands/strands_node.js @@ -57,8 +57,10 @@ export class StrandsNode { // For varying variables, we need both assignment generation AND a way to reference by identifier if (this._originalIdentifier) { - const valueDim = value?.isStrandsNode?value.dimension:(Array.isArray(value)?value.length:1); - if(valueDim!==this._originalDimension&&valueDim!==1){ + const valueDim = value?.isStrandsNode + ? value.dimension + : (Array.isArray(value) ? value.length : 1); + if (valueDim !== this._originalDimension && valueDim !== 1){ dimensionMismatchError( this._originalDimension, valueDim, @@ -117,8 +119,10 @@ export class StrandsNode { // For varying variables, create swizzle assignment if (this._originalIdentifier) { - const valueDim = value?.isStrandsNode?value.dimension:(Array.isArray(value)?value.length:1); - if(valueDim!==swizzlePattern.length&&valueDim!==1){ + const valueDim = value?.isStrandsNode + ? value.dimension + : (Array.isArray(value) ? value.length : 1); + if (valueDim !== swizzlePattern.length && valueDim !== 1){ dimensionMismatchError( swizzlePattern.length, valueDim, diff --git a/test/unit/webgpu/p5.Shader.js b/test/unit/webgpu/p5.Shader.js index 5b368ac319..249fba6f87 100644 --- a/test/unit/webgpu/p5.Shader.js +++ b/test/unit/webgpu/p5.Shader.js @@ -1363,60 +1363,60 @@ suite('WebGPU p5.Shader', function() { }); test('allows scalar broadcast when assigning a scalar to a sharedVec3 (bridge)', async () => { - await myp5.createCanvas(5, 5, myp5.WEBGPU); - - expect(() => { - myp5.baseMaterialShader().modify(() => { - let worldPosX = myp5.sharedVec3(); - myp5.getWorldInputs(inputs => { - worldPosX = inputs.position.x; // scalar → vec3, valid broadcast - return inputs; + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + let worldPosX = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + worldPosX = inputs.position.x; // scalar → vec3, valid broadcast + return inputs; + }); + }, { myp5 }); + }).not.toThrow(); }); - },{myp5}); - }).not.toThrow(); -}); -test('reports a friendly error when assigning a vec2 to a sharedVec3 (bridge)', async () => { - await myp5.createCanvas(5, 5, myp5.WEBGPU); + test('reports a friendly error when assigning a vec2 to a sharedVec3 (bridge)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); - expect(() => { - myp5.baseMaterialShader().modify(() => { - let myVec = myp5.sharedVec3(); - myp5.getWorldInputs(inputs => { - myVec = inputs.position.xy; // vec2 → vec3 mismatch - return inputs; + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position.xy; // vec2 → vec3 mismatch + return inputs; + }); + }, { myp5 }); + }).toThrow(/dimension mismatch/); }); - },{myp5}); - }).toThrow(/dimension mismatch/); -}); -test('reports a friendly error on dimension mismatch via swizzle write (bridgeSwizzle)', async () => { - await myp5.createCanvas(5, 5, myp5.WEBGPU); + test('reports a friendly error on dimension mismatch via swizzle write (bridgeSwizzle)', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); - expect(() => { - myp5.baseMaterialShader().modify(() => { - let myVec = myp5.sharedVec3(); - myp5.getWorldInputs(inputs => { - myVec.xy = inputs.position; // vec3 → 2-component swizzle mismatch - return inputs; + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec.xy = inputs.position; // vec3 → 2-component swizzle mismatch + return inputs; + }); + },{myp5}); + }).toThrow(/dimension mismatch/); }); - },{myp5}); - }).toThrow(/dimension mismatch/); -}); -test('does not error when shared variable assignment dimensions match', async () => { - await myp5.createCanvas(5, 5, myp5.WEBGPU); + test('does not error when shared variable assignment dimensions match', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); - expect(() => { - myp5.baseMaterialShader().modify(() => { - let myVec = myp5.sharedVec3(); - myp5.getWorldInputs(inputs => { - myVec = inputs.position; // vec3 → vec3, OK - return inputs; + expect(() => { + myp5.baseMaterialShader().modify(() => { + let myVec = myp5.sharedVec3(); + myp5.getWorldInputs(inputs => { + myVec = inputs.position; // vec3 → vec3, OK + return inputs; + }); + },{myp5}); + }).not.toThrow(); }); - },{myp5}); - }).not.toThrow(); -}); }); }); });