thi-ng / umbrella

⛱ Broadly scoped ecosystem & mono-repository of 198 TypeScript projects (and ~175 examples) for general purpose, functional, data driven development
https://thi.ng
Apache License 2.0
3.31k stars 144 forks source link

[shader-ast] Dependency for ternary operators, while and for loop #436

Closed sylee957 closed 7 months ago

sylee957 commented 7 months ago
import * as ast from "@thi.ng/shader-ast";

const fragColor = ast.output("vec4", "fragColor");
const f = ast.defn("bool", undefined, [], () => [ast.ret(ast.FALSE)]);
const red = ast.vec4(
  ast.float(1.0),
  ast.float(0.0),
  ast.float(0.0),
  ast.float(1.0)
);
const blue = ast.vec4(
  ast.float(0.0),
  ast.float(0.0),
  ast.float(1.0),
  ast.float(1.0)
);
ast.program([
  fragColor,
  f,
  ast.defMain(() => [
    ast.ternary(f(), ast.assign(fragColor, red), ast.assign(fragColor, blue)),
  ]),
]);

I've found an issue that program builds wrong dependency for ternary operators, when the test function contains the function calls, which gives compilation error for shaders. Ternary operators should visit .test as well as .t, .f for building the dependency graph.

I'm using the following monkey patch until this gets patched.

diff --git a/node_modules/@thi.ng/shader-ast/ast/scope.js b/node_modules/@thi.ng/shader-ast/ast/scope.js
index 3c4c0d3..b3ee8d2 100644
--- a/node_modules/@thi.ng/shader-ast/ast/scope.js
+++ b/node_modules/@thi.ng/shader-ast/ast/scope.js
@@ -1,8 +1,13 @@
 import { isArray } from "@thi.ng/checks/is-array";
 import { DGraph } from "@thi.ng/dgraph";
 import { isMat, isTerm, isVec } from "./checks.js";
-const scopedChildren = (t) => t.tag === "fn" || t.tag === "for" || t.tag == "while" ? t.scope.body : t.tag === "if" ? t.f ? [t.test, ...t.t.body, ...t.f.body] : [t.test, ...t.t.body] : void 0;
-const allChildren = (t) => scopedChildren(t) || (t.tag === "scope" ? t.body : t.tag === "ternary" ? [t.t, t.f] : t.tag === "ret" ? [t.val] : t.tag === "call" || t.tag === "call_i" ? t.args : t.tag === "sym" && t.init ? [t.init] : t.tag === "decl" ? [t.id] : t.tag === "op1" || t.tag === "swizzle" ? [t.val] : t.tag === "op2" ? [t.l, t.r] : t.tag === "assign" ? [t.r] : isVec(t) || isMat(t) ? t.val : isTerm(t.val) ? t.val : void 0);
+const scopedChildren = (t) => 
+t.tag === "while" ? [...(t.test ? [t.test] : []), ...t.scope.body] : 
+t.tag === "for" ? [...(t.init ? [t.init] : []), t.test, ...(t.iter ? [t.iter] : []), ...t.scope.body] :
+t.tag === "fn" ? t.scope.body : 
+t.tag === "if" ? t.f ? [t.test, ...t.t.body, ...t.f.body] : [t.test, ...t.t.body] : 
+void 0;
+const allChildren = (t) => scopedChildren(t) || (t.tag === "scope" ? t.body : t.tag === "ternary" ? [t.test, t.t, t.f] : t.tag === "ret" ? [t.val] : t.tag === "call" || t.tag === "call_i" ? t.args : t.tag === "sym" && t.init ? [t.init] : t.tag === "decl" ? [t.id] : t.tag === "op1" || t.tag === "swizzle" ? [t.val] : t.tag === "op2" ? [t.l, t.r] : t.tag === "assign" ? [t.r] : isVec(t) || isMat(t) ? t.val : isTerm(t.val) ? t.val : void 0);
 const walk = (visit, children, acc, tree, pre = true) => {
   if (isArray(tree)) {
     tree.forEach((x) => acc = walk(visit, children, acc, x, pre));

I also found same issues with for loop and while loop, where the dependency should be added for init, test, and iter

postspectacular commented 7 months ago

Thank you very much @sylee957 for finding & analyzing this issue! I just pushed a fix and will push a new release tomorrow!