Move glTF scene loading to core to allow for run-time loading

[fire]

Describe the project you are working on

Godot Engine and 3d multiplayer vr game

Describe the problem or limitation you are having in your project

glTF2 in core is needed for Virtual Reality and other uses in runtime asset import / export.

From Reduz

We got quite a lot of demand to make the GLTF importer part of core, given it is needed for VR and other stuff, so I have the feeling we should optimize it for performance (which we should do anyway) and have an option to load actual meshes instead of EditorImporterMesh.. so that means we would need to provide a function to parse the mesh node into a scene node and the editor supplies, otherwise it uses Mesh as a default

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

By moving gltf2 to core we can access these functions in game templates and in editor.

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

• Add Error import_scene_buffer(const PackedByteArray &p_bytes, uint32_t p_flags, int32_t p_bake_fps, Ref<GLTFState> r_state, List<String> *r_missing_deps, Error *r_err);
• Add a concept of a base path string that when empty fail early returns the code.
• Change EditorSceneImporterMeshNode3D and EditorSceneImporterMesh to use function pointers that allow switching based on function pointer validity to MeshInstance3D
• Remove PackedSceneGLTF
• Add save_buffer()
• Extensions for the GLTFDocument Add an extension system for the GLTFDocument class #3305

See also:

• Move gltf to core. godot#52501.
• Add gltf import buffer. godot#52541
• Implement a way to import a glTF scene from string/byte array #3270
• [3.x] Make GLTF module togglable when compiling release templates godot#51382
• Make GLTF module togglable when compiling release templates godot#51383
• Allow to import and load external assets at run-time (in exported projects, without extra PCK files) #1632
• GLTF for game templates. godot#52802

Future work.

• Optimize for performance
• Optimize for performance for export by not duplicating exported textures that match exactly
• Add explicit security checks and migrate any URI resolving code for textures & buffers & etc. into a common helper which has explicit "" check that returns ERR_FAIL_
• Blend shape key frames should have processing limits requested in Gltf export blend shape godot#48947

Unify GLTFDocument

	Error load_from_file(String p_path, uint32_t p_flags, int32_t p_bake_fps, Ref<GLTFState> r_state);
	Error load_from_buffer(Array p_bytes, String p_base_path, uint32_t p_flags, int32_t p_bake_fps, Ref<GLTFState> r_state);
	Error load_from_scene(Node *p_node, uint32_t p_flags, int32_t p_bake_fps, Ref<GLTFState> r_state);
	Node *write_to_scene(Ref<GLTFState> state, float p_bake_fps);
	Error write_to_filesystem(Ref<GLTFState> state, const String &p_path);
	Array write_to_buffer(Ref<GLTFState> state);

• Reading a gltf to a scene

Ref<GLTFDocument> doc;
doc.instantiate();
Ref<GLTFState> state;
state.instantiate();
Error err = doc->load_from_file(p_path, p_flags, p_bake_fps, state);
if (err != OK) {
*r_err = err;
return nullptr;
}
Node *root = doc->write_to_scene(state, p_bake_fps);

• Writing a gltf to the filesystem

Ref<GLTFDocument> doc;
doc.instantiate();
Ref<GLTFState> state;
state.instantiate();
int32_t flags = 0;
flags |= EditorSceneFormatImporter::IMPORT_USE_NAMED_SKIN_BINDS;
Error err = doc->load_from_scene(root, flags, 30.0f, state);
if (err != OK) {
ERR_PRINT(vformat("glTF2 save scene error %s.", itos(err)));
}
err = doc->write_to_filesystem(state, p_file);
if (err != OK) {
ERR_PRINT(vformat("glTF2 save scene error %s.", itos(err)));
}

• Reading a gltf buffer to a scene
• Writing a gltf to a buffer
• doc.get_state() -> GLTFState
• doc.set_state(Ref) ->state

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

Not trivial to work around in a few lines of code. This is core.

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

Many cases such as VR usage to implement controller model loading require this.

[RedSmarty]

Will this be available in 3.4?

[Calinou]

Will this be available in 3.4?

This will only be available in 4.0 as it's likely a compatibility-breaking change.

[theraot]

I tested godotengine/godot#52541 and had a look at the code.

This is what I was doing:

func _ready():
	var gltf := GLTFDocument.new()
	var gltf_state := GLTFState.new()
	gltf.load_from_scene(self, 0, 0, gltf_state)
	gltf.write_to_file("res://test.gltf", gltf_state)

I find this API more versatile. For example:

func _ready():
	var gltf := GLTFDocument.new()
	var gltf_state := GLTFState.new()
	gltf.load_from_scene(self, 0, 0, gltf_state)
	self.transform.origin.x = 50
	gltf.load_from_scene(self, 0, 0, gltf_state)
	self.transform.origin.x = 0
	gltf.write_to_file("res://test.gltf", gltf_state)

