From 93aa2a2cc4cfd08f6478e37cdbc0598c695ae199 Mon Sep 17 00:00:00 2001 From: Matty Date: Mon, 30 Sep 2024 14:43:19 -0400 Subject: [PATCH] Make `SampleCurve`/`UnevenSampleCurve` succeed at reflection (#15493) (Note: #15434 implements something very similar to this for functional curve adaptors, which is why they aren't present in this PR.) # Objective Previously, there was basically no chance that the explicitly-interpolating sample curve structs from the `Curve` API would actually be `Reflect`. The reason for this is functional programming: the structs contain an explicit interpolation `I: Fn(&T, &T, f32) -> T` which, under typical circumstances, will never be `Reflect`, which prevents the derive from realistically succeeding. In fact, they won't be a lot of other things either, notably including both`Debug` and `TypePath`, which are also required for reflection to succeed. The goal of this PR is to weaken the implementations of reflection traits for these structs so that they can implement `Reflect` under reasonable circumstances. (Notably, they will still not be `FromReflect`, which is unavoidable.) ## Solution The function fields are marked as `#[reflect(ignore)]`, and the derive macro for `Reflect` has `FromReflect` disabled. (This is not fully optimal, but we don't presently have any kind of "read-only" attribute for these fields.) Additionally, these structs receive custom `Debug` and `TypePath` implementations that display the function's (unstable!) type name instead of its value or type path (respectively). In the case of `TypePath`, this is a bit janky, but the instability of `type_name` won't generally present an issue for generics, which would have to be registered manually in the type registry anyway, which is impossible because the function type parameters cannot be named. (And in general, the "blessed" route for such cases would generally involve manually monomorphizing the function parameter away, which also allows access to `FromReflect` etc. through very ordinary use of the derive macro.) ## Testing Tests in the new `bevy_math::curve::sample_curves` module guarantee that these are actually `Reflect` under reasonable circumstances. --- ## Future changes If and when function item types become `Default`, these types will need to receive custom `FromReflect` implementations that exploit it. Such a custom implementation would also be desirable if users start doing things like wrapping function items in `Default`/`FromReflect` wrappers that still implement a `Fn` trait. Additionally, if function types become nameable in user-space, the stance on `Debug`/`TypePath` may bear reexamination, since partial monomorphization through wrappers would make implementing reflect traits for function types potentially more viable. --------- Co-authored-by: Gino Valente <49806985+MrGVSV@users.noreply.github.com> --- crates/bevy_math/src/curve/mod.rs | 203 +---------- crates/bevy_math/src/curve/sample_curves.rs | 374 ++++++++++++++++++++ 2 files changed, 381 insertions(+), 196 deletions(-) create mode 100644 crates/bevy_math/src/curve/sample_curves.rs diff --git a/crates/bevy_math/src/curve/mod.rs b/crates/bevy_math/src/curve/mod.rs index e729bd3778..5f630bc399 100644 --- a/crates/bevy_math/src/curve/mod.rs +++ b/crates/bevy_math/src/curve/mod.rs @@ -5,15 +5,19 @@ pub mod adaptors; pub mod cores; pub mod interval; +pub mod sample_curves; + +// bevy_math::curve re-exports all commonly-needed curve-related items. +pub use interval::{interval, Interval}; +pub use sample_curves::*; use adaptors::*; -pub use interval::{interval, Interval}; -use itertools::Itertools; +use cores::{EvenCore, UnevenCore}; use crate::{StableInterpolate, VectorSpace}; use core::{marker::PhantomData, ops::Deref}; -use cores::{EvenCore, EvenCoreError, UnevenCore, UnevenCoreError}; use interval::InvalidIntervalError; +use itertools::Itertools; use thiserror::Error; /// A trait for a type that can represent values of type `T` parametrized over a fixed interval. @@ -936,199 +940,6 @@ where } } -/// A curve that is defined by explicit neighbor interpolation over a set of samples. -#[derive(Clone, Debug)] -#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -pub struct SampleCurve { - core: EvenCore, - interpolation: I, -} - -impl Curve for SampleCurve -where - T: Clone, - I: Fn(&T, &T, f32) -> T, -{ - #[inline] - fn domain(&self) -> Interval { - self.core.domain() - } - - #[inline] - fn sample_unchecked(&self, t: f32) -> T { - self.core.sample_with(t, &self.interpolation) - } -} - -impl SampleCurve { - /// Create a new [`SampleCurve`] using the specified `interpolation` to interpolate between - /// the given `samples`. An error is returned if there are not at least 2 samples or if the - /// given `domain` is unbounded. - /// - /// The interpolation takes two values by reference together with a scalar parameter and - /// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and - /// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively. - pub fn new( - domain: Interval, - samples: impl IntoIterator, - interpolation: I, - ) -> Result - where - I: Fn(&T, &T, f32) -> T, - { - Ok(Self { - core: EvenCore::new(domain, samples)?, - interpolation, - }) - } -} - -/// A curve that is defined by neighbor interpolation over a set of samples. -#[derive(Clone, Debug)] -#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -pub struct SampleAutoCurve { - core: EvenCore, -} - -impl Curve for SampleAutoCurve -where - T: StableInterpolate, -{ - #[inline] - fn domain(&self) -> Interval { - self.core.domain() - } - - #[inline] - fn sample_unchecked(&self, t: f32) -> T { - self.core - .sample_with(t, ::interpolate_stable) - } -} - -impl SampleAutoCurve { - /// Create a new [`SampleCurve`] using type-inferred interpolation to interpolate between - /// the given `samples`. An error is returned if there are not at least 2 samples or if the - /// given `domain` is unbounded. - pub fn new( - domain: Interval, - samples: impl IntoIterator, - ) -> Result { - Ok(Self { - core: EvenCore::new(domain, samples)?, - }) - } -} - -/// A curve that is defined by interpolation over unevenly spaced samples with explicit -/// interpolation. -#[derive(Clone, Debug)] -#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -pub struct UnevenSampleCurve { - core: UnevenCore, - interpolation: I, -} - -impl Curve for UnevenSampleCurve -where - T: Clone, - I: Fn(&T, &T, f32) -> T, -{ - #[inline] - fn domain(&self) -> Interval { - self.core.domain() - } - - #[inline] - fn sample_unchecked(&self, t: f32) -> T { - self.core.sample_with(t, &self.interpolation) - } -} - -impl UnevenSampleCurve { - /// Create a new [`UnevenSampleCurve`] using the provided `interpolation` to interpolate - /// between adjacent `timed_samples`. The given samples are filtered to finite times and - /// sorted internally; if there are not at least 2 valid timed samples, an error will be - /// returned. - /// - /// The interpolation takes two values by reference together with a scalar parameter and - /// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and - /// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively. - pub fn new( - timed_samples: impl IntoIterator, - interpolation: I, - ) -> Result { - Ok(Self { - core: UnevenCore::new(timed_samples)?, - interpolation, - }) - } - - /// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`. - /// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`], - /// but the function inputs to each are inverses of one another. - /// - /// The samples are re-sorted by time after mapping and deduplicated by output time, so - /// the function `f` should generally be injective over the sample times of the curve. - pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleCurve { - Self { - core: self.core.map_sample_times(f), - interpolation: self.interpolation, - } - } -} - -/// A curve that is defined by interpolation over unevenly spaced samples. -#[derive(Clone, Debug)] -#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] -#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))] -pub struct UnevenSampleAutoCurve { - core: UnevenCore, -} - -impl Curve for UnevenSampleAutoCurve -where - T: StableInterpolate, -{ - #[inline] - fn domain(&self) -> Interval { - self.core.domain() - } - - #[inline] - fn sample_unchecked(&self, t: f32) -> T { - self.core - .sample_with(t, ::interpolate_stable) - } -} - -impl UnevenSampleAutoCurve { - /// Create a new [`UnevenSampleAutoCurve`] from a given set of timed samples, interpolated - /// using the The samples are filtered to finite times and - /// sorted internally; if there are not at least 2 valid timed samples, an error will be - /// returned. - pub fn new(timed_samples: impl IntoIterator) -> Result { - Ok(Self { - core: UnevenCore::new(timed_samples)?, - }) - } - - /// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`. - /// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`], - /// but the function inputs to each are inverses of one another. - /// - /// The samples are re-sorted by time after mapping and deduplicated by output time, so - /// the function `f` should generally be injective over the sample times of the curve. - pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleAutoCurve { - Self { - core: self.core.map_sample_times(f), - } - } -} - /// Create a [`Curve`] that constantly takes the given `value` over the given `domain`. pub fn constant_curve(domain: Interval, value: T) -> ConstantCurve { ConstantCurve { domain, value } diff --git a/crates/bevy_math/src/curve/sample_curves.rs b/crates/bevy_math/src/curve/sample_curves.rs new file mode 100644 index 0000000000..255fa84d10 --- /dev/null +++ b/crates/bevy_math/src/curve/sample_curves.rs @@ -0,0 +1,374 @@ +//! Sample-interpolated curves constructed using the [`Curve`] API. + +use super::cores::{EvenCore, EvenCoreError, UnevenCore, UnevenCoreError}; +use super::{Curve, Interval}; + +use crate::StableInterpolate; +use core::any::type_name; +use core::fmt::{self, Debug}; + +#[cfg(feature = "bevy_reflect")] +use bevy_reflect::{utility::GenericTypePathCell, Reflect, TypePath}; + +#[cfg(feature = "bevy_reflect")] +mod paths { + pub(super) const THIS_MODULE: &str = "bevy_math::curve::sample_curves"; + pub(super) const THIS_CRATE: &str = "bevy_math"; +} + +/// A curve that is defined by explicit neighbor interpolation over a set of evenly-spaced samples. +#[derive(Clone)] +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr( + feature = "bevy_reflect", + derive(Reflect), + reflect(where T: TypePath), + reflect(from_reflect = false, type_path = false), +)] +pub struct SampleCurve { + pub(crate) core: EvenCore, + #[cfg_attr(feature = "bevy_reflect", reflect(ignore))] + pub(crate) interpolation: I, +} + +impl Debug for SampleCurve +where + EvenCore: Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SampleCurve") + .field("core", &self.core) + .field("interpolation", &type_name::()) + .finish() + } +} + +/// Note: This is not a fully stable implementation of `TypePath` due to usage of `type_name` +/// for function members. +#[cfg(feature = "bevy_reflect")] +impl TypePath for SampleCurve +where + T: TypePath, + I: 'static, +{ + fn type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| { + format!( + "{}::SampleCurve<{},{}>", + paths::THIS_MODULE, + T::type_path(), + type_name::() + ) + }) + } + + fn short_type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| { + format!("SampleCurve<{},{}>", T::type_path(), type_name::()) + }) + } + + fn type_ident() -> Option<&'static str> { + Some("SampleCurve") + } + + fn crate_name() -> Option<&'static str> { + Some(paths::THIS_CRATE) + } + + fn module_path() -> Option<&'static str> { + Some(paths::THIS_MODULE) + } +} + +impl Curve for SampleCurve +where + T: Clone, + I: Fn(&T, &T, f32) -> T, +{ + #[inline] + fn domain(&self) -> Interval { + self.core.domain() + } + + #[inline] + fn sample_unchecked(&self, t: f32) -> T { + self.core.sample_with(t, &self.interpolation) + } +} + +impl SampleCurve { + /// Create a new [`SampleCurve`] using the specified `interpolation` to interpolate between + /// the given `samples`. An error is returned if there are not at least 2 samples or if the + /// given `domain` is unbounded. + /// + /// The interpolation takes two values by reference together with a scalar parameter and + /// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and + /// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively. + pub fn new( + domain: Interval, + samples: impl IntoIterator, + interpolation: I, + ) -> Result + where + I: Fn(&T, &T, f32) -> T, + { + Ok(Self { + core: EvenCore::new(domain, samples)?, + interpolation, + }) + } +} + +/// A curve that is defined by neighbor interpolation over a set of evenly-spaced samples, +/// interpolated automatically using [a particularly well-behaved interpolation]. +/// +/// [a particularly well-behaved interpolation]: StableInterpolate +#[derive(Clone, Debug)] +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "bevy_reflect", derive(Reflect))] +pub struct SampleAutoCurve { + pub(crate) core: EvenCore, +} + +impl Curve for SampleAutoCurve +where + T: StableInterpolate, +{ + #[inline] + fn domain(&self) -> Interval { + self.core.domain() + } + + #[inline] + fn sample_unchecked(&self, t: f32) -> T { + self.core + .sample_with(t, ::interpolate_stable) + } +} + +impl SampleAutoCurve { + /// Create a new [`SampleCurve`] using type-inferred interpolation to interpolate between + /// the given `samples`. An error is returned if there are not at least 2 samples or if the + /// given `domain` is unbounded. + pub fn new( + domain: Interval, + samples: impl IntoIterator, + ) -> Result { + Ok(Self { + core: EvenCore::new(domain, samples)?, + }) + } +} + +/// A curve that is defined by interpolation over unevenly spaced samples with explicit +/// interpolation. +#[derive(Clone)] +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr( + feature = "bevy_reflect", + derive(Reflect), + reflect(where T: TypePath), + reflect(from_reflect = false, type_path = false), +)] +pub struct UnevenSampleCurve { + pub(crate) core: UnevenCore, + #[cfg_attr(feature = "bevy_reflect", reflect(ignore))] + pub(crate) interpolation: I, +} + +impl Debug for UnevenSampleCurve +where + UnevenCore: Debug, +{ + fn fmt(&self, f: &mut fmt::Formatter<'_>) -> fmt::Result { + f.debug_struct("SampleCurve") + .field("core", &self.core) + .field("interpolation", &type_name::()) + .finish() + } +} + +/// Note: This is not a fully stable implementation of `TypePath` due to usage of `type_name` +/// for function members. +#[cfg(feature = "bevy_reflect")] +impl TypePath for UnevenSampleCurve +where + T: TypePath, + I: 'static, +{ + fn type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| { + format!( + "{}::UnevenSampleCurve<{},{}>", + paths::THIS_MODULE, + T::type_path(), + type_name::() + ) + }) + } + + fn short_type_path() -> &'static str { + static CELL: GenericTypePathCell = GenericTypePathCell::new(); + CELL.get_or_insert::(|| { + format!("UnevenSampleCurve<{},{}>", T::type_path(), type_name::()) + }) + } + + fn type_ident() -> Option<&'static str> { + Some("UnevenSampleCurve") + } + + fn crate_name() -> Option<&'static str> { + Some(paths::THIS_CRATE) + } + + fn module_path() -> Option<&'static str> { + Some(paths::THIS_MODULE) + } +} + +impl Curve for UnevenSampleCurve +where + T: Clone, + I: Fn(&T, &T, f32) -> T, +{ + #[inline] + fn domain(&self) -> Interval { + self.core.domain() + } + + #[inline] + fn sample_unchecked(&self, t: f32) -> T { + self.core.sample_with(t, &self.interpolation) + } +} + +impl UnevenSampleCurve { + /// Create a new [`UnevenSampleCurve`] using the provided `interpolation` to interpolate + /// between adjacent `timed_samples`. The given samples are filtered to finite times and + /// sorted internally; if there are not at least 2 valid timed samples, an error will be + /// returned. + /// + /// The interpolation takes two values by reference together with a scalar parameter and + /// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and + /// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively. + pub fn new( + timed_samples: impl IntoIterator, + interpolation: I, + ) -> Result { + Ok(Self { + core: UnevenCore::new(timed_samples)?, + interpolation, + }) + } + + /// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`. + /// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`], + /// but the function inputs to each are inverses of one another. + /// + /// The samples are re-sorted by time after mapping and deduplicated by output time, so + /// the function `f` should generally be injective over the sample times of the curve. + pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleCurve { + Self { + core: self.core.map_sample_times(f), + interpolation: self.interpolation, + } + } +} + +/// A curve that is defined by interpolation over unevenly spaced samples, +/// interpolated automatically using [a particularly well-behaved interpolation]. +/// +/// [a particularly well-behaved interpolation]: StableInterpolate +#[derive(Clone, Debug)] +#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))] +#[cfg_attr(feature = "bevy_reflect", derive(Reflect))] +pub struct UnevenSampleAutoCurve { + pub(crate) core: UnevenCore, +} + +impl Curve for UnevenSampleAutoCurve +where + T: StableInterpolate, +{ + #[inline] + fn domain(&self) -> Interval { + self.core.domain() + } + + #[inline] + fn sample_unchecked(&self, t: f32) -> T { + self.core + .sample_with(t, ::interpolate_stable) + } +} + +impl UnevenSampleAutoCurve { + /// Create a new [`UnevenSampleAutoCurve`] from a given set of timed samples, interpolated + /// using the The samples are filtered to finite times and + /// sorted internally; if there are not at least 2 valid timed samples, an error will be + /// returned. + pub fn new(timed_samples: impl IntoIterator) -> Result { + Ok(Self { + core: UnevenCore::new(timed_samples)?, + }) + } + + /// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`. + /// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`], + /// but the function inputs to each are inverses of one another. + /// + /// The samples are re-sorted by time after mapping and deduplicated by output time, so + /// the function `f` should generally be injective over the sample times of the curve. + pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleAutoCurve { + Self { + core: self.core.map_sample_times(f), + } + } +} + +#[cfg(test)] +mod tests { + //! These tests should guarantee (by even compiling) that `SampleCurve` and `UnevenSampleCurve` + //! can be `Reflect` under reasonable circumstances where their interpolation is defined by: + //! - function items + //! - 'static closures + //! - function pointers + use super::{SampleCurve, UnevenSampleCurve}; + use crate::{curve::Interval, VectorSpace}; + use bevy_reflect::Reflect; + + #[test] + fn reflect_sample_curve() { + fn foo(x: &f32, y: &f32, t: f32) -> f32 { + x.lerp(*y, t) + } + let bar = |x: &f32, y: &f32, t: f32| x.lerp(*y, t); + let baz: fn(&f32, &f32, f32) -> f32 = bar; + + let samples = [0.0, 1.0, 2.0]; + + let _: Box = Box::new(SampleCurve::new(Interval::UNIT, samples, foo).unwrap()); + let _: Box = Box::new(SampleCurve::new(Interval::UNIT, samples, bar).unwrap()); + let _: Box = Box::new(SampleCurve::new(Interval::UNIT, samples, baz).unwrap()); + } + + #[test] + fn reflect_uneven_sample_curve() { + fn foo(x: &f32, y: &f32, t: f32) -> f32 { + x.lerp(*y, t) + } + let bar = |x: &f32, y: &f32, t: f32| x.lerp(*y, t); + let baz: fn(&f32, &f32, f32) -> f32 = bar; + + let keyframes = [(0.0, 1.0), (1.0, 0.0), (2.0, -1.0)]; + + let _: Box = Box::new(UnevenSampleCurve::new(keyframes, foo).unwrap()); + let _: Box = Box::new(UnevenSampleCurve::new(keyframes, bar).unwrap()); + let _: Box = Box::new(UnevenSampleCurve::new(keyframes, baz).unwrap()); + } +}