Skip to content

Conversation

@viridia
Copy link
Contributor

@viridia viridia commented Oct 23, 2025

Fixes: #20579

Testing

  • Doc test
  • Also tested extensively in a separate repo, however that code was not carried over (yet).

Note: Because docs.rs was down, I could not validate the doc link urls.

@viridia viridia requested a review from NthTensor October 23, 2025 03:32
@viridia
Copy link
Contributor Author

viridia commented Oct 23, 2025

@LikeLakers2 Tried to add you as a reviewer but could not for some reason.

@viridia viridia requested a review from mockersf October 23, 2025 03:35
Copy link
Contributor

@LikeLakers2 LikeLakers2 left a comment

Choose a reason for hiding this comment

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

No worries. I hope I didn't frustrate you too much in the issue you made!

Comment on lines 589 to 593
impl<T: StableInterpolate> TryStableInterpolate for T {
fn try_interpolate_stable(&self, other: &Self, t: f32) -> Result<Self, InterpolationError> {
Ok(self.interpolate_stable(other, t))
}
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Huh, so this ended up working? I'm curious what was different about how you initially tested it, that caused the error you were reporting.

Copy link
Contributor Author

@viridia viridia Oct 23, 2025

Choose a reason for hiding this comment

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

I think it has to do with the fact that the impls are in the same module as the target types instead of being in the same module as the trait definition. But to be honest, I'm not really sure why it works.

@alice-i-cecile alice-i-cecile added C-Feature A new feature, making something new possible A-Animation Make things move and change over time A-Math Fundamental domain-agnostic mathematical operations S-Needs-Review Needs reviewer attention (from anyone!) to move forward A-UI Graphical user interfaces, styles, layouts, and widgets labels Oct 27, 2025
@github-actions
Copy link
Contributor

Your PR caused a change in the graphical output of an example or rendering test. This might be intentional, but it could also mean that something broke!
You can review it at https://pixel-eagle.com/project/B04F67C0-C054-4A6F-92EC-F599FEC2FD1D?filter=PR-21633

If it's expected, please add the M-Deliberate-Rendering-Change label.

If this change seems unrelated to your PR, you can consider updating your PR to target the latest main branch, either by rebasing or merging main into it.

Copy link
Contributor

@LikeLakers2 LikeLakers2 left a comment

Choose a reason for hiding this comment

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

Just a couple more nitpicks. These ones could reasonably be skipped if desired.


/// Error produced when the values to be interpolated are not in the same units.
#[derive(Clone, Debug)]
pub struct MismatchedUnitsError;
Copy link
Contributor

@LikeLakers2 LikeLakers2 Oct 29, 2025

Choose a reason for hiding this comment

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

You'll probably want to impl Error for MismatchedUnitsError. Error requires Display also be implemented, so I'll also include such an impl here:

impl core::error::Error for MismatchedUnitsError {}

impl core::fmt::Display for MismatchedUnitsError {
	fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
		write!(f, "cannot interpolate between two values of different units")
	}
}

Comment on lines 546 to 578
/// A trait that indicates that a value _may_ be interpolable via [`StableInterpolate`]. An
/// interpolation may fail if the values have different units - for example, attempting to
/// interpolate between [`Val::Px`] and [`Val::Percent`] will fail,
/// even though they are the same Rust type.
///
/// The motivating case for this trait is animating UI entities with [`Val`]
/// properties, which, because they are enums, cannot be interpolated in the normal way. This same
/// concept can be extended to other types as well.
///
/// Fallible interpolation can be used for animated transitions, which can be set up to fail
/// gracefully if there's a mismatch of units. For example, the a transition could smoothly
/// go from `Val::Px(10)` to `Val::Px(20)`, but if the user attempts to go from `Val::Px(10)` to
/// `Val::Percent(10)`, the animation player can detect the failure and simply snap to the new
/// value without interpolating.
///
/// An animation clip system can incorporate fallible interpolation to support a broad set of
/// sequenced parameter values. This can include numeric types, which always interpolate,
/// enum types, which may or may not interpolate depending on the units, and non-interpolable
/// types, which always jump immediately to the new value without interpolation. This meaas, for
/// example, that you can have an animation track whose value type is a boolean or a string.
///
/// Interpolation for simple number and coordinate types will always succeed, as will any type
/// that implements [`StableInterpolate`]. Types which have different variants such as
/// [`Val`] and [`Color`] will only fail if the units are different.
/// Note that [`Color`] has its own, non-fallible mixing methods, but those entail
/// automatically converting between different color spaces, and is both expensive and complex.
/// [`TryStableInterpolate`] is more conservative, and doesn't automatically convert between
/// color spaces. This produces a color interpolation that has more predictable performance.
///
/// [`Val::Px`]: https://docs.rs/bevy/latest/bevy/ui/enum.Val.html
/// [`Val::Percent`]: https://docs.rs/bevy/latest/bevy/ui/enum.Val.html
/// [`Val`]: https://docs.rs/bevy/latest/bevy/ui/struct.enum.html
/// [`Color`]: https://docs.rs/bevy/latest/bevy/color/enum.Color.html
Copy link
Contributor

Choose a reason for hiding this comment

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

I really appreciate the extensive documentation here, but now that the error type can be customized by the user, I do think it focuses a bit too much on mismatched units.

Here's how I might rewrite this documentation:

/// A type with a natural interpolation that provides strong subdivision guarantees, similar to
/// [`StableInterpolation`], but which cannot be guaranteed to interpolate correctly between two
/// arbitrary values.
///
/// The motivating case for this trait is animating UI entities with [`Val`] properties. Because
/// different `Val` instances may be of different incompatible variants, there is no guarantee that
/// we can be interpolate between two `Val`s. The same concept can be extended to other types as
/// well.
///
/// Bevy's default animation system handles erroring interpolations by snapping to the new value
/// without interpolating.
///
/// [`Val`]: https://docs.rs/bevy/latest/bevy/ui/enum.Val.html

It's not a perfect rewrite, and could probably use some work, but I hope it helps.

Copy link
Contributor

@LikeLakers2 LikeLakers2 left a comment

Choose a reason for hiding this comment

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

However, putting aside those nitpicks, I think this PR looks good. So this gets my approval.

Co-authored-by: MichiRecRoom <1008889+LikeLakers2@users.noreply.github.com>
Co-authored-by: MichiRecRoom <1008889+LikeLakers2@users.noreply.github.com>
Co-authored-by: MichiRecRoom <1008889+LikeLakers2@users.noreply.github.com>
Co-authored-by: François Mockers <francois.mockers@vleue.com>
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-Math Fundamental domain-agnostic mathematical operations A-UI Graphical user interfaces, styles, layouts, and widgets C-Feature A new feature, making something new possible S-Needs-Review Needs reviewer attention (from anyone!) to move forward

Projects

None yet

Development

Successfully merging this pull request may close these issues.

Fallible interpolation

5 participants