Skip to content

Commit 93aa2a2

Browse files
mweatherleyMrGVSV
andauthored
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 <[email protected]>
1 parent 97f2caa commit 93aa2a2

File tree

2 files changed

+381
-196
lines changed

2 files changed

+381
-196
lines changed

crates/bevy_math/src/curve/mod.rs

Lines changed: 7 additions & 196 deletions
Original file line numberDiff line numberDiff line change
@@ -5,15 +5,19 @@
55
pub mod adaptors;
66
pub mod cores;
77
pub mod interval;
8+
pub mod sample_curves;
89

9-
use adaptors::*;
10+
// bevy_math::curve re-exports all commonly-needed curve-related items.
1011
pub use interval::{interval, Interval};
11-
use itertools::Itertools;
12+
pub use sample_curves::*;
13+
14+
use adaptors::*;
15+
use cores::{EvenCore, UnevenCore};
1216

1317
use crate::{StableInterpolate, VectorSpace};
1418
use core::{marker::PhantomData, ops::Deref};
15-
use cores::{EvenCore, EvenCoreError, UnevenCore, UnevenCoreError};
1619
use interval::InvalidIntervalError;
20+
use itertools::Itertools;
1721
use thiserror::Error;
1822

1923
/// A trait for a type that can represent values of type `T` parametrized over a fixed interval.
@@ -936,199 +940,6 @@ where
936940
}
937941
}
938942

