Import/export GLTF extras to node->meta and back

[demolke]

This is useful for custom tagging of objects with properties (for example in Blender) and having this available in the editor for scripting.

• Adds import logic to propagate the parsed GLTF extras all the way to the resulting Node->meta where it creates extras dictionary
• Adds export logic to save Godot Object meta/extras dictionary into GLTF extras
• Support nodes, meshes and materials (in GLTF sense of the words)

Closes: godotengine/godot-proposals#8271

[AThousandShips]

Please open a proposal to discuss the need and use cases for this feature

[demolke]

Thanks, looks like there is one in godotengine/godot-proposals#8271, so I'll try to reuse that one

[noidexe]

I tested it with a sample scene and overall it works well but I noticed a few things:

• materials don't get metadata
• blender mesh objects with -colonly and -convcolonly lose the metadata (in my workaround I assign it to the resulting StaticBody3D)
• metadata in mesh datablocks doesn't end in the corresponding ArrayMesh resource. It seems to be added to every MeshInstance3D using the mesh. This could create collisions if a meshinstance and its assigned mesh happen to have a custom property with the same name

[fire]

We also worked on a similar pull request with meta here #39024 #40474

[demolke]

@noidexe I've uploaded new version

• GLTF material extras gets set in Godot Material
• collision node extras get set on StaticBody3D (does not support collider mesh extra CollisionShape3D)
• MeshInstance3D gets the node extras and Mesh gets the mesh extra properties

[fire]

Part of my goal is that meta import and export is symmetrical for gltf. Let me know if thats too much or you agree.

[demolke]

I was thinking the same thing. I wanted to add a test that creates a scene, serializes it to gltf, deserializes it and checks it has the right meta, but the fact the mesh re-generation is happening in ResourceImporterScene makes testing it hairy.

I also don't understand github too much, so thought I was not including it in this PR, but somehow my in-progress stuff got attached - ignore it for now, I'll try to figure out.

[demolke]

I feel bad for having you review this, apologies. I meant to say in my last comment that it needs more work - especially the test change, which I did not want to attach at all.

I guess I should mark it as draft?

[demolke]

I've added both import and export as discussed in #86183 (comment) and added a test it works as expected.

I haven't found any import/export testing to take inspiration from - it might be my unfamiliarity with Godot, but the test has to instantiate way more of Godot that I would like - the whole import process is extremely intertwined with Editor/Engine (and their subsequent dependencies).

It's also not possible to do this in-memory, so a number of files (.gltf, .bin, .scn) get written and subsequently read from disk during the test run, which I'm not happy about.

Please take a look, cheers

[aaronfranke]

@demolke Please review the CI failure and resolve all issues until the CI turns green. Only then is it ready for human review.

[warcaninos]

Sorry i'm a beginner in all of this (github/source code) i want to try out this feature so i should probably compile godot from source but i still have to wait until it's merged right ?

[AThousandShips]

See here for instructions

[lyuma]

I already reviewed this, but Aaron asked me to take another look. I think it looks good. I'd be okay merging this in its current form and iterating on it if needed.

Some feedback on @lgxm3z 's comment.

One for enabling and disabling the copying of metadata from glTF files to nodes, materials

It's not that important: I think if the data is in the file it's okay to import it; and if it's in extras it's okay to export it.

Another for automatically identifying properties recognized by Godot and applying them

I would rather work on this as extensions: I see some value in supporting additional engine functionality, but it's also non-obvious behavior and given that most properties are supported as part of the gltf spec, it will be confusing to users which properties need to be specified using extras.

nodes, materials, etc

Finally, I'm confused which objects support extras. In glTF, just about anything can have extras. In this PR, it's implemented for all types of nodes, but only meshes and material resources? Should we also add it to skins, animations and textures? Or should we add it only to nodes, not resources?

[aaronfranke]

Should we also add it to skins, animations and textures?

In general I think it makes sense to import/export extras everywhere, but it doesn't need to happen all at once. If we miss anything, we can add it in later in another PR. So we could add it in this PR, or not, I'm good either way.

[akien-mga]

Needs rebase, I tried building it locally and it failed compiling the tests with:

In file included from ./core/templates/local_vector.h:35,
                 from ./core/object/message_queue.h:36,
                 from ./core/object/object.h:35,
                 from ./core/variant/binder_common.h:35,
                 from ./core/object/method_bind.h:34,
                 from ./core/object/class_db.h:34,
                 from ./editor/editor_paths.h:34,
                 from tests/test_main.cpp:36:
./modules/gltf/tests/test_gltf_extras.h: In function 'Node* TestGltfExtras::_gltf_export_then_import(Node*, String&)':
./modules/gltf/tests/test_gltf_extras.h:62: error: no matching function for call to 'ResourceImporterScene::ResourceImporterScene(bool, bool)'
   62 |         Ref<ResourceImporterScene> import_scene = memnew(ResourceImporterScene(false, true));
