Skip to content

Commit e236b8a

Browse files
davepagurekdavepagurek
authored
Merge pull request #8223 from processing/strands-varyings
Fix varying variables in p5.strands
2 parents 8f23e69 + 403c6bb commit e236b8a

File tree

7 files changed

+509
-23
lines changed

7 files changed

+509
-23
lines changed

‎src/strands/p5.strands.js‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -25,6 +25,7 @@ function strands(p5, fn) {
2525
ctx.vertexDeclarations = new Set();
2626
ctx.fragmentDeclarations = new Set();
2727
ctx.hooks = [];
28+
ctx.globalAssignments = [];
2829
ctx.backend = backend;
2930
ctx.active = active;
3031
ctx.previousFES = p5.disableFriendlyErrors;
@@ -42,6 +43,7 @@ function strands(p5, fn) {
4243
ctx.vertexDeclarations = new Set();
4344
ctx.fragmentDeclarations = new Set();
4445
ctx.hooks = [];
46+
ctx.globalAssignments = [];
4547
ctx.active = false;
4648
p5.disableFriendlyErrors = ctx.previousFES;
4749
for (const key in ctx.windowOverrides) {

‎src/strands/strands_api.js‎

Lines changed: 37 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -149,10 +149,33 @@ export function initGlobalStrandsAPI(p5, fn, strandsContext) {
149149
strandsContext.uniforms.push({ name, typeInfo, defaultValue });
150150
return createStrandsNode(id, dimension, strandsContext);
151151
};
152+
// Shared variables with smart context detection
153+
fn[`shared${pascalTypeName}`] = function(name) {
154+
const { id, dimension } = build.variableNode(strandsContext, typeInfo, name);
155+
156+
// Initialize shared variables tracking if not present
157+
if (!strandsContext.sharedVariables) {
158+
strandsContext.sharedVariables = new Map();
159+
}
160+
161+
// Track this shared variable for smart declaration generation
162+
strandsContext.sharedVariables.set(name, {
163+
typeInfo,
164+
usedInVertex: false,
165+
usedInFragment: false,
166+
declared: false
167+
});
168+
169+
return createStrandsNode(id, dimension, strandsContext);
170+
};
171+
172+
// Alias varying* as shared* for backward compatibility
173+
fn[`varying${pascalTypeName}`] = fn[`shared${pascalTypeName}`];
152174
if (pascalTypeName.startsWith('Vec')) {
153175
// For compatibility, also alias uniformVec2 as uniformVector2, what we initially
154176
// documented these as
155177
fn[`uniform${pascalTypeName.replace('Vec', 'Vector')}`] = fn[`uniform${pascalTypeName}`];
178+
fn[`varying${pascalTypeName.replace('Vec', 'Vector')}`] = fn[`varying${pascalTypeName}`];
156179
}
157180
const originalp5Fn = fn[typeInfo.fnName];
158181
fn[typeInfo.fnName] = function(...args) {
@@ -263,11 +286,20 @@ function enforceReturnTypeMatch(strandsContext, expectedType, returned, hookName
263286
return returnedNodeID;
264287
}
265288
export function createShaderHooksFunctions(strandsContext, fn, shader) {
289+
// Add shader context to hooks before spreading
290+
const vertexHooksWithContext = Object.fromEntries(
291+
Object.entries(shader.hooks.vertex).map(([name, hook]) => [name, { ...hook, shaderContext: 'vertex' }])
292+
);
293+
const fragmentHooksWithContext = Object.fromEntries(
294+
Object.entries(shader.hooks.fragment).map(([name, hook]) => [name, { ...hook, shaderContext: 'fragment' }])
295+
);
296+
266297
const availableHooks = {
267-
...shader.hooks.vertex,
268-
...shader.hooks.fragment,
298+
...vertexHooksWithContext,
299+
...fragmentHooksWithContext,
269300
}
270301
const hookTypes = Object.keys(availableHooks).map(name => shader.hookTypes(name));
302+
271303
const { cfg, dag } = strandsContext;
272304
for (const hookType of hookTypes) {
273305
const hookImplementation = function(hookUserCallback) {
@@ -319,10 +351,13 @@ export function createShaderHooksFunctions(strandsContext, fn, shader) {
319351
const expectedTypeInfo = TypeInfoFromGLSLName[expectedReturnType.typeName];
320352
rootNodeID = enforceReturnTypeMatch(strandsContext, expectedTypeInfo, userReturned, hookType.name);
321353
}
354+
const fullHookName = `${hookType.returnType.typeName} ${hookType.name}`;
355+
const hookInfo = availableHooks[fullHookName];
322356
strandsContext.hooks.push({
323357
hookType,
324358
entryBlockID,
325359
rootNodeID,
360+
shaderContext: hookInfo?.shaderContext, // 'vertex' or 'fragment'
326361
});
327362
CFG.popBlock(cfg);
328363
}

‎src/strands/strands_codegen.js‎

Lines changed: 34 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,7 +18,7 @@ export function generateShaderCode(strandsContext) {
1818
hooksObj.uniforms[declaration] = defaultValue;
1919
}
2020

21-
for (const { hookType, rootNodeID, entryBlockID } of strandsContext.hooks) {
21+
for (const { hookType, rootNodeID, entryBlockID, shaderContext } of strandsContext.hooks) {
2222
const generationContext = {
2323
indent: 1,
2424
codeLines: [],
@@ -28,13 +28,28 @@ export function generateShaderCode(strandsContext) {
2828
tempNames: {},
2929
declarations: [],
3030
nextTempID: 0,
31+
visitedNodes: new Set(),
32+
shaderContext, // 'vertex' or 'fragment'
33+
strandsContext, // For shared variable tracking
3134
};
3235

3336
const blocks = sortCFG(cfg.outgoingEdges, entryBlockID);
3437
for (const blockID of blocks) {
3538
backend.generateBlock(blockID, strandsContext, generationContext);
3639
}
3740

41+
// Process any unvisited global assignments to ensure side effects are generated
42+
for (const assignmentNodeID of strandsContext.globalAssignments) {
43+
if (!generationContext.visitedNodes.has(assignmentNodeID)) {
44+
// This assignment hasn't been visited yet, so we need to generate it
45+
backend.generateAssignment(generationContext, strandsContext.dag, assignmentNodeID);
46+
generationContext.visitedNodes.add(assignmentNodeID);
47+
}
48+
}
49+
50+
// Reset global assignments for next hook
51+
strandsContext.globalAssignments = [];
52+
3853
const firstLine = backend.hookEntry(hookType);
3954
let returnType = hookType.returnType.properties
4055
? structType(hookType.returnType)
@@ -43,6 +58,24 @@ export function generateShaderCode(strandsContext) {
4358
hooksObj[`${hookType.returnType.typeName} ${hookType.name}`] = [firstLine, ...generationContext.codeLines, '}'].join('\n');
4459
}
4560

61+
// Finalize shared variable declarations based on usage
62+
if (strandsContext.sharedVariables) {
63+
for (const [varName, varInfo] of strandsContext.sharedVariables) {
64+
if (varInfo.usedInVertex && varInfo.usedInFragment) {
65+
// Used in both shaders - declare as varying
66+
vertexDeclarations.add(`OUT ${varInfo.typeInfo.fnName} ${varName};`);
67+
fragmentDeclarations.add(`IN ${varInfo.typeInfo.fnName} ${varName};`);
68+
} else if (varInfo.usedInVertex) {
69+
// Only used in vertex shader - declare as local variable
70+
vertexDeclarations.add(`${varInfo.typeInfo.fnName} ${varName};`);
71+
} else if (varInfo.usedInFragment) {
72+
// Only used in fragment shader - declare as local variable
73+
fragmentDeclarations.add(`${varInfo.typeInfo.fnName} ${varName};`);
74+
}
75+
// If not used anywhere, don't declare it
76+
}
77+
}
78+
4679
hooksObj.vertexDeclarations = [...vertexDeclarations].join('\n');
4780
hooksObj.fragmentDeclarations = [...fragmentDeclarations].join('\n');
4881

‎src/strands/strands_glslBackend.js‎

Lines changed: 24 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -40,6 +40,7 @@ const cfgHandlers = {
4040
}
4141
if (nodeType === NodeType.ASSIGNMENT) {
4242
glslBackend.generateAssignment(generationContext, dag, nodeID);
43+
generationContext.visitedNodes.add(nodeID);
4344
}
4445
}
4546
},
@@ -127,6 +128,7 @@ const cfgHandlers = {
127128
}
128129
if (node.nodeType === NodeType.ASSIGNMENT) {
129130
glslBackend.generateAssignment(generationContext, dag, nodeID);
131+
generationContext.visitedNodes.add(nodeID);
130132
}
131133
}
132134

@@ -195,16 +197,18 @@ export const glslBackend = {
195197
},
196198
generateAssignment(generationContext, dag, nodeID) {
197199
const node = getNodeDataFromID(dag, nodeID);
198-
// dependsOn[0] = phiNodeID, dependsOn[1] = sourceNodeID
199-
const phiNodeID = node.dependsOn[0];
200+
// dependsOn[0] = targetNodeID, dependsOn[1] = sourceNodeID
201+
const targetNodeID = node.dependsOn[0];
200202
const sourceNodeID = node.dependsOn[1];
201-
const phiTempName = generationContext.tempNames[phiNodeID];
203+
204+
// Generate the target expression (could be variable or swizzle)
205+
const targetExpr = this.generateExpression(generationContext, dag, targetNodeID);
202206
const sourceExpr = this.generateExpression(generationContext, dag, sourceNodeID);
203207
const semicolon = generationContext.suppressSemicolon ? '' : ';';
204208

205-
// Skip assignment if target and source are the same variable
206-
if (phiTempName && sourceExpr && phiTempName !== sourceExpr) {
207-
generationContext.write(`${phiTempName} = ${sourceExpr}${semicolon}`);
209+
// Generate assignment if we have both target and source
210+
if (targetExpr && sourceExpr && targetExpr !== sourceExpr) {
211+
generationContext.write(`${targetExpr} = ${sourceExpr}${semicolon}`);
208212
}
209213
},
210214
generateDeclaration(generationContext, dag, nodeID) {
@@ -246,6 +250,15 @@ export const glslBackend = {
246250
return node.value;
247251
}
248252
case NodeType.VARIABLE:
253+
// Track shared variable usage context
254+
if (generationContext.shaderContext && generationContext.strandsContext?.sharedVariables?.has(node.identifier)) {
255+
const sharedVar = generationContext.strandsContext.sharedVariables.get(node.identifier);
256+
if (generationContext.shaderContext === 'vertex') {
257+
sharedVar.usedInVertex = true;
258+
} else if (generationContext.shaderContext === 'fragment') {
259+
sharedVar.usedInFragment = true;
260+
}
261+
}
249262
return node.identifier;
250263
case NodeType.OPERATION:
251264
const useParantheses = node.usedBy.length > 0;
@@ -311,7 +324,11 @@ export const glslBackend = {
311324
}
312325
case NodeType.PHI:
313326
// Phi nodes represent conditional merging of values
314-
// They should already have been declared as temporary variables
327+
// If this phi node has an identifier (like varying variables), use that
328+
if (node.identifier) {
329+
return node.identifier;
330+
}
331+
// Otherwise, they should have been declared as temporary variables
315332
// and assigned in the appropriate branches
316333
if (generationContext.tempNames?.[nodeID]) {
317334
return generationContext.tempNames[nodeID];

‎src/strands/strands_node.js‎

Lines changed: 146 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1,13 +1,158 @@
1-
import { swizzleTrap } from './ir_builders';
1+
import { swizzleTrap, primitiveConstructorNode, variableNode } from './ir_builders';
2+
import { BaseType, NodeType, OpCode } from './ir_types';
3+
import { getNodeDataFromID, createNodeData, getOrCreateNode } from './ir_dag';
4+
import { recordInBasicBlock } from './ir_cfg';
25
export class StrandsNode {
36
constructor(id, dimension, strandsContext) {
47
this.id = id;
58
this.strandsContext = strandsContext;
69
this.dimension = dimension;
10+
11+
// Store original identifier for varying variables
12+
const dag = this.strandsContext.dag;
13+
const nodeData = getNodeDataFromID(dag, this.id);
14+
if (nodeData && nodeData.identifier) {
15+
this._originalIdentifier = nodeData.identifier;
16+
this._originalBaseType = nodeData.baseType;
17+
this._originalDimension = nodeData.dimension;
18+
}
719
}
820
copy() {
921
return createStrandsNode(this.id, this.dimension, this.strandsContext);
1022
}
23+
bridge(value) {
24+
const { dag, cfg } = this.strandsContext;
25+
const orig = getNodeDataFromID(dag, this.id);
26+
const baseType = orig?.baseType ?? BaseType.FLOAT;
27+
28+
let newValueID;
29+
if (value instanceof StrandsNode) {
30+
newValueID = value.id;
31+
} else {
32+
const newVal = primitiveConstructorNode(
33+
this.strandsContext,
34+
{ baseType, dimension: this.dimension },
35+
value
36+
);
37+
newValueID = newVal.id;
38+
}
39+
40+
// For varying variables, we need both assignment generation AND a way to reference by identifier
41+
if (this._originalIdentifier) {
42+
// Create a variable node for the target (the varying variable)
43+
const { id: targetVarID } = variableNode(
44+
this.strandsContext,
45+
{ baseType: this._originalBaseType, dimension: this._originalDimension },
46+
this._originalIdentifier
47+
);
48+
49+
// Create assignment node for GLSL generation
50+
const assignmentNode = createNodeData({
51+
nodeType: NodeType.ASSIGNMENT,
52+
dependsOn: [targetVarID, newValueID],
53+
phiBlocks: []
54+
});
55+
const assignmentID = getOrCreateNode(dag, assignmentNode);
56+
recordInBasicBlock(cfg, cfg.currentBlock, assignmentID);
57+
58+
// Track for global assignments processing
59+
this.strandsContext.globalAssignments.push(assignmentID);
60+
61+
// Simply update this node to be a variable node with the identifier
62+
// This ensures it always generates the variable name in expressions
63+
const variableNodeData = createNodeData({
64+
nodeType: NodeType.VARIABLE,
65+
baseType: this._originalBaseType,
66+
dimension: this._originalDimension,
67+
identifier: this._originalIdentifier
68+
});
69+
const variableID = getOrCreateNode(dag, variableNodeData);
70+
71+
this.id = variableID; // Point to the variable node for expression generation
72+
} else {
73+
this.id = newValueID; // For non-varying variables, just update to new value
74+
}
75+
76+
return this;
77+
}
78+
bridgeSwizzle(swizzlePattern, value) {
79+
const { dag, cfg } = this.strandsContext;
80+
const orig = getNodeDataFromID(dag, this.id);
81+
const baseType = orig?.baseType ?? BaseType.FLOAT;
82+
83+
let newValueID;
84+
if (value instanceof StrandsNode) {
85+
newValueID = value.id;
86+
} else {
87+
const newVal = primitiveConstructorNode(
88+
this.strandsContext,
89+
{ baseType, dimension: this.dimension },
90+
value
91+
);
92+
newValueID = newVal.id;
93+
}
94+
95+
// For varying variables, create swizzle assignment
96+
if (this._originalIdentifier) {
97+
// Create a variable node for the target with swizzle
98+
const { id: targetVarID } = variableNode(
99+
this.strandsContext,
100+
{ baseType: this._originalBaseType, dimension: this._originalDimension },
101+
this._originalIdentifier
102+
);
103+
104+
// Create a swizzle node for the target (myVarying.xyz)
105+
const swizzleNode = createNodeData({
106+
nodeType: NodeType.OPERATION,
107+
opCode: OpCode.Unary.SWIZZLE,
108+
baseType: this._originalBaseType,
109+
dimension: swizzlePattern.length, // xyz = 3, xy = 2, etc.
110+
swizzle: swizzlePattern,
111+
dependsOn: [targetVarID]
112+
});
113+
const swizzleID = getOrCreateNode(dag, swizzleNode);
114+
115+
// Create assignment node: myVarying.xyz = value
116+
const assignmentNode = createNodeData({
117+
nodeType: NodeType.ASSIGNMENT,
118+
dependsOn: [swizzleID, newValueID],
119+
phiBlocks: []
120+
});
121+
const assignmentID = getOrCreateNode(dag, assignmentNode);
122+
recordInBasicBlock(cfg, cfg.currentBlock, assignmentID);
123+
124+
// Track for global assignments processing in the current hook context
125+
this.strandsContext.globalAssignments.push(assignmentID);
126+
127+
// Simply update this node to be a variable node with the identifier
128+
// This ensures it always generates the variable name in expressions
129+
const variableNodeData = createNodeData({
130+
nodeType: NodeType.VARIABLE,
131+
baseType: this._originalBaseType,
132+
dimension: this._originalDimension,
133+
identifier: this._originalIdentifier
134+
});
135+
const variableID = getOrCreateNode(dag, variableNodeData);
136+
137+
this.id = variableID; // Point to the variable node, not the assignment node
138+
} else {
139+
this.id = newValueID; // For non-varying variables, just update to new value
140+
}
141+
142+
return this;
143+
}
144+
getValue() {
145+
if (this._originalIdentifier) {
146+
const { id, dimension } = variableNode(
147+
this.strandsContext,
148+
{ baseType: this._originalBaseType, dimension: this._originalDimension },
149+
this._originalIdentifier
150+
);
151+
return createStrandsNode(id, dimension, this.strandsContext);
152+
}
153+
154+
return this;
155+
}
11156
}
12157
export function createStrandsNode(id, dimension, strandsContext, onRebind) {
13158
return new Proxy(

0 commit comments

Comments
 (0)