-
-
Notifications
You must be signed in to change notification settings - Fork 149
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
VRM gets treated as glTF instead of GLB #1390
Comments
I'm going to retag this as a feature, the library currently knows nothing about .vrm or other third-party variants of .gltf/.glb. But I think I agree that this should be made to 'just work' for the .vrm file extension (other than not understanding the VRM extensions embedded in the file). I suspect it's mostly a change in the CLI, for script-based usage, Related: |
Also, probably the library should log a warning when it's asked to write a file extension it doesn't know the format of (json vs. binary). Perhaps with some way to register new extensions and their format. Like: import { Format } from '@gltf-transform/core';
io.registerFileSuffix('vrm', Format.GLB); |
I think another option could be if the check "is this a binary or a text file" is based on the GLB magic header instead of relying on file extensions (which could possibly not exist at all for entirely different reasons). |
True – that'd work for the CLI, though not in the script-based I/O case, which may not read from disk. Not sure if something like ...
... is worth trying to guarantee same binary vs. json output, I think I'd prefer to log a warning that the library doesn't know what you mean. |
Maybe I'm misunderstanding what you mean – the magic header for GLB ("glTF") is there no matter if it's just a blob or read from disk. |
In this scenario... const document = io.read('path/to/input.unknown');
...
await io.write('path/to/output.unknown', document); ... there is nothing in |
It's that what @hybridherbst means: take notice of the original file header in the io class and - if using |
Understood, but I don't think that I agree with the suggestion. Creates open-ended issues:
I'm OK with making changes to ensure .vrm is handled like .glb, and perhaps allowing registration of other extensions... but I do think that unknown extensions should trigger warnings rather than attempting auto-detection, which I don't think can be supported consistently. |
Do I understand right that the complexity here comes from the fact that
looks at the string path only, and it's not easy to add "look at the first 4 bytes to check what it actually is" since the actual file reads currently happen later / after that decision? |
From my perspective the complexity comes from trying to keep a persistent knowledge of what the original source-on-disk of a particular Document might have been. It's 'easy' at an application level, if your pipeline looks exactly like this: const document = await io.read('in.unknown');
// ... make some edits ...
await io.write('out.unknown', document); But at the library level, I don't want to make an assumption that the pipeline above is what's happening. Documents can be merged or cloned, applications might use different I/O classes for reading and writing. I think it's too much magic, when the alternative is much clearer and not particularly complex: import { writeFile } from 'node:fs';
await writeFile('out.unknown', await io.writeBinary(document)); |
OK, understood! I think I looked in the wrong place, what you meant with "adding custom filetypes" is probably that the ".vrm is to be treated as a glb file" check would have to happen somewhere around here (at the end of the pipeline, not at the beginning).
|
Describe the bug
Applying e.g. draco compression to a .vrm file results in all resouces being externalized. The VRM is then saved as a glTF json (with the vrm extension)
According to https://github.com/vrm-c/vrm-specification/blob/master/specification/0.0/README.md however the VRM is a GLB format
To Reproduce
Steps to reproduce the behavior:
gltf-transform draco "AvatarSample_A.vrm" "AvatarSample_A.vrm"
Expected behavior
VRM gets saved as GLB
Versions:
Additional context
AvatarSample_A.zip
Before
After
The text was updated successfully, but these errors were encountered: