From b15c5a023efa4e239bf34eb27399e5ae71c146db Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 11 Nov 2026 17:43:15 +0000 Subject: [PATCH] fix(engine): bound attribute extraction, NCOPY visits, xref bind lookups and PDFBOOK work; autoload from the support folder --- crates/engine/src/cmd/blocks/extract.rs | 39 +++++++++++--- crates/engine/src/cmd/blocks/xref/bind.rs | 53 +++++++++++++++----- crates/engine/src/cmd/exchange.rs | 24 +++++++++ crates/engine/src/cmd/modify2/ncopy.rs | 29 ++++++++--- crates/engine/src/cmd/modify2/tests.rs | 21 ++++++++ crates/engine/src/lisp.rs | 2 +- crates/engine/src/lisp/builtins/functions.rs | 3 +- crates/engine/src/lisp/drawing.rs | 27 ++++++++++ 8 files changed, 171 insertions(+), 27 deletions(-) diff --git a/crates/engine/src/cmd/blocks/extract.rs b/crates/engine/src/cmd/blocks/extract.rs index de85956c7..f0cdc2638 100644 --- a/crates/engine/src/cmd/blocks/extract.rs +++ b/crates/engine/src/cmd/blocks/extract.rs @@ -19,6 +19,21 @@ const MAX_TABLE_ROWS: usize = 2000; const MAX_TABLE_COLS: usize = 200; /// Attribute columns kept at most. const MAX_TAGS: usize = 190; +/// Attribute text one extraction may hold across all its references (each row owns a copy of its +/// values, constants included). Far above any real schedule; an error, never a silent cut. +const MAX_VALUE_BYTES: usize = 1 << 30; + +/// Add `add` bytes to `used`; an error past `limit`. +fn charge(used: &mut usize, add: usize, limit: usize) -> Result<()> { + *used = used.saturating_add(add); + if *used > limit { + return Err(bad( + "dataextraction", + format!("the attribute text of the block references passes {} MiB; narrow `blocks`, `tags` or `handles`", limit >> 20), + )); + } + Ok(()) +} const PARAMS: &str = "{blocks?: \"A*,B\" (block names, wildcards, default *), tags?: attribute tags to include (wildcards, default *), handles? (default: the block references in model space and every layout), combine?: bool (default true: identical rows once, with a Count column; false: one row per reference with handle, layer, position and rotation), file?: path (.csv; .txt is tab-separated), at?: [x,y] (insert a table there), title?: table title} → {columns, rows, file?, table?}"; @@ -167,6 +182,7 @@ fn found(s: &Session, p: &Value, o: &Options) -> Result> { }; let selected = selected_tags(&ents, d, o); let mut out = Vec::new(); + let mut used = 0usize; let mut consts: HashMap> = HashMap::new(); for e in &ents { let EntityKind::Insert(ins) = &e.kind else { continue }; @@ -175,18 +191,20 @@ fn found(s: &Session, p: &Value, o: &Options) -> Result> { continue; } let block = d.block(&ins.block).map(|b| b.name.clone()).unwrap_or_else(|| ins.block.clone()); + let values = values( + ins, + // Keyed by the resolved definition's exact name: `Part` and `PART` can both exist. + consts.entry(block.clone()).or_insert_with(|| d.block(&ins.block).map(|b| constants(b, &selected)).unwrap_or_default()), + &selected, + ); + charge(&mut used, values.iter().map(|(t, v)| t.len() + v.len()).sum(), MAX_VALUE_BYTES)?; out.push(Found { handle: e.handle.hex(), layer: e.common.layer.clone(), at: ins.insert.xy(), rotation: ins.rotation.to_degrees(), block: block.clone(), - values: values( - ins, - // Keyed by the resolved definition's exact name: `Part` and `PART` can both exist. - consts.entry(block.clone()).or_insert_with(|| d.block(&ins.block).map(|b| constants(b, &selected)).unwrap_or_default()), - &selected, - ), + values, }); } Ok(out) @@ -449,6 +467,15 @@ impl Interactive for ExtractM { #[cfg(test)] mod tests { + #[test] + fn attribute_text_is_charged_against_one_budget() { + let mut used = 0; + assert!(super::charge(&mut used, 60, 100).is_ok()); + assert!(super::charge(&mut used, 40, 100).is_ok()); + assert!(super::charge(&mut used, 1, 100).is_err()); + assert!(super::charge(&mut used, usize::MAX, 100).is_err()); + } + use super::*; #[test] diff --git a/crates/engine/src/cmd/blocks/xref/bind.rs b/crates/engine/src/cmd/blocks/xref/bind.rs index cdbc2af56..c217219e0 100644 --- a/crates/engine/src/cmd/blocks/xref/bind.rs +++ b/crates/engine/src/cmd/blocks/xref/bind.rs @@ -6,7 +6,7 @@ //! of nested xrefs (`xref|nested|name`) bind with every `|` written `$0$`. use std::cell::RefCell; -use std::collections::{BTreeSet, HashMap}; +use std::collections::{BTreeSet, HashMap, HashSet}; use std::sync::Arc; use cadcraft_doc::{Drawing, EntityStore, MAX_BLOCK_DEPTH, XrefStatus}; @@ -20,6 +20,16 @@ fn split(name: &str) -> Option<(&str, &str)> { /// Whether table `sym` of `d` has a symbol called `name`. fn taken(d: &Drawing, sym: Sym, name: &str) -> bool { + taken_with(d, sym, name, None) +} + +/// [`taken`], answering for blocks from `blocks` (the upper-cased names of `d`'s blocks, kept +/// current by the caller) when given: a lookup that misses scans every block, which a bind +/// naming thousands of blocks would repeat for each. +fn taken_with(d: &Drawing, sym: Sym, name: &str, blocks: Option<&HashSet>) -> bool { + if let (Sym::Block, Some(set)) = (sym, blocks) { + return set.contains(&name.to_ascii_uppercase()); + } match sym { Sym::Layer => d.layer(name).is_some(), Sym::Linetype => d.linetype(name).is_some(), @@ -32,12 +42,12 @@ fn taken(d: &Drawing, sym: Sym, name: &str) -> bool { } /// The bound name of `key|rest`: `key$n$rest` with the first free `n`, or `rest` (Insert type). -fn bound_name(d: &Drawing, sym: Sym, key: &str, rest: &str, insert: bool) -> String { +fn bound_name(d: &Drawing, sym: Sym, key: &str, rest: &str, insert: bool, blocks: Option<&HashSet>) -> String { let rest = rest.replace('|', "$0$"); if insert { return rest; } - (0..10_000).map(|n| format!("{key}${n}${rest}")).find(|c| !taken(d, sym, c)).unwrap_or_else(|| format!("{key}$${rest}")) + (0..10_000).map(|n| format!("{key}${n}${rest}")).find(|c| !taken_with(d, sym, c, blocks)).unwrap_or_else(|| format!("{key}$${rest}")) } /// Every table's symbol names, for one table at a time. @@ -53,9 +63,8 @@ fn names(d: &Drawing, sym: Sym) -> Vec { } } -/// Rename symbol `old` of table `sym` to `new`, or drop it (`new` already exists: Insert type). -fn rename_symbol(d: &mut Drawing, sym: Sym, old: &str, new: &str) { - let merge = !new.eq_ignore_ascii_case(old) && taken(d, sym, new); +/// Rename symbol `old` of table `sym` to `new`, or drop it (`merge`: `new` already exists, Insert type). +fn rename_symbol(d: &mut Drawing, sym: Sym, old: &str, new: &str, merge: bool) { macro_rules! table { ($v:expr) => {{ if merge { @@ -101,12 +110,17 @@ pub(crate) fn bind(d: &mut Drawing, key: &str, insert: bool) -> Result<(), Strin let mut map: HashMap<(Sym, String), String> = HashMap::new(); let mut bound_blocks = vec![key.clone()]; for sym in TABLES { + let mut blocks: Option> = (sym == Sym::Block).then(|| d.blocks.keys().map(|k| k.to_ascii_uppercase()).collect()); for old in names(d, sym).into_iter().filter(|n| has_prefix(n, &prefix)) { let rest = old.get(prefix.len()..).unwrap_or_default(); - let new = bound_name(d, sym, &key, rest, insert); + let new = bound_name(d, sym, &key, rest, insert, blocks.as_ref()); // Insert type: a host symbol of the bound name is used instead of the xref's. - let merged = taken(d, sym, &new); - rename_symbol(d, sym, &old, &new); + let merged = taken_with(d, sym, &new, blocks.as_ref()); + rename_symbol(d, sym, &old, &new, !new.eq_ignore_ascii_case(&old) && merged); + if let Some(set) = &mut blocks { + set.remove(&old.to_ascii_uppercase()); + set.insert(new.to_ascii_uppercase()); + } if sym == Sym::Block && !merged { bound_blocks.push(new.clone()); } @@ -242,14 +256,27 @@ fn xbind_in(d: &mut Drawing, sym: Sym, name: &str, depth: usize) -> Result = d.blocks.keys().map(|k| k.to_ascii_uppercase()).collect(); + for n in ["site$0$plan", "SITE$0$Plan", "OTHER", "SITE$1$Plan", "nothing"] { + assert_eq!(taken_with(&d, Sym::Block, n, Some(&set)), taken(&d, Sym::Block, n), "{n}"); + } + assert_eq!(bound_name(&d, Sym::Block, "SITE", "plan", false, Some(&set)), "SITE$1$plan"); + } + #[test] fn bound_names() { let mut d = Drawing::new_imperial(); - assert_eq!(bound_name(&d, Sym::Layer, "SITE", "Walls", false), "SITE$0$Walls"); + assert_eq!(bound_name(&d, Sym::Layer, "SITE", "Walls", false, None), "SITE$0$Walls"); d.ensure_layer("SITE$0$Walls"); - assert_eq!(bound_name(&d, Sym::Layer, "SITE", "walls", false), "SITE$1$walls"); - assert_eq!(bound_name(&d, Sym::Layer, "SITE", "Walls", true), "Walls"); - assert_eq!(bound_name(&d, Sym::Layer, "SITE", "TREES|Oak", false), "SITE$0$TREES$0$Oak"); + assert_eq!(bound_name(&d, Sym::Layer, "SITE", "walls", false, None), "SITE$1$walls"); + assert_eq!(bound_name(&d, Sym::Layer, "SITE", "Walls", true, None), "Walls"); + assert_eq!(bound_name(&d, Sym::Layer, "SITE", "TREES|Oak", false, None), "SITE$0$TREES$0$Oak"); assert_eq!(split("A|b|c"), Some(("A", "b|c"))); assert_eq!(split("|b"), None); } diff --git a/crates/engine/src/cmd/exchange.rs b/crates/engine/src/cmd/exchange.rs index 1b8d7c72e..a7ac13694 100644 --- a/crates/engine/src/cmd/exchange.rs +++ b/crates/engine/src/cmd/exchange.rs @@ -497,6 +497,16 @@ fn frame_books(d: &Drawing, frame_height: f64, unit: f64) -> (Vec>, u (books, skipped) } +/// Most page-by-entity renderings one PDFBOOK may ask for: every page draws the whole model through +/// its window, so the work is the frames times the model's objects (1,000 sheets of a 500,000-object +/// drawing). +const MAX_BOOK_WORK: u64 = 500_000_000; + +/// Whether `frames` pages of a model with `entities` objects stay within [`MAX_BOOK_WORK`]. +fn book_work_fits(frames: usize, entities: usize) -> bool { + (frames as u64).saturating_mul(entities as u64) <= MAX_BOOK_WORK +} + fn run_pdfbook(s: &mut Session, p: &Value) -> Result { if io().and_then(|h| h.plot).is_none() { return Err(bad("pdfbook", "plotting is not available in this build")); @@ -507,6 +517,13 @@ fn run_pdfbook(s: &mut Session, p: &Value) -> Result { if books.is_empty() { return Err(bad("pdfbook", format!("no closed polylines {frame_height} mm high (sheet frames) in model space"))); } + let sheets: usize = books.iter().map(Vec::len).sum(); + if !book_work_fits(sheets, s.doc()?.model.len()) { + return Err(bad( + "pdfbook", + format!("{sheets} sheets of a model this large are too much work for one command; plot part of the drawing at a time"), + )); + } let target = str_param(p, "path").map(str::to_string).or_else(|| s.state().ok().and_then(|st| st.path.clone())); let pb = target.as_deref().map(std::path::Path::new); let dir = pb.and_then(std::path::Path::parent).map(std::path::Path::to_path_buf).unwrap_or_default(); @@ -561,6 +578,13 @@ mod tests { Ok(()) } + #[test] + fn a_book_is_bounded_by_pages_times_objects() { + assert!(book_work_fits(1_000, 500_000)); + assert!(!book_work_fits(100_000, 100_000)); + assert!(!book_work_fits(usize::MAX, usize::MAX)); + } + #[test] fn web_export_delivers_a_download_instead_of_writing_a_path() { deliver_file("export", "Drawing1.svg", b"", Some(record)).unwrap(); diff --git a/crates/engine/src/cmd/modify2/ncopy.rs b/crates/engine/src/cmd/modify2/ncopy.rs index 9b6f9eeb2..9a449eb57 100644 --- a/crates/engine/src/cmd/modify2/ncopy.rs +++ b/crates/engine/src/cmd/modify2/ncopy.rs @@ -7,16 +7,32 @@ use super::{ // NCOPY // ===================================================================================== -/// The leaf objects of a block reference in world coordinates (nested references expanded). -fn nested_leaves(d: &cadcraft_doc::Drawing, ins: &cadcraft_doc::Insert, m: &Mat3, depth: usize, out: &mut Vec<(Common, EntityKind)>) { +/// Block entities one NCOPY may visit in all, leaves or not: references to shared, empty or +/// repeated sub-blocks multiply the visits (not the leaves) with each level of nesting. +const MAX_VISITS: usize = 1_000_000; + +/// The leaf objects of a block reference in world coordinates (nested references expanded). An +/// error when the nesting needs more than [`MAX_VISITS`] visits. +fn nested_leaves( + d: &cadcraft_doc::Drawing, + ins: &cadcraft_doc::Insert, + m: &Mat3, + depth: usize, + visits: &mut usize, + out: &mut Vec<(Common, EntityKind)>, +) -> Result<()> { if depth >= MAX_BLOCK_DEPTH || out.len() >= MAX_GEN { - return; + return Ok(()); } - let Some(b) = d.block(&ins.block) else { return }; + let Some(b) = d.block(&ins.block) else { return Ok(()) }; let mm = m.then_before(ins.transform(b.base.xy())); for e in b.entities.iter() { + *visits += 1; + if *visits > MAX_VISITS { + return Err(other("The block reference nests too many objects to copy.")); + } match &e.kind { - EntityKind::Insert(inner) => nested_leaves(d, inner, &mm, depth + 1, out), + EntityKind::Insert(inner) => nested_leaves(d, inner, &mm, depth + 1, visits, out)?, EntityKind::AttDef(_) => {} k => { let mut k = k.clone(); @@ -25,13 +41,14 @@ fn nested_leaves(d: &cadcraft_doc::Drawing, ins: &cadcraft_doc::Insert, m: &Mat3 } } } + Ok(()) } fn leaves_of(s: &Session, h: Handle) -> Result> { let e = curves::entity(s, h)?; let EntityKind::Insert(ins) = &e.kind else { return Err(other("Select a block reference.")) }; let mut out = Vec::new(); - nested_leaves(s.doc()?, ins, &Mat3::IDENTITY, 0, &mut out); + nested_leaves(s.doc()?, ins, &Mat3::IDENTITY, 0, &mut 0, &mut out)?; Ok(out) } diff --git a/crates/engine/src/cmd/modify2/tests.rs b/crates/engine/src/cmd/modify2/tests.rs index 5f65026a9..5885e1de3 100644 --- a/crates/engine/src/cmd/modify2/tests.rs +++ b/crates/engine/src/cmd/modify2/tests.rs @@ -777,3 +777,24 @@ fn offset_elliptical_arc_follows_picked_side() { let outer = max_x(json!([20, 20])); assert!((outer - 11.0).abs() < 0.05, "outer side went inward: max x = {outer}"); } + +/// Nested references multiply visits even when no leaf results; NCOPY refuses past its visit budget. +#[test] +fn ncopy_charges_every_nested_visit() { + use cadcraft_doc::{Block, Common, Entity, Insert}; + let mut s = Session::new(); + let d = s.doc_mut().unwrap(); + let mut n = 1; + for level in 0..15 { + let mut b = Block::new(&format!("B{level}")); + for _ in 0..100 { + let ins = Insert { block: format!("B{}", level + 1), ..Insert::default() }; + b.entities.push(Entity::new(Handle(n), EntityKind::Insert(ins))); + n += 1; + } + d.blocks.insert(b.name.clone(), std::sync::Arc::new(b)); + } + d.blocks.insert("B15".into(), std::sync::Arc::new(Block::new("B15"))); + let h = d.add(&Space::Model, Common::default(), EntityKind::Insert(Insert { block: "B0".into(), ..Insert::default() })).unwrap(); + assert!(s.execute("ncopy", &json!({ "handle": h.hex() })).is_err()); +} diff --git a/crates/engine/src/lisp.rs b/crates/engine/src/lisp.rs index 4d95faf77..cff65f1a6 100644 --- a/crates/engine/src/lisp.rs +++ b/crates/engine/src/lisp.rs @@ -697,7 +697,7 @@ impl Session { } pub(crate) use drawing::from_json as drawing_from_json; -pub(crate) use drawing::load_source; +pub(crate) use drawing::{load_source, support_file}; impl Session { /// A file APPLOAD loaded, by its name (with or without `.lsp`, any case). diff --git a/crates/engine/src/lisp/builtins/functions.rs b/crates/engine/src/lisp/builtins/functions.rs index 2f94218f3..08758559c 100644 --- a/crates/engine/src/lisp/builtins/functions.rs +++ b/crates/engine/src/lisp/builtins/functions.rs @@ -235,7 +235,8 @@ pub(super) fn autoload(ctx: &mut Ctx<'_>, a: Vec) -> Result Option { candidates.iter().chain(&support).find_map(super::builtins::read_text_file) } +/// The file `name` names in the support folder, when it is there: what an autoload registration +/// loads, so a same-named file in the working folder cannot take its place. `None` for an absolute +/// path, no support folder or no such file there (the general search then applies, as before). +pub fn support_file(s: &Session, name: &str) -> Option { + if std::path::Path::new(name).is_absolute() { + return None; + } + let dir = s.lisp_support_dir()?; + let candidates = + if std::path::Path::new(name).extension().is_none() { vec![format!("{name}.lsp"), name.to_string()] } else { vec![name.to_string()] }; + candidates.iter().map(|c| dir.join(c)).find(|p| p.is_file()).map(|p| p.to_string_lossy().into_owned()) +} + #[cfg(test)] mod budget_tests { use super::*; + #[test] + fn autoload_prefers_the_support_folder_file() { + let dir = std::env::temp_dir().join(format!("cadcraft-lisp-support-file-{}", std::process::id())); + std::fs::create_dir_all(&dir).unwrap(); + std::fs::write(dir.join("lazy.lsp"), "").unwrap(); + let mut s = Session::new(); + assert_eq!(support_file(&s, "lazy"), None); + s.set_lisp_support_dir(Some(dir.clone())); + assert_eq!(support_file(&s, "lazy"), Some(dir.join("lazy.lsp").to_string_lossy().into_owned())); + assert_eq!(support_file(&s, "other"), None); + assert_eq!(support_file(&s, dir.join("lazy.lsp").to_str().unwrap()), None); + let _ = std::fs::remove_dir_all(&dir); + } + /// One 64 KB string occurring many times is charged by its length each time. #[test] fn shared_strings_spend_the_conversion_budgets() {