address review: grid fallback stays on snap positions and bars; thumbnail positions in f64

crates/engine/src/clip_group_file.rs

@@ -103,7 +103,7 @@ fn decode_group_audio(
                 let bytes = crate::io::read_file(&full)?;
                 let (_, buf) = soundcraft_audio_io::decode(&bytes, Some("wav")).map_err(|err| EngineError::Io(format!("{rel}: {err}")))?;
                 let buf = crate::io::at_rate(buf, rate, crate::io::MAX_IMPORT_SAMPLES)
-                    .map_err(|r| EngineError::Io(format!("{rel}: {}", r.message("too large to import"))))?;
+                    .ok_or_else(|| EngineError::Io(format!("{rel}: too large to import")))?;
                 decoded.push((rel.clone(), buf));
                 index.insert(key, decoded.len() - 1);
                 decoded.len() - 1

crates/engine/src/cmd/sampler.rs

@@ -75,7 +75,7 @@ pub(crate) fn attach(e: &mut Engine, t: TrackId, name: &str, bytes: &[u8], path:
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
     let buf = crate::io::at_rate(buf, e.session().sample_rate.hz(), MAX_SAMPLE_VALUES)
-        .map_err(|r| EngineError::Io(format!("{name}: {}", r.message("too long for a sample (ten minutes of 192 kHz stereo at most)"))))?;
+        .ok_or_else(|| EngineError::Io(format!("{name}: too long for a sample (ten minutes of 192 kHz stereo at most)")))?;
     let channels = buf.num_channels();
     let s = e.session_mut();
     let src = crate::io::add_source(s, name, buf, path, format.format);

crates/engine/src/io/import.rs

@@ -7,48 +7,18 @@ use super::{
 /// Largest import we accept (samples per channel × channels), ~4 GiB of f32.
 pub(crate) const MAX_IMPORT_SAMPLES: usize = 1 << 30;
 
-/// The most a file's audio may grow when converted to the session rate. Sessions run at 8 kHz to
-/// 768 kHz (96×); past this a file is lying about its rate, and converting a few kilobytes would
-/// otherwise allocate and filter hundreds of megabytes.
-const MAX_RATE_EXPANSION: u32 = 128;
-
-/// Why [`at_rate`] refused a conversion.
-#[derive(Debug, PartialEq, Eq)]
-pub(crate) enum RateError {
-    /// The converted audio would hold more samples than the limit.
-    TooLarge,
-    /// The file's rate is so far below the session's that converting would grow it past [`MAX_RATE_EXPANSION`].
-    Ratio { from: u32, to: u32 },
-}
-
-impl RateError {
-    /// The refusal as a sentence: `size` says what a size refusal means to the caller.
-    pub(crate) fn message(&self, size: &str) -> String {
-        match self {
-            RateError::TooLarge => size.to_string(),
-            RateError::Ratio { from, to } => {
-                format!("its {from} Hz sample rate is too low to convert to the session's {to} Hz (at most {MAX_RATE_EXPANSION}-fold)")
-            }
-        }
-    }
-}
-
-/// `buf` converted to `rate`, unless it would then hold more than `limit` samples, or grow more
-/// than [`MAX_RATE_EXPANSION`]-fold: a file with a tiny sample rate grows by orders of magnitude.
-pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> std::result::Result<AudioBuffer, RateError> {
+/// `buf` converted to `rate`, unless it would then hold more than `limit` samples: a file with a
+/// tiny sample rate grows by orders of magnitude.
+pub(crate) fn at_rate(mut buf: AudioBuffer, rate: u32, limit: usize) -> Option<AudioBuffer> {
     let from = buf.sample_rate.max(1);
-    // widened integers: dividing first would truncate 128.86-fold down to 128
-    if u64::from(rate) > u64::from(from) * u64::from(MAX_RATE_EXPANSION) {
-        return Err(RateError::Ratio { from, to: rate });
-    }
     if buf.frames() as f64 * buf.num_channels() as f64 * f64::from(rate) / f64::from(from) > limit as f64 {
-        return Err(RateError::TooLarge);
+        return None;
     }
     if from != rate {
         buf.channels = soundcraft_dsp::offline::resample(&buf.channels, from, rate);
         buf.sample_rate = rate;
     }
-    Ok(buf)
+    Some(buf)
 }
 
 /// A relative `Audio Files/` path no other source in `s` already uses.
@@ -145,7 +115,7 @@ pub fn decode_import(path: &str, rate: u32) -> Result<DecodedImport> {
 fn decode_import_bytes(name: &str, bytes: &[u8], rate: u32) -> Result<(soundcraft_audio_io::AudioInfo, Arc<SourceAudio>)> {
     let ext = Path::new(name).extension().and_then(OsStr::to_str).map(str::to_ascii_lowercase);
     let (info, buf) = soundcraft_audio_io::decode(bytes, ext.as_deref()).map_err(|err| EngineError::Io(format!("{name}: {err}")))?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| EngineError::Io(format!("{name}: {}", e.message("file is too large to import"))))?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or_else(|| EngineError::Io(format!("{name}: file is too large to import")))?;
     if buf.frames() == 0 {
         return Err(EngineError::Io(format!("{name}: no audio")));
     }
@@ -245,22 +215,8 @@ mod rate_tests {
     #[test]
     fn a_tiny_sample_rate_is_refused_before_it_is_converted() {
         let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
-        assert!(at_rate(buf(1), 48_000, 1_000_000).is_err());
+        assert!(at_rate(buf(1), 48_000, 1_000_000).is_none());
         let ok = at_rate(buf(8_000), 48_000, 1_000_000).unwrap();
         assert_eq!((ok.sample_rate, ok.frames()), (48_000, 600));
     }
-
-    #[test]
-    fn an_extreme_rate_expansion_is_refused_whatever_the_sample_budget() {
-        let buf = |rate| AudioBuffer { sample_rate: rate, channels: vec![vec![0.5; 100]; 2] };
-        // 100 frames at 300 Hz would fit the budget as 128 000 samples, but grow 640-fold.
-        assert_eq!(at_rate(buf(300), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 300, to: 192_000 });
-        assert!(at_rate(buf(1_500), 192_000, usize::MAX).is_ok(), "128-fold is the supported extreme");
-        assert!(at_rate(buf(8_000), 768_000, usize::MAX).is_ok(), "the slowest file at the fastest session");
-        // 128.86-fold: integer division would truncate it to 128 and let it through
-        assert_eq!(at_rate(buf(1_490), 192_000, usize::MAX).unwrap_err(), RateError::Ratio { from: 1_490, to: 192_000 });
-        // a short file that is only over the sample budget is a size refusal, not a rate one
-        assert_eq!(at_rate(buf(48_000), 48_000, 10).unwrap_err(), RateError::TooLarge);
-        assert!(RateError::Ratio { from: 300, to: 192_000 }.message("big").contains("300 Hz"));
-    }
 }

crates/engine/src/io/session.rs

@@ -72,7 +72,7 @@ pub(crate) fn open_session_linked(e: &mut Engine, path: &Path, links: &Path) ->
 /// Decode one source's audio file at the session rate.
 pub(super) fn decode_source(bytes: &[u8], ext: Option<&str>, rate: u32) -> std::result::Result<Arc<SourceAudio>, String> {
     let (_, buf) = soundcraft_audio_io::decode(bytes, ext).map_err(|err| err.to_string())?;
-    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).map_err(|e| e.message("too large to load"))?;
+    let buf = at_rate(buf, rate, MAX_IMPORT_SAMPLES).ok_or("too large to load")?;
     Ok(Arc::new(SourceAudio::new(buf)))
 }
 

crates/ui-egui/src/panels/midi_editor.rs

@@ -265,20 +265,42 @@ fn draw_roll_background(painter: &egui::Painter, g: &Roll<'_>) {
         }
     }
     // Grid.
+    let step = grid_stride(grid_ticks, px_per_tick);
     let mut tick = 0;
     while tick <= len_ticks {
         let bar = tick % (TICKS_PER_QUARTER * 4) == 0;
         let x = g.x_of(tick);
-        if grid_ticks as f32 * px_per_tick > 4.0 || bar {
-            painter.line_segment(
-                [pos2(x, roll.min.y), pos2(x, roll.max.y)],
-                Stroke::new(1.0, if bar { Color32::from_rgb(80, 80, 86) } else { Color32::from_rgb(50, 50, 54) }),
-            );
-        }
-        tick += grid_ticks;
+        painter.line_segment(
+            [pos2(x, roll.min.y), pos2(x, roll.max.y)],
+            Stroke::new(1.0, if bar { Color32::from_rgb(80, 80, 86) } else { Color32::from_rgb(50, 50, 54) }),
+        );
+        tick = tick.saturating_add(step);
     }
 }
 
+/// Closest lines the grid draws, in points.
+const MIN_GRID_PX: f32 = 4.0;
+
+/// Ticks between grid lines: the grid when its lines are more than [`MIN_GRID_PX`] apart, else
+/// as few common multiples of the grid and a bar as keep that spacing (so every line is a snap
+/// position and falls on a bar). The lines drawn are then bounded by the roll's width, however long
+/// the clip, instead of by its length.
+fn grid_stride(grid_ticks: i64, px_per_tick: f32) -> i64 {
+    let bar_ticks = TICKS_PER_QUARTER * 4;
+    if grid_ticks as f32 * px_per_tick > MIN_GRID_PX {
+        return grid_ticks.max(1);
+    }
+    let grid = grid_ticks.max(1);
+    let (mut a, mut b) = (grid, bar_ticks);
+    while b != 0 {
+        (a, b) = (b, a % b);
+    }
+    let common = (grid / a).saturating_mul(bar_ticks);
+    // `as` saturates a huge or NaN quotient; a zero `px_per_tick` gives one common multiple.
+    let n = (MIN_GRID_PX / (common as f32 * px_per_tick)).ceil().max(1.0) as i64;
+    common.saturating_mul(n)
+}
+
 /// The notes, with a drag in progress applied.
 fn draw_notes(app: &SoundApp, painter: &egui::Painter, g: &Roll<'_>, base: Color32) {
     let Roll { sequence, grid_ticks, .. } = *g;
@@ -478,6 +500,24 @@ fn velocity_lane(app: &mut SoundApp, ui: &mut Ui, g: &Roll<'_>, cid: ClipId, bas
 mod tests {
     use super::*;
 
+    #[test]
+    fn grid_lines_are_bounded_by_the_roll_width_not_the_clip_length() {
+        let sixteenth = TICKS_PER_QUARTER / 4;
+        // A 1000 pt roll showing a 4-bar clip: every sixteenth is 16 pt apart and drawn.
+        assert_eq!(grid_stride(sixteenth, 1000.0 / (16.0 * TICKS_PER_QUARTER as f32)), sixteenth);
+        // The same roll over a clip a billion ticks long: at most a line every 4 pt.
+        let (len, px_per_tick) = (1_000_000_000i64, 1000.0 / 1_000_000_000.0f32);
+        let step = grid_stride(sixteenth, px_per_tick);
+        assert_eq!(step % (TICKS_PER_QUARTER * 4), 0, "lines fall on bars");
+        assert!((len / step) <= 251, "{} lines", len / step);
+        // a dotted quarter grid (1440 ticks): every line is also a grid position
+        let dotted = TICKS_PER_QUARTER * 3 / 2;
+        let step = grid_stride(dotted, px_per_tick);
+        assert_eq!((step % dotted, step % (TICKS_PER_QUARTER * 4)), (0, 0), "lines are snap positions on bars: {step}");
+        assert!((len / step) <= 251, "{} lines", len / step);
+        assert!(grid_stride(0, 0.0) >= TICKS_PER_QUARTER * 4, "a zero grid or width still advances");
+    }
+
     #[test]
     fn switching_clip_drops_selection_and_gestures() {
         let mut st = MidiEditorState { selected: vec![0, 2], clip: Some(ClipId(1)), band: Some(pos2(1.0, 2.0)), ..Default::default() };

crates/ui-egui/src/panels/video_window.rs

@@ -115,6 +115,20 @@ fn first_movie_aspect(s: &soundcraft_model::Session) -> Option<f32> {
     s.videos.first().map(|v| v.aspect() as f32)
 }
 
+/// Most thumbnails one clip draws in the Video Universe strip: 8 pt each, so more than the
+/// widest strip holds.
+const MAX_UNIVERSE_THUMBS: usize = 1 << 12;
+
+/// Left edges of the thumbnails of a clip starting at `clip_x` (each `tw` wide) that lie in
+/// `shown_min..shown_max`. The index and position are worked out in `f64`: an `f32` this far from
+/// the origin (a clip starting a billion points left) cannot step by a thumbnail's width.
+fn thumb_xs(clip_x: f32, shown_min: f32, shown_max: f32, tw: f32) -> impl Iterator<Item = f32> {
+    let (origin, tw) = (f64::from(clip_x), f64::from(tw));
+    let first = ((f64::from(shown_min) - origin) / tw).floor().max(0.0);
+    let count = ((f64::from(shown_max) - origin) / tw - first).ceil().max(0.0) as usize;
+    (0..count.min(MAX_UNIVERSE_THUMBS)).map(move |i| (origin + (first + i as f64) * tw) as f32)
+}
+
 fn universe(app: &mut SoundApp, ctx: &egui::Context) {
     app.video.poll(ctx);
     let s = app.engine.session_arc();
@@ -145,8 +159,10 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
             }
             let tw = (cr.height() * v.aspect() as f32).max(8.0);
             let hpx = ((cr.height() * ppp / 16.0).ceil() * 16.0).clamp(16.0, 256.0) as u32;
-            let mut x = cr.min.x;
-            while x < cr.max.x {
+            // Only the thumbnails inside the strip, counted rather than stepped: a clip starting
+            // far left of it has an edge so large that adding a thumbnail's width wouldn't move it.
+            let shown = cr.intersect(r);
+            for x in thumb_xs(cr.min.x, shown.min.x, shown.max.x, tw) {
                 let secs = movie_secs(&s, c.start, offset, strip_position(x, r.min.x, r.width(), end, c.start)).max(0.0);
                 if let Some(tex) = app.video.thumb(ctx, key, &v.path, secs, hpx) {
                     let tr = Rect::from_min_size(pos2(x, cr.min.y), vec2(tw, cr.height()));
@@ -154,7 +170,6 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
                     let uv = Rect::from_min_max(pos2(0.0, 0.0), pos2((vis.width() / tw).clamp(0.0, 1.0), 1.0));
                     painter.image(tex.id(), vis, uv, Color32::WHITE);
                 }
-                x += tw;
             }
         }
         // The Edit window's visible range and the playhead.
@@ -182,7 +197,18 @@ fn universe(app: &mut SoundApp, ctx: &egui::Context) {
 
 #[cfg(test)]
 mod tests {
-    use super::strip_position;
+    use super::{strip_position, thumb_xs};
+
+    #[test]
+    fn thumbnails_of_a_clip_far_left_of_the_strip_are_evenly_spaced() {
+        // 2^30 points left of the strip with 8 pt thumbnails: f32 steps of `first + i` here are 16 apart
+        let xs: Vec<f32> = thumb_xs(-1_073_741_824.0, 0.0, 80.0, 8.0).collect();
+        assert_eq!(xs.len(), 10);
+        assert!(xs.windows(2).all(|w| w[1] - w[0] == 8.0), "{xs:?}");
+        assert_eq!(xs[0], 0.0);
+        // a clip inside the strip starts at its own edge
+        assert_eq!(thumb_xs(10.0, 10.0, 34.0, 8.0).collect::<Vec<_>>(), vec![10.0, 18.0, 26.0]);
+    }
 
     // A 1024 px strip over 1_024_000 samples: 1000 samples per point, exact in f32.
     const LEFT: f32 = 100.0;