Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Support for KHR_animation_pointer #440

Open
bobbens opened this issue Dec 16, 2024 · 1 comment
Open

Support for KHR_animation_pointer #440

bobbens opened this issue Dec 16, 2024 · 1 comment

Comments

@bobbens
Copy link
Contributor

bobbens commented Dec 16, 2024

KHR_animation_pointer was ratified and merged earlier this year and would significantly boost animation potential by allowing animation things like textures and pretty much everything. I think it would be very beneficial to have support for it.

@crazyjackel
Copy link

crazyjackel commented Feb 10, 2025

I would like to second this, with some thoughts regarding implementation details:

  1. Currently GLTF-RS is not in spec due to the Index::Node on Animation Channel Target being required counter to 5.7

  2. Target Path needs to be updated to include "pointer" as a valid value; This is difficult to control via feature flag, there is a good argument to handle this similarly to how Image::MimeType is handled compared to spec by just using raw strings with comments on the allowed values. Though, we can manage it with a feature flag by making Target Path non-exhaustive, which is my preferred solution.

  3. Extensions need to be populated with the Pointer provided. The precise typing on this is difficult as it references the asset, but KHR_animation_pointer is explicit that it must be mutable in the glTF 2.0 Asset Object Model Specification, which means we can fully enumerate the precise references. I would recommend making a GLTFMutablePointer enumeration that can be serialized and deserialized from the provided pointer templates. This Enum would also need to be non-exhaustive due to extension variants that are also defined in Asset Object Specification.

I think the first step is reconciling # 1 because 'gltf-rs' is out of specification ATM... let me know if you would like me to write up a PR for this implementation.

(Edit: step 1 was done in #439 and PR #445, instead of being optional, it is instead set to max value with an is_empty being used. I missed this because this version has not been released.)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants