495798c00b
7 Commits
| Author | SHA1 | Message | Date | |
|---|---|---|---|---|
Greeble
|
7e45e8635c |
Reduce dependencies on bevy_render by preferring bevy_mesh imports (#18437)
Reduce dependencies on `bevy_render` by preferring `bevy_mesh` imports over `bevy_render` re-exports. ```diff - use bevy_render::mesh::Mesh; + use bevy_mesh::Mesh; ``` This is intended to help with #18423 (render crate restructure). Affects `bevy_gltf`, `bevy_animation` and `bevy_picking`. As part of #18423, I'm assuming there'll be a push to make crates less dependent on the big render crates. This PR seemed like a small and safe step along that path - it only changes imports and makes the `bevy_mesh` crate dependency explicit in `Cargo.toml`. Any remaining dependencies on `bevy_render` are true dependencies. ``` cargo run --example testbed_3d cargo run --example mesh_picking ``` |
||
|
Gino Valente
|
9b32e09551
|
bevy_reflect: Add clone registrations project-wide (#18307)
# Objective Now that #13432 has been merged, it's important we update our reflected types to properly opt into this feature. If we do not, then this could cause issues for users downstream who want to make use of reflection-based cloning. ## Solution This PR is broken into 4 commits: 1. Add `#[reflect(Clone)]` on all types marked `#[reflect(opaque)]` that are also `Clone`. This is mandatory as these types would otherwise cause the cloning operation to fail for any type that contains it at any depth. 2. Update the reflection example to suggest adding `#[reflect(Clone)]` on opaque types. 3. Add `#[reflect(clone)]` attributes on all fields marked `#[reflect(ignore)]` that are also `Clone`. This prevents the ignored field from causing the cloning operation to fail. Note that some of the types that contain these fields are also `Clone`, and thus can be marked `#[reflect(Clone)]`. This makes the `#[reflect(clone)]` attribute redundant. However, I think it's safer to keep it marked in the case that the `Clone` impl/derive is ever removed. I'm open to removing them, though, if people disagree. 4. Finally, I added `#[reflect(Clone)]` on all types that are also `Clone`. While not strictly necessary, it enables us to reduce the generated output since we can just call `Clone::clone` directly instead of calling `PartialReflect::reflect_clone` on each variant/field. It also means we benefit from any optimizations or customizations made in the `Clone` impl, including directly dereferencing `Copy` values and increasing reference counters. Along with that change I also took the liberty of adding any missing registrations that I saw could be applied to the type as well, such as `Default`, `PartialEq`, and `Hash`. There were hundreds of these to edit, though, so it's possible I missed quite a few. That last commit is **_massive_**. There were nearly 700 types to update. So it's recommended to review the first three before moving onto that last one. Additionally, I can break the last commit off into its own PR or into smaller PRs, but I figured this would be the easiest way of doing it (and in a timely manner since I unfortunately don't have as much time as I used to for code contributions). ## Testing You can test locally with a `cargo check`: ``` cargo check --workspace --all-features ``` |
||
|
Zachary Harrold
|
a6adced9ed
|
Deny derive_more error feature and replace it with thiserror (#16684)
# Objective - Remove `derive_more`'s error derivation and replace it with `thiserror` ## Solution - Added `derive_more`'s `error` feature to `deny.toml` to prevent it sneaking back in. - Reverted to `thiserror` error derivation ## Notes Merge conflicts were too numerous to revert the individual changes, so this reversion was done manually. Please scrutinise carefully during review. |
||
|
Stepan Koltsov
|
465d1139e7
|
Replace TwoIterators with Either in bevy_animation (#16036)
# Objective - Less code - Better iterator (implements `size_hint` for example) ## Solution - Use `either` - This change is free because `bevy_animation` depends on `bevy_asset`, which already depends on `either` ## Testing CI |
||
|
Zachary Harrold
|
814f8ec039
|
Remove thiserror from bevy_animation (#15780)
# Objective - Contributes to #15460 ## Solution - Removed `thiserror` from `bevy_animation` |
||
|
Matty
|
528ca4f95e
|
Eliminate redundant clamping from sample-interpolated curves (#15620)
# Objective Currently, sample-interpolated curves (such as those used by the glTF loader for animations) do unnecessary extra work when `sample_clamped` is called, since their implementations of `sample_unchecked` are already clamped. Eliminating this redundant sampling is a small, easy performance win which doesn't compromise on the animation system's internal usage of `sample_clamped`, which guarantees that it never samples curves out-of-bounds. ## Solution For sample-interpolated curves, define `sample_clamped` in the way `sample_unchecked` is currently defined, and then redirect `sample_unchecked` to `sample_clamped`. This is arguably a more idiomatic way of using the `cores` as well, which is nice. ## Testing Ran `many_foxes` to make sure I didn't break anything. |
||
|
Matty
|
429987ebf8
|
Curve-based animation (#15434)
# Objective This PR extends and reworks the material from #15282 by allowing arbitrary curves to be used by the animation system to animate arbitrary properties. The goals of this work are to: - Allow far greater flexibility in how animations are allowed to be defined in order to be used with `bevy_animation`. - Delegate responsibility over keyframe interpolation to `bevy_math` and the `Curve` libraries and reduce reliance on keyframes in animation definitions generally. - Move away from allowing the glTF spec to completely define animations on a mechanical level. ## Solution ### Overview At a high level, curves have been incorporated into the animation system using the `AnimationCurve` trait (closely related to what was `Keyframes`). From the top down: 1. In `animate_targets`, animations are driven by `VariableCurve`, which is now a thin wrapper around a `Box<dyn AnimationCurve>`. 2. `AnimationCurve` is something built out of a `Curve`, and it tells the animation system how to use the curve's output to actually mutate component properties. The trait looks like this: ```rust /// A low-level trait that provides control over how curves are actually applied to entities /// by the animation system. /// /// Typically, this will not need to be implemented manually, since it is automatically /// implemented by [`AnimatableCurve`] and other curves used by the animation system /// (e.g. those that animate parts of transforms or morph weights). However, this can be /// implemented manually when `AnimatableCurve` is not sufficiently expressive. /// /// In many respects, this behaves like a type-erased form of [`Curve`], where the output /// type of the curve is remembered only in the components that are mutated in the /// implementation of [`apply`]. /// /// [`apply`]: AnimationCurve::apply pub trait AnimationCurve: Reflect + Debug + Send + Sync { /// Returns a boxed clone of this value. fn clone_value(&self) -> Box<dyn AnimationCurve>; /// The range of times for which this animation is defined. fn domain(&self) -> Interval; /// Write the value of sampling this curve at time `t` into `transform` or `entity`, /// as appropriate, interpolating between the existing value and the sampled value /// using the given `weight`. fn apply<'a>( &self, t: f32, transform: Option<Mut<'a, Transform>>, entity: EntityMutExcept<'a, (Transform, AnimationPlayer, Handle<AnimationGraph>)>, weight: f32, ) -> Result<(), AnimationEvaluationError>; } ``` 3. The conversion process from a `Curve` to an `AnimationCurve` involves using wrappers which communicate the intent to animate a particular property. For example, here is `TranslationCurve`, which wraps a `Curve<Vec3>` and uses it to animate `Transform::translation`: ```rust /// This type allows a curve valued in `Vec3` to become an [`AnimationCurve`] that animates /// the translation component of a transform. pub struct TranslationCurve<C>(pub C); ``` ### Animatable Properties The `AnimatableProperty` trait survives in the transition, and it can be used to allow curves to animate arbitrary component properties. The updated documentation for `AnimatableProperty` explains this process: <details> <summary>Expand AnimatableProperty example</summary An `AnimatableProperty` is a value on a component that Bevy can animate. You can implement this trait on a unit struct in order to support animating custom components other than transforms and morph weights. Use that type in conjunction with `AnimatableCurve` (and perhaps `AnimatableKeyframeCurve` to define the animation itself). For example, in order to animate font size of a text section from 24 pt. to 80 pt., you might use: ```rust #[derive(Reflect)] struct FontSizeProperty; impl AnimatableProperty for FontSizeProperty { type Component = Text; type Property = f32; fn get_mut(component: &mut Self::Component) -> Option<&mut Self::Property> { Some(&mut component.sections.get_mut(0)?.style.font_size) } } ``` You can then create an `AnimationClip` to animate this property like so: ```rust let mut animation_clip = AnimationClip::default(); animation_clip.add_curve_to_target( animation_target_id, AnimatableKeyframeCurve::new( [ (0.0, 24.0), (1.0, 80.0), ] ) .map(AnimatableCurve::<FontSizeProperty, _>::from_curve) .expect("Failed to create font size curve") ); ``` Here, the use of `AnimatableKeyframeCurve` creates a curve out of the given keyframe time-value pairs, using the `Animatable` implementation of `f32` to interpolate between them. The invocation of `AnimatableCurve::from_curve` with `FontSizeProperty` indicates that the `f32` output from that curve is to be used to animate the font size of a `Text` component (as configured above). </details> ### glTF Loading glTF animations are now loaded into `Curve` types of various kinds, depending on what is being animated and what interpolation mode is being used. Those types get wrapped into and converted into `Box<dyn AnimationCurve>` and shoved inside of a `VariableCurve` just like everybody else. ### Morph Weights There is an `IterableCurve` abstraction which allows sampling these from a contiguous buffer without allocating. Its only reason for existing is that Rust disallows you from naming function types, otherwise we would just use `Curve` with an iterator output type. (The iterator involves `Map`, and the name of the function type would have to be able to be named, but it is not.) A `WeightsCurve` adaptor turns an `IterableCurve` into an `AnimationCurve`, so it behaves like everything else in that regard. ## Testing Tested by running existing animation examples. Interpolation logic has had additional tests added within the `Curve` API to replace the tests in `bevy_animation`. Some kinds of out-of-bounds errors have become impossible. Performance testing on `many_foxes` (`animate_targets`) suggests that performance is very similar to the existing implementation. Here are a couple trace histograms across different runs (yellow is this branch, red is main). <img width="669" alt="Screenshot 2024-09-27 at 9 41 50 AM" src="https://github.com/user-attachments/assets/5ba4e9ac-3aea-452e-aaf8-1492acc2d7fc"> <img width="673" alt="Screenshot 2024-09-27 at 9 45 18 AM" src="https://github.com/user-attachments/assets/8982538b-04cf-46b5-97b2-164c6bc8162e"> --- ## Migration Guide Most user code that does not directly deal with `AnimationClip` and `VariableCurve` will not need to be changed. On the other hand, `VariableCurve` has been completely overhauled. If you were previously defining animation curves in code using keyframes, you will need to migrate that code to use curve constructors instead. For example, a rotation animation defined using keyframes and added to an animation clip like this: ```rust animation_clip.add_curve_to_target( animation_target_id, VariableCurve { keyframe_timestamps: vec![0.0, 1.0, 2.0, 3.0, 4.0], keyframes: Keyframes::Rotation(vec![ Quat::IDENTITY, Quat::from_axis_angle(Vec3::Y, PI / 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.), Quat::IDENTITY, ]), interpolation: Interpolation::Linear, }, ); ``` would now be added like this: ```rust animation_clip.add_curve_to_target( animation_target_id, AnimatableKeyframeCurve::new([0.0, 1.0, 2.0, 3.0, 4.0].into_iter().zip([ Quat::IDENTITY, Quat::from_axis_angle(Vec3::Y, PI / 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 2.), Quat::from_axis_angle(Vec3::Y, PI / 2. * 3.), Quat::IDENTITY, ])) .map(RotationCurve) .expect("Failed to build rotation curve"), ); ``` Note that the interface of `AnimationClip::add_curve_to_target` has also changed (as this example shows, if subtly), and now takes its curve input as an `impl AnimationCurve`. If you need to add a `VariableCurve` directly, a new method `add_variable_curve_to_target` accommodates that (and serves as a one-to-one migration in this regard). ### For reviewers The diff is pretty big, and the structure of some of the changes might not be super-obvious: - `keyframes.rs` became `animation_curves.rs`, and `AnimationCurve` is based heavily on `Keyframes`, with the adaptors also largely following suite. - The Curve API adaptor structs were moved from `bevy_math::curve::mod` into their own module `adaptors`. There are no functional changes to how these adaptors work; this is just to make room for the specialized reflection implementations since `mod.rs` was getting kind of cramped. - The new module `gltf_curves` holds the additional curve constructions that are needed by the glTF loader. Note that the loader uses a mix of these and off-the-shelf `bevy_math` curve stuff. - `animatable.rs` no longer holds logic related to keyframe interpolation, which is now delegated to the existing abstractions in `bevy_math::curve::cores`. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> Co-authored-by: aecsocket <43144841+aecsocket@users.noreply.github.com> |