From 1b436894b094be3509612e33a60fb6254e1088e1 Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:27:47 +0800 Subject: [PATCH 01/12] Switch from `f32` to `Duration` to measure time --- crates/bevy_motiongfx/src/controller.rs | 40 +++-- crates/motiongfx/benches/action_storage.rs | 6 +- crates/motiongfx/examples/custom_world.rs | 8 +- crates/motiongfx/src/action.rs | 70 ++++++++- crates/motiongfx/src/action/table.rs | 5 +- crates/motiongfx/src/lib.rs | 2 + crates/motiongfx/src/pipeline.rs | 25 +-- crates/motiongfx/src/sequence.rs | 14 +- crates/motiongfx/src/time.rs | 137 +++++++++++++++++ crates/motiongfx/src/timeline.rs | 42 +++-- crates/motiongfx/src/track.rs | 143 ++++++++++++++---- crates/motiongfx_editor/src/lib.rs | 35 +++-- .../bevy_examples/examples/slide_basic.rs | 7 +- examples/bevy_examples/src/lib.rs | 8 +- .../vello_winit_example/examples/lissajous.rs | 18 ++- 15 files changed, 439 insertions(+), 121 deletions(-) create mode 100644 crates/motiongfx/src/time.rs diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index 28c71976..92361d9b 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/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,11 @@ 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); + // Derive the time from the frame counter rather than + // accumulating, so recorded frame N always lands on the + // same timestamp. player.curr_frame += 1; + timeline.set_target_time(player.frame_time()); } } } @@ -182,6 +183,21 @@ impl FixedRatePlayer { 1.0 / self.fps as f32 } + /// The exact timestamp of [`Self::curr_frame`]. + /// + /// Derived from the frame counter rather than accumulated, so a + /// given frame index always maps to the same instant no matter how + /// long the recording runs. + #[inline] + #[must_use] + pub const fn frame_time(&self) -> Duration { + let fps = if self.fps == 0 { 1 } else { self.fps as u64 }; + + Duration::from_nanos( + 1_000_000_000u64.saturating_mul(self.curr_frame) / fps, + ) + } + /// Setter method for setting [`Self::is_playing`]. #[inline] pub const fn set_playing( @@ -194,14 +210,14 @@ impl FixedRatePlayer { } #[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] @@ -209,7 +225,7 @@ impl PassivePlayer { self.track_index = track_index; } - pub fn get_time(&self) -> f32 { + pub fn get_time(&self) -> Duration { self.time } diff --git a/crates/motiongfx/benches/action_storage.rs b/crates/motiongfx/benches/action_storage.rs index 33d74f48..14210fe6 100644 --- a/crates/motiongfx/benches/action_storage.rs +++ b/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( diff --git a/crates/motiongfx/examples/custom_world.rs b/crates/motiongfx/examples/custom_world.rs index fec2eb21..809ded07 100644 --- a/crates/motiongfx/examples/custom_world.rs +++ b/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() ); } diff --git a/crates/motiongfx/src/action.rs b/crates/motiongfx/src/action.rs index 6e045fd5..09de3060 100644 --- a/crates/motiongfx/src/action.rs +++ b/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 { diff --git a/crates/motiongfx/src/action/table.rs b/crates/motiongfx/src/action/table.rs index a987caae..e2390519 100644 --- a/crates/motiongfx/src/action/table.rs +++ b/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 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()), ) } } diff --git a/crates/motiongfx/src/lib.rs b/crates/motiongfx/src/lib.rs index 709c74e6..74dfc3f0 100644 --- a/crates/motiongfx/src/lib.rs +++ b/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; diff --git a/crates/motiongfx/src/pipeline.rs b/crates/motiongfx/src/pipeline.rs index 8d77ef11..3ef566c7 100644 --- a/crates/motiongfx/src/pipeline.rs +++ b/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!( diff --git a/crates/motiongfx/src/sequence.rs b/crates/motiongfx/src/sequence.rs index d7df28a8..0ccbdde0 100644 --- a/crates/motiongfx/src/sequence.rs +++ b/crates/motiongfx/src/sequence.rs @@ -1,3 +1,5 @@ +use core::time::Duration; + use nonempty::NonEmpty; use crate::action::ActionClip; @@ -23,23 +25,23 @@ impl Sequence { /// Get the start time of the sequence. #[inline] - pub fn start(&self) -> f32 { + pub fn start(&self) -> Duration { self.clips.first().start } /// Get the end time of the sequence. #[inline] - pub fn end(&self) -> f32 { + pub fn end(&self) -> Duration { self.clips.last().end() } /// Get the duration of the sequence. #[inline] - pub fn duration(&self) -> f32 { + pub fn duration(&self) -> Duration { self.end() - self.start() } - pub(crate) fn delay(&mut self, duration: f32) { + pub(crate) fn delay(&mut self, duration: Duration) { for clip in self.clips.iter_mut() { clip.start += duration; } @@ -51,7 +53,7 @@ impl Sequence { pub fn push(&mut self, span: ActionClip) { debug_assert!( span.start >= self.end(), - "({} >= {}) `ActionClip`s shouldn't overlap!", + "({:?} >= {:?}) `ActionClip`s shouldn't overlap!", span.start, self.end(), ); @@ -73,7 +75,7 @@ impl Extend for Sequence { iter.into_iter().inspect(|clip| { debug_assert!( clip.start >= end, - "({} >= {}) `ActionClip`s shouldn't overlap!", + "({:?} >= {:?}) `ActionClip`s shouldn't overlap!", clip.start, end, ); diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs new file mode 100644 index 00000000..424368af --- /dev/null +++ b/crates/motiongfx/src/time.rs @@ -0,0 +1,137 @@ +//! Time conversion helpers. +//! +//! Motiongfx stores all timing as [`Duration`] so that the durations +//! accumulated by the track combinators and the clip offsets +//! accumulated by [`Sequence::delay`] can never disagree. Float seconds +//! are not associative, so the two accumulations used to drift apart by +//! a few ULPs, which tripped the non-overlap assertion in +//! [`Sequence::push`] and left the playhead clamp just short of the +//! final clip's end. +//! +//! Authoring in seconds is still the ergonomic default, hence +//! [`IntoDuration`]. +//! +//! [`Sequence::delay`]: crate::sequence::Sequence::delay +//! [`Sequence::push`]: crate::sequence::Sequence::push + +use core::time::Duration; + +/// Conversion into a [`Duration`] for the timing arguments of the +/// authoring API. +/// +/// Implemented for [`Duration`] itself as well as `f32`/`f64` seconds, +/// so both of these are accepted: +/// +/// ```ignore +/// action.play(0.6); +/// action.play(Duration::from_millis(600)); +/// ``` +/// +/// Seconds that a [`Duration`] cannot represent saturate rather than +/// panic: negative and non-finite values become [`Duration::ZERO`], +/// and values past [`Duration::MAX`] become [`Duration::MAX`]. This +/// keeps idioms like `set_target_time(f32::MAX)` ("seek to the end") +/// working. +pub trait IntoDuration { + fn into_duration(self) -> Duration; +} + +impl IntoDuration for Duration { + #[inline] + fn into_duration(self) -> Duration { + self + } +} + +impl IntoDuration for f32 { + #[inline] + fn into_duration(self) -> Duration { + (self as f64).into_duration() + } +} + +impl IntoDuration for f64 { + #[inline] + fn into_duration(self) -> Duration { + if self.is_nan() || self <= 0.0 { + return Duration::ZERO; + } + + // Positive and non-NaN at this point, so the only remaining + // failure is overflow (which includes `INFINITY`). + // `from_secs_f64` would panic on those instead. + Duration::try_from_secs_f64(self).unwrap_or(Duration::MAX) + } +} + +/// Offsets `time` by `delta` seconds, saturating at [`Duration::ZERO`]. +/// +/// [`Duration`] has no signed representation, so stepping a playhead +/// backwards has to go through this rather than a plain `+`. +/// +/// The offset is only as precise as `delta` itself: `0.05f32` is really +/// `0.05000000074505806`, so it lands a nanosecond off. That is +/// inherent to a float-seconds clock and is not the drift this module +/// exists to prevent — clip and track boundaries are exact +/// [`Duration`]s, so an imprecise step still resolves against them +/// consistently instead of accumulating error into the timeline. +#[inline] +pub fn offset_secs(time: Duration, delta: f32) -> Duration { + if delta >= 0.0 { + time.saturating_add(delta.into_duration()) + } else { + time.saturating_sub((-delta).into_duration()) + } +} + +#[cfg(test)] +mod tests { + use super::*; + + #[test] + fn unrepresentable_seconds_saturate_instead_of_panicking() { + assert_eq!((-1.0f32).into_duration(), Duration::ZERO); + assert_eq!(f32::NAN.into_duration(), Duration::ZERO); + assert_eq!(f32::NEG_INFINITY.into_duration(), Duration::ZERO); + // "Seek to the end" idiom: must clamp, not overflow. + assert_eq!(f32::MAX.into_duration(), Duration::MAX); + assert_eq!(f32::INFINITY.into_duration(), Duration::MAX); + } + + /// `delta` is `f32` seconds, so the result is only accurate to + /// within the rounding of that `f32` — a nanosecond or so. Exact + /// equality is deliberately not asserted here. + #[track_caller] + fn assert_near(actual: Duration, expected: Duration) { + let diff = actual.abs_diff(expected); + + assert!( + diff < Duration::from_micros(1), + "{actual:?} is not within 1us of {expected:?}", + ); + } + + #[test] + fn offset_secs_steps_both_ways() { + let time = Duration::from_millis(100); + + assert_near( + offset_secs(time, -0.05), + Duration::from_millis(50), + ); + assert_near( + offset_secs(time, 0.05), + Duration::from_millis(150), + ); + } + + #[test] + fn offset_secs_saturates_at_zero() { + let time = Duration::from_millis(100); + + // Stepping further back than the playhead has to give must + // saturate rather than wrap or panic. + assert_eq!(offset_secs(time, -10.0), Duration::ZERO); + assert_eq!(offset_secs(Duration::ZERO, -1.0), Duration::ZERO); + } +} diff --git a/crates/motiongfx/src/timeline.rs b/crates/motiongfx/src/timeline.rs index 59f84f20..d8e39718 100644 --- a/crates/motiongfx/src/timeline.rs +++ b/crates/motiongfx/src/timeline.rs @@ -1,5 +1,6 @@ use core::cmp::Ordering; use core::marker::PhantomData; +use core::time::Duration; use alloc::boxed::Box; use alloc::vec::Vec; @@ -15,6 +16,7 @@ use crate::interpolation::Interpolation; use crate::pipeline::{BakeCtx, PipelineKey, Range, SampleCtx}; use crate::registry::Registry; use crate::subject::SubjectId; +use crate::time::{self, IntoDuration}; use crate::track::Track; use crate::world::SubjectSource; @@ -35,9 +37,9 @@ pub struct Timeline { /// actions of each type, with no per-action column lookup. sample_queue: HashMap>, /// The current time of the current track. - curr_time: f32, + curr_time: Duration, /// The target time of the target track. - target_time: f32, + target_time: Duration, /// The index of the current track. curr_index: usize, /// The index of the target track. @@ -93,7 +95,7 @@ impl Timeline { > self.curr_index() { // From the start. - curr_time = 0.0; + curr_time = Duration::ZERO; ( SampleMode::End, self.curr_index()..self.target_index(), @@ -179,13 +181,12 @@ impl Timeline { Ok(index) => { let clip = &clips[index]; - let t = (self.target_time - clip.start) - / (clip.end() - clip.start); - self.queue_cache.cache( *key, clip.id, - SampleMode::Interp(t), + SampleMode::Interp( + clip.progress(self.target_time), + ), ); } // `target_time` is out of bounds. @@ -274,13 +275,13 @@ impl Timeline { /// Returns the current playback time. #[inline] - pub fn curr_time(&self) -> f32 { + pub fn curr_time(&self) -> Duration { self.curr_time } /// Returns the target playback time. #[inline] - pub fn target_time(&self) -> f32 { + pub fn target_time(&self) -> Duration { self.target_time } @@ -342,13 +343,28 @@ impl Timeline { impl Timeline { /// Set the target time of the current track, clamping the value /// within \[0.0..=track.duration\] - pub fn set_target_time(&mut self, target_time: f32) -> &mut Self { + pub fn set_target_time( + &mut self, + target_time: impl IntoDuration, + ) -> &mut Self { let duration = self.tracks[self.target_index].duration(); - self.target_time = target_time.clamp(0.0, duration); + self.target_time = target_time.into_duration().min(duration); self } + /// Steps the target time by `delta` seconds, saturating at both + /// ends of the target track. + /// + /// Prefer this over reading [`Self::target_time`] and adding to it: + /// a [`Duration`] cannot go negative, so backwards playback + /// (`delta < 0.0`) needs the saturating arithmetic this performs. + pub fn advance_secs(&mut self, delta: f32) -> &mut Self { + let target_time = time::offset_secs(self.target_time, delta); + + self.set_target_time(target_time) + } + /// Set the target track index, clamping the value within /// \[0..=track_count - 1\]. pub fn set_target_track( @@ -565,8 +581,8 @@ impl<'a, W: 'static> TimelineBuilder<'a, W> { tracks: self.tracks.into_boxed_slice(), queue_cache: QueueCache::new(), sample_queue: HashMap::new(), - curr_time: 0.0, - target_time: 0.0, + curr_time: Duration::ZERO, + target_time: Duration::ZERO, curr_index: 0, target_index: 0, _marker: PhantomData, diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index d671731f..251529dd 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -1,3 +1,5 @@ +use core::time::Duration; + use alloc::boxed::Box; use alloc::vec::Vec; use field_path::field::UntypedField; @@ -5,13 +7,14 @@ use hashbrown::HashMap; use crate::action::{ActionClip, ActionKey}; use crate::sequence::Sequence; +use crate::time::IntoDuration; pub trait TrackOrdering { /// Run all [`TrackFragment`]s one after another. fn ord_chain(self) -> TrackFragment; fn ord_all(self) -> TrackFragment; fn ord_any(self) -> TrackFragment; - fn ord_flow(self, delay: f32) -> TrackFragment; + fn ord_flow(self, delay: impl IntoDuration) -> TrackFragment; } impl TrackOrdering for T @@ -30,7 +33,7 @@ where any(self) } - fn ord_flow(self, delay: f32) -> TrackFragment { + fn ord_flow(self, delay: impl IntoDuration) -> TrackFragment { flow(delay, self) } } @@ -106,13 +109,14 @@ pub fn any( /// Run one [`Track`] after another with a fixed delay time. #[must_use = "This function consumes all the given tracks and returns a modified one."] pub fn flow( - delay: f32, + delay: impl IntoDuration, tracks: impl IntoIterator, ) -> TrackFragment { + let delay = delay.into_duration(); let mut tracks_iter = tracks.into_iter(); let mut track = tracks_iter.next().unwrap_or_default(); - let mut flow_delay = 0.0; + let mut flow_delay = Duration::ZERO; let mut final_duration = track.duration; for other_track in tracks_iter { @@ -132,7 +136,12 @@ pub fn flow( /// Run a [`Track`] after a fixed delay time. #[must_use = "This function consumes the given track and returns a modified one."] -pub fn delay(delay: f32, mut track: TrackFragment) -> TrackFragment { +pub fn delay( + delay: impl IntoDuration, + mut track: TrackFragment, +) -> TrackFragment { + let delay = delay.into_duration(); + for sequence in track.sequences.values_mut() { sequence.delay(delay); } @@ -142,14 +151,14 @@ pub fn delay(delay: f32, mut track: TrackFragment) -> TrackFragment { pub struct TrackFragment { sequences: HashMap, - duration: f32, + duration: Duration, } impl TrackFragment { pub fn new() -> Self { Self { sequences: HashMap::new(), - duration: 0.0, + duration: Duration::ZERO, } } @@ -194,8 +203,29 @@ impl TrackFragment { pub fn compile(self) -> Track { let mut sequences = self.sequences.into_iter().collect::>(); + + if sequences.is_empty() { + return Track { + field_lookups: Box::new([]), + sequence_spans: Box::new([]), + clip_arena: Box::new([]), + duration: self.duration, + }; + } + sequences.sort_by_key(|(key, _)| *key.field()); + // The combinators accumulate `duration` independently of the + // clip offsets, so pin the two together here: the playhead + // clamp in `Timeline::set_target_time` must be able to reach + // the end of the last clip. + let duration = sequences + .iter() + .map(|(_, seq)| seq.end()) + .max() + .unwrap_or(Duration::ZERO) + .max(self.duration); + let mut seq_offset = 0; let mut sequence_spans = Vec::with_capacity(sequences.len()); @@ -248,7 +278,7 @@ impl TrackFragment { field_lookups: field_lookups.into_boxed_slice(), sequence_spans: sequence_spans.into_boxed_slice(), clip_arena, - duration: self.duration, + duration, } } } @@ -284,8 +314,11 @@ pub struct Track { /// Contiguous storage of all action clips. clip_arena: Box<[ActionClip]>, - /// Total duration of the track in seconds. - duration: f32, + /// Total duration of the track. + /// + /// Guaranteed to be `>=` the end of every clip in `clip_arena`. + /// See [`TrackFragment::compile`]. + duration: Duration, } impl Track { @@ -321,7 +354,7 @@ impl Track { } #[inline] - pub fn duration(&self) -> f32 { + pub fn duration(&self) -> Duration { self.duration } } @@ -360,14 +393,18 @@ mod tests { ) } - const fn clip(duration: f32) -> ActionClip { - ActionClip::new(ActionId::PLACEHOLDER, duration) + const fn ms(millis: u64) -> Duration { + Duration::from_millis(millis) + } + + const fn clip(millis: u64) -> ActionClip { + ActionClip::new(ActionId::PLACEHOLDER, ms(millis)) } #[test] fn track_key_uniqueness() { // Sequence with 0 duration to prevent overlaps. - const DUMMY_SEQ: Sequence = Sequence::new(clip(0.0)); + const DUMMY_SEQ: Sequence = Sequence::new(clip(0)); let entity1 = DummyId(1); let entity2 = DummyId(2); @@ -403,57 +440,99 @@ mod tests { #[test] fn chain_duration_and_delay() { - let track1 = TrackFragment::single(key("a"), clip(1.0)); - let track2 = TrackFragment::single(key("b"), clip(2.0)); + let track1 = TrackFragment::single(key("a"), clip(1000)); + let track2 = TrackFragment::single(key("b"), clip(2000)); let track = [track1, track2].ord_chain(); - assert_eq!(track.duration, 3.0); + assert_eq!(track.duration, ms(3000)); let seq_b = &track.sequences[&key("b")]; // `seq_b` should be delayed by 1.0 (duration of `track1`). - assert_eq!(seq_b.start(), 1.0); + assert_eq!(seq_b.start(), ms(1000)); } #[test] fn all_duration_max() { - let track1 = TrackFragment::single(key("a"), clip(1.0)); - let track2 = TrackFragment::single(key("b"), clip(3.0)); + let track1 = TrackFragment::single(key("a"), clip(1000)); + let track2 = TrackFragment::single(key("b"), clip(3000)); let track = [track1, track2].ord_all(); - assert_eq!(track.duration, 3.0); + assert_eq!(track.duration, ms(3000)); } #[test] fn any_duration_min() { - let track1 = TrackFragment::single(key("a"), clip(1.0)); - let track2 = TrackFragment::single(key("b"), clip(3.0)); + let track1 = TrackFragment::single(key("a"), clip(1000)); + let track2 = TrackFragment::single(key("b"), clip(3000)); let track = [track1, track2].ord_any(); - assert_eq!(track.duration, 1.0); + assert_eq!(track.duration, ms(1000)); } #[test] fn flow_with_delay() { - let track1 = TrackFragment::single(key("a"), clip(1.0)); - let track2 = TrackFragment::single(key("b"), clip(1.0)); + let track1 = TrackFragment::single(key("a"), clip(1000)); + let track2 = TrackFragment::single(key("b"), clip(1000)); let track = [track1, track2].ord_flow(0.5); - assert_eq!(track.duration, 1.5); // 0.5 delay + 1.0 duration + // 0.5 delay + 1.0 duration + assert_eq!(track.duration, ms(1500)); let seq_b = &track.sequences[&key("b")]; // `seq_b` should be delayed by 0.5 - assert_eq!(seq_b.start(), 0.5); + assert_eq!(seq_b.start(), ms(500)); } #[test] fn delay_applies_offset() { - let track = TrackFragment::single(key("a"), clip(2.0)); + let track = TrackFragment::single(key("a"), clip(2000)); let track = delay(1.5, track); let seq_a = &track.sequences[&key("a")]; - assert_eq!(seq_a.start(), 1.5); - assert_eq!(seq_a.end(), 3.5); - assert_eq!(track.duration, 2.0); + assert_eq!(seq_a.start(), ms(1500)); + assert_eq!(seq_a.end(), ms(3500)); + assert_eq!(track.duration, ms(2000)); + } + + /// Chaining durations that have no exact `f32` representation used + /// to leave `TrackFragment::duration` and the clip offsets on + /// different values, because the two are accumulated separately. + /// The mismatch tripped the non-overlap assert in `Sequence::push` + /// and put `Track::duration` out of reach of the last clip's end. + #[test] + fn chain_accumulation_matches_clip_offsets() { + // 0.1s is not representable in binary floating point. + let tracks: Vec<_> = (0..10) + .map(|_| TrackFragment::single(key("a"), clip(100))) + .collect(); + + let track = tracks.ord_chain(); + + assert_eq!(track.duration, ms(1000)); + assert_eq!(track.sequences[&key("a")].end(), ms(1000)); + } + + /// `Track::duration` must always be reachable by the playhead, so + /// that the final clip can resolve to `SampleMode::End`. + #[test] + fn compile_duration_covers_last_clip_end() { + let mut fragment = + TrackFragment::single(key("a"), clip(1000)); + // Understate the duration the way a combinator would if the + // two accumulations ever diverged again. + fragment.duration = ms(999); + + let track = fragment.compile(); + + assert_eq!(track.duration(), ms(1000)); + } + + #[test] + fn compile_empty_fragment_is_not_a_panic() { + let track = TrackFragment::new().compile(); + + assert_eq!(track.duration(), Duration::ZERO); + assert!(track.sequences_spans().is_empty()); } } diff --git a/crates/motiongfx_editor/src/lib.rs b/crates/motiongfx_editor/src/lib.rs index 9f833914..bcef08ea 100644 --- a/crates/motiongfx_editor/src/lib.rs +++ b/crates/motiongfx_editor/src/lib.rs @@ -17,6 +17,8 @@ mod ui; +use core::time::Duration; + use bevy::camera::{ClearColorConfig, RenderTarget, Viewport}; use bevy::feathers::dark_theme::create_dark_theme; use bevy::feathers::theme::{ThemeBackgroundColor, UiTheme}; @@ -320,18 +322,16 @@ fn build_timeline_view( .iter() .map(|(k, s)| (k, s)) .collect(); - rows.sort_by(|a, b| { - let start = |span: &Span| { - track - .clips(*span) - .first() - .map(|c| c.start) - .unwrap_or(f32::INFINITY) - }; - start(a.1).total_cmp(&start(b.1)) + rows.sort_by_key(|(_, span)| { + track + .clips(**span) + .first() + .map(|c| c.start) + .unwrap_or(Duration::MAX) }); - let duration = track.duration(); + // The layout below is pixel math, so drop to seconds here. + let duration = track.duration().as_secs_f32(); let content_width = (duration * PIXELS_PER_SECOND).max(1.0); let content_height = TRACK_TOP_PADDING * 2.0 + rows.len() as f32 * ROW_STRIDE; @@ -358,8 +358,10 @@ fn build_timeline_view( .insert(ChildOf(name_panel)); for clip in track.clips(**span) { - let left = clip.start * PIXELS_PER_SECOND; - let width = (clip.duration * PIXELS_PER_SECOND).max(2.0); + let left = clip.start.as_secs_f32() * PIXELS_PER_SECOND; + let width = (clip.duration.as_secs_f32() + * PIXELS_PER_SECOND) + .max(2.0); commands .spawn_scene(action_box( @@ -425,7 +427,7 @@ fn on_play_pause( if let Some(timeline_id) = state.timeline && q_players.iter().any(|p| p.is_playing) && let Some(timeline) = manager.get_timeline_mut(&timeline_id) - && timeline.target_time() >= state.duration + && timeline.target_time().as_secs_f32() >= state.duration { timeline.set_target_track(0); timeline.set_target_time(0.0); @@ -448,7 +450,8 @@ fn update_playhead( let Some(timeline) = manager.get_timeline(&timeline_id) else { return; }; - let time = timeline.target_time(); + // The slider and playhead are both in seconds-as-pixels. + let time = timeline.target_time().as_secs_f32(); for mut node in &mut q_playhead { node.left = Val::Px(time * PIXELS_PER_SECOND); @@ -494,9 +497,9 @@ fn stop_at_track_end( continue; } let at_end = if player.time_scale >= 0.0 { - timeline.target_time() >= state.duration + timeline.target_time().as_secs_f32() >= state.duration } else { - timeline.target_time() <= 0.0 + timeline.target_time() == Duration::ZERO }; if at_end { player.is_playing = false; diff --git a/examples/bevy_examples/examples/slide_basic.rs b/examples/bevy_examples/examples/slide_basic.rs index ce5ce4ea..11e43283 100644 --- a/examples/bevy_examples/examples/slide_basic.rs +++ b/examples/bevy_examples/examples/slide_basic.rs @@ -1,3 +1,5 @@ +use std::time::Duration; + use bevy::camera::Hdr; use bevy::color::palettes; use bevy::prelude::*; @@ -141,14 +143,15 @@ fn slide_movement( ]) { player.set_playing(true).set_time_scale(-1.0); - if timeline.curr_time() <= 0.0 + if timeline.curr_time() == Duration::ZERO && timeline.curr_index() > 0 { // Move to the end of the previous track. let target_index = timeline.curr_index().saturating_sub(1); timeline.set_target_track(target_index); - timeline.set_target_time(f32::MAX); + // Clamped to the new track's duration. + timeline.set_target_time(Duration::MAX); } } else { player.set_playing(true).set_time_scale(1.0); diff --git a/examples/bevy_examples/src/lib.rs b/examples/bevy_examples/src/lib.rs index 191c33fb..7d494f62 100644 --- a/examples/bevy_examples/src/lib.rs +++ b/examples/bevy_examples/src/lib.rs @@ -12,16 +12,12 @@ pub fn timeline_movement( if keys.any_pressed([KeyCode::KeyD, KeyCode::ArrowRight]) { player.set_playing(false); - let target_time = - timeline.target_time() + time.delta_secs(); - timeline.set_target_time(target_time); + timeline.advance_secs(time.delta_secs()); } if keys.any_pressed([KeyCode::KeyA, KeyCode::ArrowLeft]) { player.set_playing(false); - let target_time = - timeline.target_time() - time.delta_secs(); - timeline.set_target_time(target_time); + timeline.advance_secs(-time.delta_secs()); } if keys.just_pressed(KeyCode::Space) { diff --git a/examples/vello_winit_example/examples/lissajous.rs b/examples/vello_winit_example/examples/lissajous.rs index 11173e12..5c49d4eb 100644 --- a/examples/vello_winit_example/examples/lissajous.rs +++ b/examples/vello_winit_example/examples/lissajous.rs @@ -1,5 +1,5 @@ use core::f64; -use std::time::Instant; +use std::time::{Duration, Instant}; use kurbo::{Affine, BezPath, Vec2}; use motiongfx::prelude::*; @@ -90,8 +90,8 @@ struct LissajousTableDemo { world: TableWorld, start: Instant, timeline: Timeline, - grid_duration: f32, - curve_duration: f32, + grid_duration: Duration, + curve_duration: Duration, window_size: kurbo::Size, } @@ -263,17 +263,21 @@ impl VelloDemo for LissajousTableDemo { scene: &mut vello::Scene, scale_factor: f64, ) { - let elapsed = self.start.elapsed().as_secs_f32(); + let elapsed = self.start.elapsed(); // Track 0 plays once; once done we lock to track 1 and loop it. if elapsed < self.grid_duration { self.timeline.set_target_track(0); self.timeline.set_target_time(elapsed); } else { + // `Duration` has no `Rem`, so wrap through nanoseconds. + let into_loop = (elapsed - self.grid_duration).as_nanos(); + let period = self.curve_duration.as_nanos().max(1); + self.timeline.set_target_track(1); - self.timeline.set_target_time( - (elapsed - self.grid_duration) % self.curve_duration, - ); + self.timeline.set_target_time(Duration::from_nanos( + (into_loop % period) as u64, + )); } self.timeline.queue_actions(); self.timeline From b81a6a05251849072126d4bfd557762b31016cbc Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:28:45 +0800 Subject: [PATCH 02/12] Update .gitignore --- .gitignore | 1 + 1 file changed, 1 insertion(+) diff --git a/.gitignore b/.gitignore index 909cc324..3f0a1495 100644 --- a/.gitignore +++ b/.gitignore @@ -1,2 +1,3 @@ /target /frames/*.png +*.DS_Store From b1922378569d3d5aa1e103db69766cc87255225e Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:48:13 +0800 Subject: [PATCH 03/12] `frame_time` makes better use of `Duration`'s second param --- crates/bevy_motiongfx/src/controller.rs | 78 +++++++++++++++++++++---- 1 file changed, 67 insertions(+), 11 deletions(-) diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index 92361d9b..1ff4241a 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -48,9 +48,6 @@ fn fixed_rate_player_update( { if let Some(timeline) = motiongfx.get_timeline_mut(id) { // Each frame we update the timeline according to the fps. - // Derive the time from the frame counter rather than - // accumulating, so recorded frame N always lands on the - // same timestamp. player.curr_frame += 1; timeline.set_target_time(player.frame_time()); } @@ -185,17 +182,18 @@ impl FixedRatePlayer { /// The exact timestamp of [`Self::curr_frame`]. /// - /// Derived from the frame counter rather than accumulated, so a - /// given frame index always maps to the same instant no matter how - /// long the recording runs. + /// 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 const fn frame_time(&self) -> Duration { - let fps = if self.fps == 0 { 1 } else { self.fps as u64 }; + pub fn frame_time(&self) -> Duration { + if self.fps == 0 { + return Duration::ZERO; + } - Duration::from_nanos( - 1_000_000_000u64.saturating_mul(self.curr_frame) / fps, - ) + Duration::from_secs(self.curr_frame) / self.fps as u32 } /// Setter method for setting [`Self::is_playing`]. @@ -208,6 +206,7 @@ impl FixedRatePlayer { self } } + #[derive(Default, Component)] pub struct PassivePlayer { time: Duration, @@ -233,3 +232,60 @@ impl PassivePlayer { 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); + } +} From bae0a0cce490063329274dafb1a5393f32d1c4ca Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 11:49:49 +0800 Subject: [PATCH 04/12] Fixed docs --- crates/motiongfx/src/time.rs | 15 +++++++-------- 1 file changed, 7 insertions(+), 8 deletions(-) diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index 424368af..7c03a1a8 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -1,18 +1,17 @@ //! Time conversion helpers. //! //! Motiongfx stores all timing as [`Duration`] so that the durations -//! accumulated by the track combinators and the clip offsets -//! accumulated by [`Sequence::delay`] can never disagree. Float seconds -//! are not associative, so the two accumulations used to drift apart by -//! a few ULPs, which tripped the non-overlap assertion in -//! [`Sequence::push`] and left the playhead clamp just short of the -//! final clip's end. +//! accumulated by the track combinators and the clip offsets accumulated +//! when delaying a [`Sequence`] can never disagree. Float seconds are not +//! associative, so the two accumulations used to drift apart by a few +//! ULPs, which tripped the non-overlap assertion when extending a +//! sequence and left the playhead clamp just short of the final clip's +//! end. //! //! Authoring in seconds is still the ergonomic default, hence //! [`IntoDuration`]. //! -//! [`Sequence::delay`]: crate::sequence::Sequence::delay -//! [`Sequence::push`]: crate::sequence::Sequence::push +//! [`Sequence`]: crate::sequence::Sequence use core::time::Duration; From 9efc2c5611ff4e3433bf057ad620cf621d00ecd7 Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:42:57 +0800 Subject: [PATCH 05/12] Use safe duration arithmetics, make doc concise --- crates/motiongfx/examples/custom_world.rs | 22 +++++---- crates/motiongfx/src/action.rs | 23 ++++++++-- crates/motiongfx/src/sequence.rs | 4 +- crates/motiongfx/src/time.rs | 56 ++++++++--------------- crates/motiongfx/src/timeline.rs | 6 +-- crates/motiongfx/src/track.rs | 54 +++++++++++++++++++--- crates/motiongfx_editor/src/lib.rs | 1 - 7 files changed, 101 insertions(+), 65 deletions(-) diff --git a/crates/motiongfx/examples/custom_world.rs b/crates/motiongfx/examples/custom_world.rs index 809ded07..8201a581 100644 --- a/crates/motiongfx/examples/custom_world.rs +++ b/crates/motiongfx/examples/custom_world.rs @@ -1,4 +1,5 @@ use std::collections::HashMap; +use std::time::Duration; use motiongfx::prelude::*; @@ -111,19 +112,22 @@ fn main() { // Check the values after sampling: println!("After: {:?}\n", world.subject_world); - let new_target_time = 7.0; + let new_target_time = Duration::from_secs(7); // Set target time to after total track duration timeline.set_target_time(new_target_time); - println!("timeline target time set to: {}s", new_target_time); + println!("timeline target time set to: {:?}", new_target_time); println!( - "# Before sampling \ncurrent time: {}s,\ntarget time: {}s", - timeline.curr_time().as_secs_f32(), - timeline.target_time().as_secs_f32() + "# Before sampling \ncurrent time: {:?},\ntarget time: {:?}", + timeline.curr_time(), + timeline.target_time(), + ); + println!( + "target time clamped to current track's duration ({:?})\n", + timeline.curr_track().duration() ); - println!("target time clamped to timeline duration (3s)\n"); // Queue and sample the actions. timeline.queue_actions(); @@ -133,9 +137,9 @@ fn main() { ); println!( - "# After sampling: \ncurrent time: {}s,\ntarget time: {}s\n", - timeline.curr_time().as_secs_f32(), - timeline.target_time().as_secs_f32() + "# After sampling: \ncurrent time: {:?},\ntarget time: {:?}\n", + timeline.curr_time(), + timeline.target_time(), ); } diff --git a/crates/motiongfx/src/action.rs b/crates/motiongfx/src/action.rs index 09de3060..c07cc972 100644 --- a/crates/motiongfx/src/action.rs +++ b/crates/motiongfx/src/action.rs @@ -142,16 +142,17 @@ impl ActionClip { } } + /// Saturating, so an absurd authored duration degrades to a clamped + /// timeline rather than a panic deep in playback. #[inline] pub fn end(&self) -> Duration { - self.start + self.duration + self.start.saturating_add(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. + /// Zero-duration spacer clips report `1.0` rather than `NaN`. #[inline] pub fn progress(&self, time: Duration) -> f32 { let span = self.duration.as_secs_f64(); @@ -186,6 +187,19 @@ mod clip_tests { assert_eq!(clip.progress(Duration::from_millis(3000)), 1.0); } + /// `end()` runs on every queue pass, so a saturated duration must + /// not panic there. + #[test] + fn end_saturates_instead_of_overflowing() { + let clip = ActionClip { + id: ActionId::PLACEHOLDER, + start: Duration::MAX, + duration: Duration::MAX, + }; + + assert_eq!(clip.end(), Duration::MAX); + } + #[test] fn progress_clamps_outside_the_clip() { let clip = clip(1000, 2000); @@ -194,8 +208,7 @@ mod clip_tests { 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. + /// A `NaN` here would poison the interpolated value. #[test] fn zero_duration_clip_reports_completion() { let t = clip(1000, 0).progress(Duration::from_millis(1000)); diff --git a/crates/motiongfx/src/sequence.rs b/crates/motiongfx/src/sequence.rs index 0ccbdde0..659c5ea1 100644 --- a/crates/motiongfx/src/sequence.rs +++ b/crates/motiongfx/src/sequence.rs @@ -38,12 +38,12 @@ impl Sequence { /// Get the duration of the sequence. #[inline] pub fn duration(&self) -> Duration { - self.end() - self.start() + self.end().saturating_sub(self.start()) } pub(crate) fn delay(&mut self, duration: Duration) { for clip in self.clips.iter_mut() { - clip.start += duration; + clip.start = clip.start.saturating_add(duration); } } } diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index 7c03a1a8..71c2b5b6 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -1,36 +1,24 @@ //! Time conversion helpers. //! -//! Motiongfx stores all timing as [`Duration`] so that the durations -//! accumulated by the track combinators and the clip offsets accumulated -//! when delaying a [`Sequence`] can never disagree. Float seconds are not -//! associative, so the two accumulations used to drift apart by a few -//! ULPs, which tripped the non-overlap assertion when extending a -//! sequence and left the playhead clamp just short of the final clip's -//! end. +//! Timing is stored as [`Duration`] so that the durations accumulated by +//! the track combinators and the clip offsets accumulated when delaying a +//! [`Sequence`] can never disagree. Float seconds are not associative, so +//! the two used to drift apart by a few ULPs, tripping the non-overlap +//! assertion and leaving the playhead clamp short of the final clip's end. //! -//! Authoring in seconds is still the ergonomic default, hence -//! [`IntoDuration`]. +//! [`IntoDuration`] keeps seconds as the authoring unit. //! //! [`Sequence`]: crate::sequence::Sequence use core::time::Duration; /// Conversion into a [`Duration`] for the timing arguments of the -/// authoring API. +/// authoring API, so that `play(0.6)` and +/// `play(Duration::from_millis(600))` are both accepted. /// -/// Implemented for [`Duration`] itself as well as `f32`/`f64` seconds, -/// so both of these are accepted: -/// -/// ```ignore -/// action.play(0.6); -/// action.play(Duration::from_millis(600)); -/// ``` -/// -/// Seconds that a [`Duration`] cannot represent saturate rather than -/// panic: negative and non-finite values become [`Duration::ZERO`], -/// and values past [`Duration::MAX`] become [`Duration::MAX`]. This -/// keeps idioms like `set_target_time(f32::MAX)` ("seek to the end") -/// working. +/// Unrepresentable seconds saturate rather than panic: negative and NaN +/// become [`Duration::ZERO`], overflow becomes [`Duration::MAX`]. This +/// keeps `set_target_time(f32::MAX)` ("seek to the end") working. pub trait IntoDuration { fn into_duration(self) -> Duration; } @@ -56,9 +44,8 @@ impl IntoDuration for f64 { return Duration::ZERO; } - // Positive and non-NaN at this point, so the only remaining - // failure is overflow (which includes `INFINITY`). - // `from_secs_f64` would panic on those instead. + // Positive and non-NaN, so the only remaining failure is + // overflow. `from_secs_f64` would panic on it. Duration::try_from_secs_f64(self).unwrap_or(Duration::MAX) } } @@ -68,12 +55,10 @@ impl IntoDuration for f64 { /// [`Duration`] has no signed representation, so stepping a playhead /// backwards has to go through this rather than a plain `+`. /// -/// The offset is only as precise as `delta` itself: `0.05f32` is really -/// `0.05000000074505806`, so it lands a nanosecond off. That is -/// inherent to a float-seconds clock and is not the drift this module -/// exists to prevent — clip and track boundaries are exact -/// [`Duration`]s, so an imprecise step still resolves against them -/// consistently instead of accumulating error into the timeline. +/// Only as precise as `delta` itself: `0.05f32` is really +/// `0.05000000074505806`, so it lands a nanosecond off. That is inherent +/// to a float clock, not the drift this module prevents. Clip and track +/// boundaries stay exact, so the error does not accumulate. #[inline] pub fn offset_secs(time: Duration, delta: f32) -> Duration { if delta >= 0.0 { @@ -92,14 +77,11 @@ mod tests { assert_eq!((-1.0f32).into_duration(), Duration::ZERO); assert_eq!(f32::NAN.into_duration(), Duration::ZERO); assert_eq!(f32::NEG_INFINITY.into_duration(), Duration::ZERO); - // "Seek to the end" idiom: must clamp, not overflow. assert_eq!(f32::MAX.into_duration(), Duration::MAX); assert_eq!(f32::INFINITY.into_duration(), Duration::MAX); } - /// `delta` is `f32` seconds, so the result is only accurate to - /// within the rounding of that `f32` — a nanosecond or so. Exact - /// equality is deliberately not asserted here. + /// `delta` is `f32` seconds, so exact equality is not assertable. #[track_caller] fn assert_near(actual: Duration, expected: Duration) { let diff = actual.abs_diff(expected); @@ -128,8 +110,6 @@ mod tests { fn offset_secs_saturates_at_zero() { let time = Duration::from_millis(100); - // Stepping further back than the playhead has to give must - // saturate rather than wrap or panic. assert_eq!(offset_secs(time, -10.0), Duration::ZERO); assert_eq!(offset_secs(Duration::ZERO, -1.0), Duration::ZERO); } diff --git a/crates/motiongfx/src/timeline.rs b/crates/motiongfx/src/timeline.rs index d8e39718..e85cd864 100644 --- a/crates/motiongfx/src/timeline.rs +++ b/crates/motiongfx/src/timeline.rs @@ -356,9 +356,9 @@ impl Timeline { /// Steps the target time by `delta` seconds, saturating at both /// ends of the target track. /// - /// Prefer this over reading [`Self::target_time`] and adding to it: - /// a [`Duration`] cannot go negative, so backwards playback - /// (`delta < 0.0`) needs the saturating arithmetic this performs. + /// Prefer this over adding to [`Self::target_time`]: a [`Duration`] + /// cannot go negative, so `delta < 0.0` needs saturating + /// arithmetic. pub fn advance_secs(&mut self, delta: f32) -> &mut Self { let target_time = time::offset_secs(self.target_time, delta); diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index 251529dd..47e4ef9b 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -55,7 +55,8 @@ pub fn chain( track = track.upsert_sequence(key, other_sequence); } - chain_duration += other_track.duration; + chain_duration = + chain_duration.saturating_add(other_track.duration); } track.duration = chain_duration; @@ -120,9 +121,10 @@ pub fn flow( let mut final_duration = track.duration; for other_track in tracks_iter { - flow_delay += delay; - final_duration = - (flow_delay + other_track.duration).max(final_duration); + flow_delay = flow_delay.saturating_add(delay); + final_duration = flow_delay + .saturating_add(other_track.duration) + .max(final_duration); for (key, mut sequence) in other_track.sequences { sequence.delay(flow_delay); @@ -216,9 +218,9 @@ impl TrackFragment { sequences.sort_by_key(|(key, _)| *key.field()); // The combinators accumulate `duration` independently of the - // clip offsets, so pin the two together here: the playhead - // clamp in `Timeline::set_target_time` must be able to reach - // the end of the last clip. + // clip offsets, so pin them together here: the clamp in + // `Timeline::set_target_time` must be able to reach the last + // clip's end. let duration = sequences .iter() .map(|(_, seq)| seq.end()) @@ -528,6 +530,44 @@ mod tests { assert_eq!(track.duration(), ms(1000)); } + /// `IntoDuration` saturates absurd seconds to `Duration::MAX`, so + /// the arithmetic downstream has to saturate too, or the panic just + /// moves into `chain`, `flow`, or `ActionClip::end`. + /// + /// Distinct keys per fragment: saturated clips really do overlap, + /// and the non-overlap assert is right to say so. Only the duration + /// arithmetic is under test. + #[test] + fn saturated_durations_do_not_overflow_the_combinators() { + let huge = |path: &'static str| { + TrackFragment::single( + key(path), + ActionClip::new( + ActionId::PLACEHOLDER, + f32::INFINITY.into_duration(), + ), + ) + }; + + assert_eq!(huge("a").duration, Duration::MAX); + assert_eq!( + [huge("a"), huge("b")].ord_chain().duration, + Duration::MAX + ); + assert_eq!( + [huge("a"), huge("b")].ord_flow(1.0).duration, + Duration::MAX + ); + assert_eq!( + [huge("a"), huge("b")].ord_all().duration, + Duration::MAX + ); + assert_eq!( + delay(f32::MAX, huge("a")).duration, + Duration::MAX + ); + } + #[test] fn compile_empty_fragment_is_not_a_panic() { let track = TrackFragment::new().compile(); diff --git a/crates/motiongfx_editor/src/lib.rs b/crates/motiongfx_editor/src/lib.rs index bcef08ea..81cc3912 100644 --- a/crates/motiongfx_editor/src/lib.rs +++ b/crates/motiongfx_editor/src/lib.rs @@ -450,7 +450,6 @@ fn update_playhead( let Some(timeline) = manager.get_timeline(&timeline_id) else { return; }; - // The slider and playhead are both in seconds-as-pixels. let time = timeline.target_time().as_secs_f32(); for mut node in &mut q_playhead { From ad54b0e1f4b415630307c84eb40db22db718ac4d Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:45:18 +0800 Subject: [PATCH 06/12] Make `PassivePlayer` API consistent --- crates/bevy_motiongfx/src/controller.rs | 9 +++++++-- 1 file changed, 7 insertions(+), 2 deletions(-) diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index 1ff4241a..a7b8b7c8 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -215,13 +215,18 @@ pub struct PassivePlayer { impl PassivePlayer { #[inline] - pub fn set_time(&mut self, time: impl IntoDuration) { + pub fn set_time(&mut self, time: impl IntoDuration) -> &mut Self { self.time = time.into_duration(); + self } #[inline] - pub fn set_track_index(&mut self, track_index: usize) { + pub fn set_track_index( + &mut self, + track_index: usize, + ) -> &mut Self { self.track_index = track_index; + self } pub fn get_time(&self) -> Duration { From 1cbf89dc13027ca70ff84d145f257093a9a77cfa Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Tue, 21 Jul 2026 13:52:06 +0800 Subject: [PATCH 07/12] Move tests downwards --- crates/motiongfx/src/action.rs | 44 ++++++++++++++++---------------- crates/motiongfx/src/timeline.rs | 1 + 2 files changed, 23 insertions(+), 22 deletions(-) diff --git a/crates/motiongfx/src/action.rs b/crates/motiongfx/src/action.rs index c07cc972..62f6be8d 100644 --- a/crates/motiongfx/src/action.rs +++ b/crates/motiongfx/src/action.rs @@ -166,8 +166,29 @@ impl ActionClip { } } +pub struct Segment { + /// The starting value. + pub start: T, + /// The ending value. + pub end: T, +} + +impl Segment { + pub fn new(start: T, end: T) -> Self { + Self { start, end } + } +} + +/// Determines how a [`Segment`] should be sampled. +#[derive(Debug, Clone, Copy)] +pub enum SampleMode { + Start, + End, + Interp(f32), +} + #[cfg(test)] -mod clip_tests { +mod tests { use super::*; const fn clip(start: u64, duration: u64) -> ActionClip { @@ -217,24 +238,3 @@ mod clip_tests { assert_eq!(t, 1.0); } } - -pub struct Segment { - /// The starting value. - pub start: T, - /// The ending value. - pub end: T, -} - -impl Segment { - pub fn new(start: T, end: T) -> Self { - Self { start, end } - } -} - -/// Determines how a [`Segment`] should be sampled. -#[derive(Debug, Clone, Copy)] -pub enum SampleMode { - Start, - End, - Interp(f32), -} diff --git a/crates/motiongfx/src/timeline.rs b/crates/motiongfx/src/timeline.rs index e85cd864..084bace3 100644 --- a/crates/motiongfx/src/timeline.rs +++ b/crates/motiongfx/src/timeline.rs @@ -596,5 +596,6 @@ impl<'a, W: 'static> TimelineBuilder<'a, W> { } } +// TODO: Write some unit tests. #[cfg(test)] mod tests {} From a29c680dd5ef628f0c0b960204bb20e8b1bd64c4 Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Wed, 22 Jul 2026 10:53:09 +0800 Subject: [PATCH 08/12] Create and expose `s()`, `ms()`, `ns()` shorthands --- crates/bevy_motiongfx/src/controller.rs | 22 ++++++------ crates/motiongfx/src/action.rs | 16 +++++---- crates/motiongfx/src/lib.rs | 2 +- crates/motiongfx/src/pipeline.rs | 19 +++++----- crates/motiongfx/src/time.rs | 46 +++++++++++++++++-------- crates/motiongfx/src/track.rs | 11 +++--- 6 files changed, 65 insertions(+), 51 deletions(-) diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index a7b8b7c8..6c664651 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -240,6 +240,8 @@ impl PassivePlayer { #[cfg(test)] mod tests { + use motiongfx::time::{ms, ns, s}; + use super::*; fn at(fps: u16, frame: u64) -> Duration { @@ -254,10 +256,10 @@ mod tests { #[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)); + assert_eq!(at(30, 30), s(1)); + assert_eq!(at(30, 3), ms(100)); + assert_eq!(at(25, 1), ms(40)); + assert_eq!(at(60, 90), ms(1500)); } /// The whole point of deriving from the counter: no accumulated @@ -265,12 +267,9 @@ mod tests { #[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)); + assert_eq!(at(30, 54_000), s(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) - ); + assert_eq!(at(30, 30_000_000_000), s(1_000_000_000)); } /// 1/3s is not representable in nanoseconds, so consecutive @@ -278,9 +277,8 @@ mod tests { #[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, - ); + let ideal = + ns((frame as u128 * 1_000_000_000 / 3) as u64); assert_eq!(at(3, frame).abs_diff(ideal), Duration::ZERO); } diff --git a/crates/motiongfx/src/action.rs b/crates/motiongfx/src/action.rs index 62f6be8d..e9618d23 100644 --- a/crates/motiongfx/src/action.rs +++ b/crates/motiongfx/src/action.rs @@ -189,13 +189,15 @@ pub enum SampleMode { #[cfg(test)] mod tests { + use crate::time::{ms, s}; + use super::*; const fn clip(start: u64, duration: u64) -> ActionClip { ActionClip { id: ActionId::PLACEHOLDER, - start: Duration::from_millis(start), - duration: Duration::from_millis(duration), + start: ms(start), + duration: ms(duration), } } @@ -203,9 +205,9 @@ mod tests { 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); + assert_eq!(clip.progress(ms(1000)), 0.0); + assert_eq!(clip.progress(ms(2000)), 0.5); + assert_eq!(clip.progress(ms(3000)), 1.0); } /// `end()` runs on every queue pass, so a saturated duration must @@ -226,13 +228,13 @@ mod tests { let clip = clip(1000, 2000); assert_eq!(clip.progress(Duration::ZERO), 0.0); - assert_eq!(clip.progress(Duration::from_secs(60)), 1.0); + assert_eq!(clip.progress(s(60)), 1.0); } /// A `NaN` here would poison the interpolated value. #[test] fn zero_duration_clip_reports_completion() { - let t = clip(1000, 0).progress(Duration::from_millis(1000)); + let t = clip(1000, 0).progress(ms(1000)); assert!(!t.is_nan()); assert_eq!(t, 1.0); diff --git a/crates/motiongfx/src/lib.rs b/crates/motiongfx/src/lib.rs index 74dfc3f0..4fa8b5cd 100644 --- a/crates/motiongfx/src/lib.rs +++ b/crates/motiongfx/src/lib.rs @@ -34,7 +34,7 @@ pub mod prelude { pub use crate::registry::{ AccessorRegistry, PipelineRegistry, Registry, }; - pub use crate::time::IntoDuration; + pub use crate::time::{IntoDuration, ms, ns, s}; pub use crate::timeline::{Timeline, TimelineBuilder}; pub use crate::track::{Track, TrackFragment, TrackOrdering}; pub use crate::world::SubjectSource; diff --git a/crates/motiongfx/src/pipeline.rs b/crates/motiongfx/src/pipeline.rs index 3ef566c7..cf25ee58 100644 --- a/crates/motiongfx/src/pipeline.rs +++ b/crates/motiongfx/src/pipeline.rs @@ -322,6 +322,7 @@ impl Range { #[cfg(test)] mod tests { use crate::interpolation::Interpolation; + use crate::time::s; use super::*; @@ -442,23 +443,21 @@ mod tests { #[test] fn range_overlap_behavior() { - let secs = Duration::from_secs; - let a = Range { - start: secs(0), - end: secs(5), + start: s(0), + end: s(5), }; let b = Range { - start: secs(3), - end: secs(8), + start: s(3), + end: s(8), }; let c = Range { - start: secs(6), - end: secs(10), + start: s(6), + end: s(10), }; let d = Range { - start: secs(5), - end: secs(5), + start: s(5), + end: s(5), }; // touching boundary assert!( diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index 71c2b5b6..b96987cf 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -50,6 +50,27 @@ impl IntoDuration for f64 { } } +/// Whole seconds as a [`Duration`]. +#[inline] +#[must_use] +pub const fn s(secs: u64) -> Duration { + Duration::from_secs(secs) +} + +/// Whole milliseconds as a [`Duration`]. +#[inline] +#[must_use] +pub const fn ms(millis: u64) -> Duration { + Duration::from_millis(millis) +} + +/// Whole nanoseconds as a [`Duration`]. +#[inline] +#[must_use] +pub const fn ns(nanos: u64) -> Duration { + Duration::from_nanos(nanos) +} + /// Offsets `time` by `delta` seconds, saturating at [`Duration::ZERO`]. /// /// [`Duration`] has no signed representation, so stepping a playhead @@ -72,6 +93,13 @@ pub fn offset_secs(time: Duration, delta: f32) -> Duration { mod tests { use super::*; + #[test] + fn unit_helpers_agree_with_duration_constructors() { + assert_eq!(s(2), ms(2_000)); + assert_eq!(ms(1), ns(1_000_000)); + } + + /// Floats here are the subject under test, not a time literal. #[test] fn unrepresentable_seconds_saturate_instead_of_panicking() { assert_eq!((-1.0f32).into_duration(), Duration::ZERO); @@ -87,30 +115,20 @@ mod tests { let diff = actual.abs_diff(expected); assert!( - diff < Duration::from_micros(1), + diff < ns(1_000), "{actual:?} is not within 1us of {expected:?}", ); } #[test] fn offset_secs_steps_both_ways() { - let time = Duration::from_millis(100); - - assert_near( - offset_secs(time, -0.05), - Duration::from_millis(50), - ); - assert_near( - offset_secs(time, 0.05), - Duration::from_millis(150), - ); + assert_near(offset_secs(ms(100), -0.05), ms(50)); + assert_near(offset_secs(ms(100), 0.05), ms(150)); } #[test] fn offset_secs_saturates_at_zero() { - let time = Duration::from_millis(100); - - assert_eq!(offset_secs(time, -10.0), Duration::ZERO); + assert_eq!(offset_secs(ms(100), -10.0), Duration::ZERO); assert_eq!(offset_secs(Duration::ZERO, -1.0), Duration::ZERO); } } diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index 47e4ef9b..0291c734 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -380,6 +380,7 @@ pub struct Span { #[cfg(test)] mod tests { use crate::action::{ActionId, IdRegistry, UntypedSubjectId}; + use crate::time::{ms, s}; use super::*; @@ -395,10 +396,6 @@ mod tests { ) } - const fn ms(millis: u64) -> Duration { - Duration::from_millis(millis) - } - const fn clip(millis: u64) -> ActionClip { ActionClip::new(ActionId::PLACEHOLDER, ms(millis)) } @@ -476,7 +473,7 @@ mod tests { let track1 = TrackFragment::single(key("a"), clip(1000)); let track2 = TrackFragment::single(key("b"), clip(1000)); - let track = [track1, track2].ord_flow(0.5); + let track = [track1, track2].ord_flow(ms(500)); // 0.5 delay + 1.0 duration assert_eq!(track.duration, ms(1500)); @@ -489,7 +486,7 @@ mod tests { fn delay_applies_offset() { let track = TrackFragment::single(key("a"), clip(2000)); - let track = delay(1.5, track); + let track = delay(ms(1500), track); let seq_a = &track.sequences[&key("a")]; assert_eq!(seq_a.start(), ms(1500)); @@ -555,7 +552,7 @@ mod tests { Duration::MAX ); assert_eq!( - [huge("a"), huge("b")].ord_flow(1.0).duration, + [huge("a"), huge("b")].ord_flow(s(1)).duration, Duration::MAX ); assert_eq!( From 5d3fb74a3e0cf5acc3fbe7d2c035adc9a9ee9427 Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:15:43 +0800 Subject: [PATCH 09/12] Remove `offset_secs` --- crates/bevy_motiongfx/src/controller.rs | 16 +++++++--- crates/motiongfx/src/time.rs | 41 ------------------------- crates/motiongfx/src/timeline.rs | 28 ++++++++++++----- examples/bevy_examples/src/lib.rs | 4 +-- 4 files changed, 33 insertions(+), 56 deletions(-) diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index 6c664651..f6e68b19 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -33,8 +33,14 @@ fn realtime_player_update( q_timelines.iter().filter(|(_, p)| p.is_playing) { if let Some(timeline) = motiongfx.get_timeline_mut(id) { - timeline - .advance_secs(player.time_scale * time.delta_secs()); + // Magnitude sets the step, sign picks the direction. + let delta = time.delta().mul_f64(player.time_scale.abs()); + + if player.time_scale > 0.0 { + timeline.advance_time(delta); + } else if player.time_scale < 0.0 { + timeline.rewind_time(delta); + } } } } @@ -81,7 +87,7 @@ pub struct RealtimePlayer { pub is_playing: bool, /// The time scale of the player. Set this to negative /// to play backwards. - pub time_scale: f32, + pub time_scale: f64, } impl RealtimePlayer { @@ -105,7 +111,7 @@ impl RealtimePlayer { /// Builder method for setting [`Self::time_scale`]. #[inline] #[must_use] - pub const fn with_time_scale(mut self, time_scale: f32) -> Self { + pub const fn with_time_scale(mut self, time_scale: f64) -> Self { self.time_scale = time_scale; self } @@ -121,7 +127,7 @@ impl RealtimePlayer { #[inline] pub const fn set_time_scale( &mut self, - time_scale: f32, + time_scale: f64, ) -> &mut Self { self.time_scale = time_scale; self diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index b96987cf..892f21a9 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -71,24 +71,6 @@ pub const fn ns(nanos: u64) -> Duration { Duration::from_nanos(nanos) } -/// Offsets `time` by `delta` seconds, saturating at [`Duration::ZERO`]. -/// -/// [`Duration`] has no signed representation, so stepping a playhead -/// backwards has to go through this rather than a plain `+`. -/// -/// Only as precise as `delta` itself: `0.05f32` is really -/// `0.05000000074505806`, so it lands a nanosecond off. That is inherent -/// to a float clock, not the drift this module prevents. Clip and track -/// boundaries stay exact, so the error does not accumulate. -#[inline] -pub fn offset_secs(time: Duration, delta: f32) -> Duration { - if delta >= 0.0 { - time.saturating_add(delta.into_duration()) - } else { - time.saturating_sub((-delta).into_duration()) - } -} - #[cfg(test)] mod tests { use super::*; @@ -108,27 +90,4 @@ mod tests { assert_eq!(f32::MAX.into_duration(), Duration::MAX); assert_eq!(f32::INFINITY.into_duration(), Duration::MAX); } - - /// `delta` is `f32` seconds, so exact equality is not assertable. - #[track_caller] - fn assert_near(actual: Duration, expected: Duration) { - let diff = actual.abs_diff(expected); - - assert!( - diff < ns(1_000), - "{actual:?} is not within 1us of {expected:?}", - ); - } - - #[test] - fn offset_secs_steps_both_ways() { - assert_near(offset_secs(ms(100), -0.05), ms(50)); - assert_near(offset_secs(ms(100), 0.05), ms(150)); - } - - #[test] - fn offset_secs_saturates_at_zero() { - assert_eq!(offset_secs(ms(100), -10.0), Duration::ZERO); - assert_eq!(offset_secs(Duration::ZERO, -1.0), Duration::ZERO); - } } diff --git a/crates/motiongfx/src/timeline.rs b/crates/motiongfx/src/timeline.rs index 084bace3..1a7099de 100644 --- a/crates/motiongfx/src/timeline.rs +++ b/crates/motiongfx/src/timeline.rs @@ -16,7 +16,7 @@ use crate::interpolation::Interpolation; use crate::pipeline::{BakeCtx, PipelineKey, Range, SampleCtx}; use crate::registry::Registry; use crate::subject::SubjectId; -use crate::time::{self, IntoDuration}; +use crate::time::IntoDuration; use crate::track::Track; use crate::world::SubjectSource; @@ -353,14 +353,26 @@ impl Timeline { self } - /// Steps the target time by `delta` seconds, saturating at both - /// ends of the target track. + /// Steps forward, clamping at the track's end. + pub fn advance_time( + &mut self, + time: impl IntoDuration, + ) -> &mut Self { + let target_time = + self.target_time.saturating_add(time.into_duration()); + + self.set_target_time(target_time) + } + + /// Steps backward, saturating at [`Duration::ZERO`]. /// - /// Prefer this over adding to [`Self::target_time`]: a [`Duration`] - /// cannot go negative, so `delta < 0.0` needs saturating - /// arithmetic. - pub fn advance_secs(&mut self, delta: f32) -> &mut Self { - let target_time = time::offset_secs(self.target_time, delta); + /// [`Duration`] carries no sign, hence a separate method. + pub fn rewind_time( + &mut self, + time: impl IntoDuration, + ) -> &mut Self { + let target_time = + self.target_time.saturating_sub(time.into_duration()); self.set_target_time(target_time) } diff --git a/examples/bevy_examples/src/lib.rs b/examples/bevy_examples/src/lib.rs index 7d494f62..8730ab3b 100644 --- a/examples/bevy_examples/src/lib.rs +++ b/examples/bevy_examples/src/lib.rs @@ -12,12 +12,12 @@ pub fn timeline_movement( if keys.any_pressed([KeyCode::KeyD, KeyCode::ArrowRight]) { player.set_playing(false); - timeline.advance_secs(time.delta_secs()); + timeline.advance_time(time.delta()); } if keys.any_pressed([KeyCode::KeyA, KeyCode::ArrowLeft]) { player.set_playing(false); - timeline.advance_secs(-time.delta_secs()); + timeline.rewind_time(time.delta()); } if keys.just_pressed(KeyCode::Space) { From 8c3a0f944b695ba9c6fe3882e18b6e9b874cc48b Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Wed, 22 Jul 2026 11:21:30 +0800 Subject: [PATCH 10/12] Add centi seconds shorthand as `cs()` --- crates/bevy_motiongfx/src/controller.rs | 8 ++-- crates/motiongfx/src/action.rs | 20 ++++----- crates/motiongfx/src/lib.rs | 2 +- crates/motiongfx/src/time.rs | 9 ++++ crates/motiongfx/src/track.rs | 57 ++++++++++++------------- 5 files changed, 52 insertions(+), 44 deletions(-) diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index f6e68b19..5a61a297 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -246,7 +246,7 @@ impl PassivePlayer { #[cfg(test)] mod tests { - use motiongfx::time::{ms, ns, s}; + use motiongfx::time::{cs, ns, s}; use super::*; @@ -263,9 +263,9 @@ mod tests { fn frame_time_is_exact_where_the_rate_divides() { assert_eq!(at(30, 0), Duration::ZERO); assert_eq!(at(30, 30), s(1)); - assert_eq!(at(30, 3), ms(100)); - assert_eq!(at(25, 1), ms(40)); - assert_eq!(at(60, 90), ms(1500)); + assert_eq!(at(30, 3), cs(10)); + assert_eq!(at(25, 1), cs(4)); + assert_eq!(at(60, 90), cs(150)); } /// The whole point of deriving from the counter: no accumulated diff --git a/crates/motiongfx/src/action.rs b/crates/motiongfx/src/action.rs index e9618d23..3c6a37a5 100644 --- a/crates/motiongfx/src/action.rs +++ b/crates/motiongfx/src/action.rs @@ -189,25 +189,25 @@ pub enum SampleMode { #[cfg(test)] mod tests { - use crate::time::{ms, s}; + use crate::time::{cs, s}; use super::*; - const fn clip(start: u64, duration: u64) -> ActionClip { + const fn clip(start: Duration, duration: Duration) -> ActionClip { ActionClip { id: ActionId::PLACEHOLDER, - start: ms(start), - duration: ms(duration), + start, + duration, } } #[test] fn progress_spans_the_clip() { - let clip = clip(1000, 2000); + let clip = clip(cs(100), cs(200)); - assert_eq!(clip.progress(ms(1000)), 0.0); - assert_eq!(clip.progress(ms(2000)), 0.5); - assert_eq!(clip.progress(ms(3000)), 1.0); + assert_eq!(clip.progress(cs(100)), 0.0); + assert_eq!(clip.progress(cs(200)), 0.5); + assert_eq!(clip.progress(cs(300)), 1.0); } /// `end()` runs on every queue pass, so a saturated duration must @@ -225,7 +225,7 @@ mod tests { #[test] fn progress_clamps_outside_the_clip() { - let clip = clip(1000, 2000); + let clip = clip(cs(100), cs(200)); assert_eq!(clip.progress(Duration::ZERO), 0.0); assert_eq!(clip.progress(s(60)), 1.0); @@ -234,7 +234,7 @@ mod tests { /// A `NaN` here would poison the interpolated value. #[test] fn zero_duration_clip_reports_completion() { - let t = clip(1000, 0).progress(ms(1000)); + let t = clip(cs(100), Duration::ZERO).progress(cs(100)); assert!(!t.is_nan()); assert_eq!(t, 1.0); diff --git a/crates/motiongfx/src/lib.rs b/crates/motiongfx/src/lib.rs index 4fa8b5cd..81eb8838 100644 --- a/crates/motiongfx/src/lib.rs +++ b/crates/motiongfx/src/lib.rs @@ -34,7 +34,7 @@ pub mod prelude { pub use crate::registry::{ AccessorRegistry, PipelineRegistry, Registry, }; - pub use crate::time::{IntoDuration, ms, ns, s}; + pub use crate::time::{IntoDuration, cs, ms, ns, s}; pub use crate::timeline::{Timeline, TimelineBuilder}; pub use crate::track::{Track, TrackFragment, TrackOrdering}; pub use crate::world::SubjectSource; diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index 892f21a9..6037f315 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -57,6 +57,13 @@ pub const fn s(secs: u64) -> Duration { Duration::from_secs(secs) } +/// Whole centiseconds (hundredths of a second) as a [`Duration`]. +#[inline] +#[must_use] +pub const fn cs(centis: u64) -> Duration { + Duration::from_millis(centis.saturating_mul(10)) +} + /// Whole milliseconds as a [`Duration`]. #[inline] #[must_use] @@ -78,7 +85,9 @@ mod tests { #[test] fn unit_helpers_agree_with_duration_constructors() { assert_eq!(s(2), ms(2_000)); + assert_eq!(cs(150), ms(1_500)); assert_eq!(ms(1), ns(1_000_000)); + assert_eq!(cs(u64::MAX), Duration::from_millis(u64::MAX)); } /// Floats here are the subject under test, not a time literal. diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index 0291c734..c69db24e 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -380,7 +380,7 @@ pub struct Span { #[cfg(test)] mod tests { use crate::action::{ActionId, IdRegistry, UntypedSubjectId}; - use crate::time::{ms, s}; + use crate::time::{cs, ms, s}; use super::*; @@ -396,8 +396,8 @@ mod tests { ) } - const fn clip(millis: u64) -> ActionClip { - ActionClip::new(ActionId::PLACEHOLDER, ms(millis)) + const fn clip(centis: u64) -> ActionClip { + ActionClip::new(ActionId::PLACEHOLDER, cs(centis)) } #[test] @@ -439,59 +439,59 @@ mod tests { #[test] fn chain_duration_and_delay() { - let track1 = TrackFragment::single(key("a"), clip(1000)); - let track2 = TrackFragment::single(key("b"), clip(2000)); + let track1 = TrackFragment::single(key("a"), clip(100)); + let track2 = TrackFragment::single(key("b"), clip(200)); let track = [track1, track2].ord_chain(); - assert_eq!(track.duration, ms(3000)); + assert_eq!(track.duration, cs(300)); let seq_b = &track.sequences[&key("b")]; // `seq_b` should be delayed by 1.0 (duration of `track1`). - assert_eq!(seq_b.start(), ms(1000)); + assert_eq!(seq_b.start(), cs(100)); } #[test] fn all_duration_max() { - let track1 = TrackFragment::single(key("a"), clip(1000)); - let track2 = TrackFragment::single(key("b"), clip(3000)); + let track1 = TrackFragment::single(key("a"), clip(100)); + let track2 = TrackFragment::single(key("b"), clip(300)); let track = [track1, track2].ord_all(); - assert_eq!(track.duration, ms(3000)); + assert_eq!(track.duration, cs(300)); } #[test] fn any_duration_min() { - let track1 = TrackFragment::single(key("a"), clip(1000)); - let track2 = TrackFragment::single(key("b"), clip(3000)); + let track1 = TrackFragment::single(key("a"), clip(100)); + let track2 = TrackFragment::single(key("b"), clip(300)); let track = [track1, track2].ord_any(); - assert_eq!(track.duration, ms(1000)); + assert_eq!(track.duration, cs(100)); } #[test] fn flow_with_delay() { - let track1 = TrackFragment::single(key("a"), clip(1000)); - let track2 = TrackFragment::single(key("b"), clip(1000)); + let track1 = TrackFragment::single(key("a"), clip(100)); + let track2 = TrackFragment::single(key("b"), clip(100)); - let track = [track1, track2].ord_flow(ms(500)); + let track = [track1, track2].ord_flow(cs(50)); // 0.5 delay + 1.0 duration - assert_eq!(track.duration, ms(1500)); + assert_eq!(track.duration, cs(150)); let seq_b = &track.sequences[&key("b")]; // `seq_b` should be delayed by 0.5 - assert_eq!(seq_b.start(), ms(500)); + assert_eq!(seq_b.start(), cs(50)); } #[test] fn delay_applies_offset() { - let track = TrackFragment::single(key("a"), clip(2000)); + let track = TrackFragment::single(key("a"), clip(200)); - let track = delay(ms(1500), track); + let track = delay(cs(150), track); let seq_a = &track.sequences[&key("a")]; - assert_eq!(seq_a.start(), ms(1500)); - assert_eq!(seq_a.end(), ms(3500)); - assert_eq!(track.duration, ms(2000)); + assert_eq!(seq_a.start(), cs(150)); + assert_eq!(seq_a.end(), cs(350)); + assert_eq!(track.duration, cs(200)); } /// Chaining durations that have no exact `f32` representation used @@ -503,28 +503,27 @@ mod tests { fn chain_accumulation_matches_clip_offsets() { // 0.1s is not representable in binary floating point. let tracks: Vec<_> = (0..10) - .map(|_| TrackFragment::single(key("a"), clip(100))) + .map(|_| TrackFragment::single(key("a"), clip(10))) .collect(); let track = tracks.ord_chain(); - assert_eq!(track.duration, ms(1000)); - assert_eq!(track.sequences[&key("a")].end(), ms(1000)); + assert_eq!(track.duration, cs(100)); + assert_eq!(track.sequences[&key("a")].end(), cs(100)); } /// `Track::duration` must always be reachable by the playhead, so /// that the final clip can resolve to `SampleMode::End`. #[test] fn compile_duration_covers_last_clip_end() { - let mut fragment = - TrackFragment::single(key("a"), clip(1000)); + let mut fragment = TrackFragment::single(key("a"), clip(100)); // Understate the duration the way a combinator would if the // two accumulations ever diverged again. fragment.duration = ms(999); let track = fragment.compile(); - assert_eq!(track.duration(), ms(1000)); + assert_eq!(track.duration(), cs(100)); } /// `IntoDuration` saturates absurd seconds to `Duration::MAX`, so From 08a6061763205a5ed1925d195366624dff1b3d6a Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Wed, 22 Jul 2026 19:31:25 +0800 Subject: [PATCH 11/12] Custom impl for `f32` --- crates/motiongfx/src/time.rs | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index 6037f315..bb11d221 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -33,7 +33,11 @@ impl IntoDuration for Duration { impl IntoDuration for f32 { #[inline] fn into_duration(self) -> Duration { - (self as f64).into_duration() + if self.is_nan() || self <= 0.0 { + return Duration::ZERO; + } + + Duration::try_from_secs_f32(self).unwrap_or(Duration::MAX) } } From 9bc8f1aaee12fe17b66143459f638dbd4f4c8a25 Mon Sep 17 00:00:00 2001 From: Nixon <43715558+nixonyh@users.noreply.github.com> Date: Wed, 22 Jul 2026 22:42:40 +0800 Subject: [PATCH 12/12] Remove `IntoDuration` and update examples --- crates/bevy_motiongfx/README.md | 4 +- crates/bevy_motiongfx/src/controller.rs | 5 +- crates/motiongfx/README.md | 12 ++--- crates/motiongfx/benches/action_storage.rs | 14 ++--- crates/motiongfx/docs/world.rs | 2 +- crates/motiongfx/examples/custom_world.rs | 8 +-- crates/motiongfx/src/action/table.rs | 6 +-- crates/motiongfx/src/lib.rs | 2 +- crates/motiongfx/src/time.rs | 54 ------------------- crates/motiongfx/src/timeline.rs | 21 +++----- crates/motiongfx/src/track.rs | 24 +++------ crates/motiongfx_editor/src/lib.rs | 5 +- .../bevy_examples/examples/custom_ease.rs | 2 +- .../bevy_examples/examples/custom_interp.rs | 2 +- examples/bevy_examples/examples/easings.rs | 4 +- examples/bevy_examples/examples/editor.rs | 8 +-- .../bevy_examples/examples/hello_world.rs | 8 +-- examples/bevy_examples/examples/minimal.rs | 4 +- examples/bevy_examples/examples/recording.rs | 2 +- .../bevy_examples/examples/slide_basic.rs | 16 +++--- .../vello_winit_example/examples/lissajous.rs | 14 ++--- 21 files changed, 73 insertions(+), 144 deletions(-) diff --git a/crates/bevy_motiongfx/README.md b/crates/bevy_motiongfx/README.md index 5367b33d..5f65a70c 100644 --- a/crates/bevy_motiongfx/README.md +++ b/crates/bevy_motiongfx/README.md @@ -53,7 +53,7 @@ fn build_timeline( .act(entity, path!(::translation::x), |x| { x + 6.0 }) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); @@ -90,7 +90,7 @@ fn build_timeline( path!(::base_color), |_| Srgba::RED.into(), ) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); diff --git a/crates/bevy_motiongfx/src/controller.rs b/crates/bevy_motiongfx/src/controller.rs index 5a61a297..c14c57bf 100644 --- a/crates/bevy_motiongfx/src/controller.rs +++ b/crates/bevy_motiongfx/src/controller.rs @@ -3,7 +3,6 @@ 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}; @@ -221,8 +220,8 @@ pub struct PassivePlayer { impl PassivePlayer { #[inline] - pub fn set_time(&mut self, time: impl IntoDuration) -> &mut Self { - self.time = time.into_duration(); + pub fn set_time(&mut self, time: Duration) -> &mut Self { + self.time = time; self } diff --git a/crates/motiongfx/README.md b/crates/motiongfx/README.md index e173f3bc..16a9e838 100644 --- a/crates/motiongfx/README.md +++ b/crates/motiongfx/README.md @@ -60,7 +60,7 @@ let id = 0; let action = b.act(id, path!(), |x| x + 10.0); // "Play" the action into a `TrackFragment` with a duration. -let frag = action.play(1.0); +let frag = action.play(s(1)); // Compile into a `Track` // See "Track Ordering" section for composing fragments. @@ -73,7 +73,7 @@ let mut timeline = b.compile(); timeline.bake_actions(®istry, &world); // Sample at t = 0.5, world.0[0] should now be 5.0. -timeline.set_target_time(0.5); +timeline.set_target_time(cs(50)); timeline.queue_actions(); timeline.sample_queued_actions(®istry, &mut world); @@ -147,7 +147,7 @@ let mut b = registry.create_builder::(); Animations are built up in layers: 1. An **action** says what to animate and how to transform it. -2. A **track fragment** gives the action a duration by calling `.play(seconds)`. +2. A **track fragment** gives the action a duration by calling `.play(duration)`. 3. A **track** is one or more fragments compiled together. You can order fragments before compiling (see [Track Ordering](#track-ordering)). 4. A **timeline** combines all your tracks into one playable sequence. @@ -165,7 +165,7 @@ let action = b .with_ease(ease::cubic::ease_in_out); // Play: turn the action into a fragment with a 1-second duration. -let frag = action.play(1.0); +let frag = action.play(s(1)); // Compile the fragment into a Track. let track = frag.compile(); @@ -196,7 +196,7 @@ Use `set_target_track` to jump between them. timeline.bake_actions(®istry, &subjects); // Set target time, queue, then sample. -timeline.set_target_time(0.5); +timeline.set_target_time(cs(50)); timeline.queue_actions(); timeline.sample_queued_actions(®istry, &mut subjects); @@ -261,7 +261,7 @@ use motiongfx::prelude::*; let f0 = TrackFragment::new(); let f1 = TrackFragment::new(); -let f = [f0, f1].ord_flow(0.5); +let f = [f0, f1].ord_flow(cs(50)); ``` `f1` starts 0.5 seconds after `f0` begins, regardless of how long diff --git a/crates/motiongfx/benches/action_storage.rs b/crates/motiongfx/benches/action_storage.rs index 14210fe6..7c0ca2bd 100644 --- a/crates/motiongfx/benches/action_storage.rs +++ b/crates/motiongfx/benches/action_storage.rs @@ -81,7 +81,7 @@ fn build_timeline( builder .act_builder(Id(i), path!(::x), |x| x + 72.0) .with_interp(linear_f32) - .play(1.0) + .play(s(1)) }) .collect::>(); @@ -164,7 +164,7 @@ fn bench_scrub(c: &mut Criterion) { ); } // Rewind to t=0 for the next iteration. - timeline.set_target_time(0.0); + timeline.set_target_time(s(0)); timeline.queue_actions(); timeline .sample_queued_actions(®istry, &mut world); @@ -277,7 +277,7 @@ fn build_mixed( v + 1.0 }) .with_interp(linear_f32) - .play(1.0), + .play(s(1)), builder .act_builder( Id(i), @@ -288,7 +288,7 @@ fn build_mixed( }, ) .with_interp(lerp2) - .play(1.0), + .play(s(1)), builder .act_builder( Id(i), @@ -300,7 +300,7 @@ fn build_mixed( }, ) .with_interp(lerp3) - .play(1.0), + .play(s(1)), builder .act_builder( Id(i), @@ -313,7 +313,7 @@ fn build_mixed( }, ) .with_interp(lerp4) - .play(1.0), + .play(s(1)), ] }) .collect::>(); @@ -389,7 +389,7 @@ fn bench_mixed_scrub(c: &mut Criterion) { ®istry, &mut world, ); } - timeline.set_target_time(0.0); + timeline.set_target_time(s(0)); timeline.queue_actions(); timeline .sample_queued_actions(®istry, &mut world); diff --git a/crates/motiongfx/docs/world.rs b/crates/motiongfx/docs/world.rs index e97fb0e0..30b5c68a 100644 --- a/crates/motiongfx/docs/world.rs +++ b/crates/motiongfx/docs/world.rs @@ -22,7 +22,7 @@ pub fn timeline() -> (Registry, Timeline) { let track = b .act_builder(0usize, path!(), |x| x + 10.0) .with_interp(|a: &f32, b: &f32, t| a + (b - a) * t) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); let timeline = b.compile(); diff --git a/crates/motiongfx/examples/custom_world.rs b/crates/motiongfx/examples/custom_world.rs index 8201a581..67a580ae 100644 --- a/crates/motiongfx/examples/custom_world.rs +++ b/crates/motiongfx/examples/custom_world.rs @@ -69,21 +69,21 @@ fn main() { builder .act_builder(Id(0), path!(::x), |x| x + 72.0) .with_interp(linear_f32) - .play(1.0), + .play(s(1)), [ builder .act_builder(Id(1), path!(::p0::y), |y| { y + 42.0 }) .with_interp(linear_f32) - .play(2.0), + .play(s(2)), builder .act_builder(Id(1), path!(::p1), |_| Point { x: 6.0, y: 6.0, }) .with_interp(linear_point) - .play(2.0), + .play(s(2)), ] .ord_all(), ] @@ -97,7 +97,7 @@ fn main() { timeline.bake_actions(&world.registry, &world.subject_world); // Change the target time. - timeline.set_target_time(1.5); + timeline.set_target_time(cs(150)); // Check the values before sampling: println!("Before: {:?}", world.subject_world); diff --git a/crates/motiongfx/src/action/table.rs b/crates/motiongfx/src/action/table.rs index e2390519..e7896cb9 100644 --- a/crates/motiongfx/src/action/table.rs +++ b/crates/motiongfx/src/action/table.rs @@ -1,5 +1,6 @@ use core::any::TypeId; use core::marker::PhantomData; +use core::time::Duration; use field_path::field::UntypedField; use typarena::ColumnId; @@ -16,7 +17,6 @@ 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 @@ -233,10 +233,10 @@ impl InterpActionBuilder<'_, T> { /// Confirms the configuration of the action and creates a /// [`TrackFragment`]. - pub fn play(self, duration: impl IntoDuration) -> TrackFragment { + pub fn play(self, duration: Duration) -> TrackFragment { TrackFragment::single( self.inner.key, - ActionClip::new(self.id(), duration.into_duration()), + ActionClip::new(self.id(), duration), ) } } diff --git a/crates/motiongfx/src/lib.rs b/crates/motiongfx/src/lib.rs index 81eb8838..a6637995 100644 --- a/crates/motiongfx/src/lib.rs +++ b/crates/motiongfx/src/lib.rs @@ -34,7 +34,7 @@ pub mod prelude { pub use crate::registry::{ AccessorRegistry, PipelineRegistry, Registry, }; - pub use crate::time::{IntoDuration, cs, ms, ns, s}; + pub use crate::time::{cs, ms, ns, s}; pub use crate::timeline::{Timeline, TimelineBuilder}; pub use crate::track::{Track, TrackFragment, TrackOrdering}; pub use crate::world::SubjectSource; diff --git a/crates/motiongfx/src/time.rs b/crates/motiongfx/src/time.rs index bb11d221..30adb153 100644 --- a/crates/motiongfx/src/time.rs +++ b/crates/motiongfx/src/time.rs @@ -6,54 +6,10 @@ //! the two used to drift apart by a few ULPs, tripping the non-overlap //! assertion and leaving the playhead clamp short of the final clip's end. //! -//! [`IntoDuration`] keeps seconds as the authoring unit. -//! //! [`Sequence`]: crate::sequence::Sequence use core::time::Duration; -/// Conversion into a [`Duration`] for the timing arguments of the -/// authoring API, so that `play(0.6)` and -/// `play(Duration::from_millis(600))` are both accepted. -/// -/// Unrepresentable seconds saturate rather than panic: negative and NaN -/// become [`Duration::ZERO`], overflow becomes [`Duration::MAX`]. This -/// keeps `set_target_time(f32::MAX)` ("seek to the end") working. -pub trait IntoDuration { - fn into_duration(self) -> Duration; -} - -impl IntoDuration for Duration { - #[inline] - fn into_duration(self) -> Duration { - self - } -} - -impl IntoDuration for f32 { - #[inline] - fn into_duration(self) -> Duration { - if self.is_nan() || self <= 0.0 { - return Duration::ZERO; - } - - Duration::try_from_secs_f32(self).unwrap_or(Duration::MAX) - } -} - -impl IntoDuration for f64 { - #[inline] - fn into_duration(self) -> Duration { - if self.is_nan() || self <= 0.0 { - return Duration::ZERO; - } - - // Positive and non-NaN, so the only remaining failure is - // overflow. `from_secs_f64` would panic on it. - Duration::try_from_secs_f64(self).unwrap_or(Duration::MAX) - } -} - /// Whole seconds as a [`Duration`]. #[inline] #[must_use] @@ -93,14 +49,4 @@ mod tests { assert_eq!(ms(1), ns(1_000_000)); assert_eq!(cs(u64::MAX), Duration::from_millis(u64::MAX)); } - - /// Floats here are the subject under test, not a time literal. - #[test] - fn unrepresentable_seconds_saturate_instead_of_panicking() { - assert_eq!((-1.0f32).into_duration(), Duration::ZERO); - assert_eq!(f32::NAN.into_duration(), Duration::ZERO); - assert_eq!(f32::NEG_INFINITY.into_duration(), Duration::ZERO); - assert_eq!(f32::MAX.into_duration(), Duration::MAX); - assert_eq!(f32::INFINITY.into_duration(), Duration::MAX); - } } diff --git a/crates/motiongfx/src/timeline.rs b/crates/motiongfx/src/timeline.rs index 1a7099de..1c268bd3 100644 --- a/crates/motiongfx/src/timeline.rs +++ b/crates/motiongfx/src/timeline.rs @@ -16,7 +16,6 @@ use crate::interpolation::Interpolation; use crate::pipeline::{BakeCtx, PipelineKey, Range, SampleCtx}; use crate::registry::Registry; use crate::subject::SubjectId; -use crate::time::IntoDuration; use crate::track::Track; use crate::world::SubjectSource; @@ -345,21 +344,17 @@ impl Timeline { /// within \[0.0..=track.duration\] pub fn set_target_time( &mut self, - target_time: impl IntoDuration, + target_time: Duration, ) -> &mut Self { let duration = self.tracks[self.target_index].duration(); - self.target_time = target_time.into_duration().min(duration); + self.target_time = target_time.min(duration); self } /// Steps forward, clamping at the track's end. - pub fn advance_time( - &mut self, - time: impl IntoDuration, - ) -> &mut Self { - let target_time = - self.target_time.saturating_add(time.into_duration()); + pub fn advance_time(&mut self, time: Duration) -> &mut Self { + let target_time = self.target_time.saturating_add(time); self.set_target_time(target_time) } @@ -367,12 +362,8 @@ impl Timeline { /// Steps backward, saturating at [`Duration::ZERO`]. /// /// [`Duration`] carries no sign, hence a separate method. - pub fn rewind_time( - &mut self, - time: impl IntoDuration, - ) -> &mut Self { - let target_time = - self.target_time.saturating_sub(time.into_duration()); + pub fn rewind_time(&mut self, time: Duration) -> &mut Self { + let target_time = self.target_time.saturating_sub(time); self.set_target_time(target_time) } diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index c69db24e..d5dffe68 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -7,14 +7,13 @@ use hashbrown::HashMap; use crate::action::{ActionClip, ActionKey}; use crate::sequence::Sequence; -use crate::time::IntoDuration; pub trait TrackOrdering { /// Run all [`TrackFragment`]s one after another. fn ord_chain(self) -> TrackFragment; fn ord_all(self) -> TrackFragment; fn ord_any(self) -> TrackFragment; - fn ord_flow(self, delay: impl IntoDuration) -> TrackFragment; + fn ord_flow(self, delay: Duration) -> TrackFragment; } impl TrackOrdering for T @@ -33,7 +32,7 @@ where any(self) } - fn ord_flow(self, delay: impl IntoDuration) -> TrackFragment { + fn ord_flow(self, delay: Duration) -> TrackFragment { flow(delay, self) } } @@ -110,10 +109,9 @@ pub fn any( /// Run one [`Track`] after another with a fixed delay time. #[must_use = "This function consumes all the given tracks and returns a modified one."] pub fn flow( - delay: impl IntoDuration, + delay: Duration, tracks: impl IntoIterator, ) -> TrackFragment { - let delay = delay.into_duration(); let mut tracks_iter = tracks.into_iter(); let mut track = tracks_iter.next().unwrap_or_default(); @@ -139,11 +137,9 @@ pub fn flow( /// Run a [`Track`] after a fixed delay time. #[must_use = "This function consumes the given track and returns a modified one."] pub fn delay( - delay: impl IntoDuration, + delay: Duration, mut track: TrackFragment, ) -> TrackFragment { - let delay = delay.into_duration(); - for sequence in track.sequences.values_mut() { sequence.delay(delay); } @@ -526,9 +522,8 @@ mod tests { assert_eq!(track.duration(), cs(100)); } - /// `IntoDuration` saturates absurd seconds to `Duration::MAX`, so - /// the arithmetic downstream has to saturate too, or the panic just - /// moves into `chain`, `flow`, or `ActionClip::end`. + /// Combinator arithmetic has to saturate, or a `Duration::MAX` + /// duration panics inside `chain`, `flow`, or `ActionClip::end`. /// /// Distinct keys per fragment: saturated clips really do overlap, /// and the non-overlap assert is right to say so. Only the duration @@ -538,10 +533,7 @@ mod tests { let huge = |path: &'static str| { TrackFragment::single( key(path), - ActionClip::new( - ActionId::PLACEHOLDER, - f32::INFINITY.into_duration(), - ), + ActionClip::new(ActionId::PLACEHOLDER, Duration::MAX), ) }; @@ -559,7 +551,7 @@ mod tests { Duration::MAX ); assert_eq!( - delay(f32::MAX, huge("a")).duration, + delay(Duration::MAX, huge("a")).duration, Duration::MAX ); } diff --git a/crates/motiongfx_editor/src/lib.rs b/crates/motiongfx_editor/src/lib.rs index 81cc3912..1fbd713a 100644 --- a/crates/motiongfx_editor/src/lib.rs +++ b/crates/motiongfx_editor/src/lib.rs @@ -402,7 +402,8 @@ fn on_scrub( if let Some(timeline) = manager.get_timeline_mut(&timeline_id) { timeline.set_target_track(0); - timeline.set_target_time(change.value); + timeline + .set_target_time(Duration::from_secs_f32(change.value)); } } @@ -430,7 +431,7 @@ fn on_play_pause( && timeline.target_time().as_secs_f32() >= state.duration { timeline.set_target_track(0); - timeline.set_target_time(0.0); + timeline.set_target_time(Duration::ZERO); } } diff --git a/examples/bevy_examples/examples/custom_ease.rs b/examples/bevy_examples/examples/custom_ease.rs index 3d2c97bb..6d809def 100644 --- a/examples/bevy_examples/examples/custom_ease.rs +++ b/examples/bevy_examples/examples/custom_ease.rs @@ -40,7 +40,7 @@ fn spawn_timeline( }) // A custom 10 step easing. .with_ease(|t| ((t * 10.0) as u32) as f32 / 10.0) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); diff --git a/examples/bevy_examples/examples/custom_interp.rs b/examples/bevy_examples/examples/custom_interp.rs index 57bddcaf..a22eea93 100644 --- a/examples/bevy_examples/examples/custom_interp.rs +++ b/examples/bevy_examples/examples/custom_interp.rs @@ -42,7 +42,7 @@ fn spawn_timeline( }) .with_interp(|start, end, t| arc_lerp_3d(*start, *end, t)) .with_ease(ease::cubic::ease_in_out) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); diff --git a/examples/bevy_examples/examples/easings.rs b/examples/bevy_examples/examples/easings.rs index 19fabdc1..bd1b1235 100644 --- a/examples/bevy_examples/examples/easings.rs +++ b/examples/bevy_examples/examples/easings.rs @@ -85,14 +85,14 @@ fn spawn_timeline( |x| x + 10.0, ) .with_ease(ease_fn) - .play(1.0), + .play(s(1)), b.act( sphere_mat_ids[i], path!(::emissive), move |_| red, ) .with_ease(ease_fn) - .play(1.0), + .play(s(1)), ] .ord_all() }) diff --git a/examples/bevy_examples/examples/editor.rs b/examples/bevy_examples/examples/editor.rs index c21d6e3a..c09ba716 100644 --- a/examples/bevy_examples/examples/editor.rs +++ b/examples/bevy_examples/examples/editor.rs @@ -59,10 +59,10 @@ fn spawn_timeline( .map(|&cube| { b.act(cube, path!(::scale), |_| Vec3::ONE) .with_ease(ease::back::ease_out) - .play(0.6) + .play(cs(60)) }) .collect::>() - .ord_flow(0.15); + .ord_flow(cs(15)); let spin = cubes .iter() @@ -71,10 +71,10 @@ fn spawn_timeline( Quat::from_rotation_y(std::f32::consts::PI) }) .with_ease(ease::cubic::ease_in_out) - .play(1.0) + .play(s(1)) }) .collect::>() - .ord_flow(0.1); + .ord_flow(cs(10)); let track = [grow, spin].ord_chain().compile(); b.add_tracks(track); diff --git a/examples/bevy_examples/examples/hello_world.rs b/examples/bevy_examples/examples/hello_world.rs index 3d8dd622..98c11691 100644 --- a/examples/bevy_examples/examples/hello_world.rs +++ b/examples/bevy_examples/examples/hello_world.rs @@ -67,14 +67,14 @@ fn spawn_timeline( Vec3::splat(0.9) }) .with_ease(circ_ease) - .play(1.0), + .play(s(1)), b.act( cube_id, path!(::translation::x), |x| x + 1.0, ) .with_ease(circ_ease) - .play(1.0), + .play(s(1)), b.act(cube_id, path!(::rotation), |_| { Quat::from_euler( EulerRot::XYZ, @@ -84,7 +84,7 @@ fn spawn_timeline( ) }) .with_ease(circ_ease) - .play(1.0), + .play(s(1)), ] .ord_all(); @@ -92,7 +92,7 @@ fn spawn_timeline( } } - let track = cube_tracks.ord_flow(0.01).compile(); + let track = cube_tracks.ord_flow(cs(1)).compile(); b.add_tracks(track); let timeline = b.compile(); diff --git a/examples/bevy_examples/examples/minimal.rs b/examples/bevy_examples/examples/minimal.rs index e0ccb2e7..785ae67c 100644 --- a/examples/bevy_examples/examples/minimal.rs +++ b/examples/bevy_examples/examples/minimal.rs @@ -54,13 +54,13 @@ fn build_timeline( b.act(cube_id, path!(::translation::x), |x| { x + 6.0 }) - .play(1.0), + .play(s(1)), b.act( cube_mat_id, path!(::base_color), |_| Srgba::RED.into(), ) - .play(1.0), + .play(s(1)), ] .ord_all() .compile(); diff --git a/examples/bevy_examples/examples/recording.rs b/examples/bevy_examples/examples/recording.rs index d3c7e77e..c302b7ba 100644 --- a/examples/bevy_examples/examples/recording.rs +++ b/examples/bevy_examples/examples/recording.rs @@ -112,7 +112,7 @@ fn spawn_timeline( x + Vec3::ZERO.with_x(10.0).with_z(1.0) }) .with_interp(|start, end, t| arc_lerp_3d(*start, *end, t)) - .play(1.0) + .play(s(1)) .compile(); b.add_tracks(track); diff --git a/examples/bevy_examples/examples/slide_basic.rs b/examples/bevy_examples/examples/slide_basic.rs index 11e43283..eb8a202b 100644 --- a/examples/bevy_examples/examples/slide_basic.rs +++ b/examples/bevy_examples/examples/slide_basic.rs @@ -55,7 +55,7 @@ fn spawn_timeline( let slide0 = b .act(cube, path!(::scale), |_| Vec3::ONE) .with_ease(ease::cubic::ease_out) - .play(1.0) + .play(s(1)) .compile(); let slide1 = [ @@ -66,21 +66,21 @@ fn spawn_timeline( move |_| -X_OFFSET, ) .with_ease(ease::cubic::ease_out) - .play(1.0), + .play(s(1)), b.act( cube_mat_id, path!(::base_color), move |_| palettes::tailwind::ZINC_700.into(), ) .with_ease(ease::cubic::ease_out) - .play(1.0), + .play(s(1)), ] .ord_all(), b.act(sphere, path!(::scale), |_| Vec3::ONE) .with_ease(ease::cubic::ease_out) - .play(1.0), + .play(s(1)), ] - .ord_flow(0.1) + .ord_flow(cs(10)) .compile(); b.add_tracks([slide0, slide1]); @@ -121,7 +121,7 @@ fn slide_movement( // Move to the start of the next track. let target_index = timeline.curr_index() + 1; timeline.set_target_track(target_index); - timeline.set_target_time(0.0); + timeline.set_target_time(s(0)); player.set_playing(false); } @@ -131,7 +131,7 @@ fn slide_movement( let target_index = timeline.curr_index().saturating_sub(1); timeline.set_target_track(target_index); - timeline.set_target_time(0.0); + timeline.set_target_time(s(0)); player.set_playing(false); } @@ -162,7 +162,7 @@ fn slide_movement( // Move to the start of the next track. let target_index = timeline.curr_index() + 1; timeline.set_target_track(target_index); - timeline.set_target_time(0.0); + timeline.set_target_time(s(0)); } } } diff --git a/examples/vello_winit_example/examples/lissajous.rs b/examples/vello_winit_example/examples/lissajous.rs index 5c49d4eb..38395f67 100644 --- a/examples/vello_winit_example/examples/lissajous.rs +++ b/examples/vello_winit_example/examples/lissajous.rs @@ -16,9 +16,9 @@ const CELL_H: f64 = 113.0; const CURVE_R: f64 = 42.0; const REF_R: f64 = 36.0; const DELTA: f64 = f64::consts::PI / 4.0; -const DRAW_DUR: f32 = 1.5; -const HOLD_DUR: f32 = 0.8; -const STAGGER: f32 = 0.08; // per diagonal +const DRAW_DUR: Duration = cs(150); +const HOLD_DUR: Duration = cs(80); +const STAGGER: Duration = cs(8); // per diagonal fn curve_color(c: usize) -> Color { let t = (c - 1) as f64 / (N_X - 1) as f64; @@ -169,22 +169,22 @@ impl LissajousTableDemo { let v = vert_entries.get(i).map(|&(id, p1)| { b.act(id, path!(::line::p1), move |_| p1) .with_ease(ease::cubic::ease_in_out) - .play(0.6) + .play(cs(60)) }); let h = horiz_entries.get(i).map(|&(id, p1)| { b.act(id, path!(::line::p1), move |_| p1) .with_ease(ease::cubic::ease_in_out) - .play(0.6) + .play(cs(60)) }); pair_tracks.push(match (v, h) { - (Some(v), Some(h)) => [v, h].ord_flow(0.025), + (Some(v), Some(h)) => [v, h].ord_flow(ms(25)), (Some(v), None) => v, (None, Some(h)) => h, (None, None) => unreachable!(), }); } let grid_track = - pair_tracks.into_iter().ord_flow(0.05).compile(); + pair_tracks.into_iter().ord_flow(cs(5)).compile(); b.add_tracks(grid_track); // Track 1: curves draw in and out, looped by the caller.