From bef082562087db3fba588a5cb6a9ed5dd968e529 Mon Sep 17 00:00:00 2001 From: Kevin Blackburn-Matzen Date: Sat, 18 Jul 2026 16:06:47 -0700 Subject: [PATCH] Dispose GPU resources when the SDF mesh is rebuilt 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 --- src/engine/SdfMesh.test.ts | 122 +++++++++++++++++++++++++++++++++++++ src/engine/SdfMesh.ts | 36 ++++++++--- 2 files changed, 151 insertions(+), 7 deletions(-) create mode 100644 src/engine/SdfMesh.test.ts diff --git a/src/engine/SdfMesh.test.ts b/src/engine/SdfMesh.test.ts new file mode 100644 index 0000000..21b9230 --- /dev/null +++ b/src/engine/SdfMesh.test.ts @@ -0,0 +1,122 @@ +import { describe, it, expect, beforeEach, vi } from 'vitest'; +import * as THREE from 'three'; +import { SdfMesh } from './SdfMesh'; +import { useModelerStore, type SDFDisplayData } from '../store/modelerStore'; +import type { ThreeEngine } from './ThreeEngine'; + +/** + * Three.js does not free GPU resources when an object leaves the scene graph, + * so SdfMesh must dispose the material, geometry, and textures itself on every + * rebuild. These tests pin that contract — they fail against the pre-fix code, + * which only called scene.remove(). + */ + +function display(glsl: string, textureCount = 0): SDFDisplayData { + return { + glsl, + paramCount: 1, + paramValues: [0], + textures: Array.from({ length: textureCount }, (_, i) => ({ + name: `u_tex${i}`, + width: 2, + height: 2, + data: [0, 0, 0, 0], + })), + bbMin: [-1, -1, -1], + bbMax: [1, 1, 1], + hasWarn: false, + }; +} + +/** Minimal stand-in for ThreeEngine — SdfMesh only touches `scene`. */ +function fakeEngine() { + return { scene: new THREE.Scene() } as unknown as ThreeEngine; +} + +/** The material/geometry/textures SdfMesh is currently holding. */ +function currentResources(sdfMesh: SdfMesh) { + const self = sdfMesh as unknown as { + mesh: THREE.Mesh | null; + material: THREE.ShaderMaterial | null; + }; + const textures = self.material + ? Object.values(self.material.uniforms) + .map((u) => u?.value) + .filter((v): v is THREE.Texture => v instanceof THREE.Texture) + : []; + return { mesh: self.mesh, material: self.material, textures }; +} + +beforeEach(() => { + useModelerStore.setState({ sdfDisplay: null }); +}); + +describe('SdfMesh GPU resource lifecycle', () => { + it('disposes the previous material, geometry, and textures on rebuild', () => { + useModelerStore.setState({ sdfDisplay: display('float sdf(vec3 p){return 1.0;}', 2) }); + const sdfMesh = new SdfMesh(fakeEngine()); + + const first = currentResources(sdfMesh); + expect(first.material).not.toBeNull(); + expect(first.textures).toHaveLength(2); + + const materialSpy = vi.spyOn(first.material!, 'dispose'); + const geometrySpy = vi.spyOn(first.mesh!.geometry, 'dispose'); + const textureSpies = first.textures.map((t) => vi.spyOn(t, 'dispose')); + + // A structurally different tree forces a rebuild. + useModelerStore.setState({ sdfDisplay: display('float sdf(vec3 p){return 2.0;}', 2) }); + + expect(materialSpy).toHaveBeenCalledTimes(1); + expect(geometrySpy).toHaveBeenCalledTimes(1); + for (const spy of textureSpies) expect(spy).toHaveBeenCalledTimes(1); + + // And the replacement is live, not disposed along with it. + const second = currentResources(sdfMesh); + expect(second.material).not.toBe(first.material); + expect(second.mesh).not.toBeNull(); + }); + + it('disposes when the display clears', () => { + useModelerStore.setState({ sdfDisplay: display('float sdf(vec3 p){return 1.0;}', 1) }); + const sdfMesh = new SdfMesh(fakeEngine()); + + const { material, mesh, textures } = currentResources(sdfMesh); + const materialSpy = vi.spyOn(material!, 'dispose'); + const geometrySpy = vi.spyOn(mesh!.geometry, 'dispose'); + const textureSpy = vi.spyOn(textures[0], 'dispose'); + + useModelerStore.setState({ sdfDisplay: null }); + + expect(materialSpy).toHaveBeenCalledTimes(1); + expect(geometrySpy).toHaveBeenCalledTimes(1); + expect(textureSpy).toHaveBeenCalledTimes(1); + expect(currentResources(sdfMesh).material).toBeNull(); + }); + + it('disposes on dispose()', () => { + useModelerStore.setState({ sdfDisplay: display('float sdf(vec3 p){return 1.0;}', 1) }); + const sdfMesh = new SdfMesh(fakeEngine()); + + const { material, mesh, textures } = currentResources(sdfMesh); + const materialSpy = vi.spyOn(material!, 'dispose'); + const geometrySpy = vi.spyOn(mesh!.geometry, 'dispose'); + const textureSpy = vi.spyOn(textures[0], 'dispose'); + + sdfMesh.dispose(); + + expect(materialSpy).toHaveBeenCalledTimes(1); + expect(geometrySpy).toHaveBeenCalledTimes(1); + expect(textureSpy).toHaveBeenCalledTimes(1); + }); + + it('removes the mesh from the scene when it is released', () => { + const engine = fakeEngine(); + useModelerStore.setState({ sdfDisplay: display('float sdf(vec3 p){return 1.0;}') }); + const sdfMesh = new SdfMesh(engine); + + expect(engine.scene.children).toHaveLength(1); + sdfMesh.dispose(); + expect(engine.scene.children).toHaveLength(0); + }); +}); diff --git a/src/engine/SdfMesh.ts b/src/engine/SdfMesh.ts index 05367a7..ff025ee 100644 --- a/src/engine/SdfMesh.ts +++ b/src/engine/SdfMesh.ts @@ -162,14 +162,35 @@ export class SdfMesh { this.onStoreChange(); } + /** + * Free the GPU-side resources backing the current mesh. + * + * Three.js does not release the compiled program, the geometry buffers, or + * textures when an object leaves the scene graph — `scene.remove()` only + * unlinks it. Anything that drops the current mesh must come through here, + * or every rebuild leaks a shader program, a geometry, and its textures. + */ + private releaseGpu() { + if (this.mesh) { + this.engine.scene.remove(this.mesh); + this.mesh.geometry.dispose(); + } + if (this.material) { + // Textures are reachable only via the uniforms, not through the mesh. + for (const uniform of Object.values(this.material.uniforms)) { + const value = uniform?.value; + if (value instanceof THREE.Texture) value.dispose(); + } + this.material.dispose(); + } + this.mesh = null; + this.material = null; + } + private onStoreChange() { const sdf = useModelerStore.getState().sdfDisplay; if (!sdf || !sdf.glsl) { - if (this.mesh) { - this.engine.scene.remove(this.mesh); - this.mesh = null; - this.material = null; - } + this.releaseGpu(); this.lastGlsl = ''; this.lastParamCount = 0; this.lastBBKey = ''; @@ -185,7 +206,8 @@ export class SdfMesh { } private rebuild(sdf: { glsl: string; paramCount: number; paramValues: number[]; textures: any[]; bbMin: [number,number,number]; bbMax: [number,number,number]; hasWarn: boolean }) { - if (this.mesh) this.engine.scene.remove(this.mesh); + // Dispose the previous build before allocating its replacement. + this.releaseGpu(); const initialParams = new Float32Array(sdf.paramCount); for (let i = 0; i < sdf.paramValues.length; i++) initialParams[i] = sdf.paramValues[i]; @@ -295,6 +317,6 @@ export class SdfMesh { dispose() { for (const u of this.unsubs) u(); - if (this.mesh) this.engine.scene.remove(this.mesh); + this.releaseGpu(); } }