Dispose GPU resources when the SDF mesh is rebuilt#57
Open
kmatzen wants to merge 1 commit into
Open
Conversation
Fixes #53. SdfMesh dropped its mesh in three places -- rebuild(), the cleared-display branch of onStoreChange(), and dispose() -- and each only called scene.remove(). Three.js does not free the compiled program, geometry buffers, or textures when an object leaves the scene graph, so every structural tree edit leaked a shader program, a geometry, and all of that build's textures. Viewport.tsx calls engine.dispose() on unmount, so a mount/unmount cycle compounded it, including React StrictMode's double invoke in development. All three sites now route through a releaseGpu() helper. Textures are reachable only through material.uniforms rather than from the mesh, so it walks the uniforms and disposes any THREE.Texture it finds. The tests fail 3/4 against the pre-fix code; the one that passes checks scene removal, which the old code already did. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
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
1 participant
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.
Fixes #53.
SdfMeshdropped its mesh in three places and each only calledscene.remove():rebuild()(SdfMesh.ts:188)onStoreChange()(:168)dispose()(:296)Three.js does not free the compiled program, geometry buffers, or textures when an object leaves the scene graph —
scene.remove()only unlinks it. So every structural tree edit leaked a shader program, a geometry, and all of that build's textures.Viewport.tsx:20callsengine.dispose()on unmount, which compounded it across mount/unmount cycles, including React StrictMode's double-invoke in development.The codebase already had the right pattern next door:
OutlinePass.dispose()disposes correctly, andOutlinePass.update()frees the old geometry before swapping. This is the same rule, missed in the hottest path.Change
All three sites route through one
releaseGpu()helper. Textures need explicit handling because they are reachable only throughmaterial.uniforms, not from the mesh — the helper walks the uniforms and disposes anyTHREE.Textureit finds.Verification
tsc --noEmitThe test that passes pre-fix asserts the mesh leaves the scene, which the old code already did correctly — it just never freed the GPU side. The three that fail are the disposal assertions for material, geometry, and textures across all three drop sites.
Tests construct real
threeobjects against a stub engine ({ scene });dispose()on these is CPU-side bookkeeping and needs no WebGL context, so they run headless.Related
#55 (rebuild key ignores
hasWarn/textures) should land after this — fixing that key makes rebuilds strictly more frequent, which would have amplified this leak. #32 (cache compiled shaders by tree hash) touches the same function and changes disposal from "always free the old build" to "free on eviction", so it should build on this rather than land alongside it.🤖 Generated with Claude Code