search

donmccurdy / glTF-Transform

glTF 2.0 SDK for JavaScript and TypeScript, on Web and Node.js.
https://gltf-transform.dev
MIT License
1.3k stars 145 forks source link

Prune fails to clean up accessors referenced by non-root properties #1389

Open donmccurdy opened 1 month ago

donmccurdy commented 1 month ago

Resources like Accessors may be considered 'in use' by prune() if they're referenced by unused primitives or primitive targets. Because no references exist to the primitives or primitive targets, we won't find those and dispose them when iterating over meshes, and the empty accessors are written to disk despite no real use.

The other non-root entity types in the core spec are TextureInfo, AnimationSampler, and AnimationChannel. TextureInfo won't have this issue, but the other two might. Conceivably this could also happen with extensions, but prune() doesn't currently attempt to identify unused extension resources, so that's out of scope for now.

Perhaps it would be better to do a formal tree-shake, iterating all nodes in the graph, and pruning those we can't trace to the Root? This might simplify the current implementation of prune() as well.

hybridherbst commented 1 month ago

I think doing a full prune of everything could potentially be problematic if there are other unused resources purposefully in the file (e.g. some extra materials).

donmccurdy commented 1 month ago

Agreed, it should remain at least as granular as it is now:

https://github.com/donmccurdy/glTF-Transform/blob/01a0f1fd5a241067246aec09090b7923912d5736/packages/functions/src/simplify.ts#L128-L139