GLTF: Implement `KHR_animation_pointer` for animating custom properties #94165

aaronfranke · 2024-07-10T10:33:10Z

EDIT: I suggest merging #96748 first so that we can make this work correctly for models with different UV maps.

This PR implements the glTF Object Model and the KHR_animation_pointer extension. The object model allows accessing properties of the glTF JSON using JSON pointers, mapped to Godot properties. This can be used for things like engine-agnostic scripting systems, where simple behavior graphs can be defined inside of glTF files to manipulate properties.

KHR_animation_pointer is the first extension prominently making use of the object model, to allow for animations to control properties. With vanilla glTF animations, you can animate position, rotation, scale, and weights. However, animation pointer allows animating much more. For example, you could animate the color of a light, the FOV of a camera, the albedo color of a material, the UV offset of a material, and more.

khr_anim_ptr.mp4

Test project: gltf_khr_animation_pointer.zip

These test files were created by the Khronos group, not me. The source is here: KhronosGroup/glTF-Sample-Assets#106

Both the object model and the KHR_animation_pointer extension are already final and ratified by the Khronos group. This must be core to Godot Engine because it requires a lot of low-level hooks to do it right, and other extensions need to be able to build on these features. See also godotengine/godot-proposals#10164

For ease of reviewability, I have split this PR into 8 commits:

