diff --git a/src/strands/p5.strands.js b/src/strands/p5.strands.js index e9d4cd7143..7c5813c6f2 100644 --- a/src/strands/p5.strands.js +++ b/src/strands/p5.strands.js @@ -19,6 +19,10 @@ import { initGlobalStrandsAPI, createShaderHooksFunctions, } from "./strands_api"; +import { + createStrandsShaderNameMap, + createStrandsShaderNameState +} from './strands_names'; function strands(p5, fn) { // Whether or not strands callbacks should be forced to be executed in global mode. @@ -29,14 +33,83 @@ function strands(p5, fn) { ////////////////////////////////////////////// // Global Runtime ////////////////////////////////////////////// + + /** + * @private + * @typedef {Object} StrandsContext + * @property {Object} p5 Reference to the p5 class. + * @property {Boolean} _builtinGlobalsAccessorsInstalled Whether builtin global accessors have been installed on `window`, `p5.prototype`, and `p5.Graphics.prototype` as needed. + * @property {Object} dag DAG for the current strands IR. + * @property {Object} cfg CFG for the current strands IR. + * @property {Array} uniforms Collected uniforms and their default value providers. + * @property {Object} shaderNameMap Bidirectional name map used to translate between user-facing and generated internal shader variable names. + * @property {Object} shaderNameState Numeric suffix counter used when generating internal shader names. + * @property {Set} vertexDeclarations Declarations outside the generated hook functions to prepend to vertex shader code. + * @property {Set} fragmentDeclarations Declarations outside the generated hook functions to prepend to fragment shader code. + * @property {Set} computeDeclarations Declarations outside the generated hook functions to prepend to compute shader code. + * @property {Array} hooks Collected hook IR entries to turn into shader hook source. + * @property {Object} backend Active shader backend used for code generation and backend-specific helpers. + * @property {Boolean} active Whether strands interception is currently active. + * @property {Object} renderer Renderer whose shader is being modified. + * @property {Object} baseShader Base shader being modified in the current pass. + * @property {Boolean} previousFES Previous value of `p5.disableFriendlyErrors`, restored after the pass. + * @property {Object} windowOverrides Original temporary hook targets saved from `window`. + * @property {Object} fnOverrides Original temporary hook targets saved from `p5.prototype`. + * @property {Object} graphicsOverrides Original temporary hook targets saved from `p5.Graphics.prototype`. + * @property {Number} _noiseOctaves Noise octave override captured by `noiseDetail()`. + * @property {Number} _noiseAmpFalloff Noise falloff override captured by `noiseDetail()`. + * @property {Number} _randomSeed Random seed override captured by `randomSeed()`. + * @property {Object} _builtinGlobals Cache of builtin-global uniform nodes for the current DAG. + * @property {Map} sharedVariables Shared variable metadata that tracks vertex/fragment usage to decide whether each variable becomes a local declaration or a varying. + * @property {Object} activeHook Hook currently being recorded, if any. + * @property {Boolean} _instanceIDUsedInFragment Whether fragment-stage code referenced `instanceID`, requiring it to be passed from the vertex shader to the fragment shader. + */ + + /** + * Initializes the persistent strands context. + * + * Some strands context fields should persist across multiple shader `modify()` calls. + * e.g. there is no need to set p5 class reference multiple times, + * and the builtin globals accessors should only be installed once. + * + * @private + * @param {StrandsContext} ctx The strands context object. + */ + function initPersistentStrandsContext(ctx) { + ctx.p5 = p5; + ctx._builtinGlobalsAccessorsInstalled = false; + resetTransientStrandsContext(ctx); + } + + function createBuiltinGlobalsCache(dag) { + return { + dag, + nodes: new Map(), + uniformsAdded: new Set() + }; + } + + /** + * Initializes the transient strands context for one active shader `modify()` pass. + * + * @private + * @param {StrandsContext} ctx The strands context object. + * @param {Object} backend The backend to use for shader execution. + * @param {Object} [options] Options for initializing the context. + * @param {Boolean} [options.active] Whether the context is active. + * @param {Object} [options.renderer] The renderer to use. + * @param {Object} [options.baseShader] The base shader to use. + */ function initStrandsContext( ctx, backend, - { active = false, renderer = null, baseShader = null } = {}, + { active = false, renderer = null, baseShader = null } = {} ) { ctx.dag = createDirectedAcyclicGraph(); ctx.cfg = createControlFlowGraph(); ctx.uniforms = []; + ctx.shaderNameMap = createStrandsShaderNameMap(); + ctx.shaderNameState = createStrandsShaderNameState(); ctx.vertexDeclarations = new Set(); ctx.fragmentDeclarations = new Set(); ctx.computeDeclarations = new Set(); @@ -49,23 +122,58 @@ function strands(p5, fn) { ctx.windowOverrides = {}; ctx.fnOverrides = {}; ctx.graphicsOverrides = {}; + ctx._noiseOctaves = null; + ctx._noiseAmpFalloff = null; ctx._randomSeed = null; + ctx._builtinGlobals = createBuiltinGlobalsCache(ctx.dag); + ctx.sharedVariables = new Map(); + ctx.activeHook = undefined; + ctx._instanceIDUsedInFragment = false; if (active) { p5.disableFriendlyErrors = true; } - ctx.p5 = p5; } - function deinitStrandsContext(ctx) { + /** + * Resets the transient fields of the strands context. + * + * @private + * @param {StrandsContext} ctx The strands context object. + */ + function resetTransientStrandsContext(ctx) { ctx.dag = createDirectedAcyclicGraph(); ctx.cfg = createControlFlowGraph(); ctx.uniforms = []; + ctx.shaderNameMap = createStrandsShaderNameMap(); + ctx.shaderNameState = createStrandsShaderNameState(); ctx.vertexDeclarations = new Set(); ctx.fragmentDeclarations = new Set(); ctx.computeDeclarations = new Set(); ctx.hooks = []; + ctx.backend = undefined; ctx.active = false; + ctx.renderer = null; + ctx.baseShader = null; + ctx.previousFES = p5.disableFriendlyErrors; + ctx.windowOverrides = {}; + ctx.fnOverrides = {}; + ctx.graphicsOverrides = {}; + ctx._noiseOctaves = null; + ctx._noiseAmpFalloff = null; ctx._randomSeed = null; + ctx._builtinGlobals = createBuiltinGlobalsCache(ctx.dag); + ctx.sharedVariables = new Map(); + ctx.activeHook = undefined; + ctx._instanceIDUsedInFragment = false; + } + + /** + * Deinitializes the strands context after a shader `modify()` pass is complete. + * + * @private + * @param {StrandsContext} ctx The strands context object. + */ + function deinitStrandsContext(ctx) { p5.disableFriendlyErrors = ctx.previousFES; for (const key in ctx.windowOverrides) { window[key] = ctx.windowOverrides[key]; @@ -84,10 +192,11 @@ function strands(p5, fn) { } } } + resetTransientStrandsContext(ctx); } const strandsContext = {}; - initStrandsContext(strandsContext); + initPersistentStrandsContext(strandsContext); initGlobalStrandsAPI(p5, fn, strandsContext); function withTempGlobalMode(pInst, callback) { @@ -146,12 +255,16 @@ function strands(p5, fn) { typeof shaderModifier === "string" ? `(${shaderModifier})` : `(${shaderModifier.toString()})`; - strandsCallback = transpileStrandsToJS( + const transpiledStrands = transpileStrandsToJS( p5, sourceString, options.srcLocations, scope, + this.hooks.shaderNameState ); + strandsCallback = transpiledStrands.callback; + strandsContext.shaderNameMap = transpiledStrands.shaderNameMap; + strandsContext.shaderNameState = transpiledStrands.shaderNameState; } else { strandsCallback = shaderModifier; } diff --git a/src/strands/strands_api.js b/src/strands/strands_api.js index ff5655d81d..e80dcbd0ab 100644 --- a/src/strands/strands_api.js +++ b/src/strands/strands_api.js @@ -22,6 +22,11 @@ import * as DAG from './ir_dag'; import * as FES from './strands_FES' import { getNodeDataFromID } from './ir_dag' import { StrandsNode, createStrandsNode } from './strands_node' +import { + getOrCreateInternalShaderName, + isReservedStrandsName, + STRANDS_INTERNAL_NAME_PREFIX +} from './strands_names'; const BUILTIN_GLOBAL_SPECS = { width: { typeInfo: DataType.float1, get: (p) => p.width }, @@ -212,6 +217,53 @@ function augmentFnTemporary(fn, strandsContext, name, value) { } } +/** + * Validates a shader variable name to ensure it does not use reserved prefixes. + * + * @private + * @param {Object} strandsContext The strands context object to use for name map lookup. + * @param {string} name The shader variable name to validate. + * @param {string} apiName The name of the API function calling this validation. Used for FES error message generation. + * @return {void} Throws an error if the name is invalid, otherwise returns silently + */ +function validateShaderName(strandsContext, name, apiName) { + if (typeof name !== 'string') return; + if (strandsContext.shaderNameMap?.internalToExternal?.[name]) return; + if (isReservedStrandsName(name)) { + FES.userError( + 'parameter validation error', + `${apiName}("${name}") uses the reserved internal p5.strands prefix "${STRANDS_INTERNAL_NAME_PREFIX}". ` + + 'or the reserved "gl_" prefix. Please choose a different explicit shader name.' + ); + } +} + +/** + * Resolves a shader variable name to its internal representation with validation. + * + * If the internal name does not exist yet, it will be created and stored + * in the shaderNameMap for future reference. + * + * @private + * @param {Object} strandsContext The strands context object to use for name map lookup. + * @param {string} name The shader variable name to resolve. + * @param {string} apiName The name of the API function calling this resolution. Used for FES error message generation. + * @return {string} The internal shader variable name. + */ +function resolveShaderName(strandsContext, name, apiName) { + if (typeof name !== 'string') return name; + if (strandsContext.shaderNameMap?.internalToExternal?.[name]) { + return name; + } + + validateShaderName(strandsContext, name, apiName); + return getOrCreateInternalShaderName( + strandsContext.shaderNameMap, + strandsContext.shaderNameState, + name + ); +} + ////////////////////////////////////////////// // User nodes ////////////////////////////////////////////// @@ -401,9 +453,6 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { const originalRandomSeed = fn.randomSeed; const originalMillis = fn.millis; - strandsContext._noiseOctaves = null; - strandsContext._noiseAmpFalloff = null; - augmentFn(fn, p5, 'noiseDetail', function (lod, falloff = 0.5) { if (!strandsContext.active) { return originalNoiseDetail.apply(this, arguments); @@ -463,8 +512,6 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { return createStrandsNode(id, dimension, strandsContext); }); - strandsContext._randomSeed = null; - augmentFn(fn, p5, 'randomSeed', function (seed) { if (!strandsContext.active) { return originalRandomSeed.apply(this, arguments); @@ -598,24 +645,41 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { } } augmentFn(fn, p5, `uniform${pascalTypeName}`, function(name, defaultValue) { - const { id, dimension } = build.variableNode(strandsContext, typeInfo, name); - strandsContext.uniforms.push({ name, typeInfo, defaultValue }); + const shaderName = resolveShaderName( + strandsContext, + name, + `uniform${pascalTypeName}` + ); + const { id, dimension } = build.variableNode( + strandsContext, + typeInfo, + shaderName + ); + strandsContext.uniforms.push({ + name: shaderName, + typeInfo, + defaultValue + }); return createStrandsNode(id, dimension, strandsContext); }); // Shared variables with smart context detection augmentFn(fn, p5, `shared${pascalTypeName}`, function(name) { - const { id, dimension } = build.variableNode(strandsContext, typeInfo, name); - - // Initialize shared variables tracking if not present - if (!strandsContext.sharedVariables) { - strandsContext.sharedVariables = new Map(); - } + const shaderName = resolveShaderName( + strandsContext, + name, + `shared${pascalTypeName}` + ); + const { id, dimension } = build.variableNode( + strandsContext, + typeInfo, + shaderName + ); // Track this shared variable for smart declaration generation - strandsContext.sharedVariables.set(name, { + strandsContext.sharedVariables.set(shaderName, { typeInfo, usedInVertex: false, - usedInFragment: false, + usedInFragment: false }); return createStrandsNode(id, dimension, strandsContext); @@ -669,6 +733,7 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { // Storage buffer uniform function for compute shaders fn.uniformStorage = function(name, bufferOrSchema) { + const shaderName = resolveShaderName(strandsContext, name, 'uniformStorage'); let schema = null; let defaultValue = null; @@ -696,18 +761,18 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) { const { id, dimension } = build.variableNode( strandsContext, { baseType: 'storage', dimension: 1 }, - name + shaderName ); strandsContext.uniforms.push({ - name, + name: shaderName, typeInfo: { baseType: 'storage', dimension: 1, schema }, - defaultValue, + defaultValue }); // Create StrandsNode with _originalIdentifier set (like varying variables) // This enables proper assignment node creation and ordering preservation const node = createStrandsNode(id, dimension, strandsContext); - node._originalIdentifier = name; + node._originalIdentifier = shaderName; node._originalBaseType = 'storage'; node._originalDimension = 1; node._schema = schema; @@ -822,17 +887,26 @@ function enforceReturnTypeMatch(strandsContext, expectedType, returned, hookName return returnedNodeID; } export function createShaderHooksFunctions(strandsContext, fn, shader) { - installBuiltinGlobalAccessors(strandsContext) + installBuiltinGlobalAccessors(strandsContext); + // shader.hooks.vertex/fragment/compute mix callable hook entries with stage + // metadata such as `declarations` carried over from earlier modify() calls. + const isHookEntry = ([name]) => name !== 'declarations'; // Add shader context to hooks before spreading const vertexHooksWithContext = Object.fromEntries( - Object.entries(shader.hooks.vertex).map(([name, hook]) => [name, { ...hook, shaderContext: 'vertex' }]) + Object.entries(shader.hooks.vertex) + .filter(isHookEntry) + .map(([name, hook]) => [name, { ...hook, shaderContext: 'vertex' }]) ); const fragmentHooksWithContext = Object.fromEntries( - Object.entries(shader.hooks.fragment).map(([name, hook]) => [name, { ...hook, shaderContext: 'fragment' }]) + Object.entries(shader.hooks.fragment) + .filter(isHookEntry) + .map(([name, hook]) => [name, { ...hook, shaderContext: 'fragment' }]) ); const computeHooksWithContext = Object.fromEntries( - Object.entries(shader.hooks.compute).map(([name, hook]) => [name, { ...hook, shaderContext: 'compute' }]) + Object.entries(shader.hooks.compute) + .filter(isHookEntry) + .map(([name, hook]) => [name, { ...hook, shaderContext: 'compute' }]) ); const availableHooks = { diff --git a/src/strands/strands_codegen.js b/src/strands/strands_codegen.js index 38e24c511e..c2afa6de54 100644 --- a/src/strands/strands_codegen.js +++ b/src/strands/strands_codegen.js @@ -15,6 +15,8 @@ export function generateShaderCode(strandsContext) { uniforms: {}, storageUniforms: {}, varyingVariables: [], + shaderNameMap: strandsContext.shaderNameMap, + shaderNameState: strandsContext.shaderNameState }; for (const {name, typeInfo, defaultValue} of strandsContext.uniforms) { diff --git a/src/strands/strands_names.js b/src/strands/strands_names.js new file mode 100644 index 0000000000..22b9544f02 --- /dev/null +++ b/src/strands/strands_names.js @@ -0,0 +1,120 @@ +export const STRANDS_INTERNAL_NAME_PREFIX = '_p5_strands_'; + + +/** + * @private + * @typedef {Object} StrandsShaderNameMap + * @property {Object} externalToInternal Maps external shader names to internal shader names. + * e.g. `shaderNameMap.externalToInternal['__data'] = '_p5_strands_0'` + * @property {Object} internalToExternal Maps internal shader names to external shader names. + * e.g. `shaderNameMap.internalToExternal['_p5_strands_0'] = '__data'` + */ + +/** + * Creates and returns a new `shaderNameMap` object for managing + * the mapping between external and internal shader variable names. + * + * `shaderNameMap` can resolve names in both directions: + * - `shaderNameMap.externalToInternal['__data']` -> `_p5_strands_0` + * - `shaderNameMap.internalToExternal['_p5_strands_0']` -> `__data` + * + * @private + * @return {StrandsShaderNameMap} A new `shaderNameMap` object with empty mappings. + */ +export function createStrandsShaderNameMap() { + return { + externalToInternal: {}, + internalToExternal: {} + }; +} + +/** + * @private + * @typedef {Object} StrandsShaderNameState + * @property {Number} nextSuffix - The next suffix number to use for generating unique internal shader names. + */ + +/** + * Creates and returns a new `shaderNameState` object containing the next + * suffix number for generating unique internal shader names. + * + * @private + * @param {Number} [nextSuffix] The next suffix number to use for generating + * unique internal shader names. Defaults to 0. + * @return {StrandsShaderNameState} A new `shaderNameState` object with the provided next suffix. + */ +export function createStrandsShaderNameState(nextSuffix = 0) { + // Keep suffix state in an object to allow mutation across different contexts + // without needing to return and reassign the updated value. + return { nextSuffix }; +} + +/** + * Checks if a given name is an internally reserved strands name. + * + * - `_p5_strands_` prefix is reserved for creating safe internal shader variable names. + * For example, names with two underscores are internally rewritten to `_p5_strands_`. + * - `gl_` prefix is reserved by WebGL and it is used in the shader templates. + * It shouldn't be used for user-defined variables to avoid conflicts. + * + * @private + * @param {String} name The name to check. + * @return {Boolean} True if the name is reserved, false otherwise. + */ +export function isReservedStrandsName(name) { + return ( + typeof name === 'string' && + ( + name.startsWith(STRANDS_INTERNAL_NAME_PREFIX) || + name.startsWith('gl_') + ) + ); +} + +/** + * Checks if a given name is a safe shader identifier. + * + * @private + * @param {String} name The name to check. + * @returns {Boolean} True if the name is a safe shader identifier, false otherwise. + */ +export function isSafeShaderIdentifier(name) { + return ( + typeof name === 'string' && + /^[_A-Za-z][_A-Za-z0-9]*$/.test(name) && + !name.includes('__') && + !isReservedStrandsName(name) + ); +} + +/** + * Retrieves or creates a safe internal shader name for a given external name. + * + * If the original name is already a safe shader identifier, it is returned as-is. + * Otherwise, a new unique internal name is generated, stored in the name map, and returned. + * + * @private + * @param {StrandsShaderNameMap} nameMap The shader name map object. + * @param {StrandsShaderNameState} nameState The shader name state object. + * @param {String} originalName The original external name to convert to an internal name. + * @returns {String} The internal shader name that is safe to use in shader code. + */ +export function getOrCreateInternalShaderName( + nameMap, + nameState, + originalName +) { + if (nameMap.externalToInternal[originalName]) { + return nameMap.externalToInternal[originalName]; + } + + if (isSafeShaderIdentifier(originalName)) { + return originalName; + } + + const internalName = `${STRANDS_INTERNAL_NAME_PREFIX}${nameState.nextSuffix++}`; + + nameMap.externalToInternal[originalName] = internalName; + nameMap.internalToExternal[internalName] = originalName; + return internalName; +} diff --git a/src/strands/strands_transpiler.js b/src/strands/strands_transpiler.js index 0d9310204a..52d0d8bf6d 100644 --- a/src/strands/strands_transpiler.js +++ b/src/strands/strands_transpiler.js @@ -3,6 +3,11 @@ import { ancestor, recursive } from 'acorn-walk'; import escodegen from 'escodegen'; import { UnarySymbolToName } from './ir_types'; import * as FES from './strands_FES'; +import { + createStrandsShaderNameMap, + createStrandsShaderNameState, + getOrCreateInternalShaderName +} from './strands_names'; let blockVarCounter = 0; let loopVarCounter = 0; function replaceBinaryOperator(codeSource) { @@ -512,10 +517,15 @@ const ASTCallbacks = { if (node.init.arguments.length === 0 || node.init.arguments[0].type !== 'Literal' || typeof node.init.arguments[0].value !== 'string') { + const uniformName = getOrCreateInternalShaderName( + state.shaderNameMap, + state.shaderNameState, + node.id.name + ); const uniformNameLiteral = { type: 'Literal', - value: node.id.name - } + value: uniformName + }; node.init.arguments.unshift(uniformNameLiteral); } } @@ -526,10 +536,15 @@ const ASTCallbacks = { node.init.arguments[0].type !== 'Literal' || typeof node.init.arguments[0].value !== 'string' ) { + const varyingName = getOrCreateInternalShaderName( + state.shaderNameMap, + state.shaderNameState, + node.id.name + ); const varyingNameLiteral = { type: 'Literal', - value: node.id.name - } + value: varyingName + }; node.init.arguments.unshift(varyingNameLiteral); state.varyings[node.id.name] = varyingNameLiteral; } else { @@ -1663,7 +1678,7 @@ function transformHelperFunctionEarlyReturns(ast, names) { /** * @private * @internal - * + * * Transpiles a p5.strands callback into executable JavaScript by applying * a multi-pass AST transformation pipeline. * @@ -1706,11 +1721,27 @@ function makeGuardedCallbacks(callbacks) { return guarded; } -function runNonControlFlowPass(ast, uniformCallbackNames) { - const nonControlFlowCallbacks = ({ ...ASTCallbacks }); +function runNonControlFlowPass( + ast, + uniformCallbackNames, + initialShaderNameState +) { + const nonControlFlowCallbacks = { ...ASTCallbacks }; delete nonControlFlowCallbacks.IfStatement; delete nonControlFlowCallbacks.ForStatement; - ancestor(ast, nonControlFlowCallbacks, undefined, { varyings: {}, uniformCallbackNames }); + const state = { + varyings: {}, + uniformCallbackNames, + shaderNameMap: createStrandsShaderNameMap(), + shaderNameState: createStrandsShaderNameState( + initialShaderNameState?.nextSuffix || 0 + ) + }; + ancestor(ast, nonControlFlowCallbacks, undefined, state); + return { + shaderNameMap: state.shaderNameMap, + shaderNameState: state.shaderNameState + }; } function runControlFlowPass(ast, uniformCallbackNames) { @@ -1798,11 +1829,20 @@ function buildStrandsCallback(p5, ast, scope) { -export function transpileStrandsToJS(p5, sourceString, srcLocations, scope) { +export function transpileStrandsToJS( + p5, + sourceString, + srcLocations, + scope, + initialShaderNameState +) { blockVarCounter = 0; loopVarCounter = 0; - const ast = parse(sourceString, { ecmaVersion: 2021, locations: srcLocations }); + const ast = parse(sourceString, { + ecmaVersion: 2021, + locations: srcLocations + }); throwIfLoopProtectionInserted(ast); @@ -1813,7 +1853,11 @@ export function transpileStrandsToJS(p5, sourceString, srcLocations, scope) { transformSetCallsInControlFlow(ast, uniformCallbackNames); // Pass 2: transform non-control-flow nodes (operators, varyings, uniforms, arrays) - runNonControlFlowPass(ast, uniformCallbackNames); + const { shaderNameMap, shaderNameState } = runNonControlFlowPass( + ast, + uniformCallbackNames, + initialShaderNameState + ); // Pass 3: transform helper functions with early returns to use __returnValue pattern transformHelperFunctionEarlyReturns(ast, uniformCallbackNames); @@ -1821,5 +1865,10 @@ export function transpileStrandsToJS(p5, sourceString, srcLocations, scope) { // Pass 4: transform if/for statements post-order into strandsIf/strandsFor calls runControlFlowPass(ast, uniformCallbackNames); - return buildStrandsCallback(p5, ast, scope); + const callback = buildStrandsCallback(p5, ast, scope); + return { + callback, + shaderNameMap, + shaderNameState + }; } diff --git a/src/webgl/p5.RendererGL.js b/src/webgl/p5.RendererGL.js index 78de4092f8..f297c575d5 100644 --- a/src/webgl/p5.RendererGL.js +++ b/src/webgl/p5.RendererGL.js @@ -207,6 +207,39 @@ class RendererGL extends Renderer3D { return undefined; } + + /** + * Restores the original shader variable names in a given text using + * the provided shader name map. + * + * @private + * @param {String} text The text in which to restore original shader names. + * @param {Object} shaderNameMap The shader name map object containing internal + * to external name mappings. + * @return {String} The text with original shader names restored. + */ + _restoreOriginalShaderNames(text, shaderNameMap) { + if (!text || !shaderNameMap?.internalToExternal) { + return text; + } + + let restored = text; + for (const [internalName, originalName] of Object.entries( + shaderNameMap.internalToExternal + )) { + // Restore only whole identifier matches. This assumes the internal shader + // name appears as an identifier with word boundaries in WebGL error text/source. If we + // later derive new identifiers from it by appending or prepending extra word + // characters (for example `prefix_${internalName}`, `${internalName}_suffix`), + // this regex will not match those and the restoration logic will need to be + // updated. + const pattern = new RegExp(`\\b${internalName}\\b`, 'g'); + restored = restored.replace(pattern, originalName); + } + + return restored; + } + _useShader(shader) { const gl = this.GL; gl.useProgram(shader._glProgram); @@ -1098,23 +1131,42 @@ class RendererGL extends Renderer3D { _initShader(shader) { const gl = this.GL; + const shaderNameMap = shader.hooks?.shaderNameMap; const vertShader = gl.createShader(gl.VERTEX_SHADER); gl.shaderSource(vertShader, shader.vertSrc()); gl.compileShader(vertShader); if (!gl.getShaderParameter(vertShader, gl.COMPILE_STATUS)) { + // restore original shader names in the info log and source for easier debugging + const vertexInfoLog = this._restoreOriginalShaderNames( + gl.getShaderInfoLog(vertShader), + shaderNameMap + ); + const vertexSource = this._restoreOriginalShaderNames( + shader.vertSrc(), + shaderNameMap + ); throw new Error(`Yikes! An error occurred compiling the vertex shader: ${ - gl.getShaderInfoLog(vertShader) - } in:\n\n${shader.vertSrc()}`); + vertexInfoLog + } in:\n\n${vertexSource}`); } const fragShader = gl.createShader(gl.FRAGMENT_SHADER); gl.shaderSource(fragShader, shader.fragSrc()); gl.compileShader(fragShader); if (!gl.getShaderParameter(fragShader, gl.COMPILE_STATUS)) { + // restore original shader names in the info log and source for easier debugging + const fragmentInfoLog = this._restoreOriginalShaderNames( + gl.getShaderInfoLog(fragShader), + shaderNameMap + ); + const fragmentSource = this._restoreOriginalShaderNames( + shader.fragSrc(), + shaderNameMap + ); throw new Error(`Darn! An error occurred compiling the fragment shader: ${ - gl.getShaderInfoLog(fragShader) - }`); + fragmentInfoLog + } in:\n\n${fragmentSource}`); } const program = gl.createProgram(); diff --git a/src/webgl/p5.Shader.js b/src/webgl/p5.Shader.js index c39de0bd71..4af976807c 100644 --- a/src/webgl/p5.Shader.js +++ b/src/webgl/p5.Shader.js @@ -51,6 +51,17 @@ class Shader { // Compute shader storage uniforms + default values storageUniforms: options.storageUniforms || {}, + // Maps user-facing inferred names to internal shader-facing names. + shaderNameMap: options.shaderNameMap || { + externalToInternal: {}, + internalToExternal: {} + }, + + // Tracks the next auto-generated internal shader-name suffix for chained modify() calls. + shaderNameState: options.shaderNameState || { + nextSuffix: 0 + }, + // Stores custom uniform + helper declarations as a string. declarations: options.declarations, @@ -423,6 +434,8 @@ class Shader { if (key === 'declarations') continue; if (key === 'uniforms') continue; if (key === 'storageUniforms') continue; + if (key === 'shaderNameMap') continue; + if (key === 'shaderNameState') continue; if (key === 'varyingVariables') continue; if (key === 'instanceIDVarying') continue; if (key === 'vertexDeclarations') { @@ -471,6 +484,26 @@ class Shader { (this.hooks.declarations || '') + '\n' + (hooks.declarations || ''), uniforms: Object.assign({}, this.hooks.uniforms, hooks.uniforms || {}), storageUniforms: Object.assign({}, this.hooks.storageUniforms, hooks.storageUniforms || {}), + // Preserve shader name remapping and suffix state across chained modify() + // calls so later passes keep earlier internal names and keep incrementing. + shaderNameMap: { + externalToInternal: Object.assign( + {}, + this.hooks.shaderNameMap?.externalToInternal ?? {}, + hooks.shaderNameMap?.externalToInternal ?? {} + ), + internalToExternal: Object.assign( + {}, + this.hooks.shaderNameMap?.internalToExternal ?? {}, + hooks.shaderNameMap?.internalToExternal ?? {} + ) + }, + shaderNameState: { + nextSuffix: + hooks.shaderNameState?.nextSuffix ?? + this.hooks.shaderNameState?.nextSuffix ?? + 0 + }, varyingVariables: (hooks.varyingVariables || []).concat(this.hooks.varyingVariables || []), instanceIDVarying: hooks.instanceIDVarying || this.hooks.instanceIDVarying || null, fragment: Object.assign({}, this.hooks.fragment, newHooks.fragment || {}), @@ -1098,7 +1131,10 @@ class Shader { setUniform(uniformName, data) { this.init(); - const uniform = this.uniforms[uniformName]; + const resolvedName = + this.hooks.shaderNameMap?.externalToInternal?.[uniformName] ?? + uniformName; + const uniform = this.uniforms[resolvedName]; if (!uniform) { return; } diff --git a/test/unit/webgl/p5.Shader.js b/test/unit/webgl/p5.Shader.js index 0317cf2d24..73960074ea 100644 --- a/test/unit/webgl/p5.Shader.js +++ b/test/unit/webgl/p5.Shader.js @@ -532,6 +532,41 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.strictEqual(w, myp5.width); }); + test('builtin global accessors remain usable across modify calls and after failures', () => { + myp5.createCanvas(5, 5, myp5.WEBGL); + + myp5.baseMaterialShader().modify(() => { + myp5.getPixelInputs(inputs => { + inputs.color = [myp5.mouseX / myp5.width, 0, 0, 1]; + return inputs; + }); + }, { myp5 }); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + throw new Error('intentional failure'); + }, { myp5 }); + }).toThrowError('intentional failure'); + + myp5.baseMaterialShader().modify(() => { + myp5.getPixelInputs(inputs => { + // global accessors are still usable + // meaning _builtinGlobalsAccessorsInstalled is not accidentally reset + const mxInHook = myp5.mouseX; + const wInHook = myp5.width; + // inside modify hooks, global accessors return strands nodes + assert.isTrue(mxInHook.isStrandsNode); + assert.isTrue(wInHook.isStrandsNode); + inputs.color = [1, 0, 0, 1]; + return inputs; + }); + }, { myp5 }); + + // after modify finishes, global accessors go back to ordinary p5 values + assert.isNumber(myp5.mouseX); + assert.isNumber(myp5.width); + }); + test('map() works inside a strands modify callback', () => { myp5.createCanvas(50, 50, myp5.WEBGL); const testShader = myp5.baseMaterialShader().modify(() => { @@ -701,6 +736,204 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.approximately(pixelColor[2], 153, 5); }); + test('renames only problematic inferred uniform names', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + const testShader = myp5.baseMaterialShader().modify(() => { + // Names containing two underscores are treated as problematic because of GLSL + // naming constraints, so p5.strands remaps them through a shared naming layer. + const __val = myp5.uniformFloat(() => 0.8); + const brightness = myp5.uniformFloat(() => 0.2); + myp5.getPixelInputs(inputs => { + inputs.color = [__val, brightness, brightness, 1.0]; + return inputs; + }); + }, { myp5 }); + + assert.strictEqual( + testShader.hooks.shaderNameMap.externalToInternal.__val, + '_p5_strands_0' + ); + assert.strictEqual( + testShader.hooks.shaderNameMap.internalToExternal._p5_strands_0, + '__val' + ); + assert.strictEqual( + testShader.hooks.shaderNameMap.externalToInternal.brightness, + undefined + ); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 204, 5); // Red channel should be ~204 (0.8 * 255) + assert.approximately(pixelColor[1], 51, 5); // Green channel should be ~51 (0.2 * 255) + assert.approximately(pixelColor[2], 51, 5); // Blue channel should be ~51 (0.2 * 255) + }); + + test('handles implicit uniform names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + const testShader = myp5.baseMaterialShader().modify(() => { + // using implicit uniform name with two underscores, which should be remapped + const __val = myp5.uniformFloat(); + myp5.getPixelInputs(inputs => { + inputs.color = [__val, __val, __val, 1.0]; + return inputs; + }); + }, { myp5 }); + + // setting the uniform using the original name + testShader.setUniform('__val', 0.6); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 153, 5); // Red channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[1], 153, 5); // Green channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[2], 153, 5); // Blue channel should be ~153 (0.6 * 255) + }); + + test('handles explicit uniform names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + const testShader = myp5.baseMaterialShader().modify(() => { + // using explicit uniform name with two underscores, which should be remapped + const val = myp5.uniformFloat('__val'); + myp5.getPixelInputs(inputs => { + inputs.color = [val, val, val, 1.0]; + return inputs; + }); + }, { myp5 }); + + // setting the uniform using the original name + testShader.setUniform('__val', 0.6); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = myp5.get(25, 25); + assert.approximately(pixelColor[0], 153, 5); // Red channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[1], 153, 5); // Green channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[2], 153, 5); // Blue channel should be ~153 (0.6 * 255) + }); + + test('continues inferred uniform suffixes across chained modify calls', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + const shader1 = myp5.baseMaterialShader().modify(() => { + const __first = myp5.uniformFloat(() => 0.2); + myp5.getPixelInputs(inputs => { + inputs.color.r = __first; + return inputs; + }); + }, { myp5 }); + + const shader2 = shader1.modify(() => { + const __second = myp5.uniformFloat(() => 0.8); + myp5.getWorldInputs(inputs => { + inputs.position.y += __second * 0.0; + return inputs; + }); + }, { myp5 }); + + assert.strictEqual( + shader2.hooks.shaderNameMap.externalToInternal.__first, + '_p5_strands_0' + ); + assert.strictEqual( + shader2.hooks.shaderNameMap.externalToInternal.__second, + '_p5_strands_1' + ); + assert.strictEqual( + shader2.hooks.shaderNameState.nextSuffix, + 2 + ); + }); + + test('does not leak shared/varying state into later modify calls', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const firstShader = myp5.baseMaterialShader().modify(() => { + let __worldPos = myp5.varyingVec3(); + myp5.getWorldInputs(inputs => { + __worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__worldPos / 25), 1]; + }); + }, { myp5 }); + + const internalName = + firstShader.hooks.shaderNameMap.externalToInternal.__worldPos; + assert.deepEqual( + firstShader.hooks.varyingVariables, + [`vec3 ${internalName}`] + ); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.filterColor.begin(); + myp5.filterColor.set([1, 0, 0, 1]); + myp5.filterColor.end(); + }, { myp5 }); + + assert.deepEqual( + secondShader.hooks.varyingVariables, + [] + ); + assert.notInclude(secondShader.vertSrc(), '__worldPos'); + assert.notInclude(secondShader.fragSrc(), '__worldPos'); + }); + + test('rejects explicit use of the reserved internal strands prefix', () => { + expect(() => { + myp5.baseMaterialShader().modify(() => { + const brightness = myp5.uniformFloat('_p5_strands_forbidden'); + myp5.getPixelInputs(inputs => { + inputs.color = [brightness, brightness, brightness, 1.0]; + return inputs; + }); + }, { myp5 }); + }).toThrow('reserved internal p5.strands prefix'); + }); + + test('restores original inferred names in WebGL compile errors', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + // First build a valid strands shader so the inferred name '__val' is + // remapped through the normal shaderNameMap path. + const mappedShader = myp5.baseMaterialShader().modify(() => { + const __val = myp5.uniformFloat(() => 0.8); + myp5.getWorldInputs(inputs => { + inputs.position.x += __val * 0.0; + return inputs; + }); + }, { myp5 }); + + const internalName = + mappedShader.hooks.shaderNameMap.externalToInternal.__val; + + // Then override the vertex hook with invalid GLSL using the internal + // remapped name. The `+ ;` is a deliberate syntax error, which forces + // WebGL compilation to fail and exercise original-name restoration in + // the reported error text. + const badShader = mappedShader.modify({ + 'Vertex getWorldInputs': `(Vertex inputs) { + inputs.position.x = ${internalName} + ; + return inputs; + }` + }); + + expect(() => { + myp5.shader(badShader); + myp5.plane(myp5.width, myp5.height); + }).toThrow('__val'); + }); + suite('array indexing on non-storage vectors (#8756)', () => { afterEach(() => { mockUserError.mockClear(); @@ -1951,6 +2184,30 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca }); suite('passing data between shaders', () => { + test('does not leak shared variable metadata into later shaders', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const firstShader = myp5.baseMaterialShader().modify(() => { + let worldPos = myp5.varyingVec3(); + myp5.getWorldInputs((inputs) => { + worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => [myp5.abs(worldPos / 25), 1]); + }, { myp5 }); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.filterColor.begin(); + myp5.filterColor.set([1, 0, 0, 1]); + myp5.filterColor.end(); + }, { myp5 }); + + assert.deepEqual(firstShader.hooks.varyingVariables, ['vec3 worldPos']); + assert.deepEqual(secondShader.hooks.varyingVariables, []); + assert.notInclude(secondShader.vertSrc(), 'worldPos'); + assert.notInclude(secondShader.fragSrc(), 'worldPos'); + }); + test('handle passing a value from a vertex hook to a fragment hook', () => { myp5.createCanvas(50, 50, myp5.WEBGL); myp5.pixelDensity(1); @@ -2017,6 +2274,70 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.approximately(cornerColor[2], 0, 5); }); + test('handles inferred varying names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using implicit varying name with two underscores, which should be remapped + let __worldPos = myp5.varyingVec3(); + myp5.getWorldInputs(inputs => { + __worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__worldPos / 25), 1]; + }); + }, { myp5 }); + + myp5.background(0, 0, 255); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const midColor = myp5.get(25, 25); + assert.approximately(midColor[0], 0, 5); // Red channel should be ~0 + assert.approximately(midColor[1], 0, 5); // Green channel should be ~0 + assert.approximately(midColor[2], 0, 5); // Blue channel should be ~0 + + const cornerColor = myp5.get(0, 0); + assert.approximately(cornerColor[0], 255, 5); // Red channel should be ~255 + assert.approximately(cornerColor[1], 255, 5); // Green channel should be ~255 + assert.approximately(cornerColor[2], 0, 5); // Blue channel should be ~0 + }); + + test('handles explicit varying names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using explicit varying name with two underscores, which should be remapped + let worldPos = myp5.varyingVec3('__worldPos'); + myp5.getWorldInputs(inputs => { + worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(worldPos / 25), 1]; + }); + }, { myp5 }); + + myp5.background(0, 0, 255); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const midColor = myp5.get(25, 25); + assert.approximately(midColor[0], 0, 5); // Red channel should be ~0 + assert.approximately(midColor[1], 0, 5); // Green channel should be ~0 + assert.approximately(midColor[2], 0, 5); // Blue channel should be ~0 + + const cornerColor = myp5.get(0, 0); + assert.approximately(cornerColor[0], 255, 5); // Red channel should be ~255 + assert.approximately(cornerColor[1], 255, 5); // Green channel should be ~255 + assert.approximately(cornerColor[2], 0, 5); // Blue channel should be ~0 + }); + test('handle passing a value from a vertex hook to a fragment hook as part of hook output', () => { myp5.createCanvas(50, 50, myp5.WEBGL); myp5.pixelDensity(1); @@ -2111,6 +2432,60 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca assert.approximately(centerColor[2], 255, 5); // Blue component }); + test('handles inferred shared names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using implicit shared name with two underscores, which should be remapped + let __processedNormal = myp5.sharedVec3(); + myp5.getPixelInputs(inputs => { + __processedNormal = myp5.normalize(inputs.normal); + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__processedNormal), 1]; + }); + }, { myp5 }); + + myp5.background(255, 0, 0); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const centerColor = myp5.get(25, 25); + assert.approximately(centerColor[0], 0, 5); + assert.approximately(centerColor[1], 0, 5); + assert.approximately(centerColor[2], 255, 5); + }); + + test('handles explicit shared names containing two underscores', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using explicit shared name with two underscores, which should be remapped + let processedNormal = myp5.sharedVec3('__processedNormal'); + myp5.getPixelInputs(inputs => { + processedNormal = myp5.normalize(inputs.normal); + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(processedNormal), 1]; + }); + }, { myp5 }); + + myp5.background(255, 0, 0); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const centerColor = myp5.get(25, 25); + assert.approximately(centerColor[0], 0, 5); + assert.approximately(centerColor[1], 0, 5); + assert.approximately(centerColor[2], 255, 5); + }); + test('handle passing a value from a vertex hook to a fragment hook using shared*', () => { myp5.createCanvas(50, 50, myp5.WEBGL); myp5.pixelDensity(1); @@ -2220,6 +2595,34 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca }); suite('noise()', () => { + test('noiseDetail state does not leak across modify calls', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const firstShader = myp5.baseFilterShader().modify(() => { + // changes the default noiseDetail settings + myp5.noiseDetail(7, 0.2); + myp5.getColor(() => [myp5.noise(10), 0, 0, 1]); + }, { myp5 }); + + const secondShader = myp5.baseFilterShader().modify(() => { + // does not change noiseDetail settings, so should use defaults + myp5.getColor(() => [myp5.noise(10), 0, 0, 1]); + }, { myp5 }); + + const explicitDefaultShader = myp5.baseFilterShader().modify(() => { + // uses the default noiseDetail settings + myp5.noiseDetail(4, 0.5); + myp5.getColor(() => [myp5.noise(10), 0, 0, 1]); + }, { myp5 }); + + const firstHook = firstShader.hooks.fragment['vec4 getColor']; + const secondHook = secondShader.hooks.fragment['vec4 getColor']; + const defaultHook = explicitDefaultShader.hooks.fragment['vec4 getColor']; + + assert.notStrictEqual(firstHook, secondHook); + assert.strictEqual(secondHook, defaultHook); + }); + for (let i = 1; i <= 3; i++) { test(`works with ${i}D vectors`, () => { expect(() => { @@ -2281,6 +2684,40 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca } }); + test('randomSeed state does not leak across modify calls', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const firstShader = myp5.baseFilterShader().modify(() => { + myp5.randomSeed(12); + myp5.getColor(() => [myp5.random(), 0, 0, 1]); + }, { myp5 }); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.getColor(() => [myp5.random(), 0, 0, 1]); + }, { myp5 }); + + assert.strictEqual(firstShader.hooks.uniforms['float _p5_randomSeed'](), 12); + assert.notStrictEqual(secondShader.hooks.uniforms['float _p5_randomSeed'](), 12); + }); + + test('instanceID varying does not leak across modify calls', () => { + myp5.createCanvas(50, 50, myp5.WEBGL); + + const firstShader = myp5.baseMaterialShader().modify(() => { + myp5.getFinalColor(() => { + const id = myp5.instanceID(); + return [id / 10, 0, 0, 1]; + }); + }, { myp5 }); + + const secondShader = myp5.baseMaterialShader().modify(() => { + myp5.getFinalColor(() => [1, 0, 0, 1]); + }, { myp5 }); + + assert.isDefined(firstShader.hooks.instanceIDVarying); + assert.isNull(secondShader.hooks.instanceIDVarying); + }); + test('Can use begin/end API for hooks with result', () => { myp5.createCanvas(50, 50, myp5.WEBGL); diff --git a/test/unit/webgpu/p5.Shader.js b/test/unit/webgpu/p5.Shader.js index eb9bb79990..e10899bdec 100644 --- a/test/unit/webgpu/p5.Shader.js +++ b/test/unit/webgpu/p5.Shader.js @@ -22,6 +22,41 @@ suite('WebGPU p5.Shader', function() { }); suite('p5.strands', () => { + test('builtin global accessors remain usable across modify calls and after failures', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + + myp5.baseMaterialShader().modify(() => { + myp5.getPixelInputs(inputs => { + inputs.color = [myp5.mouseX / myp5.width, 0, 0, 1]; + return inputs; + }); + }, { myp5 }); + + expect(() => { + myp5.baseMaterialShader().modify(() => { + throw new Error('intentional failure'); + }, { myp5 }); + }).toThrowError('intentional failure'); + + myp5.baseMaterialShader().modify(() => { + myp5.getPixelInputs(inputs => { + // global accessors are still usable + // meaning _builtinGlobalsAccessorsInstalled is not accidentally reset + const mxInHook = myp5.mouseX; + const wInHook = myp5.width; + // inside modify hooks, global accessors return strands nodes + assert.isTrue(mxInHook.isStrandsNode); + assert.isTrue(wInHook.isStrandsNode); + inputs.color = [1, 0, 0, 1]; + return inputs; + }); + }, { myp5 }); + + // after modify finishes, global accessors go back to ordinary p5 values + assert.isNumber(myp5.mouseX); + assert.isNumber(myp5.width); + }); + test('does not break when arrays are in uniform callbacks', async () => { await myp5.createCanvas(5, 5, myp5.WEBGPU); const myShader = myp5.baseMaterialShader().modify(() => { @@ -41,6 +76,183 @@ suite('WebGPU p5.Shader', function() { }).not.toThrowError(); }); + test('does not leak shared variable metadata into later shaders', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + + const firstShader = myp5.baseMaterialShader().modify(() => { + let __worldPos = myp5.varyingVec3(); + myp5.getWorldInputs(inputs => { + __worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => [myp5.abs(__worldPos / 25), 1]); + }, { myp5 }); + + const secondShader = myp5.baseMaterialShader().modify(() => { + myp5.getFinalColor(() => [1, 0, 0, 1]); + }, { myp5 }); + + assert.deepEqual( + firstShader.hooks.varyingVariables, + [`${firstShader.hooks.shaderNameMap.externalToInternal.__worldPos}: vec3`] + ); + assert.deepEqual(secondShader.hooks.varyingVariables, []); + assert.notInclude(secondShader.vertSrc(), '__worldPos'); + assert.notInclude(secondShader.fragSrc(), '__worldPos'); + }); + + test('renames only problematic inferred uniform names', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + const testShader = myp5.baseMaterialShader().modify(() => { + // Names containing two underscores are treated as problematic because of GLSL + // naming constraints, so p5.strands remaps them through a shared naming layer. + // WGSL itself would allow this name, but we still test the behavior in WebGPU + // because the remapping logic is shared across transpiler, strands API, codegen, + // and runtime shader handling. + const __val = myp5.uniformFloat(() => 0.8); + const brightness = myp5.uniformFloat(() => 0.2); + myp5.getPixelInputs(inputs => { + inputs.color = [__val, brightness, brightness, 1.0]; + return inputs; + }); + }, { myp5 }); + + assert.strictEqual( + testShader.hooks.shaderNameMap.externalToInternal.__val, + '_p5_strands_0' + ); + assert.strictEqual( + testShader.hooks.shaderNameMap.internalToExternal._p5_strands_0, + '__val' + ); + assert.strictEqual( + testShader.hooks.shaderNameMap.externalToInternal.brightness, + undefined + ); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = await myp5.get(25, 25); + assert.approximately(pixelColor[0], 204, 5); // Red channel should be ~204 (0.8 * 255) + assert.approximately(pixelColor[1], 51, 5); // Green channel should be ~51 (0.2 * 255) + assert.approximately(pixelColor[2], 51, 5); // Blue channel should be ~51 (0.2 * 255) + }); + + test('handles implicit uniform names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + const testShader = myp5.baseMaterialShader().modify(() => { + // using implicit uniform name with two underscores, which should be remapped + const __val = myp5.uniformFloat(); + myp5.getPixelInputs(inputs => { + inputs.color = [__val, __val, __val, 1.0]; + return inputs; + }); + }, { myp5 }); + + // setting the uniform using the original name + testShader.setUniform('__val', 0.6); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = await myp5.get(25, 25); + assert.approximately(pixelColor[0], 153, 5); // Red channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[1], 153, 5); // Green channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[2], 153, 5); // Blue channel should be ~153 (0.6 * 255) + }); + + test('handles explicit uniform names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + const testShader = myp5.baseMaterialShader().modify(() => { + // using explicit uniform name with two underscores, which should be remapped + const val = myp5.uniformFloat('__val'); + myp5.getPixelInputs(inputs => { + inputs.color = [val, val, val, 1.0]; + return inputs; + }); + }, { myp5 }); + + // setting the uniform using the original name + testShader.setUniform('__val', 0.6); + + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const pixelColor = await myp5.get(25, 25); + assert.approximately(pixelColor[0], 153, 5); // Red channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[1], 153, 5); // Green channel should be ~153 (0.6 * 255) + assert.approximately(pixelColor[2], 153, 5); // Blue channel should be ~153 (0.6 * 255) + }); + + test('continues inferred uniform suffixes across chained modify calls', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + const shader1 = myp5.baseMaterialShader().modify(() => { + const __first = myp5.uniformFloat(() => 0.2); + myp5.getPixelInputs(inputs => { + inputs.color.r = __first; + return inputs; + }); + }, { myp5 }); + + const shader2 = shader1.modify(() => { + const __second = myp5.uniformFloat(() => 0.8); + myp5.getWorldInputs(inputs => { + inputs.position.y += __second * 0.0; + return inputs; + }); + }, { myp5 }); + + assert.strictEqual( + shader2.hooks.shaderNameMap.externalToInternal.__first, + '_p5_strands_0' + ); + assert.strictEqual( + shader2.hooks.shaderNameMap.externalToInternal.__second, + '_p5_strands_1' + ); + assert.strictEqual( + shader2.hooks.shaderNameState.nextSuffix, + 2 + ); + }); + + test('does not leak shared/varying state into later modify calls', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + + const firstShader = myp5.baseMaterialShader().modify(() => { + let __worldPos = myp5.varyingVec3(); + myp5.getWorldInputs(inputs => { + __worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__worldPos / 25), 1]; + }); + }, { myp5 }); + + assert.deepEqual( + firstShader.hooks.varyingVariables, + [`${firstShader.hooks.shaderNameMap.externalToInternal.__worldPos}: vec3`] + ); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.filterColor.begin(); + myp5.filterColor.set([1, 0, 0, 1]); + myp5.filterColor.end(); + }, { myp5 }); + + assert.deepEqual( + secondShader.hooks.varyingVariables, + [] + ); + }); + suite('if statement conditionals', () => { test('handle simple if statement with true condition', async () => { await myp5.createCanvas(50, 50, myp5.WEBGPU); @@ -963,6 +1175,70 @@ suite('WebGPU p5.Shader', function() { assert.approximately(cornerColor[2], 0, 5); }); + test('handles inferred varying names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using implicit varying name with two underscores, which should be remapped + let __worldPos = myp5.varyingVec3(); + myp5.getWorldInputs(inputs => { + __worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__worldPos / 25), 1]; + }); + }, { myp5 }); + + myp5.background(0, 0, 255); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const midColor = await myp5.get(25, 25); + assert.approximately(midColor[0], 0, 5); // Red channel should be ~0 + assert.approximately(midColor[1], 0, 5); // Green channel should be ~0 + assert.approximately(midColor[2], 0, 5); // Blue channel should be ~0 + + const cornerColor = await myp5.get(0, 0); + assert.approximately(cornerColor[0], 255, 5); // Red channel should be ~255 + assert.approximately(cornerColor[1], 255, 5); // Green channel should be ~255 + assert.approximately(cornerColor[2], 0, 5); // Blue channel should be ~0 + }); + + test('handles explicit varying names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + // using explicit varying name with two underscores, which should be remapped + let worldPos = myp5.varyingVec3('__worldPos'); + myp5.getWorldInputs(inputs => { + worldPos = inputs.position.xyz; + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(worldPos / 25), 1]; + }); + }, { myp5 }); + + myp5.background(0, 0, 255); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const midColor = await myp5.get(25, 25); + assert.approximately(midColor[0], 0, 5); // Red channel should be ~0 + assert.approximately(midColor[1], 0, 5); // Green channel should be ~0 + assert.approximately(midColor[2], 0, 5); // Blue channel should be ~0 + + const cornerColor = await myp5.get(0, 0); + assert.approximately(cornerColor[0], 255, 5); // Red channel should be ~255 + assert.approximately(cornerColor[1], 255, 5); // Green channel should be ~255 + assert.approximately(cornerColor[2], 0, 5); // Blue channel should be ~0 + }); + test('handle passing a value from a vertex hook to a fragment hook as part of hook output', async () => { await myp5.createCanvas(50, 50, myp5.WEBGPU); myp5.pixelDensity(1); @@ -1058,6 +1334,58 @@ suite('WebGPU p5.Shader', function() { assert.approximately(centerColor[2], 255, 5); // Blue component }); + test('handles inferred shared names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + let __processedNormal = myp5.sharedVec3(); + myp5.getPixelInputs(inputs => { + __processedNormal = myp5.normalize(inputs.normal); + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(__processedNormal), 1]; + }); + }, { myp5 }); + + myp5.background(255, 0, 0); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const centerColor = await myp5.get(25, 25); + assert.approximately(centerColor[0], 0, 5); + assert.approximately(centerColor[1], 0, 5); + assert.approximately(centerColor[2], 255, 5); + }); + + test('handles explicit shared names containing two underscores', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + myp5.pixelDensity(1); + + const testShader = myp5.baseMaterialShader().modify(() => { + let processedNormal = myp5.sharedVec3('__processedNormal'); + myp5.getPixelInputs(inputs => { + processedNormal = myp5.normalize(inputs.normal); + return inputs; + }); + myp5.getFinalColor(() => { + return [myp5.abs(processedNormal), 1]; + }); + }, { myp5 }); + + myp5.background(255, 0, 0); + myp5.noStroke(); + myp5.shader(testShader); + myp5.plane(myp5.width, myp5.height); + + const centerColor = await myp5.get(25, 25); + assert.approximately(centerColor[0], 0, 5); + assert.approximately(centerColor[1], 0, 5); + assert.approximately(centerColor[2], 255, 5); + }); + test('handle passing a value from a vertex hook to a fragment hook using shared*', async () => { await myp5.createCanvas(50, 50, myp5.WEBGPU); myp5.pixelDensity(1); @@ -1168,6 +1496,37 @@ suite('WebGPU p5.Shader', function() { }); suite('noise()', () => { + test('noiseDetail state does not leak across modify calls', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + + const firstShader = myp5.baseFilterShader().modify(() => { + myp5.noiseDetail(7, 0.2); + myp5.getColor(() => { + return [myp5.noise(10), 0, 0, 1]; + }); + }, { myp5 }); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.getColor(() => { + return [myp5.noise(10), 0, 0, 1]; + }); + }, { myp5 }); + + const explicitDefaultShader = myp5.baseFilterShader().modify(() => { + myp5.noiseDetail(4, 0.5); + myp5.getColor(() => { + return [myp5.noise(10), 0, 0, 1]; + }); + }, { myp5 }); + + const firstHook = firstShader.hooks.fragment['vec4 getColor']; + const secondHook = secondShader.hooks.fragment['vec4 getColor']; + const defaultHook = explicitDefaultShader.hooks.fragment['vec4 getColor']; + + assert.notStrictEqual(firstHook, secondHook); + assert.strictEqual(secondHook, defaultHook); + }); + for (let i = 1; i <= 3; i++) { test(`works with ${i}D vectors`, async () => { await expect((async () => { @@ -1229,6 +1588,40 @@ suite('WebGPU p5.Shader', function() { } }); + test('randomSeed state does not leak across modify calls', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + + const firstShader = myp5.baseFilterShader().modify(() => { + myp5.randomSeed(12); + myp5.getColor(() => [myp5.random(), 0, 0, 1]); + }, { myp5 }); + + const secondShader = myp5.baseFilterShader().modify(() => { + myp5.getColor(() => [myp5.random(), 0, 0, 1]); + }, { myp5 }); + + assert.strictEqual(firstShader.hooks.uniforms['_p5_randomSeed: f32'](), 12); + assert.notStrictEqual(secondShader.hooks.uniforms['_p5_randomSeed: f32'](), 12); + }); + + test('instanceID varying does not leak across modify calls', async () => { + await myp5.createCanvas(50, 50, myp5.WEBGPU); + + const firstShader = myp5.baseMaterialShader().modify(() => { + myp5.getFinalColor(() => { + const id = myp5.instanceID(); + return [id / 10, 0, 0, 1]; + }); + }, { myp5 }); + + const secondShader = myp5.baseMaterialShader().modify(() => { + myp5.getFinalColor(() => [1, 0, 0, 1]); + }, { myp5 }); + + assert.isDefined(firstShader.hooks.instanceIDVarying); + assert.isNull(secondShader.hooks.instanceIDVarying); + }); + suite('compute shaders', () => { test('handle early return in void compute hook', async () => { await myp5.createCanvas(5, 5, myp5.WEBGPU); @@ -1268,6 +1661,50 @@ suite('WebGPU p5.Shader', function() { myp5.compute(computeShader, 1); }).not.toThrow(); }); + + test('handles inferred storage uniform names containing two underscores', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + const input = new Float32Array([1, 2, 3, 4]); + const buf = myp5.createStorage(input); + + const computeShader = myp5.buildComputeShader(() => { + // using implicit uniform storage name with two underscores, which should be remapped + const __data = myp5.uniformStorage(); + const idx = myp5.index.x; + __data[idx] = __data[idx] * 2; + }, { myp5 }); + + // setting the uniform using the original name + computeShader.setUniform('__data', buf); + myp5.compute(computeShader, 4); + + const result = await buf.read(); + for (let i = 0; i < input.length; i++) { + expect(result[i]).to.be.closeTo(input[i] * 2, 0.001); + } + }); + + test('handles explicit storage uniform names containing two underscores', async () => { + await myp5.createCanvas(5, 5, myp5.WEBGPU); + const input = new Float32Array([1, 2, 3, 4]); + const buf = myp5.createStorage(input); + + const computeShader = myp5.buildComputeShader(() => { + // using explicit uniform storage name with two underscores, which should be remapped + const data = myp5.uniformStorage('__data'); + const idx = myp5.index.x; + data[idx] = data[idx] * 2; + }, { myp5 }); + + // setting the uniform using the original name + computeShader.setUniform('__data', buf); + myp5.compute(computeShader, 4); + + const result = await buf.read(); + for (let i = 0; i < input.length; i++) { + expect(result[i]).to.be.closeTo(input[i] * 2, 0.001); + } + }); }); suite('array indexing on non-storage vectors (#8756)', () => {