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

Smooth following via generalized interpolation #13613

Closed
wants to merge 5 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
11 changes: 11 additions & 0 deletions Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -2993,6 +2993,17 @@ description = "Demonstrates how to sample random points from mathematical primit
category = "Math"
wasm = true

[[example]]
name = "smooth_follow"
path = "examples/math/smooth_follow.rs"
doc-scrape-examples = true

[package.metadata.example.smooth_follow]
name = "Smooth Follow"
description = "Demonstrates how to make an entity smoothly follow another using interpolation"
category = "Math"
wasm = true

# Gizmos
[[example]]
name = "2d_gizmos"
Expand Down
46 changes: 1 addition & 45 deletions crates/bevy_animation/src/animatable.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,3 @@
use crate::util;
use bevy_color::{Laba, LinearRgba, Oklaba, Srgba, Xyza};
use bevy_ecs::world::World;
use bevy_math::*;
Expand All @@ -16,12 +15,7 @@ pub struct BlendInput<T> {
}

/// An animatable value type.
pub trait Animatable: Reflect + Sized + Send + Sync + 'static {
/// Interpolates between `a` and `b` with a interpolation factor of `time`.
///
/// The `time` parameter here may not be clamped to the range `[0.0, 1.0]`.
fn interpolate(a: &Self, b: &Self, time: f32) -> Self;

