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

Animation Crate #482

Closed
wants to merge 31 commits into from
Closed

Animation Crate #482

wants to merge 31 commits into from

Conversation

mcpar-land
Copy link

@mcpar-land mcpar-land commented Sep 12, 2020

Adds the bevy_animation crate, components to add spline animations, and two examples.

Components

  • AnimationSplineTransform automatically animates any entity it is a part of that has the Translation, Rotation and Scale components Transform component as of Bevy 0.2.1.
  • AnimationSplineOne exposes an automatically-animated f32 value that a developer can retrieve in their own systems.
  • AnimationSplineThree fills the same function as AnimationSplineOne, but for Vec3 values.

SplineGroup

All 3 components implement the trait SplineGroup, which requires get / mutable get functions for the properties:

  • loop_style, an enum to determine if an animation plays once, loops, or 'ping-pongs' back and forth.
  • time, the current time of the animation
  • speed the speed multiplier of the animation
  • paused a boolean for pausing the animation
  • pong, a boolean that is true if the animation is ping-pong-ing in reverse.

It also offers default implementations for useful functions:

  • pause
  • play
  • toggle_pause
  • is_empty
  • start_time
  • end_time
  • duration
  • advance by a particular amount of time
  • sample at a particular time
  • current to sample at the current time

For the default SplineGroup components, I think it's about striking a balance between what values are animated frequently enough to warrant having a built-in system for animating them, and what developers should be left to hook up to generic SplineGroups themselves with custom systems, or create their own SplineGroups

Improvements

  • I am not sure if the ping-pong functionality works properly. Looping and no-looping should work, though. Ping Pong works as of 39207e4
  • The AnimationSplineThree returns samples of a struct I made, Vec3Option, instead of a raw Vec3, to make it easy to not mutate one of the three tracks (x / y / z) that has no keyframes on it. This might be better served by returning something like (Vec3, Vec3Mask) instead of making a whole new struct that may see too little use.

@memoryruins memoryruins added the C-Feature A new feature, making something new possible label Sep 13, 2020
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/vec3_option.rs Outdated Show resolved Hide resolved
(based on PR suggestions and clippy)
@voxelias
Copy link
Contributor

voxelias commented Sep 16, 2020

I can't build it on archlinux :(

cargo run --example animation_custom

ends up with:

error[E0658]: use of unstable library feature 'option_zip'
  --> crates/bevy_animation/src/spline_group.rs:85:14
   |
85 |             .zip(self.end_time())
   |              ^^^
   |
   = note: see issue #70086 <https://github.com/rust-lang/rust/issues/70086> for more information
   = help: add `#![feature(option_zip)]` to the crate attributes to enable

error: aborting due to previous error

For more information about this error, try `rustc --explain E0658`.
error: could not compile `bevy_animation`.

To learn more, run the command again with --verbose.
warning: build failed, waiting for other jobs to finish...
error: build failed

EDIT:
rustup update solved the issue

crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
crates/bevy_animation/src/spline_group.rs Outdated Show resolved Hide resolved
@cart
Copy link
Member

cart commented Sep 22, 2020

I'll be taking a look a this soon. This will require a thorough review + some thought on my part and I want to give it the time it deserves. I need to sort out some asset related stuff first, as thats blocking a number of projects.

@iwikal
Copy link
Contributor

iwikal commented Oct 18, 2020

I'm looking at extending this crate to enable animating rotations using Quat. It's tricky, since the splines crate works with Add<Self> and Mul<f32>, while the corresponding operations for a Quat in a slerp would be Mul<Self> and raising to an f32 power. I managed to get it kinda-sorta-working using a wrapper around Quat, but it's a bit buggy at the moment, and I would prefer to not have that show up in Bevy's public API. I'm thinking the best approach would be to wrap the entire splines crate in a new API that accepts Quats and f32-s specifically. Another option is to fork splines or re-implement it entirely, but that adds some extra maintenance burden.
Tangentially, we might want this API to support normalized lerp, which looks decently similar to a slerp for small rotations, and is more performant, so it's an option some people might want to use.

I also looked at piston's handling of rotating animations, and the code handles regular quaternions as well as something called "dual quaternions", linking to this article. https://dcgi.felk.cvut.cz/home/zara/papers/TCD-CS-2006-46.pdf
I'm not really sure what that is about. It seems to concern skinned meshes, which I imagine is something Bevy wants to support.

@mcpar-land
Copy link
Author

I think that a AnimationSplineQuat would be a good addition, even though I'm not the best at quaternion math. figuring out which kinds of data are common enough animation targets to warrant an included component for them will be something to be felt out over time, as there's no correct answer.

