com.unity.cloud.gltfast
com.unity.cloud.gltfast copied to clipboard
Implement extension KHR_animation_pointer
Implements KHR_animation_pointer, Closes #592.
https://user-images.githubusercontent.com/9121007/233417443-55f2dc97-fe7f-4122-9ef4-c45f50536434.mp4
Main pointers:
- [x] meshes
- [x] weights
- [x] nodes
- [x] rotation
- [x] scale
- [x] translation
- [x] weights (using duplicated version of standard code. needs refactor.)
- [x] cameras
- [x] orthographic/xmag
- [x] orthographic/ymag
- [x] orthographic/zfar
- [x] orthographic/znear
- [x] perspective/yfov
- [x] perspective/zfar
- [x] perspective/znear
- [x] materials
- [x] pbrMetallicRoughness/baseColorFactor
- [x] pbrMetallicRoughness/metallicFactor
- [x] pbrMetallicRoughness/roughnessFactor
- [x] alphaCutoff
- [x] emissiveFactor
- [x] normalTexture/scale
- [x] occlusionTexture/strength
Camera animations have been implemented with a helper GameObject due to the required recalculations needed from the raw animation values.
Extensions
In my original PR, I had implemented KHR_texture_transform
, however I will submit this as an additional PR when suitable as it necessitates moving some code from material initialisation in C#, to the shaders. I have done this for the shadergraph shaders, but not the built in one. To save changing all shadergraphs, I packed the previous vec2 rotation
property with both the new scalar rotation
and the bool flipY
that's required for KTX textures.
I will also implement KHR_lights_punctual
as a separate PR.
- [x] KHR_materials_transmission
- [x] transmissionFactor
This PR is sponsored by Envoke.
KHR_animation_pointer
is now at release candidate stage.
And merged! @atteneder if this is still something glTFast is interested in, should I look at refactoring the pointers system to be more generic and not specific to animation, now that Khronos plans to use them for the Object Model? https://github.com/KhronosGroup/glTF/blob/main/specification/2.0/ObjectModel.adoc
Animation pointer has now been ratified 🎉
Thanks a lot for this PR and getting the work started.
I agree, we need a non-animation-specific Object Model implementation. That's become even clearer after seeing KHR_interactivity getting to the public review stage using it as well.
This involves quite some string operations (which is natural, given pointers are string). I'd like to carefully review minimizing memory garbage created before merging.
The package still supports 2020 LTS. You've used range indexes instead of Substring. Unfortunately those don't work in 2020 LTS, so we either have to postpone it to the next release (that drops 2020 LTS) or change it to use Substrings.
I noticed that the new AnimationData is used, even on non-animation-pointer glTFs. Again, I need to confirm that there are no (performance) downsides whatsoever.
I've started to implement some of that feedback. I'll give an update next week.
Thanks, appreciate the review.
Object Model implementation
Absolutely. is KHR_interactivity something you'd be looking to implement in the main package too?
minimizing memory garbage
Good point, it's not something I'm an expert at, hopefully it's not terrible.
Substrings
Happy to leave that up to you! My target projects were 2021 so I didn't notice this.
AnimationData is used, even on non-animation-pointer glTFs
Yep totally understand, my thought was there ideally shouldn't be two places that animation creation happens, so I chose to pipe "legacy" animation into a new general system.
Object Model implementation
Absolutely. is KHR_interactivity something you'd be looking to implement in the main package too?
No immediate plan, but I'm keeping an eye on it.
minimizing memory garbage
Good point, it's not something I'm an expert at, hopefully it's not terrible.
Nah, I'm pretty nit-picky to be honest.
Substrings
Happy to leave that up to you! My target projects were 2021 so I didn't notice this.
On it.
AnimationData is used, even on non-animation-pointer glTFs
Yep totally understand, my thought was there ideally shouldn't be two places that animation creation happens, so I chose to pipe "legacy" animation into a new general system.
You might be right after all. I need to learn more about the tradeoffs first.
Further observations/questions
If animation channel targets are used multiple times (e.g. a camera, light or material is used on multiple nodes):
- Curves get created redundantly (not great, not terrible; pointer for optimization)
- IIRC the material is a shared one anyways. Isn't it sufficient to animate the first occurrence? A potential problem then becomes if you have multiple scene instances using the same material. An animation on instance A should not affect the material of instance B. Needs more testing and likely fixing/optimization.
- You can animate both a mesh's weights and a node's weights.
- Are curves for mesh weight animation properly duplicated/re-used for multiple nodes?
- What happens if there's a collision/both? Is this even covered in the spec?
API challenge
The PR requires to add an extensions
field to AnimationChannelTarget
. Unfortunately that conflicts with a generic one that already exists for the Newtonsoft JSON parsing schema classes. The fix requires a change that strictly speaking is an API breaking change. Even though I hardly expect an actual problem for users, policy requires raising the major version for that.
This is obviously none of your fault, but we have to address this and find a solution before moving on.