From 80cd15b474e1a35019a69cbddb695b090dba4f4b Mon Sep 17 00:00:00 2001 From: quincy-jc <184324679+quincy-jc@users.noreply.github.com> Date: Thu, 5 Nov 2026 16:24:51 +1100 Subject: [PATCH] fix(measure): bound the snap index and read one page at a time Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/document.rs | 6 + crates/measure/src/snap.rs | 107 +++++++++++++- crates/ui-egui/src/tools/measure_ui.rs | 186 ++++++++++++++++++++----- 3 files changed, 259 insertions(+), 40 deletions(-) diff --git a/crates/engine/src/document.rs b/crates/engine/src/document.rs index 987a2b627e..7124a32f36 100644 --- a/crates/engine/src/document.rs +++ b/crates/engine/src/document.rs @@ -289,6 +289,12 @@ impl Document { measure::snap::geometry(&e.cos, page).map_err(|e| e.to_string()) } + /// [`Self::measurement_paths`] within `limits`. + pub fn measurement_paths_within(&self, page: usize, limits: measure::snap::Limits) -> Result { + let e = self.editor.as_ref().ok_or("the document can't be read")?; + measure::snap::geometry_with_limits(&e.cos, page, limits).map_err(|e| e.to_string()) + } + /// [`Self::measurement_paths`] as work to run away from the UI: a heavy drawing takes /// seconds to read (millions of segments). It works on a copy of the document as it is now. pub fn measurement_paths_job(&self, page: usize) -> impl FnOnce() -> Result + Send + 'static { diff --git a/crates/measure/src/snap.rs b/crates/measure/src/snap.rs index 661b083187..95c1f2b08d 100644 --- a/crates/measure/src/snap.rs +++ b/crates/measure/src/snap.rs @@ -19,8 +19,16 @@ pub struct Limits { } impl Limits { /// Enough for heavy CAD sheets (several million segments), and - /// the renderer's own content limit. At about 40 bytes a segment, the cap holds ~320 MB. + /// the renderer's own content limit. At about 40 bytes a segment, the cap holds ~320 MB, plus + /// up to [`MAX_INDEX_ENTRIES`] in the index. pub const DEFAULT: Self = Self { segments: 8_000_000, targets: 24_000_000, bytes: 512 << 20 }; + /// What a window can read without missing frames that matter. + pub const WITHIN_A_FRAME: Self = Self { segments: 20_000, targets: 40_000, bytes: 16 << 20 }; + /// Within what the index can number. + fn indexable(self) -> Self { + let most = u32::MAX as usize / 2; + Self { segments: self.segments.min(most), targets: self.targets.min(most), ..self } + } } impl Default for Limits { fn default() -> Self { @@ -139,6 +147,9 @@ impl Geometry { } /// Dense intersection searches are capped independently from path extraction. pub fn intersection_limited(&self, at: Point, tolerance: f64) -> bool { + if tolerance.is_nan() || tolerance < 0.0 { + return false; + } let (segs, _) = self.grid.near(self, at, tolerance); segs.iter() .filter_map(|&i| self.segs.get(i as usize)) @@ -214,7 +225,7 @@ impl Geometry { } /// Snap targets are recorded only for accepted geometry, and are capped. fn target(&mut self, ends: [Point; 2], mid: Point) { - if self.targets() >= self.limits.targets { + if self.targets().saturating_add(3) > self.limits.targets { self.truncated = true; return; } @@ -507,6 +518,7 @@ pub fn geometry(doc: &Document, page_index: usize) -> Result { /// The page's geometry for snapping, within `limits`. pub fn geometry_with_limits(doc: &Document, page_index: usize, limits: Limits) -> Result { + let limits = limits.indexable(); let p = crate::page(doc, page_index)?; let resources = p.dict.get(b"Resources").map(|o| doc.resolve(o)); let empty = Dict::new(); @@ -519,7 +531,7 @@ pub fn geometry_with_limits(doc: &Document, page_index: usize, limits: Limits) - let mut geometry = walker.geometry; geometry.segs.shrink_to_fit(); geometry.tgts.shrink_to_fit(); - geometry.grid = Grid::build(&geometry); + geometry.grid = Grid::build(&geometry, MAX_INDEX_ENTRIES); Ok(geometry) } @@ -530,6 +542,9 @@ const GRID_SIDE: usize = 1024; const WIDE_CELLS: usize = 64; /// The most cells one snap reads; a larger search area (a huge tolerance) scans everything. const MAX_QUERY_CELLS: usize = 4096; +/// The most entries the segment lists of the cells hold (4 bytes each). Past it, shorter segments +/// go in the list checked at every snap: snaps get slower, and find the same. +const MAX_INDEX_ENTRIES: u64 = 64_000_000; /// A uniform grid over the geometry's box: per cell, the segments and targets touching it. #[derive(Clone, Debug, Default)] @@ -549,7 +564,7 @@ struct Grid { } impl Grid { - fn build(g: &Geometry) -> Self { + fn build(g: &Geometry, max_entries: u64) -> Self { let mut bounds: Option<[f32; 4]> = None; let mut grow = |x: f32, y: f32| { let b = bounds.get_or_insert([x, y, x, y]); @@ -576,11 +591,20 @@ impl Grid { let cells = cols * rows; // Segments: count, then fill (compressed rows). let seg_cells = |s: &[f32; 4]| grid.range(s[0].min(s[2]), s[1].min(s[3]), s[0].max(s[2]), s[1].max(s[3])); + let covered = |s: &[f32; 4]| { + let (c0, r0, c1, r1) = seg_cells(s); + (c1 - c0 + 1) * (r1 - r0 + 1) + }; + // A segment of one cell adds one entry, so the last step fits whatever the sheet. + let most_cells = [WIDE_CELLS, 16, 4] + .into_iter() + .find(|&most| g.segs.iter().map(covered).filter(|&n| n <= most).map(|n| n as u64).sum::() <= max_entries) + .unwrap_or(1); let mut counts = vec![0u32; cells]; let mut wide = Vec::new(); for (i, s) in g.segs.iter().enumerate() { let (c0, r0, c1, r1) = seg_cells(s); - if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + if (c1 - c0 + 1) * (r1 - r0 + 1) > most_cells { wide.push(i as u32); continue; } @@ -594,7 +618,7 @@ impl Grid { let mut seg_items = vec![0u32; seg_start.last().copied().unwrap_or(0) as usize]; for (i, s) in g.segs.iter().enumerate() { let (c0, r0, c1, r1) = seg_cells(s); - if (c1 - c0 + 1) * (r1 - r0 + 1) > WIDE_CELLS { + if (c1 - c0 + 1) * (r1 - r0 + 1) > most_cells { continue; } for r in r0..=r1 { @@ -673,7 +697,8 @@ impl Grid { if self.cols == 0 || self.rows == 0 { return if g.segs.is_empty() && g.tgts.is_empty() { (Vec::new(), Vec::new()) } else { all() }; } - let t = tolerance as f32; + // Rounding to `f32` can stop the box short of a candidate's cell: this much wider can't. + let t = (tolerance + (at[0].abs().max(at[1].abs()) + tolerance) * 2.0 * f64::from(f32::EPSILON)) as f32; let (x, y) = (at[0] as f32, at[1] as f32); if !(x.is_finite() && y.is_finite() && t.is_finite()) { return all(); @@ -704,3 +729,71 @@ impl Grid { (segs, tgts) } } + +#[cfg(test)] +mod grid_tests { + use super::*; + + fn geometry(segs: Vec<[f32; 4]>, tgts: Vec<[f32; 6]>, max_entries: u64) -> Geometry { + let mut g = Geometry { segs, tgts, ..Geometry::default() }; + g.grid = Grid::build(&g, max_entries); + g + } + + /// Past the allowance the index holds fewer entries and finds the same. + #[test] + fn a_bounded_index_finds_what_a_full_one_does() { + let mut segs = Vec::new(); + for i in 0..4000u16 { + let (x, y) = (f32::from(i % 64) * 15.0, f32::from(i / 64) * 15.0); + segs.push([x, y, x + 37.0, y + 41.0]); + } + let tgts = Vec::new(); + let full = geometry(segs.clone(), tgts.clone(), u64::MAX); + let budget = segs.len() as u64; + let bounded = geometry(segs, tgts, budget); + assert!(full.grid.seg_items.len() as u64 > budget, "the sheet needs more than the allowance"); + assert!(bounded.grid.seg_items.len() as u64 <= budget, "{} entries", bounded.grid.seg_items.len()); + let options = SnapOptions { endpoints: false, midpoints: false, intersections: true, paths: true }; + for i in 0..400u16 { + let at = [f64::from(i) * 2.37, f64::from(i) * 1.13]; + assert_eq!(bounded.snap(at, 6.0, options).unwrap(), full.snap(at, 6.0, options).unwrap(), "at {at:?}"); + } + } + + #[test] + fn limits_stay_within_what_the_index_can_number() { + let limits = Limits { segments: usize::MAX, targets: usize::MAX, bytes: 1 }.indexable(); + assert!(limits.segments <= u32::MAX as usize && limits.targets <= u32::MAX as usize); + assert_eq!(Limits::DEFAULT.indexable(), Limits::DEFAULT); + } + + /// A tolerance below zero finds nothing; it must not reverse the cells searched. + #[test] + fn a_negative_tolerance_finds_nothing() { + let segs = (0..4000u16) + .map(|i| [f32::from(i % 64) * 15.0, f32::from(i / 64) * 15.0, f32::from(i % 64) * 15.0 + 5.0, f32::from(i / 64) * 15.0]) + .collect(); + let g = geometry(segs, Vec::new(), u64::MAX); + assert!(g.grid.cols > 1 && g.grid.rows > 1); + assert!(!g.intersection_limited([400.0, 400.0], -200.0)); + } + + /// Near 1e8 one `f32` step is 8 units, which must not hide an end just within tolerance. + #[test] + fn rounding_doesnt_hide_a_target_at_the_edge_of_the_grid() { + let g = geometry(Vec::new(), vec![[100_000_008.0, 0.0, 100_000_016.0, 0.0, 100_000_012.0, 0.0]], u64::MAX); + let options = SnapOptions { endpoints: true, midpoints: false, intersections: false, paths: false }; + let hit = g.snap([100_000_003.999_999_9, 0.0], 4.000_000_11, options).unwrap(); + assert_eq!(hit.map(|h| h.point), Some([100_000_008.0, 0.0])); + } + + /// A target is three points, and all three must fit. + #[test] + fn a_target_is_kept_only_if_all_its_points_fit() { + let mut g = Geometry::with_limits(Limits { segments: 10, targets: 2, bytes: 1 }); + g.target([[0.0, 0.0], [1.0, 0.0]], [0.5, 0.0]); + assert_eq!(g.targets(), 0); + assert!(g.truncated); + } +} diff --git a/crates/ui-egui/src/tools/measure_ui.rs b/crates/ui-egui/src/tools/measure_ui.rs index 7eb09c7b7c..37249d0ded 100644 --- a/crates/ui-egui/src/tools/measure_ui.rs +++ b/crates/ui-egui/src/tools/measure_ui.rs @@ -186,10 +186,14 @@ fn snap_to_content( pixels_per_unit: f32, point: Point, ) -> Option<(Point, measure::snap::SnapKind)> { + let reading = tl!("Reading the drawing for snapping…"); if !load_paths(ctx, state, doc, page) { - state.error = Some(tl!("Reading the drawing for snapping…").into()); + state.error = Some(reading.into()); return None; } + if state.error.as_deref() == Some(reading) { + state.error = None; + } if let Some((_, _, Err(e))) = &state.paths { state.error = Some(e.clone()); } @@ -213,50 +217,81 @@ fn snap_to_content( None } -/// A file smaller than this has its snapping geometry read on the spot. +/// A file smaller than this may have its snapping geometry read on the spot. +#[cfg(not(target_arch = "wasm32"))] const SNAP_SMALL_FILE: usize = 8 * 1024 * 1024; /// Whether the snapping geometry of `page` as it is now is at hand, starting (on a thread) or /// collecting the read when it isn't. fn load_paths(ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize) -> bool { + use std::sync::mpsc::TryRecvError; let generation = doc.edit_generation(); let current = |p: usize, g: u64| p == page && g == generation; if state.paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { return true; } - // Too little to read to be worth a thread, and the answer is there this frame. - if doc.bytes.len() < SNAP_SMALL_FILE { - state.pending_paths = None; - state.paths = Some((page, generation, doc.measurement_paths(page))); - return true; - } - if !state.pending_paths.as_ref().is_some_and(|(p, g, _)| current(*p, *g)) { - let (sender, receiver) = std::sync::mpsc::channel(); - state.pending_paths = Some((page, generation, receiver)); - let job = doc.measurement_paths_job(page); - let repaint = ctx.clone(); - let work = move || { - let _ = sender.send(job()); - repaint.request_repaint(); + // One read at a time, each being a whole page: one of another page or edit is let finish, and + // its answer dropped. + if let Some((p, g, receiver)) = &state.pending_paths { + let answer = match receiver.try_recv() { + Ok(result) => result, + Err(TryRecvError::Empty) => return false, + Err(TryRecvError::Disconnected) => Err("the snapping read stopped unexpectedly".into()), }; - #[cfg(not(target_arch = "wasm32"))] - if let Err(e) = std::thread::Builder::new().name("pdfcraft-snap".into()).spawn(work) { - state.pending_paths = None; - state.paths = Some((page, generation, Err(e.to_string()))); + let wanted = current(*p, *g); + state.pending_paths = None; + if wanted { + state.paths = Some((page, generation, answer)); return true; } - #[cfg(target_arch = "wasm32")] - work(); } - let arrived = state.pending_paths.as_ref().and_then(|(_, _, r)| r.try_recv().ok()); - match arrived { - Some(result) => { - state.pending_paths = None; - state.paths = Some((page, generation, result)); - true - } - None => false, + start_read(ctx, state, doc, page, generation) +} + +/// What the window itself reads of `page`: a small file's page, if it is within what a frame can +/// afford. A small file can still hold millions of segments. +#[cfg(not(target_arch = "wasm32"))] +fn read_on_the_spot(doc: &Document, page: usize) -> Option> { + if doc.bytes.len() >= SNAP_SMALL_FILE { + return None; } + let read = doc.measurement_paths_within(page, measure::snap::Limits::WITHIN_A_FRAME); + (!read.as_ref().is_ok_and(|g| g.truncated)).then_some(read) +} + +/// Read `page`'s geometry, on a thread unless it is little. Whether it is at hand already. +#[cfg(not(target_arch = "wasm32"))] +fn start_read(ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize, generation: u64) -> bool { + if let Some(read) = read_on_the_spot(doc, page) { + state.paths = Some((page, generation, read)); + return true; + } + let (sender, receiver) = std::sync::mpsc::channel(); + let job = doc.measurement_paths_job(page); + let repaint = ctx.clone(); + let work = move || { + let _ = sender.send(job()); + repaint.request_repaint(); + }; + if let Err(e) = std::thread::Builder::new().name("pdfcraft-snap".into()).spawn(work) { + state.paths = Some((page, generation, Err(e.to_string()))); + return true; + } + state.pending_paths = Some((page, generation, receiver)); + false +} + +/// With no threads here, what a frame can afford; past it the drawing says it exceeds the limit. +#[cfg(target_arch = "wasm32")] +fn start_read(_ctx: &egui::Context, state: &mut MeasureView, doc: &Document, page: usize, generation: u64) -> bool { + state.paths = Some((page, generation, doc.measurement_paths_within(page, measure::snap::Limits::WITHIN_A_FRAME))); + true +} + +/// Whether a click may place a point: with snapping on, not while the drawing is being read, as +/// the point would land unsnapped. +fn takes_clicks(state: &MeasureView) -> bool { + !state.snap_enabled || state.pending_paths.is_none() } /// The Measure tools' own clicks this frame: those of a press nobody holds. @@ -325,7 +360,7 @@ pub(crate) fn page_input( point = measure::snap::constrain(*last, point); } hover = Some(point); - if clicks.single { + if clicks.single && takes_clicks(state) { if state.page != Some(page) { state.cancel(); state.page = Some(page); @@ -355,7 +390,8 @@ pub(crate) fn page_input( } } if state.page == Some(page) && !state.points.is_empty() { - if (clicks.double || !typing && ui.input(|i| i.key_pressed(egui::Key::Enter))) && matches!(tool, Tool::Perimeter | Tool::Area) { + let double = clicks.double && takes_clicks(state); + if (double || !typing && ui.input(|i| i.key_pressed(egui::Key::Enter))) && matches!(tool, Tool::Perimeter | Tool::Area) { finish(state, &mut view.pending_edit, doc, page, tool, author, units); } if !typing && ui.input(|i| i.key_pressed(egui::Key::Backspace)) { @@ -710,6 +746,11 @@ mod tests { /// A one-page PDF of `lines` short lines, written out uncompressed. fn lined_pdf(lines: usize) -> Vec { + padded_lined_pdf(lines, 0) + } + + /// [`lined_pdf`], with `padding` more bytes in the file that aren't on the page. + fn padded_lined_pdf(lines: usize, padding: usize) -> Vec { let content = "10 10 m 200 200 l S\n".repeat(lines); let objects = [ "<< /Type /Catalog /Pages 2 0 R >>".to_string(), @@ -718,6 +759,7 @@ mod tests { format!("<< /Length {} >>\nstream\n{content}\nendstream", content.len()), ]; let mut pdf = b"%PDF-1.7\n".to_vec(); + pdf.extend_from_slice(format!("%{}\n", "x".repeat(padding)).as_bytes()); let mut offsets = Vec::new(); for (i, object) in objects.iter().enumerate() { offsets.push(pdf.len()); @@ -744,7 +786,8 @@ mod tests { assert!(load_paths(&ctx, &mut state, session.get(small).unwrap(), 0), "there at once"); assert!(state.pending_paths.is_none()); - let big = lined_pdf(SNAP_SMALL_FILE / 20 + 1); + // However little is on its page. + let big = padded_lined_pdf(10, SNAP_SMALL_FILE); assert!(big.len() >= SNAP_SMALL_FILE); let big = session.open_new("big.pdf", std::sync::Arc::new(big)).unwrap(); let doc = session.get(big).unwrap(); @@ -761,6 +804,83 @@ mod tests { assert!(geometry.segment_count() > 0); } + /// A small file can hold more than a frame can read: it goes to the thread, and comes whole. + #[test] + fn a_small_file_past_a_frames_limits_is_read_on_a_thread() { + let lines = measure::snap::Limits::WITHIN_A_FRAME.segments + 1; + let pdf = lined_pdf(lines); + assert!(pdf.len() < SNAP_SMALL_FILE); + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(pdf)).unwrap(); + let doc = session.get(id).unwrap(); + let mut state = MeasureView::default(); + assert!(!load_paths(&ctx, &mut state, doc, 0), "not read on the window's thread"); + let start = std::time::Instant::now(); + while !load_paths(&ctx, &mut state, doc, 0) { + assert!(start.elapsed() < std::time::Duration::from_secs(60), "it arrives"); + std::thread::sleep(std::time::Duration::from_millis(5)); + } + let geometry = state.paths.as_ref().and_then(|(_, _, g)| g.as_ref().ok()).expect("read"); + assert_eq!(geometry.segment_count(), lines); + assert!(!geometry.truncated); + } + + /// A read of another page is let finish before the next starts, and only then is it dropped. + #[test] + fn only_one_snapping_read_runs_at_a_time() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("big.pdf", std::sync::Arc::new(lined_pdf(SNAP_SMALL_FILE / 20 + 1))).unwrap(); + let doc = session.get(id).unwrap(); + let (sender, receiver) = std::sync::mpsc::channel(); + let mut state = MeasureView { pending_paths: Some((9, 9, receiver)), ..MeasureView::default() }; + assert!(!load_paths(&ctx, &mut state, doc, 0)); + assert_eq!(state.pending_paths.as_ref().map(|(p, _, _)| *p), Some(9), "waits for it"); + sender.send(Ok(measure::snap::Geometry::default())).unwrap(); + assert!(!load_paths(&ctx, &mut state, doc, 0)); + assert_eq!(state.pending_paths.as_ref().map(|(p, _, _)| *p), Some(0), "then reads this page"); + assert!(state.paths.is_none(), "the other page's answer is dropped"); + } + + /// A read that dies says so, instead of reading on for ever. + #[test] + fn a_snapping_read_that_stops_is_an_error() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(lined_pdf(10))).unwrap(); + let doc = session.get(id).unwrap(); + let (sender, receiver) = std::sync::mpsc::channel(); + drop(sender); + let mut state = MeasureView { pending_paths: Some((0, doc.edit_generation(), receiver)), ..MeasureView::default() }; + assert!(load_paths(&ctx, &mut state, doc, 0)); + assert!(matches!(&state.paths, Some((_, _, Err(_))))); + assert!(state.pending_paths.is_none()); + } + + #[test] + fn the_reading_message_goes_when_the_drawing_is_read() { + let ctx = egui::Context::default(); + let mut session = pdfcraft_engine::Session::new(); + let id = session.open_new("small.pdf", std::sync::Arc::new(lined_pdf(10))).unwrap(); + let doc = session.get(id).unwrap(); + let mut state = MeasureView { error: Some(tl!("Reading the drawing for snapping…").into()), ..MeasureView::default() }; + let _ = snap_to_content(&ctx, &mut state, doc, 0, [5.0, 5.0], 1.0, [5.0, 5.0]); + assert_eq!(state.error, None); + } + + #[test] + fn clicks_wait_for_the_drawing_only_with_snapping_on() { + let (_sender, receiver) = std::sync::mpsc::channel(); + let mut state = MeasureView { pending_paths: Some((0, 0, receiver)), ..MeasureView::default() }; + assert!(!takes_clicks(&state)); + state.snap_enabled = false; + assert!(takes_clicks(&state)); + state.snap_enabled = true; + state.pending_paths = None; + assert!(takes_clicks(&state)); + } + #[test] fn typed_ratios_are_read_leniently() { assert_eq!(parse_ratio("1:200"), Some(200.0));