Works correctly.

---

The fact that I need GLTFState (while previously I could ignore it) highlights that the methods in GLTFDocument probably should be static, which, to be fair, File and other classes are like that, see #1101. I'd argue it is idiomatic Godot.

I don't like that it has mixed terminology. There is load and there is write (it isn't load and save nor read and write). I think read and write work better. The reason I mention this is because after load my intuition was to write save. Although, this is me being picky.

By the way, Is there a reason why flags and bake_fps are no longer optional?

[fire]

Can you write out what you think think the 6 apis should be? I like read vs write.

Can make the flag default to DEFAULT and bake_fps to 30.

[theraot]

This is what I had in mind:

Error read_from_file(String p_path, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Error read_from_buffer(Array p_bytes, String p_base_path, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Error read_from_scene(Node *p_node, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Node *write_to_scene(Ref<GLTFState> state, float p_bake_fps);
Error write_to_filesystem(Ref<GLTFState> state, const String &p_path);
Array write_to_buffer(Ref<GLTFState> state);

Just a rename of methods to have read and write in the name, and rearrange of parameters.

---

I did consider some alternatives, but, they all have drawbacks. For example if they are called file_to_state et.al. they are not neatly organized for autocomplete. If they are named state_from_ and state_to_, it makes the word state seem redundant. And if it is only from_ and to_, it reads as if they were creating or casting.

---

I think making flags and bake_fps optional in GDScript is convenient for people not using this for animation stuff. The only worry that I can think of is whether or not it makes it harder to discover them for those who need it.

[fire]

I informed that write_to_scene is different from a buffer or a file. So I was recommended using the word "generate". Maybe to make it symmetrical we can find an opposite like "convert".

[theraot]

Arguably write_to_buffer is different from write_to_filesystem in that it returns the buffer, instead of writing to an existing thing (the filesystem). In fact, given that write_to_buffer does not write to a new buffer, but returns a new one, it could use the word generate. Also, I believe it is write_to_filesystem and not write_to_file because it could be multiple files, yes? So… They are all different. But we can bring write_to_filesystem under the generate umbrella if it is generate_files. I'm not sure about an opposite for generate. However, another word that could work for adding things to the GLTFState is append.

---

In fact, submitted to your consideration, what if, GLTFDocument has a GLTFState. You could rename it to GLTFBuilder (or something like that). Then these functions are in GLTFBuilder that class, and operate implicitly with the GLTFState field.

They could be:

Error append_file(String p_path, uint32_t p_flags, int32_t p_bake_fps);
Error append_buffer(Array p_bytes, String p_base_path, uint32_t p_flags, int32_t p_bake_fps);
Error append_scene(Node *p_node, uint32_t p_flags, int32_t p_bake_fps);
Node *generate_scene(float p_bake_fps);
Error generate_files(const String &p_path);
Array generate_buffer();

[fire]

Your proposal is now unclear what is the source and sink. I don't have any suggestions that resolve this problem, but maybe some can be made.

[theraot]

Was that unclear because it lacks from, or because it lacks GLTFState as parameter? Or both?

We can address both.

How about this:

• Have GLTFState as a parameter
• Use append_from instead of append (which is to say, append_from instead of read_from)
• Use generate for those that return a result.
• And let write_to_filesystem as it was.

Like this:

Error append_from_file(String p_path, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Error append_from_buffer(Array p_bytes, String p_base_path, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Error append_from_scene(Node *p_node, Ref<GLTFState> r_state, uint32_t p_flags, int32_t p_bake_fps);
Node *generate_scene(Ref<GLTFState> state, float p_bake_fps);
Array generate_buffer(Ref<GLTFState> state);
Error write_to_filesystem(Ref<GLTFState> state, const String &p_path);

[fire]

I was distracted working on a svg importer, blender import, libcurl and other work. Sorry for the delay.

Will investigate.

[theraot]

No need to apologize, it was me who had let you without a reply.

[fire]

@theraot I updated the pull request.

[fire]

Will create new proposals for these and close this proposal as completed.

• Optimize for gltf I/O performance
• Optimize for gltf I/O performance for export by not duplicating exported textures that match exactly
• Add explicit security checks and migrate any URI resolving code for textures & buffers & etc. into a common helper which has explicit "" check that returns ERR_FAIL_
• Blend shape key frames should have processing limits requested in Gltf export blend shape godot#48947

#3757
#3756

[fire]

Closing proposal as completed. Created two extra proposals.

• Loading/Export gltf2 files causes hitches at load and is noticably slow Loading/Export gltf2 files causes hitches at load and is noticably slow #3756
• Audit and mitigate GLTF2 security issues Audit and mitigate GLTF2 security issues #3757