pub trait Animatable: Reflect + Interpolate + Sized + Send + Sync + 'static {
/// Blends one or more values together.
///
/// Implementors should return a default value when no inputs are provided here.
Expand All @@ -35,12 +29,6 @@ pub trait Animatable: Reflect + Sized + Send + Sync + 'static {
macro_rules! impl_float_animatable {
($ty: ty, $base: ty) => {
impl Animatable for $ty {
#[inline]
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
let t = <$base>::from(t);
(*a) * (1.0 - t) + (*b) * t
}

#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut value = Default::default();
Expand All @@ -60,12 +48,6 @@ macro_rules! impl_float_animatable {
macro_rules! impl_color_animatable {
($ty: ident) => {
impl Animatable for $ty {
#[inline]
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
let value = *a * (1. - t) + *b * t;
value
}

#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut value = Default::default();
Expand Down Expand Up @@ -100,11 +82,6 @@ impl_color_animatable!(Xyza);

// Vec3 is special cased to use Vec3A internally for blending
impl Animatable for Vec3 {
#[inline]
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
(*a) * (1.0 - t) + (*b) * t
}

#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut value = Vec3A::ZERO;
Expand All @@ -120,11 +97,6 @@ impl Animatable for Vec3 {
}

impl Animatable for bool {
#[inline]
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
util::step_unclamped(*a, *b, t)
}

#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
inputs
Expand All @@ -135,14 +107,6 @@ impl Animatable for bool {
}

impl Animatable for Transform {
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
Self {
translation: Vec3::interpolate(&a.translation, &b.translation, t),
rotation: Quat::interpolate(&a.rotation, &b.rotation, t),
scale: Vec3::interpolate(&a.scale, &b.scale, t),
}
}

fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut translation = Vec3A::ZERO;
let mut scale = Vec3A::ZERO;
Expand Down Expand Up @@ -173,14 +137,6 @@ impl Animatable for Transform {
}

impl Animatable for Quat {
/// Performs a slerp to smoothly interpolate between quaternions.
#[inline]
fn interpolate(a: &Self, b: &Self, t: f32) -> Self {
// We want to smoothly interpolate between the two quaternions by default,
// rather than using a quicker but less correct linear interpolation.
a.slerp(*b, t)
}

#[inline]
fn blend(inputs: impl Iterator<Item = BlendInput<Self>>) -> Self {
let mut value = Self::IDENTITY;
Expand Down
1 change: 0 additions & 1 deletion crates/bevy_animation/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
mod animatable;
mod graph;
mod transition;
mod util;

use std::cell::RefCell;
use std::collections::BTreeMap;
Expand Down
10 changes: 0 additions & 10 deletions crates/bevy_animation/src/util.rs

This file was deleted.

132 changes: 131 additions & 1 deletion crates/bevy_math/src/common_traits.rs
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
use glam::{Vec2, Vec3, Vec3A, Vec4};
use crate::{DVec2, DVec3, DVec4, Dir2, Dir3, Dir3A, Quat, Vec2, Vec3, Vec3A, Vec4};
use std::fmt::Debug;
use std::ops::{Add, Div, Mul, Neg, Sub};

Expand Down Expand Up @@ -161,3 +161,133 @@ impl NormedVectorSpace for f32 {
self * self
}
}

/// A type that can be intermediately interpolated between two given values
Copy link
Contributor

Choose a reason for hiding this comment

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

I'm slightly concerned about the fact we're adding to a foundation library a trait which is very vague, and whose implementation is relied upon by another library (animation) so is making implicit assumptions in its implementation for math types (directions and rotations use slerp, other types use lerp). I understand the drive to uniformize and remove duplicates code, but what happens when someone wants to use that trait in another library? There's almost zero guarantee on the produced value other than some vague "interpolation" mention, so if I were a crate author I'd avoid that trait because I can't reason about it. While it's located in the animation library at least there's an assumption it's used for animation, and therefore that the interpolation is linear. But here, nothing prevents me from implementing this trait as some weird random value provided I satisfy the constraints at t=0 and t=1.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

To me, using lerp/slerp for linear and rotational types has anything to do with animation per se — those are the modes of interpolation that are mathematically natural; my expectation is that if bevy_animation wanted something else specific to their use-cases, they would newtype them to customize the interpolation. In my opinion, the only concession here is for bool, but perhaps the better way forward would be to just force bevy_animation to newtype it.

As to the matter of vagueness: this is at least partially by design; for some additional context, this trait's real reason for existence is to allow for quite varied notions of interpolation, especially in light of the future Curve RFC, which presents a general framework wherein this trait allows curves to be consistently defined by discrete data. That was designed primarily with the needs of future work in bevy_math in mind (i.e. curve geometry, RMFs, reparametrizing motion, and so on), but the idea is that bevy_animation would also use it for curves in animation.

I'm not sure exactly what the concern about API consumers is; I would expect that calling interpolate on vectors or directions should produce an intuitive notion of interpolation between them, and it does. If third-party libraries want to implement weird implementations of interpolation, there is nothing anyone can do to stop them regardless, but that's true of all public trait interfaces.

On some level, I do think this is almost lower-level than bevy_math (i.e. bevy_math is kind of more of an API consumer than a producer), but I don't think there's really any better place for it right now.

Copy link
Contributor

Choose a reason for hiding this comment

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

if bevy_animation wanted something else specific to their use-cases, they would newtype them to customize the interpolation

Probably impossible, Bevy Animation works on the types it's asked to work on by the user, not the types it wants to.

has anything to do with animation per se

Maybe, but animation relies on linear interpolation. If we changed it, everything would break. And this trait doesn't provide any assurance about linearity.

I'm not sure exactly what the concern about API consumers is; I would expect that calling interpolate on vectors or directions should produce an intuitive notion of interpolation between them, and it does.

This explanation highlights very precisely what I'm worried about, the only definition here is "intuitive" and that's subjective. There's millions of values between 0. and 1., all of them can be defined as an interpretation of 0. and 1. via some function. If we don't specify the function, and just say "intuitive", then the trait is unusable because what's intuitive depends on the usage context, and it's not intrinsic such that we can have a single implementation.

this trait's real reason for existence is to allow for quite varied notions of interpolation

I don't understand how a trait with a single important per type can allow varied notions. The traditional way of doing non linear interpolation, like Bevy Animation, is to likely interpolate values using a non linear parameter t. But then your other comment states that it's linear, which prevents this pattern.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Probably impossible, Bevy Animation works on the types it's asked to work on by the user, not the types it wants to.

Actually quite easy, considering that Curves are mappable: the underlying data that is being interpolated does not need to have the same type as its output.

Maybe, but animation relies on linear interpolation. If we changed it, everything would break. And this trait doesn't provide any assurance about linearity.

No, but it makes very strong demands on interpolation nonetheless.

This explanation highlights very precisely what I'm worried about, the only definition here is "intuitive" and that's subjective. There's millions of values between 0. and 1., all of them can be defined as an interpretation of 0. and 1. via some function. If we don't specify the function, and just say "intuitive", then the trait is unusable because what's intuitive depends on the usage context, and it's not intrinsic such that we can have a single implementation.

"Some function" is not likely to satisfy the requirements of the trait implementation, which are quite strict.

/// using an auxiliary linear parameter.
Copy link
Contributor

Choose a reason for hiding this comment

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

Your comment below shows an example with a non linear t. In general I don't see any reason why we should restrict this to be linear. Unless you mean "linear interpolation", but again, no reason for that.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The parameter itself is "linear" in the sense that it is one-dimensional, and the second trait requirement treats the parameter itself linearly, since it involves interpolating f32s with lerp via interpolate, but this doesn't mean at all that the interpolation has to be linear.

That assumption is more like a constant-velocity requirement; I'm not sure if I actually like it, but its raison d'être is that it guarantees that smooth_nudge's decay parameter actually means something relatively precise.

Copy link
Contributor

Choose a reason for hiding this comment

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

The parameter itself is "linear" in the sense that it is one-dimensional,

That's "scalar" not "linear".

and the second trait requirement treats the parameter itself linearly, since it involves interpolating f32s with lerp via interpolate, but this doesn't mean at all that the interpolation has to be linear.

I don't understand this sentence, you sem to contradict yourself. If the implementation is lerp then the only source of non linearity is t so it's not linear.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I am not contradicting myself. The condition stated in the trait documentation involves linear reparametrization: if $C(p,q): \lbrack 0, 1 \rbrack \rightarrow X$ is the curve connecting $p$ and $q$ specified by interpolate, then for $a,b \in \lbrack 0, 1 \rbrack$, $a &lt; b$, $C(C(p,q)(a), C(p,q)(b))$ is demanded to be the obvious linear reparametrization of $C(p,q)|_{\lbrack a, b \rbrack}$.

(Another natural condition that is more lax is that the former merely be some reparametrization of the latter.)

///
/// The expectations for the implementing type are as follows:
/// - `interpolate(&first, &second, t)` produces `first.clone()` when `t = 0.0`
/// and `second.clone()` when `t = 1.0`.
/// - `interpolate` is self-similar in the sense that, for any values `t0`, `t1`,
/// `interpolate(interpolate(&first, &second, t0), interpolate(&first, &second, t1), t)`
/// is equivalent to `interpolate(&first, &second, interpolate(&t0, &t1, t))`.
pub trait Interpolate: Clone {
/// Interpolate between this value and the `other` given value using the parameter `t`.
/// Note that the parameter `t` is not necessarily clamped to lie between `0` and `1`.
/// However, when `t = 0.0`, `self` is recovered, while `other` is recovered at `t = 1.0`,
/// with intermediate values lying between the two in some appropriate sense.
fn interpolate(&self, other: &Self, t: f32) -> Self;

/// A version of [`interpolate`] that assigns the result to `self` for convenience.
///
/// [`interpolate`]: Interpolate::interpolate
fn interpolate_assign(&mut self, other: &Self, t: f32) {
*self = self.interpolate(other, t);
}

/// Smoothly nudge this value towards the `target` at a given decay rate. The `decay_rate`
/// parameter controls how fast the distance between `self` and `target` decays relative to
/// the units of `delta`; the intended usage is for `decay_rate` to generally remain fixed,
/// while `delta` is something like `delta_time` from an updating system. This produces a
/// smooth following of the target that is independent of framerate.
///
/// More specifically, when this is called repeatedly, the result is that the distance between
/// `self` and a fixed `target` attenuates exponentially, with the rate of this exponential
/// decay given by `decay_rate`.
///
/// For example, at `decay_rate = 0.0`, this has no effect.
/// At `decay_rate = f32::INFINITY`, `self` immediately snaps to `target`.
/// In general, higher rates mean that `self` moves more quickly towards `target`.
///
/// # Example
/// ```
/// # use bevy_math::{Vec3, Interpolate};
/// # let delta_time: f32 = 1.0 / 60.0;
/// let mut object_position: Vec3 = Vec3::ZERO;
/// let target_position: Vec3 = Vec3::new(2.0, 3.0, 5.0);
/// // Decay rate of ln(10) => after 1 second, remaining distance is 1/10th
/// let decay_rate = f32::ln(10.0);
/// // Calling this repeatedly will move `object_position` towards `target_position`:
/// object_position.smooth_nudge(&target_position, decay_rate, delta_time);
/// ```
fn smooth_nudge(&mut self, target: &Self, decay_rate: f32, delta: f32) {
self.interpolate_assign(target, 1.0 - f32::exp(-decay_rate * delta));
}
}

/// Steps between two different discrete values of any type.
/// Returns `a` if `t < 1.0`, otherwise returns `b`.
///
/// This is a common form of interpolation for discrete types.
#[inline]
fn step_unclamped<T>(a: T, b: T, t: f32) -> T {
if t < 1.0 {
a
} else {
b
}
}

impl<V> Interpolate for V
where
V: VectorSpace,
{
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
self.lerp(*other, t)
}
}

impl Interpolate for Quat {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
self.slerp(*other, t)
}
}

impl Interpolate for Dir2 {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
self.slerp(*other, t)
}
}

impl Interpolate for Dir3 {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
self.slerp(*other, t)
}
}

impl Interpolate for Dir3A {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
self.slerp(*other, t)
}
}

/// This macro is for implementing `Interpolate` on non-f32-based vector-space-like entities.
macro_rules! impl_float_interpolate {
($ty: ty, $base: ty) => {
impl Interpolate for $ty {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
let t = <$base>::from(t);
(*self) * (1.0 - t) + (*other) * t
}
}
};
}

