From 8a7d1a30401e61f1c2d0b69c2ede0b82a2cbbcce Mon Sep 17 00:00:00 2001 From: saurabh24thakur Date: Mon, 2 Mar 2026 11:07:40 +0530 Subject: [PATCH 1/3] Fix #8575: Correct builtin global accessors in p5.strands instance mode --- src/strands/strands_api.js | 77 +++++++++++++++++++++++++++++------- test/unit/webgl/p5.Shader.js | 45 ++++++++++++--------- 2 files changed, 89 insertions(+), 33 deletions(-) diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index ef4c0424c3..8826418999 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -94,24 +94,71 @@ function getBuiltinGlobalNode(strandsContext, name) { return node; } -function installBuiltinGlobalAccessors(strandsContext) { - if (strandsContext._builtinGlobalsAccessorsInstalled) return +function installBuiltinGlobalAccessors(strandsContext, fn) { + if (strandsContext._builtinGlobalsAccessorsInstalled) return; - const getRuntimeP5Instance = () => strandsContext.renderer?._pInst || strandsContext.p5?.instance + const GraphicsProto = strandsContext.p5?.Graphics?.prototype; + + // Targets where we want these accessors to be available + const targets = [window]; + if (fn) targets.push(fn); + if (GraphicsProto) targets.push(GraphicsProto); for (const name of Object.keys(BUILTIN_GLOBAL_SPECS)) { - const spec = BUILTIN_GLOBAL_SPECS[name] - Object.defineProperty(window, name, { - get: () => { - if (strandsContext.active) { - return getBuiltinGlobalNode(strandsContext, name); - } - const inst = getRuntimeP5Instance() - return spec.get(inst); - }, - }) + const spec = BUILTIN_GLOBAL_SPECS[name]; + + targets.forEach((target) => { + // Find the original descriptor if it exists on this target or its prototype chain + let originalDescriptor = null; + let curr = target; + while (curr && !originalDescriptor) { + originalDescriptor = Object.getOwnPropertyDescriptor(curr, name); + if (originalDescriptor) break; + curr = Object.getPrototypeOf(curr); + } + + Object.defineProperty(target, name, { + get: function() { + // If a shader is active, return the special uniform Node + if (strandsContext.active) { + return getBuiltinGlobalNode(strandsContext, name); + } + + // Fallback: Get the normal value (like mouseX as a number) + // If we have an original descriptor, use it to avoid recursion + if (originalDescriptor) { + if (originalDescriptor.get) { + return originalDescriptor.get.call(this); + } else if ('value' in originalDescriptor) { + return originalDescriptor.value; + } + } + + // Last resort: use the spec's getter + return spec.get(this); + }, + set: function(val) { + if (originalDescriptor && originalDescriptor.set) { + originalDescriptor.set.call(this, val); + } else { + // Shadow with a data property on the instance. + // This allows things like 'this.deltaTime = ...' to work in p5._draw + // without hitting the prototype's getter-only definition. + Object.defineProperty(this, name, { + value: val, + writable: true, + configurable: true, + enumerable: true + }); + } + }, + configurable: true, + enumerable: true + }); + }); } - strandsContext._builtinGlobalsAccessorsInstalled = true + + strandsContext._builtinGlobalsAccessorsInstalled = true; } ////////////////////////////////////////////// @@ -587,7 +634,7 @@ function enforceReturnTypeMatch(strandsContext, expectedType, returned, hookName return returnedNodeID; } export function createShaderHooksFunctions(strandsContext, fn, shader) { - installBuiltinGlobalAccessors(strandsContext) + installBuiltinGlobalAccessors(strandsContext, fn) // Add shader context to hooks before spreading const vertexHooksWithContext = Object.fromEntries( diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 2556c1d25d..24c66d3d93 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -474,25 +474,34 @@ suite('p5.Shader', function() { }).not.toThrowError(); }); -test('returns numbers for builtin globals outside hooks and a strandNode when called inside hooks', () => { - myp5.createCanvas(5, 5, myp5.WEBGL); - myp5.baseMaterialShader().modify(() => { - myp5.getPixelInputs(inputs => { - const mxInHook = window.mouseX; - const wInHook = window.width; - assert.isTrue(mxInHook.isStrandsNode); - assert.isTrue(wInHook.isStrandsNode); - inputs.color = [1, 0, 0, 1]; - return inputs; - }); - }, { myp5 }); + test('returns numbers for builtin globals outside hooks and a strandNode when called inside hooks', () => { + myp5.createCanvas(5, 5, myp5.WEBGL); + myp5.baseMaterialShader().modify(() => { + myp5.getPixelInputs(inputs => { + const mxInHook = window.mouseX; + const wInHook = window.width; + const mxInstInHook = myp5.mouseX; + const wInstInHook = myp5.width; + assert.isTrue(mxInHook.isStrandsNode, 'window.mouseX should be a StrandsNode'); + assert.isTrue(wInHook.isStrandsNode, 'window.width should be a StrandsNode'); + assert.isTrue(mxInstInHook.isStrandsNode, 'myp5.mouseX should be a StrandsNode'); + assert.isTrue(wInstInHook.isStrandsNode, 'myp5.width should be a StrandsNode'); + inputs.color = [1, 0, 0, 1]; + return inputs; + }); + }, { myp5 }); - const mx = window.mouseX; - const w = window.width; - assert.isNumber(mx); - assert.isNumber(w); - assert.strictEqual(w, myp5.width); -}); + const mx = window.mouseX; + const w = window.width; + const mxInst = myp5.mouseX; + const wInst = myp5.width; + assert.isNumber(mx); + assert.isNumber(w); + assert.isNumber(mxInst); + assert.isNumber(wInst); + assert.strictEqual(w, myp5.width); + assert.strictEqual(wInst, myp5.width); + }); test('handle custom uniform names with automatic values', () => { From 3c198668440affc04a2f10e6fd1846a9fff85e73 Mon Sep 17 00:00:00 2001 From: saurabh24thakur Date: Mon, 2 Mar 2026 14:21:29 +0530 Subject: [PATCH 2/3] Fix shader test failures: infinite recursion, struct reconstruction errors, and FES mocking in browser environment --- src/strands/ir_builders.js | 4 ++-- src/strands/strands_FES.js | 3 +++ src/strands/strands_api.js | 11 ++++++++--- test/unit/webgl/p5.Shader.js | 14 +++++--------- 4 files changed, 18 insertions(+), 14 deletions(-) diff --git a/src/strands/ir_builders.js b/src/strands/ir_builders.js index efe3908d75..2200d618a9 100644 --- a/src/strands/ir_builders.js +++ b/src/strands/ir_builders.js @@ -346,7 +346,7 @@ export function castToFloat(strandsContext, dep) { export function structConstructorNode(strandsContext, structTypeInfo, rawUserArgs) { const { cfg, dag } = strandsContext; - const { identifer, properties } = structTypeInfo; + const { properties } = structTypeInfo; if (!(rawUserArgs.length === properties.length)) { FES.userError('type error', @@ -379,7 +379,7 @@ export function structConstructorNode(strandsContext, structTypeInfo, rawUserArg }); const id = DAG.getOrCreateNode(dag, nodeData); CFG.recordInBasicBlock(cfg, cfg.currentBlock, id); - return { id, dimension: properties.length, components: structTypeInfo.components }; + return { id, dimension: properties.length, components: dependsOn }; } export function functionCallNode( diff --git a/src/strands/strands_FES.js b/src/strands/strands_FES.js index 3af0aca90b..966fae168f 100644 --- a/src/strands/strands_FES.js +++ b/src/strands/strands_FES.js @@ -4,6 +4,9 @@ export function internalError(errorMessage) { } export function userError(errorType, errorMessage) { + if (typeof window !== 'undefined' && window.mockUserError) { + window.mockUserError(errorType, errorMessage); + } const prefixedMessage = `[p5.strands ${errorType}]: ${errorMessage}`; throw new Error(prefixedMessage); } \ No newline at end of file diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index 8826418999..ca0c4af746 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -134,8 +134,12 @@ function installBuiltinGlobalAccessors(strandsContext, fn) { } } - // Last resort: use the spec's getter - return spec.get(this); + // Fallback: use the spec's getter on the active p5 instance + const instance = strandsContext.renderer?._pInst || strandsContext.p5?.instance; + if (instance && instance !== target) { + return spec.get(instance); + } + return undefined; }, set: function(val) { if (originalDescriptor && originalDescriptor.set) { @@ -733,7 +737,8 @@ export function createShaderHooksFunctions(strandsContext, fn, shader) { const newDeps = returnedNode.dependsOn.slice(); for (let i = 0; i < expectedStructType.properties.length; i++) { const expectedType = expectedStructType.properties[i].dataType; - const receivedNode = createStrandsNode(returnedNode.dependsOn[i], dag.dependsOn[retNode.id], strandsContext); + const depID = returnedNode.dependsOn[i]; + const receivedNode = createStrandsNode(depID, dag.dimensions[depID], strandsContext); newDeps[i] = enforceReturnTypeMatch(strandsContext, expectedType, receivedNode, hookType.name); } dag.dependsOn[retNode.id] = newDeps; diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 24c66d3d93..19ec1ab763 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -1,15 +1,11 @@ -import p5 from '../../../src/app.js'; import { vi } from 'vitest'; +import p5 from '../../../src/app.js'; const mockUserError = vi.fn(); -vi.mock('../../../src/strands/strands_FES', () => ({ - 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}`); } -})); +window.mockUserError = mockUserError; + + + suite('p5.Shader', function() { var myp5; From 4c63a6d55050d65b53c90852ebee0d8ba146867d Mon Sep 17 00:00:00 2001 From: saurabh24thakur Date: Tue, 3 Mar 2026 13:38:49 +0530 Subject: [PATCH 3/3] Refactor: Simplify global variable accessors and fix struct constructor components Summary of changes: - Simplified installBuiltinGlobalAccessors by removing prototype chain loop. - Moved accessor installation to initGlobalStrandsAPI for earlier availability. - Fixed invalid reference to structTypeInfo.components in ir_builders.js and added clarifying comment. - Added unit test in p5.Shader.js to verify mouseX getter on p5.prototype. --- src/strands/ir_builders.js | 1 + src/strands/strands_api.js | 40 ++++++++++++++++-------------------- test/unit/webgl/p5.Shader.js | 6 ++++++ 3 files changed, 25 insertions(+), 22 deletions(-) diff --git a/src/strands/ir_builders.js b/src/strands/ir_builders.js index 2200d618a9..1bcb90c525 100644 --- a/src/strands/ir_builders.js +++ b/src/strands/ir_builders.js @@ -379,6 +379,7 @@ export function structConstructorNode(strandsContext, structTypeInfo, rawUserArg }); const id = DAG.getOrCreateNode(dag, nodeData); CFG.recordInBasicBlock(cfg, cfg.currentBlock, id); + // components stores specify the child nodes (members) of the struct for re-binding or inspection return { id, dimension: properties.length, components: dependsOn }; } diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index ca0c4af746..d07977f612 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -105,17 +105,9 @@ function installBuiltinGlobalAccessors(strandsContext, fn) { if (GraphicsProto) targets.push(GraphicsProto); for (const name of Object.keys(BUILTIN_GLOBAL_SPECS)) { - const spec = BUILTIN_GLOBAL_SPECS[name]; - targets.forEach((target) => { - // Find the original descriptor if it exists on this target or its prototype chain - let originalDescriptor = null; - let curr = target; - while (curr && !originalDescriptor) { - originalDescriptor = Object.getOwnPropertyDescriptor(curr, name); - if (originalDescriptor) break; - curr = Object.getPrototypeOf(curr); - } + // Capture the original descriptor ONLY on the target itself (recursive-safe) + const originalDesc = Object.getOwnPropertyDescriptor(target, name); Object.defineProperty(target, name, { get: function() { @@ -124,26 +116,26 @@ function installBuiltinGlobalAccessors(strandsContext, fn) { return getBuiltinGlobalNode(strandsContext, name); } - // Fallback: Get the normal value (like mouseX as a number) - // If we have an original descriptor, use it to avoid recursion - if (originalDescriptor) { - if (originalDescriptor.get) { - return originalDescriptor.get.call(this); - } else if ('value' in originalDescriptor) { - return originalDescriptor.value; + // Fallback: Get the original value to avoid infinite recursion + if (originalDesc) { + if (originalDesc.get) { + return originalDesc.get.call(this); } + return originalDesc.value; } - // Fallback: use the spec's getter on the active p5 instance + // If no original descriptor exists on this target (e.g. window in instance mode), + // fallback to the active p5 instance if we're not already on it. const instance = strandsContext.renderer?._pInst || strandsContext.p5?.instance; if (instance && instance !== target) { - return spec.get(instance); + return instance[name]; } + return undefined; }, set: function(val) { - if (originalDescriptor && originalDescriptor.set) { - originalDescriptor.set.call(this, val); + if (originalDesc && originalDesc.set) { + originalDesc.set.call(this, val); } else { // Shadow with a data property on the instance. // This allows things like 'this.deltaTime = ...' to work in p5._draw @@ -221,6 +213,11 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { } } } + ////////////////////////////////////////////// + // Builtins, uniforms, variable constructors + ////////////////////////////////////////////// + installBuiltinGlobalAccessors(strandsContext, fn); + ////////////////////////////////////////////// // Unique Functions ////////////////////////////////////////////// @@ -638,7 +635,6 @@ function enforceReturnTypeMatch(strandsContext, expectedType, returned, hookName return returnedNodeID; } export function createShaderHooksFunctions(strandsContext, fn, shader) { - installBuiltinGlobalAccessors(strandsContext, fn) // Add shader context to hooks before spreading const vertexHooksWithContext = Object.fromEntries( diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 19ec1ab763..74602f6660 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -416,6 +416,12 @@ suite('p5.Shader', function() { }); }); suite('p5.strands', () => { + test('p5.prototype mouseX is a getter', () => { + const desc = Object.getOwnPropertyDescriptor(p5.prototype, 'mouseX'); + assert.isDefined(desc, 'Descriptor should be defined'); + assert.isDefined(desc.get, 'p5.prototype.mouseX should have a getter'); + assert.isFunction(desc.get, 'p5.prototype.mouseX getter should be a function'); + }); test('handles named function callbacks', () => { myp5.createCanvas(5, 5, myp5.WEBGL); function myMaterial() {