feat(@typegpu/gl): Implicit pointer definitions in GLSL generator - #2819
feat(@typegpu/gl): Implicit pointer definitions in GLSL generator#2819iwoplaza wants to merge 1 commit into
Conversation
|
pkg.pr.new packages benchmark commit |
Resolution Time Benchmark---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Random Branching (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.73, 1.33, 2.97, 4.73, 5.39, 8.97, 16.34, 18.83]
line [0.68, 1.37, 3.24, 4.24, 5.31, 8.89, 17.91, 17.81]
line [0.80, 1.61, 3.36, 4.87, 5.77, 9.01, 16.89, 20.65]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Linear Recursion (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.21, 0.40, 0.60, 0.70, 0.97, 0.97, 1.08, 1.21]
line [0.23, 0.41, 0.52, 0.60, 0.82, 0.87, 1.07, 1.14]
line [0.20, 0.42, 0.53, 0.65, 0.89, 0.95, 1.21, 1.37]
---
config:
themeVariables:
xyChart:
plotColorPalette: "#E63946, #3B82F6, #059669"
---
xychart
title "Full Tree (🔴 PR | 🔵 main | 🟢 release)"
x-axis "max depth" [1, 2, 3, 4, 5, 6, 7, 8]
y-axis "time (ms)"
line [0.73, 1.84, 3.31, 5.31, 9.58, 20.92, 41.08, 80.11]
line [0.82, 1.78, 3.62, 4.86, 8.67, 17.23, 38.47, 79.36]
line [0.88, 1.82, 3.60, 4.96, 9.53, 19.98, 42.81, 86.22]
|
Bundle size comparison (
|
| 🟢 Decreased (max -0.28%) | ➖ Unchanged | 🔴 Increased (max 2.79%) | ❔ Unknown |
|---|---|---|---|
| 6 | 129 | 187 | 1 |
import * as ... in PR vs import * as ... in target (did bundle size increase?):
| Test | tsdown |
|---|---|
| std_isBeingTranspiled.ts | 15.71 kB ( |
| std_getTargetShaderLanguage.ts | 15.77 kB ( |
| std_getShaderStage.ts | 15.76 kB |
import { ... } in PR vs import * as ... in PR (is the library tree-Shakeable?):
| Test | tsdown |
|---|---|
| tgpu_init.ts | 268.11 kB ( |
| tgpu_initFromDevice.ts | 267.58 kB ( |
| tgpu_resolve.ts | 168.82 kB ( |
| tgpu_resolveWithContext.ts | 168.76 kB ( |
| tgpu_bindGroupLayout.ts | 73.79 kB ( |
| tgpu_mutableAccessor.ts | 68.52 kB ( |
| tgpu_accessor.ts | 68.52 kB ( |
| tgpu_privateVar.ts | 67.21 kB ( |
| tgpu_workgroupVar.ts | 67.21 kB ( |
| tgpu_const.ts | 66.63 kB ( |
| tgpu_lazy.ts | 66.42 kB ( |
| tgpu_fragmentFn.ts | 38.92 kB ( |
| tgpu_fn.ts | 38.86 kB ( |
| tgpu_vertexFn.ts | 38.73 kB ( |
| tgpu_computeFn.ts | 38.44 kB ( |
| tgpu_vertexLayout.ts | 27.57 kB ( |
| tgpu_comptime.ts | 15.18 kB ( |
| tgpu_unroll.ts | 1.75 kB ( |
| tgpu_slot.ts | 1.70 kB ( |
If you wish to run a comparison for other, slower bundlers, run the 'Tree-shake test' from the GitHub Actions menu.
There was a problem hiding this comment.
Pull request overview
This PR adds GLSL-compatible handling for TypeGPU’s “implicit pointer” const-aliasing semantics, enabling the WebGL/GLSL fallback to preserve expected mutation behavior without native pointer support.
Changes:
- Refactors WGSL const-alias handling into an overridable
_aliasConstStatementhook. - Implements GLSL-specific aliasing rules: copy from immutable origins (e.g. uniforms), otherwise inline the aliased l-value expression and hoist runtime index expressions into temporaries to ensure single evaluation.
- Adds a dedicated
implicitPointer.test.tssuite covering immutable copies, mutable aliases, runtime index hoisting, and nested/alias-of-alias cases.
Reviewed changes
Copilot reviewed 3 out of 3 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/typegpu/src/tgsl/wgslGenerator.ts | Extracts implicit-pointer const-alias logic into _aliasConstStatement for reuse/override. |
| packages/typegpu-gl/src/glslGenerator.ts | Overrides const-alias lowering to be pointer-free in GLSL via copy-or-alias + index-hoisting. |
| packages/typegpu-gl/tests/implicitPointer.test.ts | Adds snapshot tests validating GLSL output for implicit pointer scenarios. |
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
![pullfrog[bot]](https://avatars.githubusercontent.com/in/1768019?s=60&v=4){kind=small}
There was a problem hiding this comment.
Important
The substitution + index-hoisting design is sound and matches WGSL pointer semantics in the cases I traced, but the immutable branch copies aliased values via _emitVarDecl, which emits invalid GLSL ES 3.00 whenever the aliased value is array-typed (const a = u.arr; → float a[4] = u.arr;, which GLSL ES 3.00 does not allow). See the inline comment for the fix.
Reviewed changes
- GLSL alias const statements —
_aliasConstStatementoverride inglslGenerator.ts: copies aliases of immutable memory (uniform/readonly/handle) into locals, and turns mutable-memory aliases into compile-time substitutions of the aliased expression, hoisting non-constant index expressions into once-evaluated temp variables. - Index hoisting helper — new
#hoistIndexAccesseswalks member/index accesses and replaces runtime index expressions with@index_Nreferences tied to freshly declared temps; comptime-constant indices are bound directly. - WGSL generator refactor — the implicit-pointer const handling in
wgslGenerator.tswas extracted into a protected_aliasConstStatementhook with line-identical logic (makeUniqueIdentifier(rawId, 'block')preserved); WGSL generator/constant/struct tests still pass (139 tests). - New test suite —
implicitPointer.test.tswith six snapshot tests (immutable copy, mutable substitution, runtime-index hoisting, nested member/index chains, alias-of-an-alias, local-variable alias).
I verified the edge semantics directly: index expressions are snapshotted at the declaration (later reassignment of the index variable doesn't affect the alias, matching WGSL pointer-address semantics), and alias-of-alias chains resolve correctly through the stored expressions.
DeepSeek Flash (free via Pullfrog for OSS) ｜ 𝕏
| if (immutableOrigins.includes(eq.origin)) { | ||
| const dataType = undecorateDataType(eq.dataType as d.BaseData); | ||
| const name = this.ctx.makeUniqueIdentifier(rawId, 'block'); | ||
| this.ctx.defineVariable(rawId, snip(name, dataType, 'runtime-immutable-def', false)); | ||
| return this._emitVarDecl('let', name, dataType, this.ctx.resolveSnippet(eq).value); |
There was a problem hiding this comment.
For array-typed immutable aliases this copies the array into a local, e.g. const a = uniformStruct.$.arr; (with arr a declared-uniform array field) generates float a[4] = uniformStruct.arr;. GLSL ES 3.00 does not allow initializing a variable from another array (only from array constructors), so this emits shader code that won't compile, while the equivalent code resolves fine in the WGSL generator (implicit pointer). I confirmed this exact output by resolving such a function against glOptions().
Since immutable sources can never change, substitution (treated exactly like the mutable branch — a rendered as uniformStruct.arr at every use) is semantically safe here and sidesteps the copy limitation entirely.
| uniform Boid boid; | ||
|
|
||
| vec3 foo() { | ||
| vec3 boidPos = boid.pos; |
There was a problem hiding this comment.
Consider a test covering a whole-array immutable alias (const a = uniform.arr; a[0]), which currently emits invalid GLSL as described in the comment on glslGenerator.ts:410, plus a runtime-indexed uniform element (uniform.arr[i]) to pin that the immutable copy path evaluates the index exactly once.
12cf3a1 to
1e1ea6e
Compare
1e1ea6e to
339c509
Compare
339c509 to
8715ec1
Compare
8715ec1 to
abec4f8
Compare
Closes #2679