godotengine / godot-proposals

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

Add additional hooks to GLTFDocument to support draco decompression via GDExtension #8576

Open stuarta0 opened 9 months ago

stuarta0 commented 9 months ago

Describe the project you are working on

I started looking into writing a GDExtension for Cesium integration in Godot. Rather than including the cesium-native glTF IO alongside Godot's built-in support, it may be better to just use Godot's implementation. However, Godot doesn't support the KHR_draco_mesh_compression extension used in 3D Tiles.

Describe the problem or limitation you are having in your project

Regardless of using Godot's built-in glTF logic for Cesium, it would be useful to support the KHR_draco_mesh_compression extension when loading real-time glTF models from external sources. However, it doesn't appear that there's enough granularity in the hooks provided in GLTFDocument/GLTFDocumentExtension to properly support this feature being provided as a GDExtension.

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

Add additional hooks during the load of a glTF file to allow a GDExtension to provide draco compression handling.

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

In GLTFDocument::_parse_gltf_state, the file is parsed in stages, but there are only a handful of places where GLTFDocument will iterate the registered GLTFDocumentExtensions to provide some additional handling.

To provide draco support in a way that the GLTFDocument can be agnostic to the extension, one approach could be to have a hook after the file is completely represented in Godot's data structures, but before data itself (images, meshes, etc), are loaded. Possibly just above this line. This could allow a GDExtension to update the GLTFState object by decompressing the draco compressed bufferViews, and update the relevant accessors for the mesh primitive.

Alternatively, changes could be made to _parse_meshes(), but I'm not sure there's a flexible way to achieve this, as the draco extension has requirements for the handling of accessors, in additional to the compressed buffer.

Keep in mind I'm new to Godot's codebase, so if this can be achieved via existing architecture, or if there are other concerns, please let me know.

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

Provide a completely separate implementation of glTF IO that supports draco.

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

The change described above is minimal and should allow the remainder of the feature to be provided via GDExtension.

fire commented 9 months ago

I prefer mesh opt’s implementation over draco. Whats your thoughts on that in core?

fire commented 9 months ago

Seems like we need to support both.

stuarta0 commented 9 months ago

I'm not particularly familiar with mesh opt either, but it's less about preference and more about support for third party authored models using the draco extension.

If this feature is useful for others, I feel like it would be a good project to cut my teeth on. If not, I can still hack on it before attempting something much larger like Cesium integration.

In regard to Cesium, the more I dig into it the more I may just need to use their implementation anyway, since they have their own custom extensions for their use case, i.e. EXT_mesh_features, EXT_mesh_gpu_instancing, EXT_instance_features, EXT_structural_metadata.

https://github.com/CesiumGS/3d-tiles/tree/main/specification/TileFormats/glTF

fire commented 9 months ago

I have some interest in a few of those but I need to define sone principles that determines what is in core. however exposing integration points is doable.

GeorgeS2019 commented 5 months ago

@stuarta0 https://community.cesium.com/t/cesium-for-godot/26763 https://github.com/godotengine/godot-proposals/issues/1749#issuecomment-2041684921

msedge_vpO5VpX6Qt

https://github.com/godotengine/godot-proposals/issues/6109#issuecomment-1386102543