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

Must slerping between quaternions for an animation curve always go the short way round? #1395

Closed
scurest opened this issue Jul 17, 2018 · 8 comments · Fixed by #2065
Closed

Comments

@scurest
Copy link

scurest commented Jul 17, 2018

ie. when interpolating from q1 to q2, should q2 be replaced with whichever of {q2, -q2} is closest to q1?

Please see KhronosGroup/glTF-Sample-Models#185.

The text in the spec is

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

Apparently most implementors have read this to mean that the short way round must be used but this is not implicit in the text.

And what about cubic interpolation?

@bghgary
Copy link
Contributor

bghgary commented Jul 17, 2018

And what about cubic interpolation?

I don't think there are multiple ways to interpret the cubic interpolation. The math in the appendix is what should be used to get the resulting value.

ksons pushed a commit to ksons/gltf-blender-importer that referenced this issue Aug 15, 2018
* Use empties instead of an armature for glTF nodes

This removes support for skinning.

* configure the camera from the gltf data

* camera/create_camera: intentionally return an uninitialized camera for an unexpected type

* camera/create_camera: handle a missing zfar in the perspective data

* camera/create_camera: fallback behavior to handle a missing apesctRatio

* create_camera: the glTF yfov must clearly go to the property `angle_y`
of the Blender camera, while the `aspectRatio`, if present, affects
the resolution instead of the `angle_x` (since the latter is computed
from the `angle_y` and the resolution in Blender)

*  removed auto smoothing

* Tidy up __init__ and unify the caches...

...using just one cache-lookup function instead of one for every
different type of object.

* Tidy up the mesh creator and enable RGBA colors

RGBA vertex colors are supported (required) by Blender 2.79 it
appears.

* Begin implementing a new node creator

Based on creating first a virtual forest that mirrors the scene
graph we're about to create in Blender. Details to be documented
later... if it works out :)

* Creates skins and attach them to armatures

Skinning is now working (again (again))...

* WIP animation loading

Messy and VERY slow for the moment, but functional so I'm commiting
it for my own reference.

* Put animation handling in order

Comments explained the main reasoning behind the bone function.

Also switching from keyframe_points.insert to the add/co loop
gave a **huge** performance boost.

* Use memoryviews for buffers and buffer views.

This is for faster slicing.

This doesn't make any apparent impact on import times but hopefully
it saves some memory ^^;

* Cleanup and explain how the vforest works

* Note that Blender 2.79 is needed

...because 4-component vertex colors are assumed.

Should we use some shims to papers over the Blender version
differences and get greater support? I don't have access to !=2.79
vresion atm though so it doesn't matter for now.

* Minor tweaks

* Put cameras in the scene

They don't point the right way or anything but they're there :)

* Handle creating scenes again

But have an option to just put everything into the current scene
too (I like that better).

* Add more import options

* Cache images used for texturing

This prevents loading the same one multiple times.

* Autogenerate node group data

Using the groups from the exporter, thus relieving us form writing
all our own node groups.

The group for Specular Glossiness is also part of the serialized
data, so supporting that is getting closer.

* Small work-around to enable vertex color influence

We add superfluous COLOR_0 layers to any mesh that uses a material
that is ever applied to a primitive that has a COLOR_0 attribute.
This let's us add an Attribute node to those materials without the
problem of solid red colors coming in.

