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

gltfpack: Replace images that can't be found or encoded with invalid URI #676

Merged
merged 2 commits into from
Apr 10, 2024

Conversation

zeux
Copy link
Owner

@zeux zeux commented Apr 8, 2024

This change consolidates error handling for images that can't be read, written, or encoded, and uniformly emits an invalid embedded Base64 URI in these cases.

This keeps the file invalid as per glTF spec, but allows various loaders to load it. The source file in cases like this was semantically invalid so maybe this is fine.

Ideally we would probably want to remove references to the image. Unfortunately, to preserve glTF file validity, this requires transitive removal of texture references from all materials as well as image/texture array compaction...

Fixes #675

zeux added 2 commits April 7, 2024 16:34
Instead of printing to stderr manually whenever we have a case when an
image can't be saved, call writeImageError; this unifies the error
reporting and allows us to produce JSON content that indicates an error.
This prevents errors around missing images in case we can't find the source image
or can't encode it. Loading the resulting glTF file will result in a PNG decode
error.

Unfortunately, files like this are still treated as invalid by glTF spec/validator.
@zeux
Copy link
Owner Author

zeux commented Apr 8, 2024

Removal of image/texture references is possible in theory. I think right now nothing prevents this from keeping the file valid. In addition to that change being much more invasive, I'm also worried about future extensions where you can't simply omit an image/texture reference, eg EXT_lights_image_based has an array of 6 images, so what do we do if one fails to encode?

@zeux
Copy link
Owner Author

zeux commented Apr 8, 2024

The effect of this change is that files that had invalid or missing images previously failed to parse when using three.js GLTFLoader, and now cleanly load with the following error in the console:

Screenshot 2024-04-08 at 5 29 17 PM

gltf.report also loads the file but has some odd error handling for cases like this:

Screenshot 2024-04-08 at 5 30 52 PM

@Cyphall
Copy link

Cyphall commented Apr 10, 2024

I asked for clarifications on the official GLTF repo about GLTF asset file validity in these cases, and the result is that an invalid URI indeed makes the GLTF asset file invalid but a valid URI pointing to a missing/corrupted file is valid usage, so the produced GLTF asset file must still be valid in this case.

@zeux
Copy link
Owner Author

zeux commented Apr 10, 2024

Sure; I think semantically really all of these are invalid in the same sense; glTF spec may consider some subset of these invalid cases "valid", and glTF validator may or may not disagree (for example, glTF validator will parse JPEG/PNG headers and as such a file that isn't a valid PNG can be treated as invalid by the validator).

I'm going to merge this for now as it's better than status quo; it might be better to remove image references completely after all in cases like this but this can be done separately.

@zeux zeux merged commit 78567c8 into master Apr 10, 2024
12 checks passed
@zeux zeux deleted the gltf-imgerr branch April 10, 2024 15:24
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

gltfpack: Output GLTF is invalid if input scene has missing image files
2 participants