@iwikal
Copy link
Contributor

iwikal commented Oct 19, 2020

I was thinking that AnimationSplineTransform should use Quats instead of euler angles, but maybe I'm mistaken in assuming you would never want to interpolate through euler angles? I guess that's desirable sometimes, now that I think about it, such as when animating something that rotates on a double joint with equal rotational speed on both axes of the joint, instead of along the shortest path from one orientation to the other.

@iwikal
Copy link
Contributor

iwikal commented Oct 19, 2020

A totally different approach to implementing this would be to have the actual code running be much less generic, and instead re-sample the curves more finely up front, and use simple linear interpolations between these finer keyframes. That would allow the same code to approximate all variations of these complex behaviors, with some runtime cost amortized to creation of the spline, and more memory usage.

@mcpar-land
Copy link
Author

The only thing keeping me from implementing anything with Quats so far is a lack of understanding of them on my part 😅 Swapping out for them on rotation interpolations would be better, yes, though I think an option for developers to animate in the more intuitive "Euler Space" is necessary. If we ever want to translate this over to a curve editor in a UI, those never use the four Quat values as the rotation curves.

@iwikal
Copy link
Contributor

iwikal commented Oct 22, 2020 via email

@iwikal
Copy link
Contributor

iwikal commented Oct 25, 2020

I've opened a pull request to glam that adds support for adding and subtracting quats, which would allow the splines crate to ship with a glam feature that implements all the required traits. If it turns out they're not interested in this feature, the splines crate could also potentially be refactored to do the same anyway, but without the help of the plus and minus operators, so the code gets a bit ugly.

@iwikal
Copy link
Contributor

iwikal commented Oct 27, 2020

Glam decided to merge my pull request, so in a few weeks they should have a new version released, which means splines can merge this, and then we can use Quats directly in spline keys.

@kirawi
Copy link

kirawi commented Nov 24, 2020

Haven't really looked into what the crate does since it all goes over my head, but are these components for a future animation system to build upon? 2D and 3D?

@mcpar-land
Copy link
Author

mcpar-land commented Nov 24, 2020

It's a combination of components that either provide number(s) that change over time based on a spline for you to hook up to your components with a system, and one that directly changes a transform on its own. It'll work for 2D and 3D.

@iwikal
Copy link
Contributor

iwikal commented Nov 26, 2020

Note that the spline implementation for Quats uses a normalized linear interpolation. This is fine and actually preferable when they don't rotate too far from one key to the next, but exhibits a nonlinear speed when the distance becomes greater. If the animations are imported from some kind of editor, the editor could generate lots of keys in between the user-defined poses to cover this up, but if you simply feed it quats with like 90 degree increments it will look wrong. I've looked at trying to create an extension trait to Spline that resamples sparse keys to a finer resolution, but I've only got it working for Interpolation::Linear and Interpolation::Step so far.
https://github.com/mcpar-land/bevy/pull/1/files#diff-e787f433ee133425b3bba409c0196636a2b16f12f1e597f9185769b0321eaf5aR75-R121

@iwikal
Copy link
Contributor

iwikal commented Nov 30, 2020

Looks like bevy_type_registry and bevy_property were deleted, so we need to remove all references to those. Also, I noticed some error about time.delta_seconds now being a method instead of a public field.

@mcpar-land
Copy link
Author

I did just kind of toss the merge together, sorry about that. I'll try and correct it come tomorrow.

@alice-i-cecile alice-i-cecile added help wanted A-Rendering Drawing game state to the screen labels Feb 17, 2021
@alice-i-cecile alice-i-cecile mentioned this pull request Feb 17, 2021
Base automatically changed from master to main February 19, 2021 20:44
@alice-i-cecile alice-i-cecile added the A-Animation Make things move and change over time label Apr 7, 2021
@alice-i-cecile alice-i-cecile added the S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged label Apr 23, 2021
@mockersf mockersf added the S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work label Jul 17, 2021
@alice-i-cecile
Copy link
Member

Closing out. This is useful prior art, but is extremely unlikely to be merged in its current form.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
A-Animation Make things move and change over time A-Rendering Drawing game state to the screen C-Feature A new feature, making something new possible S-Needs-Design-Doc This issue or PR is particularly complex, and needs an approved design doc before it can be merged S-Pre-Relicense This PR was made before Bevy added the Apache license. Cannot be merged or used for other work
Projects
None yet
Development

Successfully merging this pull request may close these issues.

9 participants