From 39ee2e596021e6849f62e8a12ae7c77d4bffdbc8 Mon Sep 17 00:00:00 2001 From: Tanish Manku Date: Tue, 3 Nov 2026 17:59:52 -0700 Subject: [PATCH] SHX fonts: review fixes (search the current folder for a bare drawing name, one lock for the font cache generation, no split_paths on wasm, MTEXT \U+ upper case as the layout, a point budget for repeated linetype shapes, web picks report skipped files and cap bytes read at once) --- apps/cadcraft-web/src/upload.rs | 17 ++++++++++++++++- crates/engine/src/cmd/file.rs | 2 ++ crates/fonts/src/mtext.rs | 2 +- crates/fonts/src/shx/table.rs | 7 +++---- crates/render/src/linetype_text.rs | 26 +++++++++++++++++++++++++- 5 files changed, 47 insertions(+), 7 deletions(-) diff --git a/apps/cadcraft-web/src/upload.rs b/apps/cadcraft-web/src/upload.rs index 4b4f51829..333c4906b 100644 --- a/apps/cadcraft-web/src/upload.rs +++ b/apps/cadcraft-web/src/upload.rs @@ -22,6 +22,9 @@ const ACCEPT: &str = ".dxf,.dwg,.shx"; /// Most files taken from one pick. const MAX_PICKED: u32 = 64; +/// Most bytes being read at once, all files together: a large pick must not exhaust the page's +/// memory (each file is copied once more into the app). +const MAX_READING_BYTES: u64 = 512 << 20; /// A file read in the browser (name, contents), or why it couldn't be read. type Upload = Result<(String, Vec), String>; @@ -30,12 +33,14 @@ type Upload = Result<(String, Vec), String>; #[derive(Clone)] pub struct Inbox { files: Rc>>, + /// Bytes of the files being read now. + reading: Rc>, ctx: egui::Context, } impl Inbox { pub fn new(ctx: &egui::Context) -> Self { - Inbox { files: Rc::default(), ctx: ctx.clone() } + Inbox { files: Rc::default(), reading: Rc::default(), ctx: ctx.clone() } } /// Show the browser's file picker and return at once. A chosen file arrives through @@ -72,6 +77,10 @@ impl Inbox { inbox.read(file); } } + let skipped: Vec = (MAX_PICKED..files.length()).filter_map(|i| files.get(i)).map(|f| f.name()).collect(); + if !skipped.is_empty() { + inbox.push(Err(format!("Open: at most {MAX_PICKED} files at a time; not opened: {}", skipped.join(", ")))); + } } chosen.remove(); }); @@ -101,12 +110,18 @@ impl Inbox { if let Some(e) = CadApp::open_size_error(&name, len) { return self.push(Err(e)); } + let reading = self.reading.get(); + if reading.saturating_add(len) > MAX_READING_BYTES { + return self.push(Err(format!("Open: {name} was not read: the files being read together are too large; open it on its own"))); + } + self.reading.set(reading.saturating_add(len)); let inbox = self.clone(); wasm_bindgen_futures::spawn_local(async move { let r = match wasm_bindgen_futures::JsFuture::from(file.array_buffer()).await { Ok(buf) => Ok((name, js_sys::Uint8Array::new(&buf).to_vec())), Err(e) => Err(format!("Open: could not read {name}: {}", js_error(e))), }; + inbox.reading.set(inbox.reading.get().saturating_sub(len)); inbox.push(r); }); } diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index af49e3351..606ad4e28 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -336,6 +336,8 @@ fn read_bytes(bytes: &[u8], name: &str, path: Option) -> Result String { } 'L' | 'l' | 'O' | 'o' | 'K' | 'k' => i += 2, // `\U+XXXX`: a character by its Unicode code, as the layout reads it. - 'U' | 'u' if crate::unicode_escape(&chars, i).is_some() => { + 'U' if crate::unicode_escape(&chars, i).is_some() => { if let Some(u) = crate::unicode_escape(&chars, i) { out.push(u); } diff --git a/crates/fonts/src/shx/table.rs b/crates/fonts/src/shx/table.rs index 9a002afaa..4b44927d7 100644 --- a/crates/fonts/src/shx/table.rs +++ b/crates/fonts/src/shx/table.rs @@ -140,7 +140,7 @@ pub fn drawing_dirs() -> Vec { pub fn set_search_path(list: &str) { let dirs: Vec = list .split(';') - .flat_map(|p| if cfg!(windows) { vec![PathBuf::from(p)] } else { std::env::split_paths(p).collect() }) + .flat_map(|p| if cfg!(any(windows, target_arch = "wasm32")) { vec![PathBuf::from(p)] } else { std::env::split_paths(p).collect() }) .filter(|d| !d.as_os_str().is_empty() && !d.to_string_lossy().trim().is_empty()) .take(MAX_DIRS) .collect(); @@ -184,7 +184,7 @@ pub fn generation() -> u64 { /// The SHX font file a style or linetype names, if CADCraft can find and read it. pub fn find(name: &str) -> Option> { let mut k = key(name)?; - let (provider, dirs) = { + let (provider, dirs, generation) = { let t = table(); // The font map renames once (a map can't loop). if let Some(to) = t.map.iter().find(|(from, _)| *from == k).and_then(|(_, to)| key(to)) { @@ -197,9 +197,8 @@ pub fn find(name: &str) -> Option> { return r.clone(); } let dirs: Vec = t.drawing_dirs.iter().chain(t.search_path.iter()).cloned().collect(); - (t.provider.clone(), dirs) + (t.provider.clone(), dirs, t.generation) }; - let generation = generation(); let font = provider .and_then(|p| p.load(&k)) .or_else(|| read_from(&dirs, &k)) diff --git a/crates/render/src/linetype_text.rs b/crates/render/src/linetype_text.rs index 97770c2b2..9f753ed11 100644 --- a/crates/render/src/linetype_text.rs +++ b/crates/render/src/linetype_text.rs @@ -25,6 +25,9 @@ use cadcraft_geom::Vec2; /// Upper bound on the texts and shapes drawn along one run (hostile scales; `apply` caps the /// dashes). const MAX_TEXTS: usize = 10_000; +/// Most points all the placed texts and shapes of one polyline hold together (a large shape +/// repeated many times). +const MAX_POINTS: usize = 2_000_000; /// One text or shape element of the pattern, shaped once at the origin. struct Element { @@ -56,10 +59,12 @@ pub fn texts(d: &Drawing, pts: &[Vec2], lt: &Linetype, scale: f64, min_pattern: } let mut walk = Walk { pts, i: 0, start: 0.0 }; let mut base = 0.0; + let mut points = 0usize; 'repeats: while base <= len { for el in &elements { let s = base + el.at; - if s > len + 1e-9 || out.len() >= MAX_TEXTS { + points = points.saturating_add(point_count(&el.local)); + if s > len + 1e-9 || out.len() >= MAX_TEXTS || points > MAX_POINTS { break 'repeats; } let Some((p, dir)) = walk.at(s) else { break 'repeats }; @@ -74,6 +79,10 @@ pub fn texts(d: &Drawing, pts: &[Vec2], lt: &Linetype, scale: f64, min_pattern: out } +fn point_count(s: &Shaped) -> usize { + s.strokes.iter().map(Vec::len).chain(s.glyphs.iter().flatten().map(Vec::len)).fold(0, usize::saturating_add) +} + /// The shape element `el` at the origin (scaled by the element's and the linetype's scale), if /// its shape file is found and has the shape. fn shape(d: &Drawing, el: &cadcraft_doc::DashElement, scale: f64) -> Option { @@ -253,6 +262,21 @@ mod tests { assert!(texts(&d, &[Vec2::ZERO, Vec2::new(2.0, 0.0)], <, 1.0, 0.0).is_empty()); } + #[test] + fn a_large_shape_repeated_many_times_is_bounded() { + // A shape of 1,000 one-unit vectors there and back: about 1,000 points each time. + let body: Vec<&str> = (0..500).flat_map(|_| ["010", "018"]).collect(); + let src = format!("*131,1002,ZIGZAG\n1,{},0\n", body.join(",")); + cadcraft_fonts::shx::register("cadcraft-render-big.shx", &cadcraft_fonts::shx::compile(&src).unwrap()).unwrap(); + let d = Drawing::new_imperial(); + let mut lt = fence("cadcraft-render-big.shx"); + lt.pattern[1].shape = Some(131); + // 3,000 repeats would hold about 3 million points. + let t = texts(&d, &[Vec2::ZERO, Vec2::new(2850.0, 0.0)], <, 1.0, 0.0); + let points: usize = t.iter().map(point_count).sum(); + assert!(!t.is_empty() && t.len() < 3000 && points <= MAX_POINTS, "{} shapes, {points} points", t.len()); + } + #[test] fn text_in_an_shx_style_uses_the_font_metrics() { // A text font written for this test: A is 4 wide with a gap of 2, cap height 6.