Skip to content

Commit 7df621e

Browse files
committed
Fix for loops not traversing minified code properly
1 parent b511667 commit 7df621e

3 files changed

Lines changed: 87 additions & 33 deletions

File tree

src/strands/strands_for.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -359,7 +359,7 @@ export class StrandsFor {
359359
CFG.popBlock(cfg);
360360

361361
const loopVarNode = createStrandsNode(phiNode.id, phiNode.dimension, this.strandsContext);
362-
this.bodyResults = this.bodyCb(loopVarNode, phiVars);
362+
this.bodyResults = this.bodyCb(loopVarNode, phiVars) || {};
363363
for (const key in this.bodyResults) {
364364
this.bodyResults[key] = this.strandsContext.p5.strandsNode(this.bodyResults[key]);
365365
}

src/strands/strands_transpiler.js

Lines changed: 57 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -679,46 +679,71 @@ const ASTCallbacks = {
679679

680680
// Analyze which outer scope variables are assigned in the loop body
681681
const assignedVars = new Set();
682-
const analyzeBlock = (body, parentLocalVars = new Set()) => {
683-
if (body.type !== 'BlockStatement') return;
684682

685-
// First pass: collect variable declarations within this block
686-
const localVars = new Set([...parentLocalVars]);
687-
for (const stmt of body.body) {
683+
// Helper function to check if a block contains strands control flow calls as immediate children
684+
const blockContainsStrandsControlFlow = (node) => {
685+
if (node.type !== 'BlockStatement') return false;
686+
return node.body.some(stmt => {
687+
// Check for variable declarations with strands control flow init
688688
if (stmt.type === 'VariableDeclaration') {
689-
for (const decl of stmt.declarations) {
690-
if (decl.id.type === 'Identifier') {
691-
localVars.add(decl.id.name);
692-
}
693-
}
689+
const match = stmt.declarations.some(decl =>
690+
decl.init?.type === 'CallExpression' &&
691+
(
692+
(
693+
decl.init?.callee?.type === 'MemberExpression' &&
694+
decl.init?.callee?.object?.type === 'Identifier' &&
695+
decl.init?.callee?.object?.name === '__p5' &&
696+
(decl.init?.callee?.property?.name === 'strandsFor' ||
697+
decl.init?.callee?.property?.name === 'strandsIf')
698+
) ||
699+
(
700+
decl.init?.callee?.type === 'Identifier' &&
701+
(decl.init?.callee?.name === '__p5.strandsFor' ||
702+
decl.init?.callee?.name === '__p5.strandsIf')
703+
)
704+
)
705+
);
706+
return match
707+
}
708+
return false;
709+
});
710+
};
711+
712+
// First pass: collect all variable declarations in the body
713+
const localVars = new Set();
714+
ancestor(bodyFunction.body, {
715+
VariableDeclarator(node, ancestors) {
716+
// Skip if we're inside a block that contains strands control flow
717+
if (ancestors.some(blockContainsStrandsControlFlow)) return;
718+
if (node.id.type === 'Identifier') {
719+
localVars.add(node.id.name);
694720
}
695721
}
722+
});
723+
724+
// Second pass: find assignments to non-local variables using acorn-walk
725+
ancestor(bodyFunction.body, {
726+
AssignmentExpression(node, ancestors) {
727+
// Skip if we're inside a block that contains strands control flow
728+
if (ancestors.some(blockContainsStrandsControlFlow)) {
729+
return
730+
}
696731

697-
// Second pass: find assignments to non-local variables
698-
for (const stmt of body.body) {
699-
if (stmt.type === 'ExpressionStatement' &&
700-
stmt.expression.type === 'AssignmentExpression') {
701-
const left = stmt.expression.left;
702-
if (left.type === 'Identifier') {
703-
// Direct variable assignment: x = value
704-
if (!localVars.has(left.name)) {
705-
assignedVars.add(left.name);
706-
}
707-
} else if (left.type === 'MemberExpression' &&
708-
left.object.type === 'Identifier') {
709-
// Property assignment: obj.prop = value (includes swizzles)
710-
if (!localVars.has(left.object.name)) {
711-
assignedVars.add(left.object.name);
712-
}
732+
const left = node.left;
733+
if (left.type === 'Identifier') {
734+
// Direct variable assignment: x = value
735+
if (!localVars.has(left.name)) {
736+
assignedVars.add(left.name);
737+
}
738+
} else if (left.type === 'MemberExpression' &&
739+
left.object.type === 'Identifier') {
740+
// Property assignment: obj.prop = value (includes swizzles)
741+
if (!localVars.has(left.object.name)) {
742+
assignedVars.add(left.object.name);
713743
}
714-
} else if (stmt.type === 'BlockStatement') {
715-
// Recursively analyze nested block statements, passing down local vars
716-
analyzeBlock(stmt, localVars);
717744
}
718745
}
719-
};
720-
721-
analyzeBlock(bodyFunction.body);
746+
});
722747

723748
if (assignedVars.size > 0) {
724749
// Add copying, reference replacement, and return statements similar to if statements

test/unit/webgl/p5.Shader.js

Lines changed: 29 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -898,6 +898,35 @@ suite('p5.Shader', function() {
898898
assert.approximately(pixelColor[2], 0, 5); // 0.0 * 255 = 0
899899
});
900900

901+
test('handle for loop modifying multiple variables after minification', () => {
902+
myp5.createCanvas(50, 50, myp5.WEBGL);
903+
904+
const testShader = myp5.baseMaterialShader().modify(() => {
905+
myp5.getPixelInputs(inputs => {
906+
let red = myp5.float(0.0);
907+
let green = myp5.float(0.0);
908+
909+
for (let i = 0; i < 4; i++) {
910+
// Note the comma!
911+
red = red + 0.125, // 4 * 0.125 = 0.5
912+
green = green + 0.25; // 4 * 0.25 = 1.0
913+
}
914+
915+
inputs.color = [red, green, 0.0, 1.0];
916+
return inputs;
917+
});
918+
}, { myp5 });
919+
920+
myp5.noStroke();
921+
myp5.shader(testShader);
922+
myp5.plane(myp5.width, myp5.height);
923+
924+
const pixelColor = myp5.get(25, 25);
925+
assert.approximately(pixelColor[0], 127, 5); // 0.5 * 255 ≈ 127
926+
assert.approximately(pixelColor[1], 255, 5); // 1.0 * 255 = 255
927+
assert.approximately(pixelColor[2], 0, 5); // 0.0 * 255 = 0
928+
});
929+
901930
test('handle for loop with conditional inside', () => {
902931
myp5.createCanvas(50, 50, myp5.WEBGL);
903932

0 commit comments

Comments
 (0)