Rework Mesh handling on scene importing. #44319
Conversation
reduz
force-pushed
the
integrate-meshoptimizer
branch
4 times, most recently
from
aa8ae4b
to
1a09a20
Compare
| @@ -1440,7 +1444,8 @@ Error EditorSceneImporterGLTF::_parse_materials(GLTFState &state) { | |||
| if (d.has("name")) { | |||
| material->set_name(d["name"]); | |||
| } | |||
| material->set_flag(StandardMaterial3D::FLAG_ALBEDO_FROM_VERTEX_COLOR, true); | |||
| //don't do this here only if vertex color exists | |||
There was a problem hiding this comment.
since the albedo code was touched i have to reverify that the test cases don't regress #21087 (comment)
There was a problem hiding this comment.
This flag is less optimal if turned on, so I only make it on when needed
| if (surfaces[i].lods.size()) { | ||
| Dictionary lods; | ||
| for (int j = 0; j < surfaces[i].lods.size(); j++) { | ||
| lods[surfaces[i].lods[j].distance] = surfaces[i].lods[j].indices; |
There was a problem hiding this comment.
There was a problem hiding this comment.
All this will be generated internally in Godot, the extensions in GLTF will be ignored
| } else { | ||
| idx = *idxptr; | ||
| } | ||
|
|
||
| index_array.push_back(idx); | ||
| } | ||
|
|
||
| vertex_array.clear(); | ||
| vertex_array = new_vertices; | ||
|
|
||
| format |= Mesh::ARRAY_FORMAT_INDEX; |
There was a problem hiding this comment.
Will you be adding a quantized mesh format? You'd have to read up what gltf quantized requires. KhronosGroup/glTF#1670
There was a problem hiding this comment.
Here are a list of relevant specs from gltf.
- https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Vendor/EXT_mesh_gpu_instancing
- https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Vendor/MSFT_lod
- https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_draco_mesh_compression
- https://github.com/KhronosGroup/glTF/tree/master/extensions/2.0/Khronos/KHR_mesh_quantization
There was a problem hiding this comment.
Nope, we do our own optimizations on import
There was a problem hiding this comment.
EXT GPU Instancing can be supported with a multimesh, but to be honest all this stuff seem like unsupported by authoring software anyway
There was a problem hiding this comment.
To be clear, I'm asking about the benefit of smaller in memory sizes for meshes. I don't think it's possible to use a smaller type for vertices. [Since you decided to only have have one (compressed) mesh format.]
As an example, a static PBR-ready mesh typically requires POSITION (12 bytes), TEXCOORD (8 bytes), NORMAL (12 bytes) and TANGENT (16 bytes) for each vertex, for a total of 48 bytes. With this extension, it is possible to use SHORT to store position and texture coordinate data (8 and 4 bytes, respectively) and BYTE to store normal and tangent data (4 bytes each), for a total of 20 bytes per vertex with often negligible quality impact.
There was a problem hiding this comment.
Godot 4.0 uses:
- 12 bytes for position. At some point we added quantizing for this as optional, but it just generally looks bad and users had a lot of problems. Meshes had visible seams, small details get deformed easily, etc. Its just not worh it.
- 4 bytes for normal (Godot 4.0 uses ARGB2101010)
- 4 bytes for tangent (also ARGB2101010)
- 8 bytes for UV. Again, we s upported having less, but to be honest users nowadays mostly use larger textures than 1024, so using half float for UV looks pretty terrible.
This way, Godot uses 12+4+4+8=28 bytes per vertex instead of 20. Its honestly not much of a problem and things just work.
| ClassDB::bind_method(D_METHOD("optimize_indices_for_cache"), &SurfaceTool::optimize_indices_for_cache); | ||
|
|
||
| ClassDB::bind_method(D_METHOD("get_max_axis_length"), &SurfaceTool::get_max_axis_length); | ||
| ClassDB::bind_method(D_METHOD("generate_lod", "nd_threshold", "target_index_count"), &SurfaceTool::generate_lod, DEFVAL(3)); |
There was a problem hiding this comment.
nd_threshold is confusing to people who don't read documentation.
There was a problem hiding this comment.
You can't understand this without reading doc anyway
reduz
force-pushed
the
integrate-meshoptimizer
branch
2 times, most recently
from
ac84242
to
93017fe
Compare
-Reworked how meshes are treated by importer by using EditorSceneImporterMesh and EditorSceneImporterMeshNode. Instead of Mesh and MeshInstance, this allows more efficient processing of meshes before they are actually registered in the RenderingServer. -Integrated MeshOptimizer -Reworked internals of SurfaceTool to use arrays, making it more performant and easy to run optimizatons on.
reduz
force-pushed
the
integrate-meshoptimizer
branch
from
93017fe
to
77a045e
Compare
|
Thanks! |
-Reworked how meshes are treated by importer by using EditorSceneImporterMesh and EditorSceneImporterMeshNode. Instead of Mesh and MeshInstance, this allows more efficient processing of meshes before they are actually registered in the RenderingServer.
-Integrated MeshOptimizer
-Reworked internals of SurfaceTool to use arrays, making it more performant and easy to run optimizatons on.