review splitter/linker so they don't mutate previous pass locals

hxsl/Cache.hx

@@ -257,6 +257,10 @@ class Cache {
 		}
 		#end
 
+		var prev = s;
+		var splitter = new hxsl.Splitter();
+		var s = try splitter.split(s) catch( e : Error ) { e.msg += "\n\nin\n\n"+Printer.shaderToString(s); throw e; };
+
 		// params tracking
 		var paramVars = new Map();
 		for( v in linker.allVars )
@@ -266,11 +270,10 @@ class Cache {
 				default:
 				}
 				var inf = shaderDatas[v.instanceIndex];
-				paramVars.set(v.id, { instance : inf.index, index : inf.inst.params.get(v.merged[0].id) } );
+				var nv = @:privateAccess splitter.varMap.get(v.v);
+				paramVars.set(nv == null ? v.id : nv.id, { instance : inf.index, index : inf.inst.params.get(v.merged[0].id) } );
 			}
 
-		var prev = s;
-		var s = try new hxsl.Splitter().split(s) catch( e : Error ) { e.msg += "\n\nin\n\n"+Printer.shaderToString(s); throw e; };
 
 		#if debug
 		Printer.check(s.vertex,[prev]);

hxsl/Linker.hx

@@ -318,25 +318,6 @@ class Linker {
 		cur.onStack = false;
 	}
 
