godotengine / godot-proposals

Godot Improvement Proposals (GIPs)
MIT License
1.07k stars 69 forks source link

Unify Tangent Generation in 3D Importers #8696

Open fire opened 6 months ago

fire commented 6 months ago

Describe the project you are working on

Godot Engine is a game engine where different importers are used to load 3D models. The current system has each importer generating tangents independently, leading to inconsistencies and inefficiencies.

Describe the problem or limitation you are having in your project

The tangent generation process is not unified across all importers. This leads to inconsistencies in the way tangents are generated and can cause visual artifacts in the rendered models. Additionally, some importers rely on SurfaceTool for tangent generation which is disliked by the reviewer team.

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

The proposed feature is to unify the tangent generation process at the ImporterMesh level. This will ensure that all importers generate tangents in a consistent manner for a better design. By removing the dependency on SurfaceTool it will have a more clean design.

The time profiles says that Mikktspace tangent generation's slowness scale dwarfs everything though but I will make that a separate proposal.

### Tasks
- [ ] Add tangent generation to ImporterMesh. Do not touch CSG / SurfaceTool / MeshDataTool
### Tasks
- [ ] Remove dependency on SurfaceTool for Collada, OBJ, GLTF, UFBX
- [ ] Do not generate any normals or tangents in any of the SceneImporters
- [ ] In EditorImporterScene, do the generation there by calling the functions added to ImporterMesh, where it matters, before the conversion to ArrayMesh
- [ ] For runtime conversion of ImporterMesh to ArrayMesh in GLTFDocument (and eventually FBXDocument) use the code in ImporterMesh. Avoid SurfaceTool.
- [ ] FBX Document has 2 callsites for SurfaceTool.
- [ ] GLTF Document has 3 callsites for SurfaceTool
- [ ] FBX Document has generate tangents
- [ ] GLTF Document has generate tangents
- [ ] Collada has 1 callsite for SurfaceTool
- [ ] .Obj has 1 callsite for Generate Normals
- [ ] .DAE has 1 callsite for Generate Normals

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

Ensure that all importers don't generate tangents, and that the 3d scene pipeline only does the generation in Ref\<ImporterMesh>.

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

This enhancement will be used every time a model is imported, which is a fundamental part of using the engine. While it's possible to manually generate tangents after importing a model, doing so would be less efficient and could still lead to inconsistencies.

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

Yes, this should be a core feature because it affects the fundamental process of importing 3D models. It's not something that can be easily added on top of the existing system without modifying the importers themselves.

fire commented 6 months ago

From a test a few months ago I did a profile using the fbx importer for surface improvements. I think that surface tool code quality will improve, but not import performance. Since the mikktspace generation is the majority of the time and the surface tool usage is a small sliver.

We discussed removing the callbacks and there's a pr for those changes.

image

https://profiler.firefox.com/public/vw7s6qja0gkxqex44pqj6gkyt1zwvy1y7ahn70r/flame-graph/?globalTrackOrder=0&hiddenLocalTracksByPid=21679-0wxk&symbolServer=http%3A%2F%2F127.0.0.1%3A3000%2F00z3c60vnfnligbg3hszq21q8pqkqy2v0wfyfgr&thread=0&v=10