GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108

hybridherbst · 2022-05-23T15:08:36Z

The following PRs are rolled into this one and ideally are merged first:

Description

This PR adds support for KHR_animation_pointer. We wanted to put this out early for discussion. There's a good amount of refactoring to be done (especially figuring out how to do property binding in a clean/extensible way, and how to deal with animated cameras/lights/materials that three might duplicate in certain cases).

three.js fork with built drag-and-drop viewer can be found here:
https://needle.tools/three.js/examples/?q=loader_mu#webgl_loader_multiple

Some sample models are here:
KhronosGroup/glTF-Sample-Models#353

To quickly test, just download some of the GLB files from the sample-models fork and drop them into the three-js fork above :)

Known Limitations

If lights or cameras are used on multiple nodes, three.js duplicates those. This currently isn't supported here. When animating lights/cameras is wanted, each node should have a unique light/camera.
Three.js only allows texture transforms on map and aoMap, not on other properties. This means they can be animated but don't have an effect. Custom shaders can still use those properties, so I think it's valuable to still import their animations (this is incomplete right now).
Materials might be duplicated depending on what features a particular mesh has (with/without vertex colors, normals, tangents). Tracks would need to be duplicated in this case as well to point to those new materials.

Next steps
Before doing the remaining implementation (which mostly consists of mapping glTF JSON Pointer strings to three.js property names), I think a couple of points are worth discussing:

currently PropertyBinding.findNode is monkey patched to support materials and node uuids. It may be that custom extensions need to have callbacks into this (e.g. KHR_animation_pointer wants to resolve materials). I'm unsure if findNode itself should be extended to support more cases or a callback mechanism be introduced (I'd prefer the callback).
~~lights currently don't work, getDependency doesn't have 'lights' right now (another candidate for a callback for findNode).~~
naming-wise it's a bit weird that pendingNodes may now contain other things than nodes (e.g. materials). Should we rename this to pendingDependencies?
given that at some point three.js may also want to export with KHR_animation_pointer, I'm unsure of where all the path and property remapping should happen. I guess for now import is good to tackle first to find all edge cases and then later talk about export.
some properties in three.js work differently - e.g. Light.penumbra is calculated from inner and outer spot angle. Not sure what the right approach would be during animation. Precalculating this as array seems rather complicated (inner and outer spot could have separate keyframe times etc.).
Light.angle also needs to be converted from rad to deg, which can probably be done when creating the Track array.

cc @donmccurdy @elalish @mrdoob and probably others :) I'm really looking forward to having support for this, opens up many possibilities.

Babylon and Gestaltor seem to have also started implementations.

Implement KHR_animation_pointer BabylonJS/Babylon.js#12544

This contribution is funded by Needle and prefrontal cortex

mrdoob · 2022-05-24T07:53:12Z

The name of the extension is kind of confusing.
At first I thought it was a way of making gltf elements react to the pointer.

hybridherbst · 2022-05-24T08:26:57Z

Fully agree – it has been renamed a couple of times already and the name hasn't necessarily gotten better.
In case a good name comes to mind for "mostly arbitrary glTF property animation", please comment here:

KHR_animation_pointer KhronosGroup/glTF#2147

donmccurdy

🎉 Thanks for opening this PR @hybridherbst! We'd likely want to wait on merging anything until the extension is ratified, or nearly so. It's helpful to see how this might look for discussion in the meantime.

The features required by KHR_animation_pointer are pretty complex to support (nothing wrong with this PR, it is just a very complex extension in practice) and so I'll flag a few likely issues in comments below.

donmccurdy · 2022-05-24T15:20:42Z

examples/jsm/loaders/GLTFLoader.js

+					const pathStart = path.substring( 0, pathIndex );
+					targetProperty = path.substring( pathIndex );
+
+					// TODO implement all properties and/or find a better way to have a good mapping here


Supporting all properties may be a non-goal for three.js I think. Especially as animated extension properties come into the mix. We can try to support whichever properties seem helpful to three.js users, and are not overly complex to include.

No need to change these in the draft PR now, but a few things to flag:

alphaCutoff animation can trigger shader recompilation

we have very limited support for UV transforms, all textures except aoMap must share the same transform. I wonder if we should avoid animating those properties until that is improved.

metallicFactor seems unlikely to be animated much?

donmccurdy · 2022-05-24T15:21:44Z

examples/jsm/loaders/GLTFLoader.js

+							targetProperty = 'morphTargetInfluences';
+							break;
+
+						// TODO matrix


I think animation is required to target TRS properties, not a matrix?

Good question. @UX3D-nopper is /matrix intentionally included in KHR_animation_pointer?

donmccurdy · 2022-05-24T15:25:59Z

