diff --git a/Cargo.lock b/Cargo.lock index 3f85f54..96f388f 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -3644,6 +3644,7 @@ dependencies = [ "hashbrown 0.17.1", "libm", "nonempty", + "tracing", "typarena", ] diff --git a/crates/motiongfx/Cargo.toml b/crates/motiongfx/Cargo.toml index f0e5d9d..d642f38 100644 --- a/crates/motiongfx/Cargo.toml +++ b/crates/motiongfx/Cargo.toml @@ -16,6 +16,7 @@ hashbrown = { workspace = true } nonempty = { workspace = true } typarena = { workspace = true } libm = { workspace = true } +tracing = { workspace = true, optional = true } [dev-dependencies] criterion = { version = "0.5", features = ["html_reports"] } @@ -28,5 +29,6 @@ harness = false workspace = true [features] -default = ["std"] +default = ["std", "tracing"] +tracing = ["dep:tracing"] std = [] diff --git a/crates/motiongfx/src/sequence.rs b/crates/motiongfx/src/sequence.rs index 659c5ea..b2bf675 100644 --- a/crates/motiongfx/src/sequence.rs +++ b/crates/motiongfx/src/sequence.rs @@ -49,42 +49,86 @@ impl Sequence { } impl Sequence { + /// Reports every clip from `first_new` on that starts before the + /// clip listed ahead of it, and every earlier clip it overlaps. + #[cfg(feature = "tracing")] + fn report_conflicts_from(&self, first_new: usize) { + // The latest end time among all clips before the new clips. + let mut max_end = self + .clips + .iter() + .take(first_new) + .map(ActionClip::end) + .max() + .unwrap_or(Duration::ZERO); + + for (index, clip) in + self.clips.iter().enumerate().skip(first_new) + { + if let Some(prev) = index + .checked_sub(1) + .and_then(|prev_index| self.clips.get(prev_index)) + .filter(|prev| clip.start < prev.start) + { + tracing::error!( + "`ActionClip` {} starts at {:?}, before clip {} at {:?} on the same field", + index, + clip.start, + index - 1, + prev.start, + ); + } + + // No earlier clip reaches this clip, so nothing can + // overlap it and the scan is skipped. + if clip.start < max_end { + for (before_index, before) in + self.clips.iter().enumerate().take(index) + { + if clip.start < before.end() + && before.start < clip.end() + { + tracing::error!( + "`ActionClip` {} ({:?}..{:?}) overlaps clip {} ({:?}..{:?}) on the same field", + index, + clip.start, + clip.end(), + before_index, + before.start, + before.end(), + ); + } + } + } + + max_end = max_end.max(clip.end()); + } + } + + /// Appends a clip, reporting any conflict it introduces. #[inline] pub fn push(&mut self, span: ActionClip) { - debug_assert!( - span.start >= self.end(), - "({:?} >= {:?}) `ActionClip`s shouldn't overlap!", - span.start, - self.end(), - ); - self.clips.push(span); + + #[cfg(feature = "tracing")] + self.report_conflicts_from(self.clips.len() - 1); } } impl Extend for Sequence { + /// Appends clips, reporting any conflict they introduce. #[inline] fn extend>( &mut self, iter: T, ) { - #[cfg(debug_assertions)] - let mut end = self.end(); - #[cfg(debug_assertions)] - let iter = { - iter.into_iter().inspect(|clip| { - debug_assert!( - clip.start >= end, - "({:?} >= {:?}) `ActionClip`s shouldn't overlap!", - clip.start, - end, - ); - - end = clip.end(); - }) - }; + #[cfg(feature = "tracing")] + let first_new = self.clips.len(); self.clips.extend(iter); + + #[cfg(feature = "tracing")] + self.report_conflicts_from(first_new); } } diff --git a/crates/motiongfx/src/track.rs b/crates/motiongfx/src/track.rs index dd4c053..eb58ef9 100644 --- a/crates/motiongfx/src/track.rs +++ b/crates/motiongfx/src/track.rs @@ -253,7 +253,7 @@ impl TrackFragment { )); field = key.field(); - field_offset = field_len; + field_offset += field_len; field_len = 0; } field_len += 1; @@ -582,4 +582,61 @@ mod tests { assert_eq!(track.duration(), Duration::ZERO); assert!(track.sequences_spans().is_empty()); } + + #[test] + fn field_spans_cover_every_lane() { + const DUMMY: Sequence = Sequence::new(clip(0)); + + let fa = UntypedField::placeholder_with_path("a"); + let fb = UntypedField::placeholder_with_path("b"); + let fc = UntypedField::placeholder_with_path("c"); + + let mut ids = IdRegistry::new(); + let s1 = ids.register_instance(DummyId(1)); + let s2 = ids.register_instance(DummyId(2)); + + let k = |sid, field| { + ActionKey::new( + UntypedSubjectId::new::(sid), + field, + ) + }; + + let track = TrackFragment::new() + .upsert_sequence(k(s1, fa), DUMMY.clone()) + .upsert_sequence(k(s2, fa), DUMMY.clone()) + .upsert_sequence(k(s1, fb), DUMMY.clone()) + .upsert_sequence(k(s1, fc), DUMMY.clone()) + .compile(); + + // Fields in the order `compile` sorts them into. + let mut covered = Vec::new(); + + for (field, expected) in [(fa, 2), (fb, 1), (fc, 1)] { + let spans = track + .lookup_field_spans(field) + .expect("field was compiled in"); + + assert_eq!(spans.len(), expected, "{field:?}"); + assert!( + spans.iter().all(|(key, _)| *key.field() == field), + "{field:?} span points at the wrong lanes", + ); + + covered.extend(spans.iter().map(|(key, _)| *key)); + } + + // Every lane, once: a drifted offset repeats one and skips + // another. + let lanes = track + .sequences_spans() + .iter() + .map(|(key, _)| *key) + .collect::>(); + + assert_eq!( + covered, lanes, + "field spans must cover every lane" + ); + } }