GPU-native unit tests for TSL - #34331
Merged
Merged
Conversation
🖼️ E2E screenshot tests✅ All examples render correctly again (run). |
Owner
|
5c81184 should fix the E2E screenshot issue. |
Contributor
Author
|
BTW after this is merged, I have already 8 more PRs of bug fixes in TSL that this new functionality has found. I will do them as separate PRs that each add the relevant unit tests and fix the uncovered bug at the same time. Otherwise I worry that it will not allow for a good review of each of the bugs. |
This was referenced Aug 21, 2026
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 21, 2026
…nditional temp node reference
addFlowCodeHierarchy() read flowCodeBlock off the node's builder data and
immediately called .get() on it. flowCodeBlock is created lazily, by
addLineFlowCodeBlock(), and only when the node's first build happened
inside some enclosing conditional block. If a cached temp node's first
build instead happened at the top level of a function body (no enclosing
block at all), flowCodeBlock was never initialized -- and if that same
node was later referenced again from inside an If(), TempNode's cached-value
fast path (fixed in the prior commit on this branch) would call
addFlowCodeHierarchy() and crash outright dereferencing undefined.
When flowCodeBlock is undefined, needsFlowCode is now set to false
directly instead of dereferencing it -- the mathematically correct answer,
not just a crash-avoidance guard: flowCodeBlock being undefined means the
node's assignment line was flowed unconditionally at the top level, which
is in scope from every block nested inside it, so no re-flow is ever
needed.
Depends on this branch's prior commit (the TempNode.js fix): without it,
TempNode's cached-value path never calls addFlowCodeHierarchy() at all, so
this crash is unreachable -- confirmed empirically, which is why this PR
is built on top of fix/tempnode-sibling-branch-caching rather than directly
on tsl-unit-tests.
Adds a TSL unit test (TSLNeutralToneMapping.tests.js) using the real,
unmodified neutralToneMapping() library function, which hits exactly this
pattern and reliably crashed on both [webgpu] and [webgl] before this fix.
Verified: reverting just this commit's NodeBuilder.js change (keeping the
TempNode.js fix) reproduces the exact crash ("Cannot read properties of
undefined (reading 'get')") and wrong values described above; restoring it
fixes both, full addons suite passes clean (61/61) either way this commit
is or isn't the one under test.
Built on top of fix/tempnode-sibling-branch-caching, which is itself built
on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 21, 2026
…nditional temp node reference
addFlowCodeHierarchy() read flowCodeBlock off the node's builder data and
immediately called .get() on it. flowCodeBlock is created lazily, by
addLineFlowCodeBlock(), and only when the node's first build happened
inside some enclosing conditional block. If a cached temp node's first
build instead happened at the top level of a function body (no enclosing
block at all), flowCodeBlock was never initialized -- and if that same
node was later referenced again from inside an If(), TempNode's cached-value
fast path (fixed in the prior commit on this branch) would call
addFlowCodeHierarchy() and crash outright dereferencing undefined.
When flowCodeBlock is undefined, needsFlowCode is now set to false
directly instead of dereferencing it -- the mathematically correct answer,
not just a crash-avoidance guard: flowCodeBlock being undefined means the
node's assignment line was flowed unconditionally at the top level, which
is in scope from every block nested inside it, so no re-flow is ever
needed.
Depends on this branch's prior commit (the TempNode.js fix): without it,
TempNode's cached-value path never calls addFlowCodeHierarchy() at all, so
this crash is unreachable -- confirmed empirically, which is why this PR
is built on top of fix/tempnode-sibling-branch-caching rather than directly
on tsl-unit-tests.
Adds a TSL unit test (TSLNeutralToneMapping.tests.js) using the real,
unmodified neutralToneMapping() library function, which hits exactly this
pattern and reliably crashed on both [webgpu] and [webgl] before this fix.
Verified: reverting just this commit's NodeBuilder.js change (keeping the
TempNode.js fix) reproduces the exact crash ("Cannot read properties of
undefined (reading 'get')") and wrong values described above; restoring it
fixes both, full addons suite passes clean (61/61) either way this commit
is or isn't the one under test.
Built on top of fix/tempnode-sibling-branch-caching, which is itself built
on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
WGSL's faceForward built-in is camelCase (faceForward), but MathNode.FACEFORWARD is the (correct, GLSL-spelled) all-lowercase string 'faceforward', and WGSLNodeBuilder's wgslMethods table had no entry mapping it -- so every WebGPU-backend call to faceForward() failed to compile. Add the missing wgslMethods entry, mirroring the existing inversesqrt -> inverseSqrt entry. Adds a two-backend TSL unit test (TSLFaceForward.tests.js) that fails on [webgpu] and passes on [webgl] before this fix, and passes on both after. Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
…ead of float determinant(m) computes and codegens a scalar (float) at the shader level, but MathNode.DETERMINANT was missing from the list of methods special-cased to report 'float' from generateNodeType(). It fell through to getInputType(), which for a single-matrix-argument call returns the matrix's own type (e.g. 'mat3') -- so determinant(mat3).getNodeType() answered 'mat3', not 'float'. Add MathNode.DETERMINANT alongside LENGTH/DISTANCE/DOT in that branch. Adds a TSL unit test (TSLDeterminant.tests.js) that failed with 'gpuTest: type mismatch -- comparing "mat3" against "float"' before this fix. Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
…y reads as zero A node promoted to a cached temp variable (anything deriving from TempNode) had its assignment statement flowed into whichever code-block was active the first time it was referenced. A later reference from a sibling conditional block (an If/Else that doesn't share an ancestor with the first reference's block) never actually executed that assignment, so the 'cached' variable silently read its default-initialized value (0) there -- no compile error, no runtime warning. NodeBuilder.addFlowCodeHierarchy() exists specifically to re-flow a cached node's assignment into a new block on repeat reference, and the generic (non-TempNode) caching path in Node.build() already calls it -- this adds the same call to TempNode.build()'s own specialized propertyName-based caching path. Adds a TSL unit test (TSLMatrixInverseRoundTrip.tests.js) whose harness writes each mat4 column from its own If branch, all referencing the same shared M*inverse(M) expression -- before this fix, only the diagonal entries came out correct and the rest silently read zero. Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
…nditional temp node reference
addFlowCodeHierarchy() read flowCodeBlock off the node's builder data and
immediately called .get() on it. flowCodeBlock is created lazily, by
addLineFlowCodeBlock(), and only when the node's first build happened
inside some enclosing conditional block. If a cached temp node's first
build instead happened at the top level of a function body (no enclosing
block at all), flowCodeBlock was never initialized -- and if that same
node was later referenced again from inside an If(), TempNode's cached-value
fast path (fixed in the prior commit on this branch) would call
addFlowCodeHierarchy() and crash outright dereferencing undefined.
When flowCodeBlock is undefined, needsFlowCode is now set to false
directly instead of dereferencing it -- the mathematically correct answer,
not just a crash-avoidance guard: flowCodeBlock being undefined means the
node's assignment line was flowed unconditionally at the top level, which
is in scope from every block nested inside it, so no re-flow is ever
needed.
Depends on this branch's prior commit (the TempNode.js fix): without it,
TempNode's cached-value path never calls addFlowCodeHierarchy() at all, so
this crash is unreachable -- confirmed empirically, which is why this PR
is built on top of fix/tempnode-sibling-branch-caching rather than directly
on tsl-unit-tests.
Adds a TSL unit test (TSLNeutralToneMapping.tests.js) using the real,
unmodified neutralToneMapping() library function, which hits exactly this
pattern and reliably crashed on both [webgpu] and [webgl] before this fix.
Verified: reverting just this commit's NodeBuilder.js change (keeping the
TempNode.js fix) reproduces the exact crash ("Cannot read properties of
undefined (reading 'get')") and wrong values described above; restoring it
fixes both, full addons suite passes clean (61/61) either way this commit
is or isn't the one under test.
Built on top of fix/tempnode-sibling-branch-caching, which is itself built
on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
…e() NaN exponent gain() had two stacked bugs: (1) it used a native JS ternary (x.lessThan(0.5) ? A : B) on a TSL Node, which is always truthy in JS, so it unconditionally evaluated to A regardless of x's runtime value -- no GPU branching at all; (2) even with correct branching it was built from parabola() (pow(4x(1-x), k)), which does not satisfy the documented 'k=1 is the identity curve' contract. Rewrote gain() using select() (real GPU branching) and pow(2x, k) (matching Inigo Quilez's reference definition). pcurve()'s exponent used native JS division (1.0 / a) on a Node, always producing NaN (Node has no valueOf()/toString() numeric coercion). Fixed by using TSL's div() node constructor. Adds a TSL unit test (TSLGainPcurve.tests.js) covering both functions. Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
mat4(v0,v1,v2,v3) (four vec4 args) takes its arguments as *columns* (standard GLSL/WGSL constructor semantics) -- unlike TSL's other flat 16-scalar mat4(...) constructor, which is row-major. RotateNode's 3D rotation matrices were written as if the vec4-args form were also row-major (each vec4(...) spelled out a textbook rotation-matrix row), so the constructor consumed each row as a column instead -- the resulting matrix was the transpose of the intended one, i.e. the same rotation run backwards, on all three axes. Rewrote all three per-axis matrices (rotationXMatrix/Y/Z) column-by-column so the columns passed to mat4(...) are the actual columns of the intended row-major rotation matrix. Adds a TSL unit test (TSLRotate.tests.js) cross-checking the 3D single-axis rotation against the (already correct) 2D rotate() case on all three axes. Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
bhouston
added a commit
to bhouston/three.js
that referenced
this pull request
Aug 22, 2026
sinc(x, k) == sin(arg)/arg where arg = PI*(k*x - 1), which has a removable
singularity at arg == 0 (limit is 1, the function's peak). The
implementation evaluated the division unconditionally, so a
compile-time-constant input hitting the peak exactly made WGSL's compiler
constant-fold the whole expression and hit a hard shader-compile error
('0.0 / 0.0' cannot be represented as 'abstract-float'); non-constant
runtime inputs would silently produce NaN at that one value.
Guarded the division with select(abs(arg).lessThan(1e-6), 1.0,
sin(arg).div(arg)), returning the analytic limit at the singularity.
select() alone is not sufficient -- WGSL's compile-time constant folding
evaluates the sin(arg)/arg sub-expression regardless of which runtime
branch select()/if would actually take, so arg is also routed through
.toVar() to stop it being recognized as a foldable compile-time constant.
Adds a TSL unit test (TSLSinc.tests.js) whose peak assertion
(sinc(1/k, k) == 1) deliberately exercises the exact singularity point.
Built on top of the tsl-unit-tests branch (three.js PR mrdoob#34331).
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
TSL/node unit tests today are mock-only — they check bookkeeping against fake renderers, never actual shader output. This adds a harness that runs real TSL expressions on the GPU and asserts on the results, with a QUnit-style API:
If things fail, it tells you why and what tests:
gpuFuzzTestdispatches an assertion across many GPU-generated cases in one shot:256 independent cases, generated and checked entirely on the GPU, in one dispatch.
How it works
Each
assert.*()call gets its own row in a pair ofvec4storage buffers (actual/expected), written by one compute invocation per row. A tiny internal node asks the real TSL builder what type the expression resolved to (getNodeType(builder)), so no type needs to be declared by hand — even through composed function calls. After the dispatch, the harness reads both buffers back and does the comparison, tolerancing, and per-component diffing on the CPU, where it's cheap to produce good error messages.Scalars and vecN types are a single row. Matrix types (mat3/mat4) don't fit in one
vec4, so they're represented as multiple "columns" — mat3 as 3 x vec3, mat4 as 4 x vec4 — each zero-padded and written/read as its own row, with per-component diff labels likecol0.xon failure.gpuFuzzTestsites that assert on a matrix need to opt in viamaxColumnsPerSite(3 or 4) since the column count is a resource budget, not just a convenience.Both entry points run on WebGPU and WebGPURenderer's WebGL2 fallback backend by default (one QUnit test per backend, e.g.
... [webgpu],... [webgl]) — narrow to one explicitly only for a node that's deliberately WebGPU-only. Each backend's availability is detected at runtime: if a backend fails to initialize in a given environment (no GPU, missing driver, etc.), its tests skip with a warning instead of failing the build — GitHub-hosted CI runners, for instance, can support one backend but not the other depending on the image.Scope
test/unit/addons/tsl/gpu-test-utils.js— the harness (gpuTest,gpuFuzzTest,Kind)test/unit/addons/tsl/GPUTest.tests.js— example teststest/unit/puppeteer.unit.js— prints failing test names/messages (previously only aggregate counts)npm run test-unit-addonsPuppeteer/headless CI job — no new infrastructureBased on the GPU testing method used in threeify (render-and-readback shader tests, one row of a texture per test).
(BTW I have already found 8 TSL bugs, a few serious, using this framework, but I will submit them as separate PRs to allow for proper review of each one.)
CC: @sunag