examples/jsm/loaders/GLTFLoader.js

+							targetProperty = 'far';
+							break;
+						case 'aspect':
+							break;


Similar to above, will have to call camera.updateProjectionMatrix() after any of these changes.

I'm not sure what I think about animation of the aspect ratio. glTF lacks any meaningful way to express "let the camera inherit the aspect ratio from the application", animating this property kind of seems like stacking blocks on top of something that is already not working properly.

donmccurdy · 2022-05-24T15:35:04Z

examples/jsm/loaders/GLTFLoader.js

+			if ( useExtension )
+				nodeDependency = this.getAnimationPointerDependency( target );
+			else
+				nodeDependency = this.getDependency( 'node', name );


One of the larger hurdles on this extension will be that glTF definitions do not translate 1:1 into three.js objects —

Materials will be cloned depending on the context in which they're used. The same glTF material applied to (1) points, (2) lines, (3) mesh with vertex colors, (4) mesh with vertex normals, (5) mesh with vertex tangents, will result in 5x copies of the material.

Lights and Cameras will be cloned if they're attached at >1 Node.

Probably similar for KHR_audio if that gets ratified?

Do you have an idea on how to tackle these in the context of this extension? I'd assume that when they are split, the correct additional tracks would need to be created.

I first thought one could be keeping some importUUID around, which could then be used during findNode to map back to objects, but I don't think one track can target multiple objects (by design).

donmccurdy · 2022-05-24T15:35:34Z

examples/jsm/loaders/GLTFLoader.js

+					parts[ 2 ] = node.uuid;
+					animationPointerPropertyPath = parts.join( '.' );
+					if ( animationPointerDebug )
+						console.log( node, inputAccessor, outputAccessor, target, animationPointerPropertyPath );


The trouble with targeting animation to the UUID is that many users will clone the model before attaching one or more copies to their scene. The cloned objects will have new UUIDs, and so the same animation clips cannot target them. We prefer to target based on unique names for that reason, but glTF doesn't require unique names. And in this case (see above) we may need to animate multiple objects, even if only a single source glTF definition was animated. 😕

Switched to node.name ?? node.uuid, similar to other places.

takahirox · 2022-05-25T08:17:23Z

Hi, thanks for working on it.

I didn't look into the spec and change deeply yet, but let me share my rough thoghts so far.

If the extension doesn't greatly fit to Three.js API, it may be good to consider partial support
If the extension may not be so popular, the implementation can be too hacky, or the extension doesn't fit to Three.js API well, it may be good to make the extension handler as a plugin and release at an external repository like this

takahirox · 2022-05-25T08:20:56Z

examples/jsm/loaders/GLTFLoader.js

+};
+
+// HACK monkey patching findNode to ensure we can map to other types required by KHR_animation_pointer.
+const find = PropertyBinding.findNode;


I don't think monkey patching is a good idea. Maybe good to consider either

Update the core findNode

Give up the perfect extension support and consider partial feature support that the existing findNode can handle

If we really need monkey patching, it may be good to do it only if KHR_animation_pointer extension is used

The existing findNode can't handle most of what animation_pointer requires unfortunately. I agree that it would be better to update findNode as mentioned in my original post, looking forward to ideas which parts should move there. Ideally there's a callback that for example extensions can tap into.

I changed the implementation to only monkey patch if the extension is actually used.

takahirox · 2022-05-25T08:26:25Z

examples/jsm/loaders/GLTFLoader.js

+
+			let nodeDependency = undefined;
+			if ( useExtension )
+				nodeDependency = this.getAnimationPointerDependency( target );


It may be good to consider the plugin system for handling a extension. It provides an extensibility while it keeps the simplicity for processing the glTF core in the loader unless the extension handler really needs to be in the loader core. If the current Plugin API doesn't fit to process the extension, please consider to propose a good new API for handling animation extension.

Yes, as written in my original message that was the plan.
I now refactored it to be contained entirely in an animation, and added two new callbacks, see

GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108 (comment)

hybridherbst · 2022-05-31T23:24:38Z

