Mesh Import should normalize all normal data from every importer

[RevoluPowered]

Describe the project you are working on

Godot Engine Development

Describe the problem or limitation you are having in your project

We have normal data coming to the renderer with 0,0,0 this causes octahedral compression to fail, we'd like to eliminate all sources of invalid normals.

Describe the feature / enhancement and how it helps to overcome the problem or limitation

I've checked all importers:

• obj
• fbx
• glb
• collada

They all will happily import 0,0,0 normal data.

Describe how your proposal will work, with code, pseudo-code, mock-ups, and/or diagrams

Implement a simple function to normalize the normal data in the importer interface.

Ideas of a places to put this:

• set_mesh()
• add_surface
• add_surface_to_arrays

If this enhancement will not be used often, can it be worked around with a few lines of script?

Not really, there are millions of assets out there, many thousands with bad normal data.

Is there a reason why this should be core and not an add-on in the asset library?

YES. 🍰 its core that would need a tiny edit.

[The-O-King]

I think this is a good idea, and does fix the 0 length normal issue!

Some questions-

1. if we do get a 0 length normal, what should happen? Should we return an error? Warning?
2. Are we only doing this for imported meshes or all possible mesh update paths? For example someone mentioned procedural meshes possibly having 0 length normals? If all possible paths should be normalized, perhaps having checks in VisualServer::mesh_add_surface_from_arrays to normalize/catch ALL scenarios

I would also expand this to include the tangent vector since it needs to be treated the same way as the normal vector in this case

[fire]

Yes!

[RevoluPowered]

I think this is a good idea, and does fix the 0 length normal issue!

Some questions-

1. if we do get a 0 length normal, what should happen? Should we return an error? Warning?
2. Are we only doing this for imported meshes or all possible mesh update paths? For example someone mentioned procedural meshes possibly having 0 length normals? If all possible paths should be normalized, perhaps having checks in VisualServer::mesh_add_surface_from_arrays to normalize/catch ALL scenarios

I would also expand this to include the tangent vector since it needs to be treated the same way as the normal vector in this case

1. Yep we should just warn them and import it anyway, there are many models you can download with invalid data in them because somesoftware did x and should have done y during adding a modifier.
2. We should do this for all meshes, not just imported ones if we do this for add_surfaces_to_arrays it should cover dynamic meshes we generate in the editor (like the grid) later when we add true procedural meshes we can also add the filter there.

Expanding to tangents sounds good. We can also decide on a #define for a standard normal to be used when we have a 0,0,0 normal, and also a standard to be used for invalid tangent

const Vector3 DEFAULT_NORMAL = Vector3(0,0,1);
const Vector3 DEFAULT_TANGENT = Vector3(someone tell me a good default)

[clayjohn]

I think we should do two things:

1. Have an option in import options to normalize normals and
2. Have a warning when invalid normals are used

That way if users are using bad normals the importer will warn them and point them to the setting to force normalize.

The force normalize option can detect Vector3(0,0,0) normals and replace them with an arbitrary default normal (like (0, 1, 0))

[RevoluPowered]

doing some housekeeping of my own proposals, feel free to ask if you want it re-opened.

we did some work to improve this and I believe it's not been an issue since 4.1.dev.