Adding a KHR_animation2 extension

[UX3D-nopper]

No description provided.

[donmccurdy]

I think what would be valuable is if the extension would specify a "minimum set of guaranteed properties that can be animated when this extension is present".

If a minimum set were defined, I'd suggest it be much shorter than the "valid targets" list. The list is there to put an upper bound on the implementation complexity – that upper bound is still quite high. In an ideal world with perfect foresight, this extension would have been part of base glTF 2.0 and new extensions (like punctual lights) would simply opt-in to which of their properties could be animated. I see this extension's goal as setting that baseline now. It would be nice if existing extensions (e.g. KHR_lights_punctual) could be grandfathered in somehow without entirely new extensions.

[hybridherbst]

Would this mean that for every "animation extension - existing other extension" pair, there needs to be a separate "other extension _ animation" extension to cover which values are animated? Or would the goal be to retrospectively state for each other extensions which values can be animated?

Alternative would be to have this extension here specify something like

"if a viewer supports KHR_texture_transforms and KHR_animation2, the following properties can be animated:"

for the current set of KHR extensions.

Otherwise it feels like this is running into undefined behaviour pretty much immediately... for example, animating texture transforms is what I was most looking forward to from an animation extension. But now contrary to my understanding it doesn't sound like that is covered at all.

Another example would be animating emission strength: I think it would just be natural that viewers who support both KHR_animation2 and KHR_emissive_strength support animating emmisive strength, spec or not. If that isn't written anywhere it's just "random/undefined" though and nobody can rely on it.

(side note for reference, USD simply allows animating everything... not sure if that was looked into as part of this proposal)

[donmccurdy]

Would this mean that for every "animation extension - existing other extension" pair, there needs to be a separate "other extension _ animation" extension to cover which values are animated?...

No — or at least I'm hoping not. Adding an "animation" appendix to ratified extensions does not seem crazy to me, nor does adding an appendix to this extension clarifying behavior for all previously-ratified extensions. Future extensions should probably specify animation-ready properties, perhaps just as a general statement and not specifically bound to KHR_animation2.

USD simply allows animating everything...

USD permits a lot of things that will not work outside its own runtime; it isn't a portable runtime format in that sense. It is focused on not losing an ounce of authoring data, as an interchange format understandably would be.

In my opinion, it's better to set some upper bounds on complexity for clients. Animation that requires restructuring a glTF file or recompiling shaders is probably a bad thing, and the work of avoiding this should fall more on authoring workflows than on clients.

That said, the lack of a practical "update" path for glTF extensions is a particular obstacle in the case of animation. The burden of defining a list of animatable properies perfectly, on the first try, is not a desirable design constraint. 😕

[UX3D-nopper]

Would this mean that for every "animation extension - existing other extension" pair, there needs to be a separate "other extension _ animation" extension to cover which values are animated?...

No — or at least I'm hoping not. Adding an "animation" appendix to ratified extensions does not seem crazy to me, nor does adding an appendix to this extension clarifying behavior for all previously-ratified extensions. Future extensions should probably specify animation-ready properties, perhaps just as a general statement and not specifically bound to KHR_animation2.

USD simply allows animating everything...

USD permits a lot of things that will not work outside its own runtime; it isn't a portable runtime format in that sense. It is focused on not losing an ounce of authoring data, as an interchange format understandably would be.

In my opinion, it's better to set some upper bounds on complexity for clients. Animation that requires restructuring a glTF file or recompiling shaders is probably a bad thing, and the work of avoiding this should fall more on authoring workflows than on clients.

That said, the lack of a practical "update" path for glTF extensions is a particular obstacle in the case of animation. The burden of defining a list of animatable properies perfectly, on the first try, is not a desirable design constraint. 😕

Hmm, maybe we add all current KHR_* extensions and mention, what property is animatable or not.
Any future extension has to mention, if KHR_animation2 is supported, which prperty can be animated.

[hybridherbst]

An example of how I think this could look:

KHR_lights_punctual

Property	Animatable
name	❌
color	✅
intensity	✅
type	❌
range	✅
spot.innerConeAngle	✅
spot.outerConeAngle	✅

KHR_texture_transform

Property	Animatable
offset	✅
rotation	✅
scale	✅
texCoord	❌

KHR_draco_mesh_compression
❌ No animatable properties.

KHR_materials_clearcoat

