Skip to content
Open
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
8 changes: 8 additions & 0 deletions src/webgl/p5.Shader.js
Original file line number Diff line number Diff line change
Expand Up @@ -560,6 +560,14 @@ p5.Shader = class {
const initializer = this.hooks.uniforms[key];
let value;
if (initializer instanceof Function) {
// Check for common outside variable references
const funcString = initializer.toString();
if (funcString.includes('mouseX') || funcString.includes('windowWidth') ||
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we'll want to go with a different approach than manually trying to make a list of variable names to exclude. A user could create a variable named anything they want, so we need something more robust to capture that. The test code in the issue was just an example.

We may need to detect variable declarations in the transpilation phase (https://github.com/processing/p5.js/blob/dev-2.0/src/strands/strands_transpiler.js) and then see if there are references to variables not included in that.

Copy link
Author

Choose a reason for hiding this comment

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

Thanks @davepagurek for the feedback! You're absolutely right - checking for hardcoded variable names isn't robust. I've removed that implementation.

Following your suggestion, I need to implement this during the transpilation phase in strands_transpiler.js. The approach should:

  1. Track variable declarations during transpilation to build a scope
  2. Analyze uniform initializer functions to see which variables they reference
  3. Check if any referenced variables aren't in the strand's scope
  4. Provide helpful errors

Could you provide some guidance on how to best integrate this with the existing transpiler? Should I add a new callback to the ASTCallbacks object, or would it be better to analyze the AST separately before/after transpilation?
Thanks!

Copy link
Contributor

Choose a reason for hiding this comment

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

Since nothing actually needs to be rewritten, this could be a new phase (maybe run before actual transpilation), but feel free to reuse a lot of the structure of the transpiler, since acorn will be a valuable tool in detecting these. You can use AST Explorer and change the AST to Acorn in the header to see how Acorn parses code, which can help you figure out what structures to look for when detecting declarations.

This is also the kind of thing that would really benefit from unit tests so we can see what's covered.

funcString.includes('state_') || funcString.includes('pixelate')) {
p5._friendlyError(
`p5.strands: Variable referenced in uniform "${name}" is not accessible in shader context.`
);
}
value = initializer();
} else {
value = initializer;
Expand Down