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

Assets with required extensions should log a warning #151

Open
bghgary opened this issue Nov 20, 2020 · 7 comments
Open

Assets with required extensions should log a warning #151

bghgary opened this issue Nov 20, 2020 · 7 comments

Comments

@bghgary
Copy link

bghgary commented Nov 20, 2020

I recently encountered some files that don't work in certain viewers due to required extensions in the asset. I wonder if it makes sense to put a warning in the report when this happens? We show that there is a required extension, but everything appears green and so people expect them to work everywhere.

@emackey
Copy link
Member

emackey commented Nov 23, 2020

For extensions that don't need to be required, sure. For example, Draco without a fallback always needs requiredExtensions, but things like material properties really don't. For example, failing to parse a clearcoat extension won't damage a viewer's ability to show the model with no clearcoat on it, so clearcoat (and most of PBR Next) should never be marked as required.

I think "required" should be reserved for cases where the viewer might potentially crash or choke if it doesn't understand the contents of the extension, such as Draco, WebP, BasisU etc. (when fallbacks are not present of course). It doesn't help anyone to place "required" on for example punctual lights or material variants, where a viewer could safely ignore the extension and just be down a couple features.

@bghgary
Copy link
Author

bghgary commented Nov 23, 2020

I'm not sure I've explained my thoughts properly. I'm suggesting that the validator issue a warning for assets with required extensions (such as Draco, WebP, BasisU) indicating that these assets may not be portable since not all clients will have these extensions implemented.

@emackey
Copy link
Member

emackey commented Nov 23, 2020

Hmm. I don't think it would be a good idea to issue any warnings on a completely valid Draco or BasisU asset.

Even though compatibility is possibly reduced, I think it's too much "crying wolf" if the validator were to put yellow flags on every Draco and BasisU asset. I'd like warnings to stand out as indicative of defects.

@bghgary
Copy link
Author

bghgary commented Nov 23, 2020

Maybe a warning is too strong. I think the validator should say something though. Would an info be okay?

@lexaknyazev
Copy link
Member

The validation report already contains that information, so adding an extra message there would be redundant. Imo, this issue should be handled on UI/UX side.

@bghgary
Copy link
Author

bghgary commented Nov 23, 2020

Ok, how about updating https://github.khronos.org/glTF-Validator/?

image

Add something here? The main issue is since everything is green, people are expecting this asset to work everywhere, but that is not true.

@lexaknyazev
Copy link
Member

I agree that non-portable assets shouldn't show as green with no user-facing messages.

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

No branches or pull requests

3 participants