Repository navigation
fix(@typegpu/gl): release unused uniforms and destroyed textures - #3258
Kaleidosium wants to merge 2 commits into
Conversation
There was a problem hiding this comment.
🟢 Approval recommended
No blocking issues were identified, and focused regression tests cover the changed ownership, binding, and cleanup behavior.
No open issues 🎉
What changed in this PR
Fixes #3153 by allowing unused uniforms and destroyed texture wrappers to be collected while a WebGL root remains alive.
Changes:
- Discover uniform bindings from shader references instead of root-wide history.
- Unregister destroyed textures through callbacks that weakly reference the texture registry.
- Add regression coverage for resource lifetimes, uniform bindings, and native cleanup.
| File | Description |
|---|---|
packages/typegpu-gl/tests/webglFallback.test.ts |
Verify texture and framebuffer deletion exactly once by identity. |
packages/typegpu-gl/tests/uniforms.test.ts |
Cover cross-stage bindings, writes, optimized-out uniforms, and program-specific locations. |
packages/typegpu-gl/tests/garbageCollection.test.ts |
Verify uniform, buffer, root, and texture collection behavior. |
packages/typegpu-gl/src/webglTexture.ts |
Invoke and clear the destruction callback. |
packages/typegpu-gl/src/tgpuRootWebGL.ts |
Remove uniform history and introduce weakly referenced texture-registry cleanup. |
Review effort: Balanced
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
tgpuRootWebGL.ts— uniform retention: dropped the root-wide#uniformsregistry.TgpuWebGLRenderPipelineImplnow discovers bindings by iteratingcrossShaderStageState.globalIdentifierMapand filteringinstanceof WebGLUniformImpl, so acreateUniformresult is no longer pinned for the root's lifetime; only pipelines that actually bind a uniform retain it, and optimized-out uniforms (getUniformLocation === null) are not retained.tgpuRootWebGL.ts/webglTexture.ts— texture retention:#texturesis now aSet.createTextureinstalls anonDestroycallback that holds aWeakRefto the registry, andWebGLTextureImpl.destroy()invokes it exactly once after native deletion and then clears it. A surviving texture handle therefore cannot keep the registry or its sibling textures alive.- Tests: added GC regression tests (
garbageCollection.test.ts), a uniform-binding test that exercises both stages, per-program locations, and optimized-out uniforms (uniforms.test.ts), plus an exactly-once texture/framebuffer teardown test (webglFallback.test.ts).
Traced the reference graph by hand: the texture callback captures only the WeakRef, so no strong edge leads from a texture back to the set or root, and pipeline #uniformBindings retain only the uniforms they bind. root.destroy() iterating the set while onDestroy deletes the current entry is safe JS Set semantics. Verified locally: the full typegpu-gl suite passes (142 passed, 2 skipped), tsc passes, and oxlint reports no issues on the changed files.
ℹ️ TgpuRootWebGL.destroy() no longer destroys uniforms
With the root-wide uniform registry gone, destroy() no longer calls uniform.destroy(). That is behavior-neutral today because WebGLUniformImpl.destroy() is a no-op, but the file carries a TODO(#2510) to make uniforms UBO-backed — at which point root teardown would silently stop releasing uniform GPU resources.
Technical details
# Uniform cleanup owner is no longer the root
## Affected sites
- `packages/typegpu-gl/src/tgpuRootWebGL.ts` (`destroy()`) — iterates buffers, textures, and samplers, but no longer uniforms.
- `packages/typegpu-gl/src/tgpuRootWebGL.ts` (`WebGLUniformImpl.destroy()`) — currently a no-op.
## Required outcome
- None for this PR (no observable behavior change). When #2510 lands and `WebGLUniformImpl` allocates native UBOs, uniform teardown must be owned somewhere — either revisit root-level tracking (e.g. a `WeakRef` registry mirroring `#textures`) or have the pipeline release bindings it created.
## Open questions for the human
- Is uniform resource lifetime intended to be owned by the pipeline rather than the root going forward?deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏
…haring The test assumed a second pipeline built from the same descriptor links a second program. With a program cache (software-mansion#3257) both pipelines share one, so the lookup failed. Check that every upload targets a location queried on the program bound for that draw, and give the second pipeline a different fragment shader so the per-program check stays meaningful.
There was a problem hiding this comment.
✅ No new issues found.
Reviewed changes
uniforms.test.ts— program-sharing-agnostic binding assertions: replaced the program-index-baseduniformLocationhelper with anuploadedValues()helper that decodes each recordeduniform1f/uniform4fvupload back to its{ program, name }and asserts the program matches the one bound by the draw'suseProgram. The second pipeline now uses a different fragment shader so its linked program differs from the first, keeping the test valid whether or not identical-GLSL pipelines share one linked program (#3257).
The change is test-only; the production fix and other test files are byte-identical to the prior review. Ran the three touched GL suites locally: 27 passed, 2 skipped.
deepseek-v4.1-flash (free via Pullfrog for OSS) | 𝕏

Fixes #3153.
Long-lived WebGL roots retained unused uniforms and destroyed texture wrappers until root destruction.
Discover uniform bindings from shader references instead of keeping a root-wide uniform history, and unregister textures when they are destroyed. Cleanup callbacks reference the texture registry weakly, allowing surviving texture handles to outlive the root without retaining sibling textures.
Tests:
main: unused uniforms and their buffers, uniforms no longer used by any pipeline, destroyed textures, and optimized-out uniforms are all collected while their root stays alive.mainas well: a retained texture keeps neither its root nor its siblings alive, a retained destroyed texture releases its cleanup references, uniforms from both stages bind to locations of the program being drawn and pick up later writes, and remaining textures and framebuffers are destroyed exactly once byroot.destroy().Behaviour changes:
mainit was declared but never uploaded, so it silently read as zero. This isn't covered by a dedicated test yet.root.destroy()no longer destroys uniforms. That does nothing today, but will need revisiting once explore(@typegpu/gl): Try out implementing uniforms using Uniform Buffer Objects #2510 backs uniforms with real GPU buffers.Not addressed here: samplers still accumulate in the root's sampler cache for the root's lifetime.
Validation:
@typegpu/gl0.12.7 /typegpu0.12.8) and main (7c853fa3) using Metal and SwiftShader.threadsandvmThreadspools.One existing documentation example exceeded its five-second timeout during the unrestricted parallel run. It passed unchanged in isolation, and the complete built suite passed with four workers and unchanged timeout limits.