-	function uniqueLocals( expr : TExpr, locals : Map < String, Bool > ) : Void {
-		switch( expr.e ) {
-		case TVarDecl(v, _):
-			if( locals.exists(v.name) ) {
-				var k = 2;
-				while( locals.exists(v.name + k) )
-					k++;
-				v.name += k;
-			}
-			locals.set(v.name, true);
-		case TBlock(el):
-			var locals = [for( k in locals.keys() ) k => true];
-			for( e in el )
-				uniqueLocals(e, locals);
-		default:
-			expr.iter(uniqueLocals.bind(_, locals));
-		}
-	}
-
 	public function link( shadersData : Array<ShaderData> ) : ShaderData {
 		debug("---------------------- LINKING -----------------------");
 		varMap = new Map();
@@ -347,11 +328,12 @@ class Linker {
 
 		var dupShaders = new Map();
 		shadersData = [for( s in shadersData ) {
-			var s = s, sreal = s;
-			if( dupShaders.exists(s) )
-				s = Clone.shaderData(s);
-			dupShaders.set(s, sreal);
-			s;
+			if( dupShaders.exists(s.name) )
+				Clone.shaderData(s);
+			else {
+				dupShaders.set(s.name, s);
+				s;
+			}
 		}];
 
 		// globalize vars
@@ -507,7 +489,6 @@ class Linker {
 					exprs.push(s.body);
 				}
 			var expr = { e : TBlock(exprs), t : TVoid, p : exprs.length == 0 ? null : exprs[0].p };
-			uniqueLocals(expr, new Map());
 			return {
 				kind : kind,
 				ref : v,
@@ -522,8 +503,8 @@ class Linker {
 		];
 
 		// make sure the first merged var is the original for duplicate shaders
-		for( s in dupShaders.keys() ) {
-			var sreal = dupShaders.get(s);
+		for( s in shadersData ) {
+			var sreal = dupShaders.get(s.name);
 			if( s == sreal ) continue;
 			for( i in 0...s.vars.length )
 				allocVar(s.vars[i],null).merged.unshift(sreal.vars[i]);

hxsl/Splitter.hx

@@ -2,6 +2,7 @@ package hxsl;
 using hxsl.Ast;
 
 private class VarProps {
+	public var origin : TVar;
 	public var v : TVar;
 	public var read : Int;
 	public var write : Int;
@@ -42,20 +43,14 @@ class Splitter {
 				throw "assert";
 			}
 
-		// perform a first mapVars before we map fragment shader vars		
-		vfun = {
-			ret : vfun.ret,
-			ref : vfun.ref,
-			kind : vfun.kind,
-			args : vfun.args,
-			expr : mapVars(vfun.expr),
-		};
-
+		var vafterMap = [];
 		for( inf in Lambda.array(vvars) ) {
 			var v = inf.v;
+			if( inf.local ) continue;
 			switch( v.kind ) {
 			case Var, Local:
-				v.kind = fvars.exists(v.id) ? Var : Local;
+				var fv = fvars.get(inf.origin.id);
+				v.kind = fv != null && fv.read > 0 ? Var : Local;
 			default:
 			}
 			switch( v.kind ) {
@@ -65,29 +60,39 @@ class Splitter {
 					var nv : TVar = {
 						id : Tools.allocVarId(),
 						name : v.name,
-						kind : v.kind,
+						kind : Local,
 						type : v.type,
 					};
-					vars = vvars;
-					var ninf = get(nv);
-					v.kind = Local;
+					uniqueName(nv);
+					varMap.set(inf.origin, nv);
+
+					var ninf = new VarProps(nv);
+					ninf.read++;
+					vvars.set(nv.id, ninf);
+
 					var p = vfun.expr.p;
-					var e = { e : TBinop(OpAssign, { e : TVar(nv), t : nv.type, p : p }, { e : TVar(v), t : v.type, p : p } ), t : nv.type, p : p };
-					addExpr(vfun, e);
-					checkExpr(e);
-					if( nv.kind == Var ) {
-						var old = fvars.get(v.id);
-						varMap.set(v, nv);
-						fvars.remove(v.id);
-						var np = new VarProps(nv);
-						np.read = old.read;
-						np.write = old.write;
-						fvars.set(nv.id, np);
-					}
+					var e = { e : TBinop(OpAssign, { e : TVar(v), t : nv.type, p : p }, { e : TVar(nv), t : v.type, p : p } ), t : nv.type, p : p };
+					vafterMap.push(() -> addExpr(vfun, e));
+
+					if( v.kind == Var )
+						vafterMap.push(() -> varMap.set(inf.origin, v)); // restore
 				}
 			default:
 			}
 		}
+
+		// perform a first mapVars before we map fragment shader vars
+		vfun = {
+			ret : vfun.ret,
+			ref : vfun.ref,
+			kind : vfun.kind,
+			args : vfun.args,
+			expr : mapVars(vfun.expr),
+		};
+
+		for( f in vafterMap )
+			f();
+
 		var finits = [];
 		var todo = [];
 		for( inf in fvars ) {
@@ -102,22 +107,24 @@ class Splitter {
 					type : v.type,
 				};
 				uniqueName(nv);
-				var i = vvars.get(v.id);
+				var i = vvars.get(inf.origin.id);
 				if( i == null ) {
 					i = new VarProps(v);
-					vvars.set(v.id, i);
+					vvars.set(inf.origin.id, i);
 				}
 				i.read++;
-				var vp = new VarProps(nv);
-				vp.write = 1;
-				vvars.set(nv.id, vp);
-				var fp = new VarProps(nv);
-				fp.read = 1;
-				todo.push(fp);
+				varMap.set(inf.origin, nv);
+
+				var ninf = new VarProps(nv);
+				ninf.origin = inf.origin;
+
+				// make sure it's listed in variables
+				fvars.set(inf.origin.id, ninf);
+				vvars.set(nv.id, ninf);
+
 				addExpr(vfun, { e : TBinop(OpAssign, { e : TVar(nv), t : v.type, p : vfun.expr.p }, { e : TVar(v), t : v.type, p : vfun.expr.p } ), t : v.type, p : vfun.expr.p } );
-				varMap.set(v, nv);
-				inf.local = true;
 			case Var if( inf.write > 0 ):
+				// prevent error when writing to a varying
 				var nv : TVar = {
 					id : Tools.allocVarId(),
 					name : v.name,
@@ -126,12 +133,10 @@ class Splitter {
 				};
 				uniqueName(nv);
 				finits.push( { e : TVarDecl(nv, { e : TVar(v), t : v.type, p : ffun.expr.p } ), t:TVoid, p:ffun.expr.p } );
-				varMap.set(v, nv);
+				varMap.set(inf.origin, nv);
 			default:
 			}
 		}
-		for( v in todo )
-			fvars.set(v.v.id, v);
 
 		// final check
 		for( v in vvars )
@@ -139,12 +144,6 @@ class Splitter {
 		for( v in fvars )
 			checkVar(v, false, vvars, ffun.expr.p);
 
-		// support for double mapping v -> v1 -> v2
-		for( v in varMap.keys() ) {
-			var v2 = varMap.get(varMap.get(v));
-			if( v2 != null )
-				varMap.set(v, v2);
-		}
 		ffun = {
 			ret : ffun.ret,
 			ref : ffun.ref,
@@ -161,21 +160,22 @@ class Splitter {
 			ffun.expr = { e : TBlock(finits), t : TVoid, p : ffun.expr.p };
 		}
 
-		var vvars = [for( v in vvars ) if( !v.local ) v.v];
-		var fvars = [for( v in fvars ) if( !v.local ) v.v];
+		var vvars = [for( v in vvars ) if( !v.local ) v];
+		var fvars = [for( v in fvars ) if( !v.local ) v];
 		// make sure we sort the inputs the same way they were sent in
-		vvars.sort(function(v1, v2) return v1.id - v2.id);
-		fvars.sort(function(v1, v2) return v1.id - v2.id);
+		inline function getId(v:VarProps) return v.origin == null ? v.v.id : v.origin.id;
+		vvars.sort(function(v1, v2) return getId(v1) - getId(v2));
+		fvars.sort(function(v1, v2) return getId(v1) - getId(v2));
 
 		return {
 			vertex : {
 				name : "vertex",
-				vars : vvars,
+				vars : [for( v in vvars ) v.v],
 				funs : [vfun],
 			},
 			fragment : {
 				name : "fragment",
-				vars : fvars,
+				vars : [for( v in fvars ) v.v],
 				funs : [ffun],
 			},
 		};
@@ -196,7 +196,8 @@ class Splitter {
 			throw new Error("Variable " + v.v.name + " is used without being initialized", p);
 		case Var:
 			if( !vertex ) {
-				var i = vvars.get(v.v.id);
+				var i = vvars.get(v.origin.id);
+				if( i != null && i.v.kind == Input ) return;
 				if( i == null || i.write == 0 ) throw new Error("Varying " + v.v.name + " is not written by vertex shader",p);
 			}
 		default:
@@ -210,7 +211,7 @@ class Splitter {
 			v2 == null ? e : { e : TVar(v2), t : e.t, p : e.p };
 		case TVarDecl(v, init):
 			var v2 = varMap.get(v);
-			v2 == null ? e.map(mapVars) : { e : TVarDecl(v2,mapVars(init)), t : e.t, p : e.p };
+			v2 == null ? e.map(mapVars) : { e : TVarDecl(v2,init == null ? null : mapVars(init)), t : e.t, p : e.p };
 		case TFor(v, it, loop):
 			var v2 = varMap.get(v);
 			v2 == null ? e.map(mapVars) : { e : TFor(v2,mapVars(it),mapVars(loop)), t : e.t, p : e.p };
@@ -222,24 +223,23 @@ class Splitter {
 	function get( v : TVar ) {
 		var i = vars.get(v.id);
 		if( i == null ) {
-			var v2 = varMap.get(v);
-			if( v2 != null )
-				return get(v2);
-			var oldName = v.name;
-			uniqueName(v);
-			if( v.kind == Local && oldName != v.name ) {
-				// variable renamed : restore its name and create a new one
-				var nv : TVar = {
-					id : Tools.allocVarId(),
-					name : v.name,
-					kind : v.kind,
-					type : v.type,
-				};
+			var nv = varMap.get(v);
+			if( nv == null ) {
+				if( v.kind == Global || v.kind == Output || v.kind == Input )
+					nv = v;
+				else {
+					nv = {
+						id : Tools.allocVarId(),
+						name : v.name,
+						kind : v.kind,
+						type : v.type,
+					};
+					uniqueName(nv);
+				}
 				varMap.set(v,nv);
-				v.name = oldName;
-				v = nv;
 			}
-			i = new VarProps(v);
+			i = new VarProps(nv);
+			i.origin = v;
 			vars.set(v.id, i);
 		}
 		return i;
@@ -248,7 +248,6 @@ class Splitter {
 	function uniqueName( v : TVar ) {
 		if( v.kind == Global || v.kind == Output || v.kind == Input )
 			return;
-		v.parent = null;
 		var n = varNames.get(v.name);
 		if( n != null && n != v ) {
 			var prefix = v.name;