godot
godot copied to clipboard
Mesh importer tangents
Do not merge for 4.3.
https://github.com/godotengine/godot-proposals/issues/8696
- [x] Blend shapes have special processing
See https://github.com/godotengine/godot/pull/83648 for a test case.
Looking for review.
- Not sure if I should be forcing dummy tangents.
- The conditions for generating tangents is duplicated and different between importers previously. now it's unified.
The linked PR appears to be for removing SurfaceTool from the import process, but this seems to increase reliance on the SurfaceTool instead of removing it. Is there a reason for the change in plan?
The idea is we first consolidate all the uses of Surface Tool in one place in the scene importer from GLTF, Obj, FBX and Collada. Then, we can optimize that entire chunk of code as a whole by in-lining it etc. Like split the problem.
I read the proposal for this carefully, and although it said there is a desire to remove surface tool, it also said that duplication of code is an issue among the importers so there's two problems.
Edited:
When I posted this to @lyuma, he wasn't able to review the code because the in-lined version of the "SurfaceTool::generate_tangent" was too lengthy and there's a lack of reviewers on this.
Conversation between Bqq and Lyuma on Tangents
-
Broken Tangents and Blend Shape Normals in DCC: Lyuma expressed concern about the perception that some Digital Content Creation (DCC) tools may produce broken tangents or blend shape normals+tangents.
-
Godot's Import Settings Limitations: According to Lyuma, Godot's import settings are too limited. Users have to choose between 'Ensure Tangents' or not. However, if blend shapes exist, this can completely break.
-
bqq's Approach for Tangents: bqq shared his personal approach for handling tangents, which is to discard the DCC tangents unconditionally and recompute new ones with mikktspace. He believes this should be an option, not the default.
-
Godot's Lack of Option for Recomputing Tangents: Lyuma pointed out that Godot doesn't provide the option to recompute tangents. If DCC tangents exist, Godot will always use them, leaving no way to fix these.
Suggested Improvements
- A dropdown option for handling tangents like "always generate", "generate if missing", "ignore if missing", "remove always(?)".
- Improvement in Godot's import settings to handle tangents better.
A dropdown option for handling tangents like "always generate", "generate if missing", "ignore if missing", "remove always(?)".
A Remove Always option makes sense to minimize mesh file size/memory usage if you don't intend on using normal or height maps on the mesh in question.
Remove Always option makes sense to minimize mesh file size/memory usage
The problem is that godot did something in 4.2 that made mesh format more rigid and broke many models unless they have normals and tangents. I don't want to add this until we fix the bugs in the rendering server
I understand the rendering team has a lot bigger issues to contend with right now but it would be nice to have a discussion with them on this issue at some point and figure out if there is a way to make mesh formats less rigid so we can optimize models that don't need tangents or normals.
Remove Always option makes sense to minimize mesh file size/memory usage
The problem is that godot did something in 4.2 that made mesh format more rigid and broke many models unless they have normals and tangents. I don't want to add this until we fix the bugs in the rendering server
I understand the rendering team has a lot bigger issues to contend with right now but it would be nice to have a discussion with them on this issue at some point and figure out if there is a way to make mesh formats less rigid so we can optimize models that don't need tangents or normals.
Can you be more specific what issues you are seeing? AFAIK we have fixed all the issues with tangents in the RenderingServer already. There aren't any more bug reports. If you are seeing issues arising from the new mesh compression stuff, I'd welcome a bug report. Fixing these tangent issues is a very high priority for me.
Can you be more specific what issues you are seeing? AFAIK we have fixed all the issues with tangents in the RenderingServer already. There aren't any more bug reports. If you are seeing issues arising from the new mesh compression stuff, I'd welcome a bug report. Fixing these tangent issues is a very high priority for me.
@clayjohn I am talking about the regression in 4.2 on meshes with blend shapes without tangents. That regression is still present in my 1-week-old build.
Ensure Tangents on: works
Ensure Tangents off: mesh fails to render
I'm reluctant to offer an option in the import settings which will break meshes (I know we have Ensure Tangents, but we need to keep it for compatibility and it defaults to on)
Can you be more specific what issues you are seeing? AFAIK we have fixed all the issues with tangents in the RenderingServer already. There aren't any more bug reports. If you are seeing issues arising from the new mesh compression stuff, I'd welcome a bug report. Fixing these tangent issues is a very high priority for me.
@clayjohn I am talking about the regression in 4.2 on meshes with blend shapes without tangents. That regression is still present in my 1-week-old build. Ensure Tangents on: works
Ensure Tangents off: mesh fails to render
I'm reluctant to offer an option in the import settings which will break meshes (I know we have Ensure Tangents, but we need to keep it for compatibility and it defaults to on)
Very interesting. Is there a bug report I can take a look at? I can fix this if I have access to a bug report or MRP. I haven't seen this issue reported before though
@clayjohn documented in #86384 mrp: https://github.com/godotengine/godot/files/13738417/MeshTangents.zip
and mrp from 85960: https://github.com/godotengine/godot/files/13623984/resource_loader_test.zip (run and press end key)
@clayjohn documented in #86384 mrp: https://github.com/godotengine/godot/files/13738417/MeshTangents.zip
and mrp from 85960: https://github.com/godotengine/godot/files/13623984/resource_loader_test.zip (run and press end key)
Thanks! I added it to the list of release blockers. I'll take a closer look during the week