From dc9eea2695be7d8200e06453e49cfd85c48a3e1d Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 16:20:58 -0500 Subject: [PATCH] Small bounds: outline simplification, table row insertion, note ids, xtask stats (#2232) * fix: iterative outline simplification, bulk row insertion, linear note ids, no symlink walking in xtask stats * Apply review fixes - crates/doc/src/tests.rs - crates/geom/src/trace.rs * Apply review fixes - crates/geom/src/trace.rs * address review: cover a maximal note id in tests, format trace test --------- Co-authored-by: aiongg <67543387+aiongg@users.noreply.github.com> --- crates/doc/src/endnotes.rs | 6 ++- crates/doc/src/notes.rs | 6 ++- crates/doc/src/table.rs | 7 ++++ crates/doc/src/table/edit.rs | 12 +++--- crates/doc/src/tests.rs | 29 +++++++++++++++ crates/geom/src/trace.rs | 71 +++++++++++++++++++++++++++--------- xtask/src/stats.rs | 4 +- 7 files changed, 107 insertions(+), 28 deletions(-) diff --git a/crates/doc/src/endnotes.rs b/crates/doc/src/endnotes.rs index b15bb745c..4c7a60a39 100644 --- a/crates/doc/src/endnotes.rs +++ b/crates/doc/src/endnotes.rs @@ -103,8 +103,12 @@ impl Story { /// Insert empty endnotes for `n` new reference characters before which `k` references sit. pub(crate) fn endnotes_inserted(&mut self, k: usize, n: usize) { + // As for footnotes: the largest id kept as ids are given out, and the free-id search past + // the largest one (`+ 1` overflowed there). + let mut max = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0); for i in 0..n { - let id = self.endnotes.iter().map(|x| x.id).max().unwrap_or(0) + 1; + let id = max.checked_add(1).unwrap_or_else(|| crate::ids::next_local_id(self.endnotes.iter().map(|x| x.id))); + max = max.max(id); let mut text = Story::new(StoryId(0)); text.chars[0].format = CharFormat::default(); self.endnotes.insert((k + i).min(self.endnotes.len()), Arc::new(Footnote { id, text })); diff --git a/crates/doc/src/notes.rs b/crates/doc/src/notes.rs index 0811c549c..0e6b0f03b 100644 --- a/crates/doc/src/notes.rs +++ b/crates/doc/src/notes.rs @@ -186,8 +186,12 @@ impl Story { /// Insert empty footnotes for `n` new reference characters before which `k` references sit. pub(crate) fn notes_inserted(&mut self, k: usize, n: usize) { + // The largest id so far, kept as ids are given out (finding it again for every new note + // was quadratic in a batch); past the largest id, the free-id search as before. + let mut max = self.notes.iter().map(|x| x.id).max().unwrap_or(0); for i in 0..n { - let id = self.next_note_id(); + let id = max.checked_add(1).unwrap_or_else(|| self.next_note_id()); + max = max.max(id); let mut text = Story::new(StoryId(0)); text.chars[0].format = CharFormat::default(); self.notes.insert((k + i).min(self.notes.len()), Arc::new(Footnote { id, text })); diff --git a/crates/doc/src/table.rs b/crates/doc/src/table.rs index 3782c0181..4c726b743 100644 --- a/crates/doc/src/table.rs +++ b/crates/doc/src/table.rs @@ -264,6 +264,13 @@ mod tests { t.check().unwrap(); assert_eq!(t.cell(1, 1).unwrap().row_span, 3); assert_eq!(t.nrows(), 5); + // Several rows at once go in together, in order, inside the merged region. + t.insert_rows(2, 3); + t.check().unwrap(); + assert_eq!(t.cell(1, 1).unwrap().row_span, 6); + assert_eq!(t.nrows(), 8); + t.delete_rows(2, 3); + t.check().unwrap(); // Header stays first. t.insert_rows(0, 1); t.check().unwrap(); diff --git a/crates/doc/src/table/edit.rs b/crates/doc/src/table/edit.rs index 8b140fde1..a4a1c876d 100644 --- a/crates/doc/src/table/edit.rs +++ b/crates/doc/src/table/edit.rs @@ -35,13 +35,11 @@ impl Table { } }) .collect(); - for k in 0..n { - self.rows.insert(at + k, row.clone()); - let i = (at + k) * nc; - for (c, cell) in template.iter().enumerate() { - self.cells.insert(i + c, cell.clone()); - } - } + // All the new rows and cells in one move each (one at a time moved every later cell + // again for each one). + self.rows.splice(at..at, std::iter::repeat_n(row, n)); + let i = at * nc; + self.cells.splice(i..i, (0..n).flat_map(|_| template.iter().cloned())); self.set_regions(®ions); } diff --git a/crates/doc/src/tests.rs b/crates/doc/src/tests.rs index dd6805f41..ac050a457 100644 --- a/crates/doc/src/tests.rs +++ b/crates/doc/src/tests.rs @@ -568,6 +568,35 @@ fn set_story_text_prunes_story_hyperlinks_and_placeholders() { assert_eq!(d.data_merge.placeholders[0].id, 20); } +/// Many note references put in at once each get a note with an id of its own, in one pass. +#[test] +fn many_note_references_get_their_notes_at_once() { + let n = 20_000; + let mut st = Story::new(StoryId(1)); + let refs = format!("{}{}", FOOTNOTE_REF.to_string().repeat(n), crate::ENDNOTE_REF.to_string().repeat(n)); + st.insert(0, &refs); + assert_eq!((st.notes.len(), st.endnotes.len()), (n, n)); + let ids: std::collections::HashSet = st.notes.iter().map(|x| x.id).collect(); + assert_eq!(ids.len(), n); + let ids: std::collections::HashSet = st.endnotes.iter().map(|x| x.id).collect(); + assert_eq!(ids.len(), n); +} + +/// An existing note whose id is `u64::MAX` can't be counted past: the next note still gets an id +/// of its own. +#[test] +fn note_ids_survive_a_maximal_existing_id() { + let mut st = Story::new(StoryId(1)); + st.insert(0, &FOOTNOTE_REF.to_string()); + st.insert(0, &crate::ENDNOTE_REF.to_string()); + Arc::make_mut(&mut st.notes[0]).id = u64::MAX; + Arc::make_mut(&mut st.endnotes[0]).id = u64::MAX; + st.insert(0, &FOOTNOTE_REF.to_string()); + st.insert(0, &crate::ENDNOTE_REF.to_string()); + assert_ne!(st.notes[0].id, st.notes[1].id); + assert_ne!(st.endnotes[0].id, st.endnotes[1].id); +} + /// Exceptions are read in one pass: breaks are char positions of the lowercased word, a break /// at the start, at the end or twice in a row counts once or not at all, and a long exception /// full of breaks takes no longer than its length. diff --git a/crates/geom/src/trace.rs b/crates/geom/src/trace.rs index 292f82e87..4a085211d 100644 --- a/crates/geom/src/trace.rs +++ b/crates/geom/src/trace.rs @@ -81,33 +81,68 @@ fn simplify_closed(p: &[Point], tol: f64) -> Vec { a } +/// Ramer–Douglas–Peucker on `p`, keeping its ends. Spans wait on a stack of their own rather +/// than in nested calls: an outline of many points can split one point at a time, deeper than +/// the call stack goes. A tolerance that isn't a number or is below zero counts as zero (each +/// split must take a point strictly inside its span, or the span would never get shorter). fn rdp(p: &[Point], tol: f64) -> Vec { - if p.len() < 3 { - return p.to_vec(); - } - let (a, b) = (p[0], p[p.len() - 1]); - let d = b - a; - let len = d.hypot(); - let (mut best, mut idx) = (0.0, 0); - for (i, q) in p.iter().enumerate().take(p.len() - 1).skip(1) { - let dist = if len < 1e-12 { (*q - a).hypot() } else { ((*q - a).cross(d) / len).abs() }; - if dist > best { - (best, idx) = (dist, i); + let (Some(_), Some(last)) = (p.first(), p.len().checked_sub(1)) else { return Vec::new() }; + let tol = if tol >= 0.0 { tol } else { 0.0 }; + let mut keep = vec![false; p.len()]; + keep[0] = true; + keep[last] = true; + let mut spans = vec![(0, last)]; + while let Some((s, e)) = spans.pop() { + if e < s + 2 { + continue; + } + let (a, b) = (p[s], p[e]); + let d = b - a; + let len = d.hypot(); + let (mut best, mut idx) = (0.0, s); + for (i, q) in p.iter().enumerate().take(e).skip(s + 1) { + let dist = if len < 1e-12 { (*q - a).hypot() } else { ((*q - a).cross(d) / len).abs() }; + if dist > best { + (best, idx) = (dist, i); + } + } + if best > tol && idx > s { + keep[idx] = true; + spans.push((idx, e)); + spans.push((s, idx)); } } - if best <= tol { - return vec![a, b]; - } - let mut left = rdp(&p[..=idx], tol); - left.pop(); - left.extend(rdp(&p[idx..], tol)); - left + p.iter().zip(keep).filter_map(|(q, k)| k.then_some(*q)).collect() } #[cfg(test)] mod tests { use super::*; + /// A tolerance below zero or NaN ends (each split shrinks its span), and a 200,000-point + /// path simplifies to completion. + #[test] + fn simplification_ends_for_any_tolerance_and_depth() { + let line: Vec = (0..10).map(|i| Point::new(f64::from(i), 0.0)).collect(); + for tol in [-1.0, f64::NAN, 0.0] { + assert_eq!(rdp(&line, tol).len(), 2, "{tol}"); + } + // A long convex curve simplifies to completion. + let n = 200_000; + let curve: Vec = (0..n).map(|i| Point::new(f64::from(i), (f64::from(i) * 1e-3).powi(3))).collect(); + assert!(rdp(&curve, 1e-12).len() > 2); + // A zigzag of growing amplitude: every split takes the point next to the span's end. + let zigzag: Vec = (0..10_000) + .map(|i| { + let x = f64::from(i); + Point::new(x, if i % 2 == 0 { x } else { -x }) + }) + .collect(); + assert_eq!(rdp(&zigzag, 0.0).len(), zigzag.len()); + assert!(rdp(&[], 1.0).is_empty()); + assert_eq!(rdp(&line[..1], 1.0).len(), 1); + } + #[test] fn traces_a_disc_and_its_hole() { let (w, h) = (40, 40); diff --git a/xtask/src/stats.rs b/xtask/src/stats.rs index f40b1300a..d9379327a 100644 --- a/xtask/src/stats.rs +++ b/xtask/src/stats.rs @@ -18,7 +18,9 @@ fn walk(dir: &Path, c: &mut Counts) { let p = e.path(); let name = e.file_name(); let name = name.to_string_lossy(); - if p.is_dir() { + // A link to a folder isn't followed: links can lead back up the tree or to the same + // folder many times over. + if e.file_type().is_ok_and(|t| t.is_dir()) { // Nested fuzz workspaces and build output are not part of the crate. if name != "target" && name != "fuzz" && !name.starts_with('.') { walk(&p, c);