Property	Animatable
clearcoatFactor	✅
clearcoatTexture	❌
clearcoatRoughnessFactor	✅
clearcoatRoughnessTexture	❌
clearcoatNormalTexture	❌

KHR_materials_ior

Property	Animatable
ior	✅

@UX3D-nopper I could offer to complete the list above if that's helpful?

[UX3D-nopper]

An example of how I think this could look:

KHR_lights_punctual

Property Animatable
name ❌
color ✅
intensity ✅
type ❌
range ✅
spot.innerConeAngle ✅
spot.outerConeAngle ✅
KHR_texture_transform

Property Animatable
offset ✅
rotation ✅
scale ✅
texCoord ❌
KHR_draco_mesh_compression ❌ No animatable properties.

KHR_materials_clearcoat

Property Animatable
clearcoatFactor ✅
clearcoatTexture ❌
clearcoatRoughnessFactor ✅
clearcoatRoughnessTexture ❌
clearcoatNormalTexture ❌
KHR_materials_ior

Property Animatable
ior ✅
@UX3D-nopper I could offer to complete the list above if that's helpful?

Please and do not forget to add your name!

[donmccurdy]

I think a list like that would be a good thing — wherever it ends up being located. The choices above look very reasonable to me. Thanks!

[UX3D-nopper]

I mean the goal is to get this forward and that things have beend dicusses and prepared. If this is final, you never know.

[hybridherbst]

Think I got them all -
List of all animatable properties in existing KHR extensions: ux3d#17

Independent of what the wording is going to be (e.g. "recommended", "required", "optional") it's actually pretty helpful to have this as list.

[emackey]

Two questions, and I apologize if either of these have already been covered here:

1. Why the name KHR_animation2? Could it be KHR_property_animation?

2. Does this animation extension allow any new behaviors with animating translate/rotate/scale? The glTF format has a solid history of NOT allowing multiple ways to set the same setting, and I wonder, are we violating that here? Doing so creates fragmentation, and bloats engines that have already implemented the classic animation system, so it may be worth excluding T/R/S from this new animation system and instead providing some kind of linkage to the classic one where needed.

[UX3D-nopper]

Two questions, and I apologize if either of these have already been covered here:

1. Why the name KHR_animation2? Could it be KHR_property_animation?
2. Does this animation extension allow any new behaviors with animating translate/rotate/scale? The glTF format has a solid history of NOT allowing multiple ways to set the same setting, and I wonder, are we violating that here? Doing so creates fragmentation, and bloats engines that have already implemented the classic animation system, so it may be worth excluding T/R/S from this new animation system and instead providing some kind of linkage to the classic one where needed.

Regarding 1:
animation2 - to express a new approach to address any animation property.

Regarding 2:
In the current spec, there is this description of the node:
"When undefined, the animated object MAY be defined by an extension."
https://github.com/KhronosGroup/glTF/blob/main/specification/2.0/schema/animation.channel.target.schema.json#L11
This fact of the specification is used to define this extension and to be backward comaptible:
https://github.com/KhronosGroup/glTF/blob/a70e8b5e7055d4d48219311362e1596d438c9390/extensions/2.0/Khronos/KHR_animation2/README.md#extension-compatibility-and-fallback-behavior

[emackey]

Suggestions from today's call:

• Rename to KHR_animations_properties KHR_animation_pointer
• Add a new enum pointer

        "target" : {
            "path" : "pointer"
            "extensions": {
                "KHR_animation_pointer" : {
                    "pointer" : "/materials/0/pbrMetallicRoughness/baseColorFactor"
                }
            }
        }

[UX3D-nopper]

Thank u

[lexaknyazev]

KHR_animations_properties

The material extensions are using materials (plural) for historical reasons but I think we agreed to use singular going forward.

The suggestion was KHR_animation_pointer. Isn't it more precise wrt the extension functionality?

[emackey]

Ah, OK, KHR_animation_pointer is good.

[javagl]

The animation.sampler.schema.json documentation of LINEAR currently says

When targeting a rotation, spherical linear interpolation (slerp) **SHOULD** be used to interpolate quaternions.

I just looked at the latest README.md, and apparently, node rotations are (right now) the only case where a quaternion is animated. But I assume that the slerp-interpolation should always be used when interpolating a quaternion. This might warrant adding a hint in the README.md as well, maybe somewhere near https://github.com/KhronosGroup/glTF/blob/a70e8b5e7055d4d48219311362e1596d438c9390/extensions/2.0/Khronos/KHR_animation2/README.md#json-pointer, which currently says

