diff --git a/crates/engine/src/cmd/event.rs b/crates/engine/src/cmd/event.rs index 3622141d..e4b81233 100644 --- a/crates/engine/src/cmd/event.rs +++ b/crates/engine/src/cmd/event.rs @@ -402,6 +402,22 @@ mod tests { assert!((s.tempo.tempo_at_tick(tick) - 133.5).abs() < 1e-9); } + /// Changing the initial meter keeps a later meter change at its bar (issue #250). + #[test] + fn earlier_meter_change_keeps_later_meter_on_its_bar() { + use soundcraft_time::BarBeat; + let mut e = crate::Engine::default(); + assert_eq!(e.session().sample_rate.hz(), 48_000); + e.execute("event.meter", &json!({"numerator": 3, "denominator": 4, "at": 192_000})).unwrap(); + e.execute("event.meter", &json!({"numerator": 6, "denominator": 4, "at": 0})).unwrap(); + let map = &e.session().tempo; + let later = map.meters().get(1).copied().unwrap(); + assert_eq!((later.numerator, later.denominator), (3, 4)); + assert_eq!(map.bar_beat_at_tick(later.tick), BarBeat { bar: 3, beat: 1, tick: 0 }); + let bar2_beat3 = BarBeat { bar: 2, beat: 3, tick: 0 }; + assert_eq!(map.bar_beat_at_tick(map.tick_at_bar_beat(bar2_beat3)), bar2_beat3); + } + fn marker_name(e: &crate::Engine, id: u64) -> String { e.session().markers.iter().find(|m| m.id.0 == id).map(|m| m.name.clone()).unwrap_or_default() } diff --git a/crates/time/src/tempo.rs b/crates/time/src/tempo.rs index 5b87a3a7..b179a6d6 100644 --- a/crates/time/src/tempo.rs +++ b/crates/time/src/tempo.rs @@ -96,13 +96,31 @@ impl TempoMap { } /// Insert or replace the meter at `tick` (snapped to the bar line at or before it). + /// Later meter changes keep their bar numbers, so they stay on bar lines. pub fn set_meter(&mut self, tick: i64, numerator: u32, denominator: u32) -> Result<(), TimeError> { if !(1..=64).contains(&numerator) || ![1, 2, 4, 8, 16, 32, 64].contains(&denominator) { return Err(TimeError::Meter(numerator, denominator)); } let tick = if tick <= 0 { 0 } else { self.bar_start_tick(self.bar_beat_at_tick(tick).bar) }; - let ev = MeterEvent { tick, numerator, denominator }; - match self.meters.binary_search_by(|e| e.tick.cmp(&tick)) { + let later = self.take_meters_after(tick); + self.put_meter(MeterEvent { tick, numerator, denominator }); + self.restore_meters_at_bars(later); + Ok(()) + } + + /// Remove a meter event (the one at tick 0 is kept). Later meter changes keep their bar numbers. + pub fn remove_meter(&mut self, tick: i64) -> bool { + if tick == 0 || !self.meters.iter().any(|e| e.tick == tick) { + return false; + } + let later = self.take_meters_after(tick); + self.meters.retain(|e| e.tick != tick); + self.restore_meters_at_bars(later); + true + } + + fn put_meter(&mut self, ev: MeterEvent) { + match self.meters.binary_search_by(|e| e.tick.cmp(&ev.tick)) { Ok(i) => { if let Some(e) = self.meters.get_mut(i) { *e = ev; @@ -110,16 +128,21 @@ impl TempoMap { } Err(i) => self.meters.insert(i, ev), } - Ok(()) } - pub fn remove_meter(&mut self, tick: i64) -> bool { - if tick == 0 { - return false; + /// Remove the meters after `tick`, returning each with the bar it starts. + fn take_meters_after(&mut self, tick: i64) -> Vec<(i64, MeterEvent)> { + let later: Vec<_> = self.meters.iter().filter(|e| e.tick > tick).map(|e| (self.bar_beat_at_tick(e.tick).bar, *e)).collect(); + self.meters.retain(|e| e.tick <= tick); + later + } + + /// Put meters back at the start of their bars under the current earlier meters, in bar order. + fn restore_meters_at_bars(&mut self, later: Vec<(i64, MeterEvent)>) { + for (bar, ev) in later { + let tick = self.bar_start_tick(bar); + self.put_meter(MeterEvent { tick, ..ev }); } - let before = self.meters.len(); - self.meters.retain(|e| e.tick != tick); - before != self.meters.len() } /// Tempo in effect at `tick`. diff --git a/crates/time/tests/meter_grid_contract.rs b/crates/time/tests/meter_grid_contract.rs new file mode 100644 index 00000000..5d224324 --- /dev/null +++ b/crates/time/tests/meter_grid_contract.rs @@ -0,0 +1,72 @@ +//! Meter changes stay on bar lines when an earlier meter is edited (issue #250). + +use soundcraft_time::{BarBeat, MeterEvent, SampleRate, TempoMap}; + +fn assert_positions_round_trip(map: &TempoMap) { + for bar in 1..=5 { + let meter = map.meter_at_tick(map.bar_start_tick(bar)); + for beat in 1..=i64::from(meter.numerator) { + let position = BarBeat { bar, beat, tick: 0 }; + assert_eq!(map.bar_beat_at_tick(map.tick_at_bar_beat(position)), position); + } + } +} + +fn meter_bars(map: &TempoMap) -> Vec<(i64, u32, u32)> { + map.meters() + .iter() + .map(|m| { + let bb = map.bar_beat_at_tick(m.tick); + assert_eq!((bb.beat, bb.tick), (1, 0), "meter at tick {} is off the bar line", m.tick); + (bb.bar, m.numerator, m.denominator) + }) + .collect() +} + +#[test] +fn earlier_meter_edit_preserves_bar_positions() { + let mut map = TempoMap::default(); + let at = map.samples_to_ticks(192_000, SampleRate::HZ_48000); + assert_eq!(at, 7680); + map.set_meter(at, 3, 4).unwrap(); + assert_positions_round_trip(&map); + map.set_meter(0, 6, 4).unwrap(); + assert_positions_round_trip(&map); + // The 3/4 change stays at bar 3, now two 6/4 bars in. + assert_eq!(meter_bars(&map), vec![(1, 6, 4), (3, 3, 4)]); + assert_eq!(map.meters().get(1).map(|m| m.tick), Some(2 * 5760)); +} + +#[test] +fn inserting_and_removing_meters_keeps_later_bars() { + let mut map = TempoMap::default(); + map.set_meter(map.bar_start_tick(5), 5, 4).unwrap(); + map.set_meter(map.bar_start_tick(3), 3, 4).unwrap(); + assert_eq!(meter_bars(&map), vec![(1, 4, 4), (3, 3, 4), (5, 5, 4)]); + // Inserting 7/8 at bar 2 keeps 3/4 at bar 3 and 5/4 at bar 5. + map.set_meter(map.bar_start_tick(2), 7, 8).unwrap(); + assert_eq!(meter_bars(&map), vec![(1, 4, 4), (2, 7, 8), (3, 3, 4), (5, 5, 4)]); + assert_positions_round_trip(&map); + // Removing 3/4 leaves 5/4 at bar 5. + let three_four = map.meters().iter().find(|m| m.numerator == 3).map(|m| m.tick).unwrap(); + assert!(map.remove_meter(three_four)); + assert_eq!(meter_bars(&map), vec![(1, 4, 4), (2, 7, 8), (5, 5, 4)]); + assert_positions_round_trip(&map); +} + +#[test] +fn initial_meter_first_and_restoring_controls() { + // Setting the initial meter first, then the later change at the same raw tick, snaps to a bar line. + let mut map = TempoMap::default(); + map.set_meter(0, 6, 4).unwrap(); + map.set_meter(7680, 3, 4).unwrap(); + assert_eq!(map.meters().get(1), Some(&MeterEvent { tick: 5760, numerator: 3, denominator: 4 })); + assert_positions_round_trip(&map); + // Restoring 4/4 after the edit puts the 3/4 change back where it started. + let mut map = TempoMap::default(); + map.set_meter(7680, 3, 4).unwrap(); + map.set_meter(0, 6, 4).unwrap(); + map.set_meter(0, 4, 4).unwrap(); + assert_eq!(map.meters().get(1), Some(&MeterEvent { tick: 7680, numerator: 3, denominator: 4 })); + assert_positions_round_trip(&map); +}