I believe the support added here is now "feature complete" for the core spec and all currently ratified extensions (minus some texture transforms that three doesn't support anyways). There's still open questions regarding the currently required use of PropertyBinding/monkey patching, and what to do when materials and/or lights/cameras are duplicated during import (and would thus require creating multiple tracks). Looking for input on these!

I refactored everything into a self-contained extension (GLTFAnimationPointerExtension), and had to add two new callbacks for it:

loadAnimationTargetFromChannel( animationChannel )
createAnimationTracks( node, inputAccessor, outputAccessor, sampler, target )

hybridherbst · 2022-06-04T21:39:29Z

Made a separate PR with the required callbacks to allow for KHR_animation_pointer:

GLTFLoader: add hooks for animation #24193

hybridherbst · 2022-10-10T10:02:54Z

The PRs that would need to be merged so KHR_animation_pointer can at least be used as an external extension would be:

cc @donmccurdy @takahirox

donmccurdy · 2022-10-10T20:49:08Z

I think I would prefer that we exclude KHR_animation_pointer from the threejs repository unless/until it has started the ratification process with Khronos. I'm not aware if it's more than a proposal at this stage. Adding hooks or other extension API changes required to support the extension externally, as in the other PRs, would be welcome.

hybridherbst · 2022-10-10T21:56:51Z

Understood. You are right that it's currently just a proposal, however, both Gestaltor and Babylon have already implemented it and there's multiple sample assets that I made. I'll ask what the blocker is for moving forward.

examples/jsm/loaders/GLTFLoader.js

+
+		}
+
+		if ( targetId === null || isNaN( targetId ) ) {


…nto GLTFParser

add some TODO comments, rename pointer path property wip support for camera and light properties, added morph target support formatting, fix alphaTest, add opacity track splitting added some comments, added some material ext animation properties fixed Ior > IOR, test switching to node names to avoid duplication problems (but incomplete yet, needs full paths) fix: use "pointer" property instead of "path" refactor: move to extension, add new callbacks move remaining pieces into extension, rename methods fix: orthographic/perspective camera, pbrMetallicRoughness prefix, clearcoat/sheen/specular extensions rm change on PATH_PROPERTIES only patch PropertyBindings if KHR_animation_pointer is actually used fix: fall back to original findNode implementation earlier if special cases don't yield results introduce animationPointerResolver to allow configuring KHR_animation_pointer from outside fix: glTF camera fov is in radians, not degrees fix: use QuaternionKeyframeTrack instead of ColorKeyframeTrack if property ends with .quaternion use extension hooks

GitHubDragonFly · 2024-01-08T21:42:27Z

@hybridherbst just as an FYI, on my end I have dared using your GLTF Loader version from this PR in my GLTF Viewer and it seems to be working fine for the Animate All The Things example that you have available for testing.

Whenever you might get back to dealing with this arbitrary extension, and in addition to the warning listed by the bot above, there is one other thing to correct related to CUBICSPLINE comparison on lines 1300 and 4921, where interpolation should be replaced with sampler.interpolation.

Khronos InterpolationTest example shows this correction as necessary to be fully functional.

elalish · 2024-01-08T22:05:11Z

The good news is the glTF interactivity WG is now pushing KHR_animation_pointer as a dependency for the interactivity extension. It should be going through the ratification process shortly, which will require implementations like this. Thanks @hybridherbst!

hybridherbst · 2024-01-09T22:50:41Z

@GitHubDragonFly the version on our latest branch (2412881) should be better and lilkely already has that fix.

@elalish happy to hear – let me know when a good time would be to pick up this PR again and bring it up to speed with our latest changes.

elalish · 2024-03-02T00:15:43Z

@hybridherbst I think now is great - did you see that KHR_animation_pointer is going through ratification now? So there are already two implementations out there somewhere. Would be awesome to have in Three!

mrdoob added this to the r??? milestone May 24, 2022

hybridherbst changed the title ~~GLTFLoader: add KHR_animation_pointer support~~ GLTFLoader: add arbitrary property animation through KHR_animation_pointer support May 24, 2022

donmccurdy reviewed May 24, 2022

View reviewed changes

takahirox reviewed May 25, 2022

View reviewed changes

hybridherbst force-pushed the khr-animation-pointer branch from 9743045 to 7e0d1de Compare May 30, 2022 12:18

hybridherbst mentioned this pull request Jun 4, 2022

GLTFLoader: add hooks for animation #24193

Open

hybridherbst mentioned this pull request Jun 20, 2022

GLTFLoader: add getDependency( type, index ) implementation #24252

Merged

hybridherbst force-pushed the khr-animation-pointer branch from 6f69066 to d6adc29 Compare July 5, 2022 13:37

donmccurdy mentioned this pull request Aug 24, 2022

PropertyBinding: Allow map as target object. #24537

Merged

hybridherbst mentioned this pull request Sep 6, 2022

PropertyBinding: fix map property binding regression introduced in #24537 #24603

Merged

hybridherbst force-pushed the khr-animation-pointer branch from 3c019f1 to b9a0b83 Compare September 7, 2022 00:17

hybridherbst mentioned this pull request Oct 10, 2022

GLTFLoader: Prevent load exception for assets that animation.target.node is undefined #24770

Merged

hybridherbst marked this pull request as ready for review October 10, 2022 10:04

marwie mentioned this pull request Oct 25, 2022

Export animated objects from Unity as Glb file got an error when imported into ThreeJS via GLTFLoader needle-tools/needle-engine-support#100

Closed

hybridherbst force-pushed the khr-animation-pointer branch from b9a0b83 to 1a881e9 Compare November 24, 2022 23:56

takahirox mentioned this pull request Nov 28, 2022

PropertyBinding: Support more arbitary objects #25028

Open

hybridherbst mentioned this pull request Jan 24, 2023

PropertyBinding falls back to root node for incorrect paths/nodes #25286

Closed

hybridherbst force-pushed the khr-animation-pointer branch from 1a881e9 to 2d05374 Compare March 27, 2023 22:48

github-advanced-security bot found potential problems Mar 27, 2023

View reviewed changes

examples/jsm/loaders/GLTFLoader.js

}