For a scalar value, the JSON Pointer targets the glTF property.
This means, the property is replaced by the interpolated value.

For any other case, the JSON Pointer targets the glTF property as well but must be an array property.
This means, that the two or more elements do replace the values in the array.

and could be extended with

The type of the interpolation is determined by the animation sampler. If the sampler defines a LINEAR interpolation mode, and the interpolated value is a qaternion, then a spherical linear interpolation (slerp) **SHOULD** be used to compute the interpolated value.

(If this is considered to be relevant, I can also put it into a PR)

---

An aside: I'm still a bit surprised that setting/adding "path": "pointer" is supposed to be valid. Right now, this will cause a validation warning like

"code": "VALUE_NOT_IN_LIST",
"message": "Invalid value 'pointer'. Valid values are ('translation', 'rotation', 'scale', 'weights').",
"severity": 1,
"pointer": "/animations/0/channels/0/target/path"

This is related to KhronosGroup/glTF-Validator#172 (where the KHR_animation_pointer extension might to "hook" in, to extend the set of allowed values), and raises some other low-level schema questions, but may not have to be discussed here.

[emackey]

An aside: I'm still a bit surprised that setting/adding "path": "pointer" is supposed to be valid. Right now, this will cause a validation warning

I can confirm that adding enum values does not violate JSON schema, because of the foresight of including the catch-all string as the last in the list. I remember having this discussion back in 2016~2017 in the early days of VSCode's JSON schema validation integration, and future extended enum values were explicitly the reason this was added back in the day.

As for the validator's current warning, note that it is indeed just a warning ("severity": 1) and not an error ("severity": 0). The validator can be updated to relax this when the animations extension is present.

However, I do think it wise to require this extension be listed in both extensionsUsed and extensionsRequired, similar to when the webp and/or Draco extensions are used without fallback. Implementations will indeed require a new enum value in this field to handle the new animation, and old implementations are not guaranteed to safely ignore it (although they can and probably should be updated to ignore unknown values in all of glTF's enums, which all allow future values).

[donmccurdy]

Implementations will indeed require a new enum value in this field to handle the new animation...

The current specification also includes another carve-out for extensions: When "node" isn’t defined, channel SHOULD be ignored. This might give us some room to avoid extensionsRequired here.

A bit of a semantic question about how extensionsRequired should be interpreted by clients — I expect few implementations can practically support animation of all available properties defined here. Under what circumstances are clients expected to refuse to attempt loading, when the extension is given in extensionsRequired?

[emackey]

The extensionsRequired list is just a basic "My loader can parse this" requirement, not "My viewer has 100% coverage." If they've added the pointer enum, then their loader can parse it, and they're good to accept it in this list. I do like your idea about avoiding even this requirement though.

[javagl]

I can confirm that adding enum values does not violate JSON schema, because of the foresight of including the catch-all string as the last in the list. I remember having this discussion back in 2016~2017 in the early days of VSCode's JSON schema validation integration, and future extended enum values were explicitly the reason this was added back in the day.

The context: #899 (comment)

This certainly makes sense (and is valid) on the schema level. But I wondered about this on the level of validation. Just to confirm: I assume that it is not intended to add "pointer" as a new enum value in the core spec, right? Instead, it will only be listed as one new enum value via the extension. (The last statements didn't seem entirely clear, but maybe I misinterpreted something here)

[bghgary]

Just to confirm: I assume that it is not intended to add "pointer" as a new enum value in the core spec, right?

This has to be right. We can't modify the core spec. That said, I'm not sure if it's possible to augment the schema for this enum (e.g., such that vscode will be able to show them).

[emackey]

That said, I'm not sure if it's possible to augment the schema for this enum (e.g., such that vscode will be able to show them).

Correct, it cannot go in core schema, only the extension.

VSCode has always needed a transformed copy of the glTF schema, for a number of reasons including enum/extended descriptions and direct hookup of various extension schemas into core. There's a script that automates this process. Once we see some implementations embrace this extension, I'll probably update that script to inject the new enum value, making it available to VSCode autocomplete/tooltip users. I'm sure a future version of the validator will issue a warning if some user selects the new enum value without applying the extension that enables that value.

[emackey]

Continued in #2147