From a2e87ec0d4d59f6b6581647e5380bfe7535c7bfc Mon Sep 17 00:00:00 2001 From: Kevin Blackburn-Matzen Date: Sat, 18 Jul 2026 16:37:50 -0700 Subject: [PATCH] Include hasWarn and textures in the shader rebuild key Fixes #55. The rebuild decision compared only glsl and paramCount, but rebuild() bakes two more inputs into the material: hasWarn selects the fragment source via buildFrag, and each texture becomes a uniform. An evaluation producing identical GLSL and param count but a different hasWarn therefore never reached the GPU, leaving the thin-wall highlight showing the previous evaluation's state. Correction to the issue as I filed it: the textures half is latent, not live. codegen declares `textures`, resets it, and returns it, but nothing ever pushes to it, and nothing else in src/worker/sdf references textures -- so sdf.textures is always [] at runtime and cannot currently go stale. Only hasWarn is a live bug. The texture key is included anyway: it costs nothing while the array is empty, and it means the key is already correct when textures get wired up rather than being a trap waiting for that change. The texture key hashes content (FNV-1a) rather than just dimensions, so a content edit at unchanged width/height still invalidates. Tests fail 2/3 against the base branch. The third asserts that an identical display does NOT rebuild, which both satisfy -- it guards against over-invalidating, since each rebuild recompiles a shader and disposes the previous one. Co-Authored-By: Claude Opus 4.8 --- src/engine/SdfMesh.test.ts | 47 +++++++++++++++++++++++++++++++++++++- src/engine/SdfMesh.ts | 43 ++++++++++++++++++++++++++++++++-- 2 files changed, 87 insertions(+), 3 deletions(-) diff --git a/src/engine/SdfMesh.test.ts b/src/engine/SdfMesh.test.ts index 21b9230..4e299eb 100644 --- a/src/engine/SdfMesh.test.ts +++ b/src/engine/SdfMesh.test.ts @@ -11,7 +11,11 @@ import type { ThreeEngine } from './ThreeEngine'; * which only called scene.remove(). */ -function display(glsl: string, textureCount = 0): SDFDisplayData { +function display( + glsl: string, + textureCount = 0, + overrides: Partial = {}, +): SDFDisplayData { return { glsl, paramCount: 1, @@ -25,6 +29,7 @@ function display(glsl: string, textureCount = 0): SDFDisplayData { bbMin: [-1, -1, -1], bbMax: [1, 1, 1], hasWarn: false, + ...overrides, }; } @@ -120,3 +125,43 @@ describe('SdfMesh GPU resource lifecycle', () => { expect(engine.scene.children).toHaveLength(0); }); }); + +describe('SdfMesh rebuild cache key', () => { + const GLSL = 'float sdf(vec3 p){return 1.0;}'; + + it('rebuilds when hasWarn changes at identical GLSL', () => { + useModelerStore.setState({ sdfDisplay: display(GLSL, 0, { hasWarn: false }) }); + const sdfMesh = new SdfMesh(fakeEngine()); + const before = currentResources(sdfMesh).material; + + // Same GLSL and param count; only the warn flag differs. It is compiled + // into the fragment source, so the material must be rebuilt. + useModelerStore.setState({ sdfDisplay: display(GLSL, 0, { hasWarn: true }) }); + + expect(currentResources(sdfMesh).material).not.toBe(before); + }); + + it('rebuilds when texture content changes at identical dimensions', () => { + const withData = (data: number[]) => + display(GLSL, 0, { textures: [{ name: 'u_tex0', width: 2, height: 2, data }] }); + + useModelerStore.setState({ sdfDisplay: withData([0, 0, 0, 0]) }); + const sdfMesh = new SdfMesh(fakeEngine()); + const before = currentResources(sdfMesh).material; + + useModelerStore.setState({ sdfDisplay: withData([1, 2, 3, 4]) }); + + expect(currentResources(sdfMesh).material).not.toBe(before); + }); + + it('does not rebuild when nothing baked into the material changed', () => { + useModelerStore.setState({ sdfDisplay: display(GLSL) }); + const sdfMesh = new SdfMesh(fakeEngine()); + const before = currentResources(sdfMesh).material; + + // A fresh object with identical contents must not force a recompile. + useModelerStore.setState({ sdfDisplay: display(GLSL) }); + + expect(currentResources(sdfMesh).material).toBe(before); + }); +}); diff --git a/src/engine/SdfMesh.ts b/src/engine/SdfMesh.ts index ff025ee..91ccaa4 100644 --- a/src/engine/SdfMesh.ts +++ b/src/engine/SdfMesh.ts @@ -145,12 +145,39 @@ ${hasWarn ? ` float warnDist = abs(sdfWarn(p)); `; } +/** FNV-1a over the texture payload, so content edits at unchanged dimensions + * still invalidate the material. */ +function hashBytes(data: number[]): number { + let h = 0x811c9dc5; + for (let i = 0; i < data.length; i++) { + h ^= data[i] & 0xff; + h = Math.imul(h, 0x01000193); + } + return h >>> 0; +} + +/** + * Cache key for the texture uniforms baked into the material. + * + * Note that codegen never populates `textures` today — the array is declared, + * reset, and returned, but nothing pushes to it — so this is always '' at + * runtime and costs nothing. It is here so the key is already correct when + * textures do get wired up, rather than being a latent stale-render bug + * waiting for that change. + */ +function textureKey(textures: { name: string; width: number; height: number; data: number[] }[] | undefined): string { + if (!textures || textures.length === 0) return ''; + return textures.map((t) => `${t.name}:${t.width}x${t.height}:${hashBytes(t.data)}`).join('|'); +} + export class SdfMesh { private engine: ThreeEngine; private mesh: THREE.Mesh | null = null; private material: THREE.ShaderMaterial | null = null; private lastGlsl = ''; private lastParamCount = 0; + private lastHasWarn = false; + private lastTextureKey = ''; private lastBBKey = ''; private unsubs: (() => void)[] = []; @@ -193,14 +220,26 @@ export class SdfMesh { this.releaseGpu(); this.lastGlsl = ''; this.lastParamCount = 0; + this.lastHasWarn = false; + this.lastTextureKey = ''; this.lastBBKey = ''; return; } - // Rebuild shader only if GLSL structure changed - if (sdf.glsl !== this.lastGlsl || sdf.paramCount !== this.lastParamCount) { + // Rebuild when anything baked into the material changes. hasWarn selects + // the fragment source via buildFrag, and the textures become uniforms, so + // neither can be left out of the key without rendering a stale material. + const texKey = textureKey(sdf.textures); + if ( + sdf.glsl !== this.lastGlsl || + sdf.paramCount !== this.lastParamCount || + sdf.hasWarn !== this.lastHasWarn || + texKey !== this.lastTextureKey + ) { this.lastGlsl = sdf.glsl; this.lastParamCount = sdf.paramCount; + this.lastHasWarn = sdf.hasWarn; + this.lastTextureKey = texKey; this.rebuild(sdf); } }