Cache glTF model scenes#909
Conversation
|
The latest updates on your projects. Learn more about Vercel for GitHub.
|
![graphite-app[bot]](https://avatars.githubusercontent.com/in/158384?s=60&v=4){kind=small}
| test("normalizes cachebust_origin without dropping meaningful query params", () => { | ||
| expect( | ||
| normalizeGltfModelCacheKey( | ||
| "https://assets.tscircuit.com/chip.glb?foo=1&cachebust_origin=abc&bar=2", | ||
| ), | ||
| ).toBe("https://assets.tscircuit.com/chip.glb?foo=1&bar=2") | ||
|
|
||
| expect( | ||
| normalizeGltfModelCacheKey("/models/chip.glb?cachebust_origin=abc"), | ||
| ).toBe("/models/chip.glb") | ||
| }) | ||
|
|
||
| test("clones cached scenes with independent material instances", () => { | ||
| const material = new THREE.MeshStandardMaterial({ color: 0x00ff00 }) | ||
| const original = new THREE.Group() | ||
| original.add(new THREE.Mesh(new THREE.BoxGeometry(1, 1, 1), material)) | ||
|
|
||
| const firstClone = cloneSceneWithIndependentMaterials(original) | ||
| const secondClone = cloneSceneWithIndependentMaterials(original) | ||
| const firstMesh = firstClone.children[0] as THREE.Mesh | ||
| const secondMesh = secondClone.children[0] as THREE.Mesh | ||
|
|
||
| expect(firstMesh.material).not.toBe(material) | ||
| expect(secondMesh.material).not.toBe(material) | ||
| expect(firstMesh.material).not.toBe(secondMesh.material) | ||
| }) |
There was a problem hiding this comment.
This file contains two test(...) calls (one at line 8 and another at line 20), which violates the rule that a *.test.ts file may have AT MOST one test(...). Please split this into two separate numbered files, e.g. gltf-model-cache1.test.ts (for the cache key normalization test) and gltf-model-cache2.test.ts (for the independent material cloning test).
Spotted by Graphite (based on custom rule: Custom rule)
Is this helpful? React 👍 or 👎 to let us know.
| const loader = new GLTFLoader() | ||
| const promise = loader.loadAsync(gltfUrl).then((gltf) => { | ||
| cache.set(cacheKey, { promise, scene: gltf.scene }) | ||
| return gltf.scene | ||
| }) | ||
|
|
||
| cache.set(cacheKey, { promise, scene: null }) | ||
|
|
||
| const scene = await promise | ||
| return cloneSceneWithIndependentMaterials(scene) |
There was a problem hiding this comment.
Failed model loads are permanently cached, preventing retries. When loader.loadAsync(gltfUrl) rejects (network error, 404, invalid file, etc.), the rejected promise is stored in the cache. All subsequent calls with the same URL will hit line 68-70 and re-throw the cached rejection forever.
Fix by removing failed entries from cache:
const loader = new GLTFLoader()
const promise = loader.loadAsync(gltfUrl)
.then((gltf) => {
cache.set(cacheKey, { promise, scene: gltf.scene })
return gltf.scene
})
.catch((error) => {
cache.delete(cacheKey)
throw error
})
cache.set(cacheKey, { promise, scene: null })
const scene = await promise
return cloneSceneWithIndependentMaterials(scene)| const loader = new GLTFLoader() | |
| const promise = loader.loadAsync(gltfUrl).then((gltf) => { | |
| cache.set(cacheKey, { promise, scene: gltf.scene }) | |
| return gltf.scene | |
| }) | |
| cache.set(cacheKey, { promise, scene: null }) | |
| const scene = await promise | |
| return cloneSceneWithIndependentMaterials(scene) | |
| const loader = new GLTFLoader() | |
| const promise = loader.loadAsync(gltfUrl).then((gltf) => { | |
| cache.set(cacheKey, { promise, scene: gltf.scene }) | |
| return gltf.scene | |
| }).catch((error) => { | |
| cache.delete(cacheKey) | |
| throw error | |
| }) | |
| cache.set(cacheKey, { promise, scene: null }) | |
| const scene = await promise | |
| return cloneSceneWithIndependentMaterials(scene) |
Spotted by Graphite
Is this helpful? React 👍 or 👎 to let us know.
Co-authored-by: Copilot <223556219+Copilot@users.noreply.github.com>
|
Addressed the Graphite review feedback in f380a8b:
Validation is green on the PR now: Format Check, Test Node Bundle Load, Type Check, and Vercel. Preview/demo links: |
1 participant
Summary
Tests
/claim #93