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

Skeletal animations require position, rotation and scale on every key, and instead they should be independent of each other #6065

Closed
kdrnic opened this issue Feb 13, 2015 · 8 comments

Comments

@kdrnic
Copy link

kdrnic commented Feb 13, 2015

Currently skeletal animation keys as passed to THREE.Animation constructor are like this:

{
   pos: [ , , ],
   rot: [ , , , ],
   scl: [ , , ],
   time: 
};

the problem is that it is required that both pos, rot and scl fields be present.
This is problematic depending on the animation.
Suppose we have a bone animated like this:
Frame 1) Set rotation, set position
Frame 2) Interpolate rotation, interpolate position
Frame 3) Set rotation, interpolate position
Frame 5) Interpolate rotation, interpolate position
Frame 6) Interpolate rotation, set position
Frame 7) Interpolate rotation, interpolate position
Frame 8) Set rotation, interpolate position
etc. etc.
it is impossible to pass such animation to three, because some keys will have either rotation or position set and expect the other to be interpolated from values defined in a previous key.

If a key is passed without the pos array, it will give errors of "Cannot read property '0' of undefined" in three.js:29612.

If a key is passed without the rot array, it will give errors of "Cannot read property 'x' of undefined" in three.js:728.

This has become quite annoying as I am trying to correctly implement a loader for a format that allows keyframes to have only position or only rotation set.

@mrdoob
Copy link
Owner

mrdoob commented Feb 18, 2015

So the workaround is to create interpolated values in the data or zeroed values?

@makc
Copy link
Contributor

makc commented Feb 19, 2015

to create interpolated values

you would have a whole layer of code on top of this to make it work. people will want to export curves. makes sense from the file size point of view.

@mrdoob
Copy link
Owner

mrdoob commented Feb 19, 2015

Yes. I just wanted to double-check that I understood the problem.

@Dynasty9
Copy link

With the current setup is is possible to animate the upper half with one animation and the lower with another?

@kdrnic
Copy link
Author

kdrnic commented Apr 5, 2015

@mrdoob wouldn't this be more fit as an Improvement rather than a suggestion (I am talking about the label)?

@mrdoob
Copy link
Owner

mrdoob commented Apr 5, 2015

😊

@kdrnic
Copy link
Author

kdrnic commented Jul 22, 2015

kdrnic/three-mm3d#1 was closed by making a function applied to the data before it is used to construct THREE.Animation. It checks for keys lacking pos or rot, looks for the closest previous key and closest next key not lacking, and interpolates linearly.

@donmccurdy
Copy link
Collaborator

The THREE.Animation class no longer exists, and as far as I know this case works correctly with the new animation system. Please reopen if this is still reproducible.

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

No branches or pull requests

5 participants