impl_float_interpolate!(f64, f64);
impl_float_interpolate!(DVec2, f64);
impl_float_interpolate!(DVec3, f64);
impl_float_interpolate!(DVec4, f64);

// This is slightly cursed but necessary for unifying with an `Animatable` implementation for `bool`
impl Interpolate for bool {
#[inline]
fn interpolate(&self, other: &Self, t: f32) -> Self {
step_unclamped(*self, *other, t)
}
}
6 changes: 3 additions & 3 deletions crates/bevy_math/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -50,9 +50,9 @@ pub mod prelude {
},
direction::{Dir2, Dir3, Dir3A},
primitives::*,
BVec2, BVec3, BVec4, EulerRot, FloatExt, IRect, IVec2, IVec3, IVec4, Mat2, Mat3, Mat4,
Quat, Ray2d, Ray3d, Rect, Rotation2d, URect, UVec2, UVec3, UVec4, Vec2, Vec2Swizzles, Vec3,
Vec3Swizzles, Vec4, Vec4Swizzles,
BVec2, BVec3, BVec4, EulerRot, FloatExt, IRect, IVec2, IVec3, IVec4, Interpolate, Mat2,
Mat3, Mat4, Quat, Ray2d, Ray3d, Rect, Rotation2d, URect, UVec2, UVec3, UVec4, Vec2,
Vec2Swizzles, Vec3, Vec3Swizzles, Vec4, Vec4Swizzles,
};
}

Expand Down
12 changes: 11 additions & 1 deletion crates/bevy_transform/src/components/transform.rs
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
use super::GlobalTransform;
use bevy_ecs::{component::Component, reflect::ReflectComponent};
use bevy_math::{Affine3A, Dir3, Mat3, Mat4, Quat, Vec3};
use bevy_math::{Affine3A, Dir3, Interpolate, Mat3, Mat4, Quat, Vec3};
use bevy_reflect::prelude::*;
use bevy_reflect::Reflect;
use std::ops::Mul;
Expand Down Expand Up @@ -559,3 +559,13 @@ impl Mul<Vec3> for Transform {
self.transform_point(value)
}
}

impl Interpolate for Transform {
fn interpolate(&self, other: &Self, t: f32) -> Self {
Transform {
translation: self.translation.interpolate(&other.translation, t),
rotation: self.rotation.interpolate(&other.rotation, t),
scale: self.scale.interpolate(&other.scale, t),
}
}
}
1 change: 1 addition & 0 deletions examples/README.md
Original file line number Diff line number Diff line change
Expand Up @@ -323,6 +323,7 @@ Example | Description
[Random Sampling](../examples/math/random_sampling.rs) | Demonstrates how to sample random points from mathematical primitives
[Rendering Primitives](../examples/math/render_primitives.rs) | Shows off rendering for all math primitives as both Meshes and Gizmos
[Sampling Primitives](../examples/math/sampling_primitives.rs) | Demonstrates all the primitives which can be sampled.
[Smooth Follow](../examples/math/smooth_follow.rs) | Demonstrates how to make an entity smoothly follow another using interpolation

## Reflection

Expand Down
Loading