Sketchfab model validation failes

[gleblebedev]

How to reproduce:

Read the generated gltf file available from: https://sketchfab.com/3d-models/littlest-tokyo-94b24a60dc1b48248de50bf087c0f042 (License: CC Attribution)

SharpGLTF throws an exception: SharpGLTF.Validation.ModelException: 'Accessor[104] TANGENT[1364] length 0 is not unit length'

I don't think it's required to have Tangent or Normal as a unit length vector. There are cases when special effects can be achieved with custom values in the data. The library shouldn't try guess what user want.

[gleblebedev]

At least there should be mode with no validation.

[vpenades]

Hi, thanks for writing!

I've downloaded the model and checked it with GLTF Validator , which throws +3000 errors.

It seems there's an issue with sketchfab gltf exporter, someone has reported the issue with that same model at sketchfab forums.

Checking the model myself, the problem with normals is that they're zero length, probably due to degenerated triangles, which is pretty bad.

But, as stated in sketchfab's forum, having duplicated animation keys is much worse because it may break an engine animation system.

About skipping validation, I have mixed feelings about it: If I skip validation, it means I will have to load invalid data, so I would have to, either add an expensive sanitization step after loading the model, or add a lot of runtime checks to handle invalid data, which would make the code even more complicated, - just because some other exporter did not export the model correctly -

I'm not agaist adding sanitization code to fix invalid gltf files so they can be loaded, but we're at a point were all gltf exporters should do an extra effort to prevent exporting invalid gltf files, I believe it's still too soon to expect importers to fix other's exporters errors.

[gleblebedev]

It's fine to throw if, for example, a mesh is referencing a missing buffer view. I don't like an idea to have a data validation as a blocker. Zero length normal is absolutely acceptable value. It could mean that you don't want a ligh source to lit the vertex as dot product will always be zero in this case. Or you may want it to be overstretched. It is also ok to have zero-area triangles as they could be bound to a different bones and be used to fill the gap when bones are pulled apart.

I believe a developer of an app should decide what data is valid.

I understand it is useful to have this kind of validation during development to check that there are no obvious issues with accessors.

[vpenades]

The idea behind gltf is that exporters are responsible of ensuring that the exported data is 100% correct and free of errors, so importers and engines can trust the model and simply render it with minimal checks.

Obviosly we're not in that state yet, since there's a lot of exporters exporting invalid gltf data.

I am worried that if I begin being able to load invalid data, the developers responsible of fixing their exporters will be less pressured to fix their exporters: "why I should fix my exporter, if all the other importers and engines are taking measures to fix my invalid data on load?"

I agree with you that variable length normals can be interesting to control how the light affects a certain vertex, but this usage is not defined in the gltf schema, keep in mind that this is not about usage, but about standards, and standards with loose specifications usually have a short life. You might interpret non unit length normals as a lighting feature, while another engine might interpret it as an error.

Take a look at the "unlit" material; it does nothing!, it's an empty extension, but it's there to inform the importer engines that they should treat the material in a special way.

Now, it could be nice to have an extension in the normals accessor, something like "VariableLengthNormalExtension", so if that extension exists, the loading engine knows it must expect non unit length normals, and prepare the vertex shader for that feature... in case the extension is not present, it would treat non unit length normals as errors. This is how things are done in gltf.

[gleblebedev]

Hmm... It must be that I missed it from the spec. I thought the data is the same as in Collada where I can define any vertex elements with arbitrary names and format.

Could you please provide me with a link where the spec restricts element values?

[gleblebedev]

On a GitHub I found info about W component of tangent should be +1 or -1... Did not find anything about length of the xyz.

It also says that normal should be "normalised".

[vpenades]

I've just issued a question to khronos members to clarify about the normals and tangets requiring being unit lenght.... if non unit length is indeed allowed, then they should fix the gltf validator to stop throwing errors and I'll fix SharpGLTF code accordingly.

It'll take a while, I'll keep you informed.

[gleblebedev]

Thanks a million!

What you would recommend to do in the meanwhile? To fork the lib and make adjustments?

[vpenades]

I would not recomend you to do that, there's many gltf files that are 100% valid, if the library fails to load a model it's because it's an incorrect gltf file, it should not be trusted and it can only give trouble to your engine.

So if the library gives you an error when loading a gltf file, just inform the user the model is not valid.

Exporters will keep improving over time, today, both Sketchfab and Blender exporters have several issues that will be adressed in the next month, just wait for them to catch up.

if you want variable length normals, this is something that needs to be specified with a gltf extension, because you need to inform any engine loading the model that your normals have variable length as a lighting feature, and not because it's an export error, I could propose the extension to the khronos board and see if it's accepted.

[vpenades]

It seems both zero length normals and tangents are disallowed: https://github.com/KhronosGroup/glTF/issues/1461

[gleblebedev]

Ok, it looks like I have to continue this conversation there :)

Just for a reference - here is what one can achieve with control over normal vectors: https://www.youtube.com/watch?v=yhGjCzxJV3E

[vpenades]

Sure you can do a lot of tricks with normals, but the key point here is that the vertex and pixel shaders must be informed of these tricks. I already saw the guilty Gear video you posted, it's pretty famous actually... but they probably used custom vertex and pixel shaders to achieve that effect.

What I mean is that you cannot create a gltf with variable length normals, without informing the engine in some way that you want to do that normals trick.

Actually, most engines will re-normalize the normals thinking that it's a model with invalid data.

And now, thinking about how shaders work, if you want to do a zero length normal to achieve pitch black light, the normal would become unnormalizable at the pixel shader, and it would break several aspects of the shader, producing visual artifacts, so a better, safer way to achieve what you want to do is to propose a new gltf normal attribute with XYZW components, where the XYZ would be the normalized normal, and the W component is the light scale factor. Tangents already work that way.

[gleblebedev]

I see you've added "skipValidation" as an argument. Thanks :)

[vpenades]

Yes, we're having lots of issues too, trying to load Sketchfab meshes and files from other sources.

Notice that Skip Validation simply skips the validation steps, it does not fix the model if it has issues, so if the model has serious problems that prevent displaying it correctly, there's little you can do.

That's why Skip Validation should be used as a last resort, in case you absolutely need to load a model, and you can handle yourself all potential errors.

In the future, I would like to include an option that detects and fixes non critical issues instead of throwing an error, but this will take time.