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

Use LINEAR interpolation when sampling anims #417

Merged
merged 3 commits into from Apr 15, 2019

Conversation

pjoe
Copy link
Contributor

@pjoe pjoe commented Apr 10, 2019

Not sure why this ever used STEP :)

With LINEAR you can get decent result with sampling every few frames (e.g. 3-4 depending on your animation), instead of the chunky result you will get with STEP

@pjoe
Copy link
Contributor Author

pjoe commented Apr 10, 2019

Sample file, with sampling every 4 frames:

anim_sampling.zip

Tried capturing the difference in gifs

STEP:
step

LINEAR:
linear

Copy link
Contributor

@UX3D-becher UX3D-becher left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Although the glTF specification to my knowledge would allow single keyframes with linear interpolation, some engines expect at least two. In order to avoid breaking things for these probably very common cases, we have to resort to 'STEP' interpolation.
A user that does not want STEP interpolation at the moment can bake the animation manually.
If we want to automate the baking process, animations with only a single control point have to explicitly be handled

@pjoe
Copy link
Contributor Author

pjoe commented Apr 10, 2019

@UX3D-becher OK, I understand the issue, but for end user this really should be handled automatically. I'll try to see how difficult it is to handle the case with only 1 keyframe.

Having to manually bake anims is a complication of the workflow, and also to some extend makes the export process less 'non-destructive'.

@pjoe
Copy link
Contributor Author

pjoe commented Apr 10, 2019

@UX3D-becher something like this should do it 92c50f7

@emackey
Copy link
Member

emackey commented Apr 10, 2019

@UX3D-becher I find that reasoning a little odd. Wouldn't it be better to file bug reports against engines that can't handle an individual linear keyframe?

I guess having a workaround here doesn't hurt much, but thus far the pattern has been to insist on filing issues and fixing problems rather than bending the format spec to accommodate bugs.

Copy link
Member

@emackey emackey left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm in favor of this, with or without the one-keyframe workaround.

@emackey
Copy link
Member

emackey commented Apr 10, 2019

Fixes #369

@donmccurdy
Copy link
Contributor

Agreed that LINEAR interpolation should be used for baking whenever possible. Engines are not rendering at 24-30 fps, which the keyframes usually are. Whether single-keyframe tracks should use STEP or LINEAR I'm not sure.

@donmccurdy
Copy link
Contributor

donmccurdy commented Apr 10, 2019

Hm, but this will go to STEP for all channels, if any channel has a single keyframe? I might prefer LINEAR in that case, unless it's easy enough to specify per-channel.

@emackey
Copy link
Member

emackey commented Apr 10, 2019

Ouch. Would definitely prefer LINEAR in that case.

@pjoe
Copy link
Contributor Author

pjoe commented Apr 10, 2019

I am all for always using LINEAR and would consider it a weird edge case / bug if engines don't handle it correctly. So I can easily revert last commit if we agree on this? Or try to add further detection of single channel (won't there always be 3 or 4 together?) Anyway this PR improves behavior in most common cases (more then 1 key), so can also be merged as is and we can further tackle the single key/STEP issue in separate PR

@pjoe
Copy link
Contributor Author

pjoe commented Apr 11, 2019

After thinking/testing this some more:

I believe the best option is to use LINEAR if any of the channels in a sampler has more than 1 keyframe,
since loc/scale/rot will get each their sampler and glTF will batch together each subcomponent, i.e. glTF doesn't support having different interpolation (or number of keys) for e.g. loc.x and loc.y anyway.

If anyone can come up with a case where this would break - speak up 😄

Otherwise I think it is good to go now

@emackey
Copy link
Member

emackey commented Apr 12, 2019

Any objections @UX3D-becher @donmccurdy ?

@donmccurdy
Copy link
Contributor

Sounds good to me. I could imagine CUBICSPLINE causing problems with a single keyframe, but STEP and LINEAR seem completely reasonable. If there's a bug in e.g. threejs we can certainly fix those.

@emackey emackey merged commit 0b1eb68 into KhronosGroup:master Apr 15, 2019
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

Successfully merging this pull request may close these issues.

None yet

4 participants