KhronosGroup / glTF-Validator

Tool to validate glTF assets.
Apache License 2.0
366 stars 60 forks source link

Mesh data validation #19

Open lexaknyazev opened 7 years ago

lexaknyazev commented 7 years ago

Validator should be able to detect technical issues with mesh data such as degenerate or incomplete primitives. As of 55a960559a6b4cf4f9ad0d598443477943b792fc, only degenerate triangles are reported.

Next possible features (in no particular order):

Thoughts? @pjcozzi @bghgary @emackey @javagl @UX3D-nopper

pjcozzi commented 7 years ago

These all sound good - I could see others such as manifold meshes for 3D printing, consistent winding order, etc. These would be "best practices", but not enforced by the spec.

Some of these will not be super fast so it would be good to still have a fast path validation option for apps that want to run validation on import/export.

Also, it would be cool to make these stages in gltf-pipeline, and then for the validator to use gltf-pipeline as a dependency, but I don't think that is realistic in the short-term with the validator using Dart and gltf-pipeline using Node.js with an in-progress 2.0 branch.

javagl commented 7 years ago

Originally, the validator reported each degenerate triangle individually. After the linked commit, it only reports the number of degenerate triangles.

This is similar to errors regarding the min/max values for accessors: When they are wrong (e.g. all values are >0, but the maximum value is set to 0), then an error is emitted for each wrong value (which can be a lot).

Is this an issue?

lexaknyazev commented 7 years ago

@javagl

This is similar to errors regarding the min/max values for accessors: When they are wrong (e.g. all values are >0, but the maximum value is set to 0), then an error is emitted for each wrong value (which can be a lot).

There're two distinct validation issues with bounds:

javagl commented 7 years ago

I think that seeing actual numbers is useful in this case.

I basically agree with that. It's just a case of potentially many errors being generated for a "tiny" mistake (i.e. a wrong max value). A single message like "There is a value of 5.5 at index 45, which is greater than the maximum of 1.0, and there are 48343 more values exceeding the maximum" could alleviate this. But I can imagine that this may be fiddly to implement, and it's probably not urgent.

emackey commented 7 years ago

For what it's worth, I don't see the two issues mentioned above as being separate. Let's just treat it as if the min or max value is the one always assumed to be wrong, as the vertex data is more likely correct.

If half the mesh sticks out past max, the correct fix is not to flatten one side of the model, it's to adjust min/max.

lexaknyazev commented 7 years ago

If half the mesh sticks out past max, the correct fix is not to flatten one side of the model, it's to adjust min/max.

Sure. I think that some users may want to mute/demote the first case while keeping the second one as error.

emackey commented 7 years ago

I think that some users may want to mute/demote the first case while keeping the second one as error.

Ah, one is more severe than the other, good point. Still no need to repeat the error message per vertex, IMHO, since it seems wrong to blame the vertex data rather than the min/max calculation. But keeping two separate issue types with configurable severities makes sense.