vpenades / SharpGLTF

glTF reader and writer for .NET Standard
MIT License
470 stars 75 forks source link

System.NotFiniteNumberException: Normal is invalid #11

Closed Imaver closed 5 years ago

Imaver commented 5 years ago

Hi vpenades,

Could you please have a look at the issue that I currently struggling with?

I found one object that produces SYstem.NotFiniteNumberException

It occurs on model.CreateMeshes(meshBuilder) method. " at SharpGLTF._Extensions.ValidateNormal(Vector3 normal, String msg)\r\n at SharpGLTF.Geometry.VertexTypes.VertexPositionNormal.Validate()\r\n at SharpGLTF.Geometry.Vertex3.Validate()\r\n at SharpGLTF.Geometry.PrimitiveBuilder4.Validate()\r\n at SharpGLTF.Geometry.MeshBuilder4.Validate()\r\n at SharpGLTF.Schema2.Schema2Toolkit.CreateMeshes[TMaterial,TvP,TvM,TvS](ModelRoot root, Func2 materialEvaluator, MeshBuilder`4[] meshBuilders)\r\n at GltfExporter.GLTFExporter.<>c__DisplayClass8_0.b__0(MaterialObejct3D materialObject3D, Object context) in C:\Work\Dev\Source\GltfExporter\GLTFExporter.cs:line 662"

I debugged the normals that are going in while method primitive.AddTriangle(vertexPositionNormal1, vertexPositionNormal2, vertexPositionNormal3); is executing, and found no infinite float values or Nan value. All normals are normalized to 1 before passed to this method.

Attaching the file with normal output. NormalAddLog.txt

Could you please add validate to primitive.AddTriangle method? So that it will be easier to debug. With current workflow, I am adding all triangles first and then validate method throws an exception on model.CreateMeshes(meshBuilder)

If validation was checked on AddTriangle method, it would be easier to see the exact incorrect normal

Also, it is quite strange that this exception is happening because Normals look good

vpenades commented 5 years ago

MeshBuilderhas a property called MeshBuilder.StrictMode that can be set to true just after creating a new MeshBuilder.

When set to true, it will throw exceptions immediately.

Alternatively, VertexPositionNormalalso has a .Validate() method that will throw exceptions if the normal is not finite, or not unit lenght.

@Imaver Give it a try and let me know what you find.

Imaver commented 5 years ago

Thank you very much! Will try it tomorrow and let you know

Imaver commented 5 years ago

Hi, vpenades, The normal that produces exception has zero values (0,0,0)

My suggestion would be to add more descriptive exception message in this case so that it is easier to debug when mesh builder mode is set to not strict. Maybe: "Normal is invalid, zero normals are not supported"?

This is a minor improvement suggestion, kindly let me know if this makes sense

vpenades commented 5 years ago

Maybe the ScrictMode should be renamed to DebugMode or something like that.

It's still in development, and certainly, when normal (or tangent) is zero, it should throw an exception in script mode, or do nothing in non scrict mode.

The idea behihd non strict mode is to prevent crashes in environments where the library is being used unatended, or in production, where you want a user to import a model that has invalid data, but you don't want to crash the application.

something I'm considering is to add a warnings section in the meshbuilder object... so if strict mode is NOT set, errors will be logged as warnings.

Imaver commented 5 years ago

Thank you for the extensive explanation