three.js
three.js copied to clipboard
GLTFLoader: add arbitrary property animation through KHR_animation_pointer support
The following PRs are rolled into this one and ideally are merged first:
- #24252
- #24193
- #24603
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: https://github.com/KhronosGroup/glTF-Sample-Models/pull/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 iffindNode
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 topendingDependencies
? - 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.
- https://github.com/BabylonJS/Babylon.js/issues/12544
This contribution is funded by Needle and prefrontal cortex
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.
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:
- https://github.com/KhronosGroup/glTF/pull/2147
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
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 )
Made a separate PR with the required callbacks to allow for KHR_animation_pointer:
- https://github.com/mrdoob/three.js/pull/24193
The PRs that would need to be merged so KHR_animation_pointer
can at least be used as an external extension would be:
- #24252
- #24193
cc @donmccurdy @takahirox
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.
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.
@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.
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!
@GitHubDragonFly the version on our latest branch (24128818278145748255e7ed8fb6ff083ec9da6b) 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.
@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!