if ( targetId === null || isNaN( targetId ) ) {

Check warning

Code scanning / CodeQL

Comparison between inconvertible types

Variable 'targetId' cannot be of type null, but it is compared to [an expression](1) of type null.

hybridherbst force-pushed the khr-animation-pointer branch 2 times, most recently from b7504d6 to 3e5ac4c Compare March 28, 2023 17:24

hybridherbst and others added 3 commits May 23, 2023 23:53

introduce GLTFLoader extension methods for animations

86b87bf

move getArrayFromAccessor and createCubicSplineTrackInterpolant i…

5df1e19

…nto GLTFParser

hybridherbst force-pushed the khr-animation-pointer branch from 3e5ac4c to 1152049 Compare May 23, 2023 22:22

hybridherbst mentioned this pull request May 24, 2023

GLTFLoader: Refactor animation methods to be accessible from extensions on parser #26126

Merged

Mugen87 mentioned this pull request May 11, 2024

Ability to target ShaderMaterial uniforms via animations #28333

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108

GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108

hybridherbst commented May 23, 2022 •

edited

Loading

mrdoob commented May 24, 2022

hybridherbst commented May 24, 2022 •

edited

Loading

donmccurdy left a comment

donmccurdy May 24, 2022 •

edited

Loading

donmccurdy May 24, 2022

hybridherbst May 31, 2022

donmccurdy May 24, 2022 •

edited

Loading

donmccurdy May 24, 2022

hybridherbst May 31, 2022 •

edited

Loading

donmccurdy May 24, 2022

hybridherbst May 31, 2022

takahirox commented May 25, 2022

takahirox May 25, 2022 •

edited

Loading

hybridherbst May 31, 2022

hybridherbst May 31, 2022

takahirox May 25, 2022

hybridherbst May 31, 2022 •

edited

Loading

hybridherbst commented May 31, 2022 •

edited

Loading

hybridherbst commented Jun 4, 2022

hybridherbst commented Oct 10, 2022 •

edited

Loading

donmccurdy commented Oct 10, 2022

hybridherbst commented Oct 10, 2022

GitHubDragonFly commented Jan 8, 2024

elalish commented Jan 8, 2024

hybridherbst commented Jan 9, 2024

elalish commented Mar 2, 2024

GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108

Are you sure you want to change the base?

GLTFLoader: add arbitrary property animation through KHR_animation_pointer support #24108

Conversation

hybridherbst commented May 23, 2022 • edited Loading

mrdoob commented May 24, 2022

hybridherbst commented May 24, 2022 • edited Loading

donmccurdy left a comment

Choose a reason for hiding this comment

donmccurdy May 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

donmccurdy May 24, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hybridherbst May 31, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

takahirox commented May 25, 2022

takahirox May 25, 2022 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

hybridherbst May 31, 2022 • edited Loading

Choose a reason for hiding this comment

hybridherbst commented May 31, 2022 • edited Loading

hybridherbst commented Jun 4, 2022

hybridherbst commented Oct 10, 2022 • edited Loading

donmccurdy commented Oct 10, 2022

hybridherbst commented Oct 10, 2022

GitHubDragonFly commented Jan 8, 2024

elalish commented Jan 8, 2024

hybridherbst commented Jan 9, 2024

elalish commented Mar 2, 2024

hybridherbst commented May 23, 2022 •

edited

Loading

hybridherbst commented May 24, 2022 •

edited

Loading

donmccurdy May 24, 2022 •

edited

Loading

donmccurdy May 24, 2022 •

edited

Loading

hybridherbst May 31, 2022 •

edited

Loading

takahirox May 25, 2022 •

edited

Loading

hybridherbst May 31, 2022 •

edited

Loading

hybridherbst commented May 31, 2022 •

edited

Loading

hybridherbst commented Oct 10, 2022 •

edited

Loading