Skip to content

Commit

Permalink
Eliminate redundant clamping from sample-interpolated curves (#15620)
Browse files Browse the repository at this point in the history
# 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.
  • Loading branch information
mweatherley authored Oct 3, 2024
1 parent 46180a7 commit 528ca4f
Show file tree
Hide file tree
Showing 4 changed files with 77 additions and 15 deletions.
8 changes: 4 additions & 4 deletions crates/bevy_animation/src/animation_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1021,14 +1021,14 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
// `UnevenCore::sample_with` is implicitly clamped.
self.core.sample_with(t, <T as Animatable>::interpolate)
}

#[inline]
fn sample_clamped(&self, t: f32) -> T {
// Sampling by keyframes is automatically clamped to the keyframe bounds.
self.sample_unchecked(t)
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

Expand Down
42 changes: 36 additions & 6 deletions crates/bevy_animation/src/gltf_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,10 +23,15 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
self.core
.sample_with(t, |x, y, t| if t >= 1.0 { y.clone() } else { x.clone() })
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

impl<T> SteppedKeyframeCurve<T> {
Expand Down Expand Up @@ -57,7 +62,7 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> V {
fn sample_clamped(&self, t: f32) -> V {
match self.core.sample_interp_timed(t) {
// In all the cases where only one frame matters, defer to the position within it.
InterpolationDatum::Exact((_, v))
Expand All @@ -69,6 +74,11 @@ where
}
}
}

#[inline]
fn sample_unchecked(&self, t: f32) -> V {
self.sample_clamped(t)
}
}

impl<T> CubicKeyframeCurve<T> {
Expand Down Expand Up @@ -112,7 +122,7 @@ impl Curve<Quat> for CubicRotationCurve {
}

#[inline]
fn sample_unchecked(&self, t: f32) -> Quat {
fn sample_clamped(&self, t: f32) -> Quat {
let vec = match self.core.sample_interp_timed(t) {
// In all the cases where only one frame matters, defer to the position within it.
InterpolationDatum::Exact((_, v))
Expand All @@ -125,6 +135,11 @@ impl Curve<Quat> for CubicRotationCurve {
};
Quat::from_vec4(vec.normalize())
}

#[inline]
fn sample_unchecked(&self, t: f32) -> Quat {
self.sample_clamped(t)
}
}

impl CubicRotationCurve {
Expand Down Expand Up @@ -170,7 +185,7 @@ where
}

#[inline]
fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
fn sample_iter_clamped(&self, t: f32) -> impl Iterator<Item = T> {
match self.core.sample_interp(t) {
InterpolationDatum::Exact(v)
| InterpolationDatum::LeftTail(v)
Expand All @@ -182,6 +197,11 @@ where
}
}
}

#[inline]
fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
self.sample_iter_clamped(t)
}
}

impl<T> WideLinearKeyframeCurve<T> {
Expand Down Expand Up @@ -219,7 +239,7 @@ where
}

#[inline]
fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
fn sample_iter_clamped(&self, t: f32) -> impl Iterator<Item = T> {
match self.core.sample_interp(t) {
InterpolationDatum::Exact(v)
| InterpolationDatum::LeftTail(v)
Expand All @@ -234,6 +254,11 @@ where
}
}
}

#[inline]
fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
self.sample_iter_clamped(t)
}
}

impl<T> WideSteppedKeyframeCurve<T> {
Expand Down Expand Up @@ -269,7 +294,7 @@ where
self.core.domain()
}

fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
fn sample_iter_clamped(&self, t: f32) -> impl Iterator<Item = T> {
match self.core.sample_interp_timed(t) {
InterpolationDatum::Exact((_, v))
| InterpolationDatum::LeftTail((_, v))
Expand All @@ -285,6 +310,11 @@ where
),
}
}

#[inline]
fn sample_iter_unchecked(&self, t: f32) -> impl Iterator<Item = T> {
self.sample_iter_clamped(t)
}
}

/// An error indicating that a multisampling keyframe curve could not be constructed.
Expand Down
10 changes: 9 additions & 1 deletion crates/bevy_color/src/color_gradient.rs
Original file line number Diff line number Diff line change
Expand Up @@ -54,13 +54,21 @@ impl<T> Curve<T> for ColorCurve<T>
where
T: Mix + Clone,
{
#[inline]
fn domain(&self) -> Interval {
self.core.domain()
}

fn sample_unchecked(&self, t: f32) -> T {
#[inline]
fn sample_clamped(&self, t: f32) -> T {
// `EvenCore::sample_with` clamps the input implicitly.
self.core.sample_with(t, T::mix)
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

#[cfg(test)]
Expand Down
32 changes: 28 additions & 4 deletions crates/bevy_math/src/curve/sample_curves.rs
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,15 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
// `EvenCore::sample_with` is implicitly clamped.
self.core.sample_with(t, &self.interpolation)
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

impl<T, I> SampleCurve<T, I> {
Expand Down Expand Up @@ -143,10 +149,16 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
// `EvenCore::sample_with` is implicitly clamped.
self.core
.sample_with(t, <T as StableInterpolate>::interpolate_stable)
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

impl<T> SampleAutoCurve<T> {
Expand Down Expand Up @@ -242,9 +254,15 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
// `UnevenCore::sample_with` is implicitly clamped.
self.core.sample_with(t, &self.interpolation)
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

impl<T, I> UnevenSampleCurve<T, I> {
Expand Down Expand Up @@ -301,10 +319,16 @@ where
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
fn sample_clamped(&self, t: f32) -> T {
// `UnevenCore::sample_with` is implicitly clamped.
self.core
.sample_with(t, <T as StableInterpolate>::interpolate_stable)
}

#[inline]
fn sample_unchecked(&self, t: f32) -> T {
self.sample_clamped(t)
}
}

impl<T> UnevenSampleAutoCurve<T> {
Expand Down

0 comments on commit 528ca4f

Please sign in to comment.