939-
/// A curve that is defined by explicit neighbor interpolation over a set of samples.
940-
#[derive(Clone, Debug)]
941-
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
942-
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
943-
pub struct SampleCurve<T, I> {
944-
core: EvenCore<T>,
945-
interpolation: I,
946-
}
947-
948-
impl<T, I> Curve<T> for SampleCurve<T, I>
949-
where
950-
T: Clone,
951-
I: Fn(&T, &T, f32) -> T,
952-
{
953-
#[inline]
954-
fn domain(&self) -> Interval {
955-
self.core.domain()
956-
}
957-
958-
#[inline]
959-
fn sample_unchecked(&self, t: f32) -> T {
960-
self.core.sample_with(t, &self.interpolation)
961-
}
962-
}
963-
964-
impl<T, I> SampleCurve<T, I> {
965-
/// Create a new [`SampleCurve`] using the specified `interpolation` to interpolate between
966-
/// the given `samples`. An error is returned if there are not at least 2 samples or if the
967-
/// given `domain` is unbounded.
968-
///
969-
/// The interpolation takes two values by reference together with a scalar parameter and
970-
/// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and
971-
/// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively.
972-
pub fn new(
973-
domain: Interval,
974-
samples: impl IntoIterator<Item = T>,
975-
interpolation: I,
976-
) -> Result<Self, EvenCoreError>
977-
where
978-
I: Fn(&T, &T, f32) -> T,
979-
{
980-
Ok(Self {
981-
core: EvenCore::new(domain, samples)?,
982-
interpolation,
983-
})
984-
}
985-
}
986-
987-
/// A curve that is defined by neighbor interpolation over a set of samples.
988-
#[derive(Clone, Debug)]
989-
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
990-
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
991-
pub struct SampleAutoCurve<T> {
992-
core: EvenCore<T>,
993-
}
994-
995-
impl<T> Curve<T> for SampleAutoCurve<T>
996-
where
997-
T: StableInterpolate,
998-
{
999-
#[inline]
1000-
fn domain(&self) -> Interval {
1001-
self.core.domain()
1002-
}
1003-
1004-
#[inline]
1005-
fn sample_unchecked(&self, t: f32) -> T {
1006-
self.core
1007-
.sample_with(t, <T as StableInterpolate>::interpolate_stable)
1008-
}
1009-
}
1010-
1011-
impl<T> SampleAutoCurve<T> {
1012-
/// Create a new [`SampleCurve`] using type-inferred interpolation to interpolate between
1013-
/// the given `samples`. An error is returned if there are not at least 2 samples or if the
1014-
/// given `domain` is unbounded.
1015-
pub fn new(
1016-
domain: Interval,
1017-
samples: impl IntoIterator<Item = T>,
1018-
) -> Result<Self, EvenCoreError> {
1019-
Ok(Self {
1020-
core: EvenCore::new(domain, samples)?,
1021-
})
1022-
}
1023-
}
1024-
1025-
/// A curve that is defined by interpolation over unevenly spaced samples with explicit
1026-
/// interpolation.
1027-
#[derive(Clone, Debug)]
1028-
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
1029-
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
1030-
pub struct UnevenSampleCurve<T, I> {
1031-
core: UnevenCore<T>,
1032-
interpolation: I,
1033-
}
1034-
1035-
impl<T, I> Curve<T> for UnevenSampleCurve<T, I>
1036-
where
1037-
T: Clone,
1038-
I: Fn(&T, &T, f32) -> T,
1039-
{
1040-
#[inline]
1041-
fn domain(&self) -> Interval {
1042-
self.core.domain()
1043-
}
1044-
1045-
#[inline]
1046-
fn sample_unchecked(&self, t: f32) -> T {
1047-
self.core.sample_with(t, &self.interpolation)
1048-
}
1049-
}
1050-
1051-
impl<T, I> UnevenSampleCurve<T, I> {
1052-
/// Create a new [`UnevenSampleCurve`] using the provided `interpolation` to interpolate
1053-
/// between adjacent `timed_samples`. The given samples are filtered to finite times and
1054-
/// sorted internally; if there are not at least 2 valid timed samples, an error will be
1055-
/// returned.
1056-
///
1057-
/// The interpolation takes two values by reference together with a scalar parameter and
1058-
/// produces an owned value. The expectation is that `interpolation(&x, &y, 0.0)` and
1059-
/// `interpolation(&x, &y, 1.0)` are equivalent to `x` and `y` respectively.
1060-
pub fn new(
1061-
timed_samples: impl IntoIterator<Item = (f32, T)>,
1062-
interpolation: I,
1063-
) -> Result<Self, UnevenCoreError> {
1064-
Ok(Self {
1065-
core: UnevenCore::new(timed_samples)?,
1066-
interpolation,
1067-
})
1068-
}
1069-
1070-
/// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`.
1071-
/// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`],
1072-
/// but the function inputs to each are inverses of one another.
1073-
///
1074-
/// The samples are re-sorted by time after mapping and deduplicated by output time, so
1075-
/// the function `f` should generally be injective over the sample times of the curve.
1076-
pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleCurve<T, I> {
1077-
Self {
1078-
core: self.core.map_sample_times(f),
1079-
interpolation: self.interpolation,
1080-
}
1081-
}
1082-
}
1083-
1084-
/// A curve that is defined by interpolation over unevenly spaced samples.
1085-
#[derive(Clone, Debug)]
1086-
#[cfg_attr(feature = "serialize", derive(serde::Serialize, serde::Deserialize))]
1087-
#[cfg_attr(feature = "bevy_reflect", derive(bevy_reflect::Reflect))]
1088-
pub struct UnevenSampleAutoCurve<T> {
1089-
core: UnevenCore<T>,
1090-
}
1091-
1092-
impl<T> Curve<T> for UnevenSampleAutoCurve<T>
1093-
where
1094-
T: StableInterpolate,
1095-
{
1096-
#[inline]
1097-
fn domain(&self) -> Interval {
1098-
self.core.domain()
1099-
}
1100-
1101-
#[inline]
1102-
fn sample_unchecked(&self, t: f32) -> T {
1103-
self.core
1104-
.sample_with(t, <T as StableInterpolate>::interpolate_stable)
1105-
}
1106-
}
1107-
1108-
impl<T> UnevenSampleAutoCurve<T> {
1109-
/// Create a new [`UnevenSampleAutoCurve`] from a given set of timed samples, interpolated
1110-
/// using the The samples are filtered to finite times and
1111-
/// sorted internally; if there are not at least 2 valid timed samples, an error will be
1112-
/// returned.
1113-
pub fn new(timed_samples: impl IntoIterator<Item = (f32, T)>) -> Result<Self, UnevenCoreError> {
1114-
Ok(Self {
1115-
core: UnevenCore::new(timed_samples)?,
1116-
})
1117-
}
1118-
1119-
/// This [`UnevenSampleAutoCurve`], but with the sample times moved by the map `f`.
1120-
/// In principle, when `f` is monotone, this is equivalent to [`Curve::reparametrize`],
1121-
/// but the function inputs to each are inverses of one another.
1122-
///
1123-
/// The samples are re-sorted by time after mapping and deduplicated by output time, so
1124-
/// the function `f` should generally be injective over the sample times of the curve.
1125-
pub fn map_sample_times(self, f: impl Fn(f32) -> f32) -> UnevenSampleAutoCurve<T> {
1126-
Self {
1127-
core: self.core.map_sample_times(f),
1128-
}
1129-
}
1130-
}
1131-
1132943
/// Create a [`Curve`] that constantly takes the given `value` over the given `domain`.
1133944
pub fn constant_curve<T: Clone>(domain: Interval, value: T) -> ConstantCurve<T> {
1134945
ConstantCurve { domain, value }

0 commit comments

Comments
 (0)