[p5.js 2.0+ Bug Report]: stale `sharedVariables` leak into later p5.strands shaders

[eupthere]

Most appropriate sub-area of p5.js?

• Accessibility
• Color
• Core/Environment/Rendering
• Data
• DOM
• Events
• Image
• IO
• Math
• Typography
• Utilities
• WebGL
• WebGPU
• p5.strands
• Build process
• Unit testing
• Internationalization
• Friendly errors
• Other (specify if possible)

p5.js version

2.3.0

Web browser and version

Firefox 151.0.2

Operating system

MacOSX 26.5.1

Steps to reproduce this

Steps:

1. Create a p5.strands shader using shared*() or varying*() with a name containing __, for example let __worldPos = varyingVec3();.
2. Build another unrelated shader afterward in the same p5 instance, without using __worldPos at all.
3. The later shader can fail compilation because the old shared/varying declaration leaks into the generated shader source.

Snippet:

function setup() {
  createCanvas(400, 400, WEBGL);

  const firstShader = baseMaterialShader().modify(() => {
    let __worldPos = varyingVec3();
    getWorldInputs((inputs) => {
      __worldPos = inputs.position.xyz;
      return inputs;
    });
    getFinalColor(() => {
      return [abs(__worldPos / 25), 1];
    });
  });

  console.log('firstShader varyings:', firstShader?.hooks?.varyingVariables);

  const secondShader = baseFilterShader().modify(() => {
    filterColor.begin();
    filterColor.set([1, 0, 0, 1]);
    filterColor.end();
  });

  console.log('secondShader varyings:', secondShader?.hooks?.varyingVariables);

  background(255);
  filter(secondShader);

  noLoop();
}

In the generated shader source, the later shader unexpectedly contains:

OUT vec3 __worldPos;

even though the later shader callback never mentions worldPos or __worldPos.

Additionally, logging the shader hook metadata shows that the second shader already contains the stale varying before it is ever compiled:

console.log(firstShader?.hooks?.varyingVariables);
// ["vec3 __worldPos"]

console.log(secondShader?.hooks?.varyingVariables);
// ["vec3 __worldPos"]

Suspected cause

sharedVariables in p5.strands appears to be lazily created in src/strands/strands_api.js, but it does not seem to be reset in initStrandsContext() / deinitStrandsContext() in src/strands/p5.strands.js.

p5.js/src/strands/strands_api.js

Lines 549 to 552 in beff79e

	// Initialize shared variables tracking if not present
	if (!strandsContext.sharedVariables) {
	strandsContext.sharedVariables = new Map();
	}

p5.js/src/strands/p5.strands.js

Lines 32 to 87 in beff79e

	function initStrandsContext(
	ctx,
	backend,
	{ active = false, renderer = null, baseShader = null } = {},
	) {
	ctx.dag = createDirectedAcyclicGraph();
	ctx.cfg = createControlFlowGraph();
	ctx.uniforms = [];
	ctx.vertexDeclarations = new Set();
	ctx.fragmentDeclarations = new Set();
	ctx.computeDeclarations = new Set();
	ctx.hooks = [];
	ctx.backend = backend;
	ctx.active = active;
	ctx.renderer = renderer;
	ctx.baseShader = baseShader;
	ctx.previousFES = p5.disableFriendlyErrors;
	ctx.windowOverrides = {};
	ctx.fnOverrides = {};
	ctx.graphicsOverrides = {};
	ctx._randomSeed = null;
	if (active) {
	p5.disableFriendlyErrors = true;
	}
	ctx.p5 = p5;
	}
	function deinitStrandsContext(ctx) {
	ctx.dag = createDirectedAcyclicGraph();
	ctx.cfg = createControlFlowGraph();
	ctx.uniforms = [];
	ctx.vertexDeclarations = new Set();
	ctx.fragmentDeclarations = new Set();
	ctx.computeDeclarations = new Set();
	ctx.hooks = [];
	ctx.active = false;
	ctx._randomSeed = null;
	p5.disableFriendlyErrors = ctx.previousFES;
	for (const key in ctx.windowOverrides) {
	window[key] = ctx.windowOverrides[key];
	}
	for (const key in ctx.fnOverrides) {
	fn[key] = ctx.fnOverrides[key];
	}
	// Clean up the hooks temporarily installed on p5.Graphics.prototype (#8549)
	const GraphicsProto = p5.Graphics?.prototype;
	if (GraphicsProto) {
	for (const key in ctx.graphicsOverrides) {
	if (ctx.graphicsOverrides[key] === undefined) {
	delete GraphicsProto[key];
	} else {
	GraphicsProto[key] = ctx.graphicsOverrides[key];
	}
	}
	}
	}

That suggests stale shared*() / varying*() bookkeeping may leak across modify() calls on the reused strands context object, which can affect later tests/shader builds.

Expected behavior

Each modify() call should start with a fresh per-build sharedVariables state, so declarations from earlier shader builds do not leak into later unrelated shaders.

Additional context

This surfaced while working on a separate fix for inferred uniform names containing __ (#8794). As new regression tests were added for varying*(), shared*(), and related shader-side identifiers, unrelated later tests started failing because stale sharedVariables state appeared to leak into subsequent modify() calls. That makes debugging and writing regression coverage harder, because a test can fail with declarations originating from a previous shader build rather than from its own callback.

Unrelated tests failing:

Unexpected shared variable(__worldPos) added to the shader source:

[davepagurek]

Good catch @eupthere! Are you interested in trying to handle this while working on the other issue too? Possibly by just resetting the shared variable tracking when we initialize a p5.strands context?

[eupthere]

@davepagurek Yes! Then I will include the fix to the PR for #8794 .

The fix should be simple, but I have some questions to understand the design a bit better.

Right now some strandsContext fields are initialized/reset in p5.strands.js, while others are attached from strands_api.js (for example sharedVariables, _builtinGlobals, _builtinGlobalsAccessorsInstalled, activeHook) or glsl/wgsl backends(_instanceIDUsedInFragment), and that makes me wonder what the intended rule is for which fields belong which file/module.

Related to that, p5.strands.js seems to own init/deinit, but if it also has to reset fields that are created inside strands_api.js, the responsibility boundary feels a little awkward to me. It makes me wonder whether context lifecycle management is intended to live primarily in p5.strands.js, or whether some amount of lifecycle state in the API layer was an intentional tradeoff. I’d like to understand the reasoning/tradeoffs behind that split.

[davepagurek]

The original version of strands had everything about the strands context initialized in p5.strands.js, and I believe some of the later cases were added in strands_api because they related to specific methods which patched in e.g. a uniform that could then be reused within that same method if called again. But then over time that grew from a one-off into a system used by multiple methods, so now there's a bunch of that in strands_api.

I suspect probably they should all go in the same initialization overall to avoid cases like this where we forget to clear them due to them being managed in another spot. I think probably all the cases of lazy initialization could just be explicitly initialized? Let me know if that sounds right to you!

[eupthere]

Thanks for the explanation! I think most strandsContext fields could be initialized explicitly in p5.strands.js, with a few exceptions like _builtinGlobalsAccessorsInstalled. That one does not look like per-modify() state, so I assume it should not be reset on each run.