Switch from f32 to Duration to measure time

.gitignore

@@ -1,2 +1,3 @@
 /target
 /frames/*.png
+*.DS_Store

crates/bevy_motiongfx/src/controller.rs

@@ -1,6 +1,9 @@
+use core::time::Duration;
+
 use bevy_app::prelude::*;
 use bevy_ecs::prelude::*;
 use bevy_time::prelude::*;
+use motiongfx::time::IntoDuration;
 
 use crate::MotionGfxSystems;
 use crate::manager::{MotionGfxManager, TimelineId};
@@ -30,10 +33,8 @@ fn realtime_player_update(
         q_timelines.iter().filter(|(_, p)| p.is_playing)
     {
         if let Some(timeline) = motiongfx.get_timeline_mut(id) {
-            let target_time = timeline.target_time()
-                + player.time_scale * time.delta_secs();
-
-            timeline.set_target_time(target_time);
+            timeline
+                .advance_secs(player.time_scale * time.delta_secs());
         }
     }
 }
@@ -47,11 +48,8 @@ fn fixed_rate_player_update(
     {
         if let Some(timeline) = motiongfx.get_timeline_mut(id) {
             // Each frame we update the timeline according to the fps.
-            let target_time =
-                timeline.target_time() + player.delta_secs();
-
-            timeline.set_target_time(target_time);
             player.curr_frame += 1;
+            timeline.set_target_time(player.frame_time());
         }
     }
 }
@@ -182,6 +180,22 @@ impl FixedRatePlayer {
         1.0 / self.fps as f32
     }
 
+    /// The exact timestamp of [`Self::curr_frame`].
+    ///
+    /// A given frame index always maps to the same instant no matter
+    /// how long the recording runs.
+    ///
+    /// Returns [`Duration::ZERO`] when [`Self::fps`] is zero.
+    #[inline]
+    #[must_use]
+    pub fn frame_time(&self) -> Duration {
+        if self.fps == 0 {
+            return Duration::ZERO;
+        }
+
+        Duration::from_secs(self.curr_frame) / self.fps as u32
+    }
+
     /// Setter method for setting [`Self::is_playing`].
     #[inline]
     pub const fn set_playing(
@@ -192,28 +206,86 @@ impl FixedRatePlayer {
         self
     }
 }
+
 #[derive(Default, Component)]
 pub struct PassivePlayer {
-    time: f32,
+    time: Duration,
     track_index: usize,
 }
 
 impl PassivePlayer {
     #[inline]
-    pub fn set_time(&mut self, time: f32) {
-        self.time = time;
+    pub fn set_time(&mut self, time: impl IntoDuration) {
+        self.time = time.into_duration();
     }
 
     #[inline]
     pub fn set_track_index(&mut self, track_index: usize) {
         self.track_index = track_index;
     }
 
-    pub fn get_time(&self) -> f32 {
+    pub fn get_time(&self) -> Duration {
         self.time
     }
 
     pub fn get_track(&self) -> usize {
         self.track_index
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    fn at(fps: u16, frame: u64) -> Duration {
+        FixedRatePlayer {
+            fps,
+            curr_frame: frame,
+            is_playing: false,
+        }
+        .frame_time()
+    }
+
+    #[test]
+    fn frame_time_is_exact_where_the_rate_divides() {
+        assert_eq!(at(30, 0), Duration::ZERO);
+        assert_eq!(at(30, 30), Duration::from_secs(1));
+        assert_eq!(at(30, 3), Duration::from_millis(100));
+        assert_eq!(at(25, 1), Duration::from_millis(40));
+        assert_eq!(at(60, 90), Duration::from_millis(1500));
+    }
+
+    /// The whole point of deriving from the counter: no accumulated
+    /// error, so a long render lands on exact second boundaries.
+    #[test]
+    fn frame_time_does_not_drift_over_a_long_render() {
+        // 30 minutes at 30fps.
+        assert_eq!(at(30, 54_000), Duration::from_secs(1800));
+        // Well past where the old nanosecond-based form overflowed.
+        assert_eq!(
+            at(30, 30_000_000_000),
+            Duration::from_secs(1_000_000_000)
+        );
+    }
+
+    /// 1/3s is not representable in nanoseconds, so consecutive
+    /// frames differ by a nanosecond. The error must not compound.
+    #[test]
+    fn frame_time_stays_within_a_nanosecond_of_ideal() {
+        for frame in [1u64, 2, 1_000, 999_999] {
+            let ideal = Duration::from_nanos(
+                (frame as u128 * 1_000_000_000 / 3) as u64,
+            );
+
+            assert_eq!(at(3, frame).abs_diff(ideal), Duration::ZERO);
+        }
+    }
+
+    /// A rate of zero frames per second advances no time, rather than
+    /// dividing by zero or standing in some invented rate.
+    #[test]
+    fn zero_fps_yields_zero() {
+        assert_eq!(at(0, 0), Duration::ZERO);
+        assert_eq!(at(0, 5), Duration::ZERO);
+    }
+}

crates/motiongfx/benches/action_storage.rs

@@ -156,8 +156,7 @@ fn bench_scrub(c: &mut Criterion) {
                 // the start so successive iterations are identical.
                 b.iter(|| {
                     for frame in 0..=FRAMES {
-                        let t =
-                            duration * frame as f32 / FRAMES as f32;
+                        let t = duration * frame / FRAMES;
                         timeline.set_target_time(black_box(t));
                         timeline.queue_actions();
                         timeline.sample_queued_actions(
@@ -383,8 +382,7 @@ fn bench_mixed_scrub(c: &mut Criterion) {
 
                 b.iter(|| {
                     for frame in 0..=FRAMES {
-                        let t =
-                            duration * frame as f32 / FRAMES as f32;
+                        let t = duration * frame / FRAMES;
                         timeline.set_target_time(black_box(t));
                         timeline.queue_actions();
                         timeline.sample_queued_actions(

crates/motiongfx/examples/custom_world.rs

@@ -120,8 +120,8 @@ fn main() {
 
     println!(
         "# Before sampling \ncurrent time: {}s,\ntarget time: {}s",
-        timeline.curr_time(),
-        timeline.target_time()
+        timeline.curr_time().as_secs_f32(),
+        timeline.target_time().as_secs_f32()
     );
     println!("target time clamped to timeline duration (3s)\n");
 
@@ -134,8 +134,8 @@ fn main() {
 
     println!(
         "# After sampling: \ncurrent time: {}s,\ntarget time: {}s\n",
-        timeline.curr_time(),
-        timeline.target_time()
+        timeline.curr_time().as_secs_f32(),
+        timeline.target_time().as_secs_f32()
     );
 }
 

crates/motiongfx/src/action.rs

@@ -1,4 +1,5 @@
 use core::any::TypeId;
+use core::time::Duration;
 
 use alloc::boxed::Box;
 use field_path::field::UntypedField;
@@ -125,26 +126,83 @@ pub type EaseFn = fn(t: f32) -> f32;
 #[derive(Debug, Clone, Copy)]
 pub struct EaseStorage(pub EaseFn);
 
-#[derive(Debug, Clone, Copy, PartialEq)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq)]
 pub struct ActionClip {
     pub id: ActionId,
-    pub start: f32,
-    pub duration: f32,
+    pub start: Duration,
+    pub duration: Duration,
 }
 
 impl ActionClip {
-    pub const fn new(id: ActionId, duration: f32) -> Self {
+    pub const fn new(id: ActionId, duration: Duration) -> Self {
         Self {
             id,
-            start: 0.0,
+            start: Duration::ZERO,
             duration,
         }
     }
 
     #[inline]
-    pub fn end(&self) -> f32 {
+    pub fn end(&self) -> Duration {
         self.start + self.duration
     }
+
+    /// Normalized progress of `time` through this clip, in
+    /// \[0.0..=1.0\].
+    ///
+    /// Zero-duration clips (used as spacers) report `1.0` rather than
+    /// dividing by zero.
+    #[inline]
+    pub fn progress(&self, time: Duration) -> f32 {
+        let span = self.duration.as_secs_f64();
+
+        if span == 0.0 {
+            return 1.0;
+        }
+
+        let elapsed = time.saturating_sub(self.start).as_secs_f64();
+        (elapsed / span).clamp(0.0, 1.0) as f32
+    }
+}
+
+#[cfg(test)]
+mod clip_tests {
+    use super::*;
+
+    const fn clip(start: u64, duration: u64) -> ActionClip {
+        ActionClip {
+            id: ActionId::PLACEHOLDER,
+            start: Duration::from_millis(start),
+            duration: Duration::from_millis(duration),
+        }
+    }
+
+    #[test]
+    fn progress_spans_the_clip() {
+        let clip = clip(1000, 2000);
+
+        assert_eq!(clip.progress(Duration::from_millis(1000)), 0.0);
+        assert_eq!(clip.progress(Duration::from_millis(2000)), 0.5);
+        assert_eq!(clip.progress(Duration::from_millis(3000)), 1.0);
+    }
+
+    #[test]
+    fn progress_clamps_outside_the_clip() {
+        let clip = clip(1000, 2000);
+
+        assert_eq!(clip.progress(Duration::ZERO), 0.0);
+        assert_eq!(clip.progress(Duration::from_secs(60)), 1.0);
+    }
+
+    /// Zero-duration clips are used as spacers; they must not yield
+    /// `NaN` and poison the interpolated value.
+    #[test]
+    fn zero_duration_clip_reports_completion() {
+        let t = clip(1000, 0).progress(Duration::from_millis(1000));
+
+        assert!(!t.is_nan());
+        assert_eq!(t, 1.0);
+    }
 }
 
 pub struct Segment<T> {

crates/motiongfx/src/action/table.rs

@@ -16,6 +16,7 @@ use super::{
 use crate::ThreadSafe;
 use crate::resources::Resources;
 use crate::subject::SubjectId;
+use crate::time::IntoDuration;
 use crate::track::TrackFragment;
 
 /// Phantom marker distinguishing [`ActionId`]'s [`GenId`] domain from
@@ -232,10 +233,10 @@ impl<T> InterpActionBuilder<'_, T> {
 
     /// Confirms the configuration of the action and creates a
     /// [`TrackFragment`].
-    pub fn play(self, duration: f32) -> TrackFragment {
+    pub fn play(self, duration: impl IntoDuration) -> TrackFragment {
         TrackFragment::single(
             self.inner.key,
-            ActionClip::new(self.id(), duration),
+            ActionClip::new(self.id(), duration.into_duration()),
         )
     }
 }

crates/motiongfx/src/lib.rs

@@ -11,6 +11,7 @@ pub mod registry;
 mod resources;
 pub mod sequence;
 pub mod subject;
+pub mod time;
 pub mod timeline;
 pub mod track;
 pub mod world;
@@ -33,6 +34,7 @@ pub mod prelude {
     pub use crate::registry::{
         AccessorRegistry, PipelineRegistry, Registry,
     };
+    pub use crate::time::IntoDuration;
     pub use crate::timeline::{Timeline, TimelineBuilder};
     pub use crate::track::{Track, TrackFragment, TrackOrdering};
     pub use crate::world::SubjectSource;

crates/motiongfx/src/pipeline.rs

@@ -2,6 +2,7 @@ pub mod func_pointers;
 
 use core::any::TypeId;
 use core::marker::PhantomData;
+use core::time::Duration;
 
 use func_pointers::{BakeFnPtr, SampleFnPtr};
 
@@ -305,10 +306,10 @@ where
     }
 }
 
-#[derive(Default, Debug, PartialEq, Clone, Copy)]
+#[derive(Default, Debug, PartialEq, Eq, Clone, Copy)]
 pub struct Range {
-    pub start: f32,
-    pub end: f32,
+    pub start: Duration,
+    pub end: Duration,
 }
 
 impl Range {
@@ -441,21 +442,23 @@ mod tests {
 
     #[test]
     fn range_overlap_behavior() {
+        let secs = Duration::from_secs;
+
         let a = Range {
-            start: 0.0,
-            end: 5.0,
+            start: secs(0),
+            end: secs(5),
         };
         let b = Range {
-            start: 3.0,
-            end: 8.0,
+            start: secs(3),
+            end: secs(8),
         };
         let c = Range {
-            start: 6.0,
-            end: 10.0,
+            start: secs(6),
+            end: secs(10),
         };
         let d = Range {
-            start: 5.0,
-            end: 5.0,
+            start: secs(5),
+            end: secs(5),
         }; // touching boundary
 
         assert!(