GLTF: Move the component type enum into GLTFAccessor
- Also includes exposing and documenting the component types.
GLTF: Add more accessor component types
- Not actually used in any code path when exporting, but allows importing more data types. This will become useful in the future for extensions like KHR_interactivity that can utilize data encoded with these types, or if new extensions use these data types for their buffers.
GLTF: Clean up animation code to make way for KHR_animation_pointer
- This commit will be the most annoying to review. It overhauls the GLTF animation import and export code, so it's hard to get a detailed look at the changes from the diff.
- Some specifics: Use double for animation times consistently (in the old code it was converting between double and float back and forth several times), rename Track to NodeTrack (to make way for pointer tracks), add static methods to GLTFAnimation to convert between Godot and GLTF interpolation types, move bone and Skeleton3D bone handling code to its own function _node_and_or_bone_to_gltf_node_index, overhaul and simplify _convert_animation.
GLTF: Prerequisite cleanups before KHR_animation_pointer
- Add get_scene_node_path and has_additional_data to GLTFNode, fix code order in GLTFDocumentExtension, remove center of mass ignore warning in physics (it's supported now), rename d to mesh_dict in mesh import code.
GLTF: Implement the glTF Object Model to support JSON pointer properties
- This is the exciting/interesting one, and is the one that exposes APIs for using the object model.
- Includes support for handling object model pointers present in the core glTF spec and in a few extensions such as lights, texture transform, and emissive strength.
GLTF: Implement the glTF Object Model in physics extension
- Allows for animating things like the mass of a physics body or the radius of a sphere collider.
- I had to rework a bit of the export code to allow for shape indices to be known earlier in the process.
GLTF: Add functions to encode and decode Variants to/from accessors
- Required to import pointer tracks as Variant arrays for use with Godot's animation system.
GLTF: Implement KHR_animation_pointer for animating custom properties
- Bringing all of it together!

If desired, I can move some of these into a separate PR.

JekSun97 · 2024-07-10T14:16:48Z

Wow, I didn’t even know that GLTF could store such data, I was surprised by the waterfall, how does it work? UVs are recorded and animated?

adamscott · 2024-07-10T14:39:02Z

Could you add this link when talking about JSON pointers in your intro? I had to search about the concept.
https://datatracker.ietf.org/doc/html/rfc6901

aaronfranke · 2024-07-10T16:42:25Z

@JekSun97 The UV offset values can be stored in glTF in a KHR_texture_transform. This is then mapped to Godot's BaseMaterial3D uv1_offset. This PR adds support for importing and exporting animations that animate this property, but it's already possible to store a frozen non-animated version of this value before this PR.

fire · 2024-09-17T18:11:41Z

https://github.com/KhronosGroup/glTF/tree/main/extensions/2.0/Khronos/KHR_animation_pointer is a ratified official extension so as soon as the Godot Engine portion is done, we can merge. Some of the other extensions are still in draft stage.

lyuma

There are currently several cases involving TYPE_VALUE tracks for :scale, :rotation, :quaternion, :position and :transform that I think do not work for cubic interpolation.

I think these cases were also broken before, so I do not want fixing this to block this PR. Just making a note of where the issues are.

modules/gltf/gltf_document.cpp

lyuma · 2024-10-04T04:49:00Z

modules/gltf/gltf_document.cpp

+					bool last = false;
+					while (true) {
+						Quaternion rotation;
+						Error err = p_godot_animation->try_rotation_track_interpolate(p_godot_anim_track_index, time, &rotation);


Both the old and the new code have broken cubic interpolation TYPE_VALUE trs tracks.

This is incorrect if track is TRACK_VALUE:
See scene/resources/animation.cpp:

ERR_FAIL_COND_V(t->type != TYPE_ROTATION_3D, ERR_INVALID_PARAMETER);

lyuma · 2024-10-04T04:50:45Z

modules/gltf/gltf_document.cpp

+					bool last = false;
+					while (true) {
+						Vector3 position;
+						Error err = p_godot_animation->try_position_track_interpolate(p_godot_anim_track_index, time, &position);


Both the old and the new code have broken cubic interpolation TYPE_VALUE trs tracks.

ERR_FAIL_COND_V(t->type != TYPE_POSITION_3D, ERR_INVALID_PARAMETER);

lyuma · 2024-10-04T04:51:04Z

modules/gltf/gltf_document.cpp

+					bool last = false;
+					while (true) {
+						Vector3 scale;
+						Error err = p_godot_animation->try_scale_track_interpolate(p_godot_anim_track_index, time, &scale);


Both the old and the new code have broken cubic interpolation TYPE_VALUE trs tracks.

ERR_FAIL_COND_V(t->type != TYPE_SCALE_3D, ERR_INVALID_PARAMETER);

lyuma · 2024-10-04T04:59:11Z

modules/gltf/gltf_document.cpp

+						err = p_godot_animation->try_rotation_track_interpolate(p_godot_anim_track_index, time, &rotation);
+						ERR_CONTINUE(err != OK);
+						err = p_godot_animation->try_scale_track_interpolate(p_godot_anim_track_index, time, &scale);
+						ERR_CONTINUE(err != OK);


Both the old and the new code have broken cubic interpolation TYPE_VALUE trs tracks.

This is incorrect if track is TRACK_VALUE:
See scene/resources/animation.cpp:

ERR_FAIL_COND_V(t->type != TYPE_ROTATION_3D, ERR_INVALID_PARAMETER);

modules/gltf/gltf_document.cpp

lyuma · 2024-10-04T05:28:47Z

modules/gltf/structures/gltf_node.cpp

+	if (p_handle_skeletons && skeleton != -1) {
+		// Special case for skeleton nodes, skip all bones so that the path is to the Skeleton3D node.
+		// A path that would otherwise be `A/B/C/Bone1/Bone2/Bone3` becomes `A/B/C/Skeleton3D:Bone3`.
+		subpath.append(get_name());


Test animating blend shapes or materials on a MeshInstance3D that is a child of a BoneAttachment3D (bone) which does not have skin

modules/gltf/structures/gltf_node.cpp

modules/gltf/structures/gltf_object_model_property.h

lyuma · 2024-10-04T06:20:20Z

modules/gltf/extensions/physics/gltf_document_extension_physics.cpp

+		// At this point, we have found a node with the shape index we were looking for.
+		if (_will_gltf_shape_become_subnode(p_state, gltf_node, node_index)) {
+			Vector<StringName> sname_path = node_path.get_names();
+			sname_path.append(gltf_node->get_name() + "Shape");


If this causes a name collision, the physics animations will not work as expected, since "Shape" will already be in use.

We should consider reserving node name + "Shape" when importing nodes.

lyuma

Reviewed together in a call with @fire and @aaronfranke (took 3 hours!)

This seems great! No major issues. It would be good to get it merged so we can start testing it for imports and exports

lyuma · 2024-10-04T06:57:21Z

modules/gltf/gltf_document.cpp

+			const int track_index = animation->get_track_count();
+			animation->add_track(Animation::TYPE_VALUE);


this will cause individually animated .../weights/N tracks to be TYPE_VALUE, but we should convert them to TYPE_BLEND_SHAPE with the name of the blend shape (will have to find the mesh and get the shape name)

modules/gltf/structures/gltf_object_model_property.h

modules/gltf/structures/gltf_animation.cpp

modules/gltf/structures/gltf_node.cpp

modules/gltf/gltf_document.cpp

modules/gltf/structures/gltf_animation.cpp

modules/gltf/doc_classes/GLTFObjectModelProperty.xml

modules/gltf/gltf_document.cpp

Repiteo · 2024-10-30T00:08:42Z

It says in the queue that this is awaiting final touches, but that might be outdated. Is this ready for merge in your eyes @aaronfranke?

aaronfranke · 2024-10-30T06:07:32Z

@Repiteo There are some minor things, but they can all be resolved after merge. Every remaining problem pointed out above is actually a pre-existing problem, not something introduced by this PR.

I'd personally like to avoid making this PR too much larger, and it would be nice to avoid conflicting with myself too much, so merging this before fixing the other things makes sense. I did leave a note that I suggest merging #96748 first so that we can make UV map animations work correctly. I'll either need to update this PR or that PR.

lyuma

I was asked to give another approval. I compared the diffs since last review and they seem fine.

Every remaining problem pointed out above is actually a pre-existing problem, not something introduced by this PR.

To be clear, there are two new problems. They are both minor and not blockers.

individually animated .../weights/N tracks to be TYPE_VALUE, but we should convert them to TYPE_BLEND_SHAPE with the name of the blend shape
We should consider reserving node name + "Shape" when importing nodes.

I would be happy to see this merged.

Repiteo · 2024-11-05T01:45:24Z

This one has more thorough review than #96748 & a handful of approvals to boot, so that PR can be expanded to handle the minor problems @lyuma mentioned after this is merged

Add get_scene_node_path and has_additional_data to GLTFNode, remove center of mass ignore warning in physics (it's supported now), rename `d` to `mesh_dict` in mesh import code.

Repiteo · 2024-11-05T03:58:14Z

Thanks!

aaronfranke added feature proposal needs testing topic:import topic:animation topic:3d labels Jul 10, 2024

aaronfranke added this to the 4.4 milestone Jul 10, 2024

aaronfranke requested a review from a team as a code owner July 10, 2024 10:33

aaronfranke mentioned this pull request Jul 10, 2024

Implement KHR_animation_pointer in GLTF for animating custom properties godotengine/godot-proposals#10164

Open

aaronfranke force-pushed the gltf-khr-anim-ptr branch from 7641446 to 27bdcad Compare July 10, 2024 16:38

aaronfranke mentioned this pull request Jul 15, 2024

Consider Using a more generic referencing system KhronosGroup/glTF#1518

Closed

aaronfranke mentioned this pull request Aug 2, 2024

glTF roadmap - what would you like to see next in glTF? KhronosGroup/glTF#1051

Open

aaronfranke force-pushed the gltf-khr-anim-ptr branch 4 times, most recently from 998e973 to c87f4db Compare August 9, 2024 14:15

This was referenced Aug 19, 2024

Create blender import/export plugin for resonite packages Yellow-Dog-Man/Resonite-Issues#2335

Open

Implement support for glTF as an interchange format (import/export, Khronos/OMI extensions) Yellow-Dog-Man/Resonite-Issues#2778

Open

aaronfranke force-pushed the gltf-khr-anim-ptr branch from c87f4db to c2e9314 Compare August 21, 2024 04:18

aaronfranke mentioned this pull request Aug 29, 2024

Add get_supported_gltf_extensions to GLTFDocument #96251

Merged

aaronfranke force-pushed the gltf-khr-anim-ptr branch from c2e9314 to e3113da Compare August 29, 2024 10:47

aaronfranke force-pushed the gltf-khr-anim-ptr branch 2 times, most recently from 8a74c2b to d0d4675 Compare September 17, 2024 11:26

aaronfranke force-pushed the gltf-khr-anim-ptr branch from d0d4675 to 8155c65 Compare September 17, 2024 22:06

fire mentioned this pull request Oct 3, 2024

Add tests from gltf_khr_animation_pointer.zip godotengine/godot-tests#33

Open

aaronfranke force-pushed the gltf-khr-anim-ptr branch from 8155c65 to 127d7e0 Compare October 4, 2024 04:56

aaronfranke requested a review from a team as a code owner October 4, 2024 04:56

lyuma reviewed Oct 4, 2024

View reviewed changes

aaronfranke force-pushed the gltf-khr-anim-ptr branch from 127d7e0 to f56a23d Compare October 4, 2024 06:17

lyuma reviewed Oct 4, 2024

View reviewed changes

lyuma approved these changes Oct 4, 2024

View reviewed changes

aaronfranke force-pushed the gltf-khr-anim-ptr branch from f56a23d to 2c93a33 Compare October 4, 2024 07:38

akien-mga changed the title ~~GLTF: Implement KHR_animation_pointer for animating custom properties~~ GLTF: Implement KHR_animation_pointer for animating custom properties Oct 4, 2024

AThousandShips approved these changes Oct 4, 2024

View reviewed changes

aaronfranke force-pushed the gltf-khr-anim-ptr branch from 2c93a33 to e5cb94b Compare October 4, 2024 22:40

aaronfranke mentioned this pull request Oct 5, 2024

Import custom attributes with GLTFDocumentExtension #97756

Draft

lyuma approved these changes Oct 30, 2024

View reviewed changes

fire approved these changes Nov 4, 2024

View reviewed changes

aaronfranke added 8 commits November 4, 2024 17:55

GLTF: Move the component type enum into GLTFAccessor

4b0085a

GLTF: Add more accessor component types

c7391d2

GLTF: Clean up animation code to make way for KHR_animation_pointer

834189a

GLTF: Prerequisite cleanups before KHR_animation_pointer

cd367b3

Add get_scene_node_path and has_additional_data to GLTFNode, remove center of mass ignore warning in physics (it's supported now), rename `d` to `mesh_dict` in mesh import code.

GLTF: Implement the glTF Object Model to support JSON pointer properties

4b66a25

GLTF: Implement the glTF Object Model in physics extension

11576b8

GLTF: Add functions to encode and decode Variants to/from accessors

d373d20

GLTF: Implement KHR_animation_pointer for animating custom properties

0513943

aaronfranke force-pushed the gltf-khr-anim-ptr branch from e5cb94b to 0513943 Compare November 5, 2024 01:56

Repiteo merged commit ef8aafc into godotengine:master Nov 5, 2024
20 checks passed

aaronfranke deleted the gltf-khr-anim-ptr branch November 5, 2024 11:59

aaronfranke mentioned this pull request Nov 6, 2024

GLTF: Read material texture "texCoord" property on import #96748

Open

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GLTF: Implement `KHR_animation_pointer` for animating custom properties #94165

GLTF: Implement `KHR_animation_pointer` for animating custom properties #94165

aaronfranke commented Jul 10, 2024 •

edited

Loading

JekSun97 commented Jul 10, 2024

adamscott commented Jul 10, 2024

aaronfranke commented Jul 10, 2024

fire commented Sep 17, 2024

lyuma left a comment

lyuma Oct 4, 2024

lyuma Oct 4, 2024

lyuma Oct 4, 2024

lyuma Oct 4, 2024

lyuma Oct 4, 2024

lyuma Oct 4, 2024

lyuma left a comment

lyuma Oct 4, 2024

Repiteo commented Oct 30, 2024

aaronfranke commented Oct 30, 2024

lyuma left a comment

Repiteo commented Nov 5, 2024

Repiteo commented Nov 5, 2024

		const int track_index = animation->get_track_count();
		animation->add_track(Animation::TYPE_VALUE);

GLTF: Implement KHR_animation_pointer for animating custom properties #94165

GLTF: Implement KHR_animation_pointer for animating custom properties #94165

Conversation

aaronfranke commented Jul 10, 2024 • edited Loading

JekSun97 commented Jul 10, 2024

adamscott commented Jul 10, 2024

aaronfranke commented Jul 10, 2024

fire commented Sep 17, 2024

lyuma left a comment

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

lyuma left a comment

Choose a reason for hiding this comment

lyuma Oct 4, 2024

Choose a reason for hiding this comment

Repiteo commented Oct 30, 2024

aaronfranke commented Oct 30, 2024

lyuma left a comment

Choose a reason for hiding this comment

Repiteo commented Nov 5, 2024

Repiteo commented Nov 5, 2024

GLTF: Implement `KHR_animation_pointer` for animating custom properties #94165

GLTF: Implement `KHR_animation_pointer` for animating custom properties #94165

aaronfranke commented Jul 10, 2024 •

edited

Loading