Merge pull request #8641 from processing/fix/shared-variable

Fix a bug where shared strands variables are detected in the wrong spot

Author: davepagurek

src/strands/p5.strands.js

@@ -41,7 +41,6 @@ function strands(p5, fn) {
     ctx.vertexDeclarations = new Set();
     ctx.fragmentDeclarations = new Set();
     ctx.hooks = [];
-    ctx.globalAssignments = [];
     ctx.backend = backend;
     ctx.active = active;
     ctx.renderer = renderer;
@@ -63,7 +62,6 @@ function strands(p5, fn) {
     ctx.vertexDeclarations = new Set();
     ctx.fragmentDeclarations = new Set();
     ctx.hooks = [];
-    ctx.globalAssignments = [];
     ctx.active = false;
     p5.disableFriendlyErrors = ctx.previousFES;
     for (const key in ctx.windowOverrides) {

src/strands/strands_api.js

@@ -427,7 +427,6 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) {
         typeInfo,
         usedInVertex: false,
         usedInFragment: false,
-        declared: false
       });
 
       return createStrandsNode(id, dimension, strandsContext);

src/strands/strands_codegen.js

@@ -47,18 +47,6 @@ export function generateShaderCode(strandsContext) {
       backend.generateBlock(blockID, strandsContext, generationContext);
     }
 
-    // Process any unvisited global assignments to ensure side effects are generated
-    for (const assignmentNodeID of strandsContext.globalAssignments) {
-      if (!generationContext.visitedNodes.has(assignmentNodeID)) {
-        // This assignment hasn't been visited yet, so we need to generate it
-        backend.generateAssignment(generationContext, strandsContext.dag, assignmentNodeID);
-        generationContext.visitedNodes.add(assignmentNodeID);
-      }
-    }
-
-    // Reset global assignments for next hook
-    strandsContext.globalAssignments = [];
-
     const firstLine = backend.hookEntry(hookType);
     let returnType;
     if (hookType.returnType.properties) {

src/strands/strands_node.js

@@ -69,9 +69,6 @@ export class StrandsNode {
       const assignmentID = getOrCreateNode(dag, assignmentNode);
       recordInBasicBlock(cfg, cfg.currentBlock, assignmentID);
 
-      // Track for global assignments processing
-      this.strandsContext.globalAssignments.push(assignmentID);
-
       // Simply update this node to be a variable node with the identifier
       // This ensures it always generates the variable name in expressions
       const variableNodeData = createNodeData({
@@ -135,9 +132,6 @@ export class StrandsNode {
       const assignmentID = getOrCreateNode(dag, assignmentNode);
       recordInBasicBlock(cfg, cfg.currentBlock, assignmentID);
 
-      // Track for global assignments processing in the current hook context
-      this.strandsContext.globalAssignments.push(assignmentID);
-
       // Simply update this node to be a variable node with the identifier
       // This ensures it always generates the variable name in expressions
       const variableNodeData = createNodeData({

test/unit/webgl/p5.Shader.js

@@ -1770,6 +1770,38 @@ test('returns numbers for builtin globals outside hooks and a strandNode when ca
         assert.approximately(centerColor[2], 255, 5); // Blue component
       });
 
+      test('handle passing a value between fragment hooks only while having a vertex hook', () => {
+        myp5.createCanvas(50, 50, myp5.WEBGL);
+        myp5.pixelDensity(1);
+
+        const testShader = myp5.baseMaterialShader().modify(() => {
+          let processedNormal = myp5.sharedVec3();
+          myp5.objectInputs.begin();
+          myp5.objectInputs.position += [0, 0, 0];
+          myp5.objectInputs.end();
+
+          myp5.pixelInputs.begin();
+          processedNormal = myp5.normalize(myp5.pixelInputs.normal);
+          myp5.pixelInputs.end();
+
+          myp5.finalColor.begin();
+          // Use the processed normal to create a color - should be [0, 0, 1] for plane facing camera
+          myp5.finalColor.set([myp5.abs(processedNormal), 1]);
+          myp5.finalColor.end();
+        }, { myp5 });
+
+        myp5.background(255, 0, 0); // Red background to distinguish from result
+        myp5.noStroke();
+        myp5.shader(testShader);
+        myp5.plane(myp5.width, myp5.height);
+
+        // Normal of plane facing camera should be [0, 0, 1], so color should be [0, 0, 255]
+        const centerColor = myp5.get(25, 25);
+        assert.approximately(centerColor[0], 0, 5);   // Red component
+        assert.approximately(centerColor[1], 0, 5);   // Green component
+        assert.approximately(centerColor[2], 255, 5); // Blue component
+      });
+
       test('handle passing a value from a vertex hook to a fragment hook using shared*', () => {
         myp5.createCanvas(50, 50, myp5.WEBGL);
         myp5.pixelDensity(1);

test/unit/webgpu/p5.Shader.js

@@ -978,6 +978,38 @@ suite('WebGPU p5.Shader', function() {
         assert.approximately(centerColor[2], 255, 5); // Blue component
       });
 
+      test('handle passing a value between fragment hooks only while having a vertex hook', async () => {
+        await myp5.createCanvas(50, 50, myp5.WEBGPU);
+        myp5.pixelDensity(1);
+
+        const testShader = myp5.baseMaterialShader().modify(() => {
+          let processedNormal = myp5.sharedVec3();
+          myp5.objectInputs.begin();
+          myp5.objectInputs.position += [0, 0, 0];
+          myp5.objectInputs.end();
+
+          myp5.pixelInputs.begin();
+          processedNormal = myp5.normalize(myp5.pixelInputs.normal);
+          myp5.pixelInputs.end();
+
+          myp5.finalColor.begin();
+          // Use the processed normal to create a color - should be [0, 0, 1] for plane facing camera
+          myp5.finalColor.set([myp5.abs(processedNormal), 1]);
+          myp5.finalColor.end();
+        }, { myp5 });
+
+        myp5.background(255, 0, 0); // Red background to distinguish from result
+        myp5.noStroke();
+        myp5.shader(testShader);
+        myp5.plane(myp5.width, myp5.height);
+
+        // Normal of plane facing camera should be [0, 0, 1], so color should be [0, 0, 255]
+        const centerColor = await myp5.get(25, 25);
+        assert.approximately(centerColor[0], 0, 5);   // Red component
+        assert.approximately(centerColor[1], 0, 5);   // Green component
+        assert.approximately(centerColor[2], 255, 5); // Blue component
+      });
+
       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);