./core/os/memory.h:125:51: note: in definition of macro 'memnew'
  125 | #define memnew(m_class) _post_initialize(new ("") m_class)
      |                                                   ^~~~~~~
In file included from ./modules/gltf/tests/test_gltf_extras.h:39,
                 from ./modules/modules_tests.gen.h:42,
                 from tests/test_main.cpp:162:
./editor/import/3d/resource_importer_scene.h:309: note: candidate: 'ResourceImporterScene::ResourceImporterScene(const String&, bool)'
  309 |         ResourceImporterScene(const String &p_scene_import_type = "PackedScene", bool p_singleton = false);
./editor/import/3d/resource_importer_scene.h:309: note:   no known conversion for argument 1 from 'bool' to 'const String&'

(implies building with scons tests=yes)

[demolke]

Needs rebase, I tried building it locally and it failed compiling the tests with:

Done, thanks for taking a look.

Finally, I'm confused which objects support extras. In glTF, just about anything can have extras. In this PR, it's implemented for all types of nodes, but only meshes and material resources? Should we also add it to skins, animations and textures? Or should we add it only to nodes, not resources?

I think eventually yes, all the resources should have extras supported - adding it to node vs adding it to mesh (or other resource) are distinctly different use cases. You could put generic information on the "enemy" mesh re-used everywhere and have specific extras on the node.

Once this get submitted I would like to get consensus on the bones PR #87150 and then I can look into the other types.

[akien-mga]

Thanks!

[warcaninos]

Great work i've tested it and it just works™ i just have one question, when i import a .blend there is a root node that is made with the name of the .blend, what is this root node in blender ?
I've tried adding custom properties to the : workspace, collection, scene and none of those correspond to the root node.

[fire]

we force the name of the blend file into the root node name

[warcaninos]

So it does not correspond to anything on the blender side ? Then i should open a proposal to at least have the root node inherit the custom properties of the scene, because in an exported gltf you already have this data available.

[aaronfranke]

@warcaninos That root node corresponds to the Blender scene itself. Blender scenes allow multiple root nodes, which is incompatible with Godot scenes that have exactly one root node.

The "proper" fix for this is to add a feature to Blender to allow it to optionally flag scenes as having a single root node. This is implemented in Godot here, it just needs Blender to add support for it. Meaning, you should open a proposal to Blender, not a proposal to Godot.

[warcaninos]

Unless there is something i missunderstood, when you export a gltf scene from godot it doesn't get the metadata as extra.

[aaronfranke]

@warcaninos It only gets metadata named extras, not all metadata.

[warcaninos]

I've tried metadata with the name "extras" wich automatically get called "Extras" with values of type
string: test - Does not show up in the gltf
dictionary: string: Node, string: MeshInstance3D; Does not show up in the gltf
Array of dictionary with the same dictionary up here and it still does not show up in the gltf.
I've searched in the documentation but since this is a feature that is still in a dev branch there is nothing.

[aaronfranke]

@warcaninos Are you using the master branch of Godot (what will become Godot 4.4)? It's not in Godot 4.3.

[warcaninos]

@aaronfranke I'm using 4.4 dev2 import extras does work so i assume exporting should work too.

[warcaninos]

Should i open an issue about this ?

[aaronfranke]

@warcaninos I can't reproduce the problem. The scene in Godot:

The exported file (formatted with Prettier):

[warcaninos]

I switched to 4.4 dev 3 and i do not have this problem anymore. Thanks for your time.

[bryab]

Hello, this works great! But it doesn't work for lights, as these are not 'nodes' but are 'lights' in the 'KHR_lights_punctual' extension. Otherwise the, the 'extras' dictionary is stored in exactly the same manner as with nodes in the GLTF.

[Zireael07]

@bryab Please open an issue. A comment on a long closed PR is unlikely to be seen

[aaronfranke]

@bryab On import, it would make sense to merge those into the node metadata. However, on export, it isn't clear whether to give metadata to the node or the light, so this would be an import-only feature.

[demolke]

I'll respond here to make it clear what works, but let's have further discussion on an issue as mentioned.

Custom properties on the node (in Blender naming on the Object) are supported.

	"nodes":[
		{
			"extensions":{
				"KHR_lights_punctual":{
					"light":0
				}
			},
			"extras":{
				"prop_on_object":1.0  // this is supported
			},
			"name":"Light",
                        ....
		},
	]
	"extensions":{
		"KHR_lights_punctual":{
			"lights":[
				{
					"color":[1,1,1],
					"intensity":54351.41306588226,
					"type":"point",
					"name":"Light",
					"extras":{
						"prop_on_light":1.0 // this isn't
					}
				}
			]
		}
	},

So if you're looking for a way to send this information back and forth, you can achieve that on the object/node.