COLOR_1, etc. are still not supported (well, they're imported of
course, they just don't influence the materials).

* Tighten up scene graph creation

- Avoid creating dummy vnodes for meshes when possible.
- Rotate cameras to their (correct?) position
- Some slightly tighter docs.
- Some more robustness in realizing vnodes; it would probably be
  possible to use an armature for the whole forest again with minimal
  code changes

* Fix some typos in animation.py

Animating non-bone locations should work now. Curve clean-up should
be working too.

* Avoid bone->nonbone->bone child relations

The vtree beneath an armature should now be all bones except
possibly at the leaves, and we should realize them correctly(?)
now.

Also fix a typo with mesh names.

* Rename node.py -> scene.py

Avoids confusion with the similarly named node_groups stuff.

* Large push towards full material implementation

Support for pbrSpecularGlossiness was added here too.

* Fill in sampler properties

Such as they are, anyway.

* Support KHR_materials_unlit extension

* Gate animation importing behind a flag...

...since it's still weird. Make the options prettier and ignore
scenes by default too.

* Allow for >4 vertex weights

...by using multiple attributes sets. We anticipate here, since
though there is a PR for this, it hasn't been merged into the spec
yet.

* Bugfix: do not modify vertex color accessor arrays

If the accessor was used again later in the glTF, we would have
seen a modified array since we hard-modified the accessor elements.
Avoid doing this.

* Bugfix: don't set vertex weights of 0.0

If the joints were eg. [0,0,0,0] and the weights were [1,0,0,0],
previously we would overwrite the weight for 0 three times so it
ended up as 0. Now it will correctly be 1.

* Support MSFT_texture_dds extension.

We hardly have to do anything since Blender will already handle DDS
textures so we might as well. Untested though.

* Implement KHR_texture_transform

Untested.

* Greatly simplify animation importing

I initially thought the pose TRS was computed from (final TRS) *
(rest TRS)^{-1} (ie. the opposite order of multiplication from what
I now believe is correct.) Under this scheme, the pose components
could depend on multiple final components, which required bringing
the curves under a common domain. But under the correct order of
multiplication each pose components depends only on its corresponding
final component (and the rest TRS) so that turns out to have been
over-complicated and, thank God, we can finally get rid of it!

Though, alas! it does not fix Monster.

* Add COLOR_X, etc. layers in order of X

This ensures that COLOR_0 is always the first (and ensures Blender
thinks its the default, for eg. TEXCOORD_0 and UV maps?) and avoids
any gaps in the sequence of X.

* Slightly better bones

- Pick better bone lengths (when the bones are rotated the armature
  actually looks plausible now)
- Report a warning when bones had non-unit scalings in the glTF

* Only add vertices referenced by the indices (#29)

* Work-around re vertex colors for Blender 2.79.0

* Set animation curve interpolation

Also fix a typo that mysteriously did not prevent Blender from
figuring out what the fcurves were targeting.

* Allow the user to rotate bones

For example, try rotating the bones in CesiumMan or Monster by 90d
in the X direction to get the skeleton to look correct.

I am somewhat uncertain of the math for this.

* Rotate the short way for linear interpolation

See KhronosGroup/glTF/issues/1395. Observe the difference in
AnimatedTriangle or the low-poly fox.

* Be noiser when we put a node in the wrong place

Just so the user knows. Very minor, theoreteical fix for bone
lengths too.

* Polish armature/bone creation pass

The restriction about overlapping armatures is lifted and we no
longer require a skeleton property.

* Add groups to animations

This makes it more like what you'd by adding keyframes by hitting I

* Fix action creation

Fix a typo that created superfluous actions.
Always make the first animation the one that plays by default.
Fix setting of the default action on armatures.
Don't let Blender delete unused actions.

* Fix regression of #16
@pjcozzi
Copy link
Member

pjcozzi commented Jan 24, 2019

Also see #1515

@stevenvergenz
Copy link

Any official word on this? Is short-way around assumed? This is inconsistent between players right now (Windows 3D Viewer vs Unity).

@lexaknyazev
Copy link
Member

lexaknyazev commented Oct 3, 2021

Two consecutive rotation quaternions define a great circle on which the intermediate values are located. Technically, the sign of their dot product defines whether the short or the long path is selected for the interpolated values.

For example with three intermediate points,

  • Positive dot product

    • q0 = [0, 0, 0.7071, 0.7071] (+90° around the Z axis)
    • q1 = [0, 0, 1, 0] (+180° around the Z axis)
    • Interpolated angles (each step is +22.5°): 90°, 112.5°, 135°, 157.5°, 180°.
  • Negative dot product

    • q0 = [0, 0, 0.7071, 0.7071] (+90° around the Z axis)
    • q1 = [0, 0, -1, 0] (-180° around the Z axis)
    • Interpolated angles (each step is -67.5°): 90°, 22.5°, 315°, 247.5°, 180°.

Since the spec is silent on this topic, we have three options:

  • State that each two consecutive quaternions denote the short path, regardless of their dot product sign.
    • Pros: seems like most engines already assume that.
    • Cons: slightly less efficient data storage in some cases, extra runtime logic, some engines may have different assumptions.
  • State that consecutive quaternions must not have negative dot products, leave interpretation of such data undefined.
    • Pros: no engine updates.
    • Cons: invalidating some models (including our samples), putting the extra burden on exporters.
  • State that the path selection depends on the dot product sign.
    • Pros: potentially more efficient data storage, less runtime operations.
    • Cons: likely breaking the ecosystem.

@emackey @bghgary @donmccurdy @javagl WDYT?

@emackey
Copy link
Member

emackey commented Oct 4, 2021

@scurest As a point of reference, are any of these better or worse for Blender's sake? Do you happen to have an opinion on which would be best for the glTF ecosystem?

@scurest
Copy link
Author

scurest commented Oct 4, 2021

IMO (1) (two consecutive quaternions denote the short path, regardless of their dot product sign) is already the case (based on the sample and what implementations actually do) and just needs to be added to the text.

I don't think there are any particular implications for Blender though beyond that obviously (1) is already what is implemented there.

@emackey
Copy link
Member

emackey commented Oct 4, 2021

Agreed, I think sticking to (1) for the sake of minimal breakage is the right path here. Thanks @scurest.

@lexaknyazev
Copy link
Member

If we specify (1) as the required interpretation, I think we should still recommend exporters to generate continuous quaternion sequence.

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

Successfully merging a pull request may close this issue.

6 participants