Skip to content
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

Consider using non-destructive buffer IO for .gltf files #1588

Open
zeux opened this issue Dec 21, 2024 · 2 comments
Open

Consider using non-destructive buffer IO for .gltf files #1588

zeux opened this issue Dec 21, 2024 · 2 comments
Labels
feature New enhancement or request package:cli

Comments

@zeux
Copy link

zeux commented Dec 21, 2024

When glTF-Transform is used to process some input glTF files into an output file, it can overwrite the .bin file used by the sources, yielding them invalid.

I'm not sure if this is a fully general property or specific to some transforms; I think I hit this before with different transforms, but today I've hit this with merge:

gltf-transform bistro.gltf bistro-animations.gltf bistro-merged.gltf

... produces a valid .gltf file, but also keeps the file name for the buffer as bistro.bin, without renaming it to bistro-merged.bin instead, which clobbers the old data, makes bistro.gltf invalid as it refers to invalid offsets in the file, and doesn't present a way to recover unless the file is under source control (which it was, thankfully 😅)

This is not a problem when saving to GLB, but that embeds textures and makes the resulting file harder to hand-edit, so I often use .gltf as a destination and sometimes forget this will happen.

I'm not sure to what extent this is the workflow that people just expect: my expectations have been that when the output is foo.gltf, the output buffer will be foo.bin - that is probably unrealistic to expect if the file used multiple .bin buffers, but I'm not sure if this is an outlier. Especially for commands like merge that require an extra command line flag to produce more than one buffer in the output it feels wrong to reuse the original file name.

@zeux zeux added the feature New enhancement or request label Dec 21, 2024
@donmccurdy donmccurdy added this to the 🗄️ Backlog milestone Dec 22, 2024
@donmccurdy
Copy link
Owner

donmccurdy commented Dec 22, 2024

Thanks @zeux! At the level of the JS library (@gltf-transform/core) I think it would be important to keep the URLs of buffers and images as-is, as much as possible. But for the CLI (@gltf-transform/cli) maybe overwriting these files isn't the right default, good points here. Perhaps the CLI should check for conflicts before output, and display a prompt if necessary?

$ gltf-transform bistro.gltf bistro-animations.gltf bistro-merged.gltf

Output to the target directory will overwrite the following files:

(list of files)

Do you want to continue? y/n

I agree that the foo.gltf/foo.bin convention is convenient and intuitive, just hard to generalize to multiple .bin files as you mention. Maybe the reminder if files are going to be overwritten would be enough to avoid most accidental clobbering of .bin resources?

@zeux
Copy link
Author

zeux commented Dec 22, 2024

Yeah if you're looking for a general heuristic then maybe if the output file name does not match the input file name (assuming you support smth like gltf-transform dedup bistro.gltf bistro.gltf where the user's intent is clearly to clobber the data anyhow), then prompt, but maybe also offer a "rename" option? Not sure if this is too difficult.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
feature New enhancement or request package:cli
Projects
None yet
Development

No branches or pull requests

2 participants