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

GLTFLoader: Make error when loading textures with no data clear #21177

Merged
merged 1 commit into from
Feb 1, 2021
Merged

GLTFLoader: Make error when loading textures with no data clear #21177

merged 1 commit into from
Feb 1, 2021

Conversation

zeux
Copy link
Contributor

@zeux zeux commented Jan 29, 2021

If GLTFLoader is used to load a texture with no uri or bufferView, it
uses an empty uri which results in FileLoader loading the HTML document
from which the call originates. The resulting document is then fed into
an actual image loader such as KTX2Loader or BitmapLoader, which tends
to generate hard to understand errors.

This change detects the error early and throws a special message to make
debugging easier.

If GLTFLoader is used to load a texture with no uri or bufferView, it
uses an empty uri which results in FileLoader loading the HTML document
from which the call originates. The resulting document is then fed into
an actual image loader such as KTX2Loader or BitmapLoader, which tends
to generate hard to understand errors.

This change detects the error early and throws a special message to make
debugging easier.
@mrdoob mrdoob requested a review from donmccurdy January 30, 2021 00:20
@mrdoob mrdoob added this to the r126 milestone Jan 30, 2021
@donmccurdy
Copy link
Collaborator

What is a scenario where this error might occur? I don't think we'd want to go out of the way to include English error messages in the bundle to warn about malformed assets — there are an unending number of ways the asset might be invalid — but if this could indicate user error, like failing to register an extension handler, then I'm in favor of the change.

@zeux
Copy link
Contributor Author

zeux commented Feb 1, 2021

The scenario here is that either the source glTF file is invalid, or the image data comes from a non-optional image extension that three.js doesn't implement. (note that files with unknown required extensions aren't rejected)

I agree that we don't necessarily need to guard against every possible issue like that but I found the actual flow here (loading empty URI results in reinterpreting HTML data as image contents) very surprising, hence the change.

@mrdoob mrdoob merged commit 20d1f60 into mrdoob:dev Feb 1, 2021
@mrdoob
Copy link
Owner

mrdoob commented Feb 1, 2021

Thanks!

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.

4 participants