aleleba/cadcraft
1
0
Fork 0
mirror of https://github.com/storytold/cadcraft.git synced 2026-11-03 00:42:46 -06:00
Code Issues Packages Projects Releases Wiki Activity

fix(engine): merge another drawing's tables and blocks in linear time

Browse Source 
INSERT of a file, IMPORT, paste and COMPARE export each checked every incoming layer, linetype, style and block name by scanning the growing destination vector, and restarted the anonymous block name search from 1 at every collision, so a compact drawing with many distinct names made the command quadratic. The four copies of that code become one helper (cmd/merge.rs) that looks names up in a set built once per table and continues each generated prefix from the last number it allocated. Xref loading does the same for its dependent symbols, and no longer rescans the block table after a fetch that imported nothing.
This commit is contained in:
Dylan M. Taylor
2026-11-03 05:01:17 +00:00
parent 5ed192555c
commit b50db10bf7
7 changed files with 140 additions and 138 deletions
Download Patch File Download Diff File Expand all files Collapse all files
crates/engine/src/cmd.rs
+1
View File
@@ -22,6 +22,7 @@ mod inquiry;
mod layer;
mod layout;
mod lisp;
mod merge;
mod mleader_content;
mod mleader_edit;
mod modify;
crates/engine/src/cmd/blocks/insert_file.rs
+3 -38
View File
@@ -6,7 +6,7 @@
//! (names this drawing already has keep this drawing's definitions). The file is read through the
//! host's io hooks like OPEN, so the engine stays I/O-agnostic.
use std::collections::{BTreeMap, BTreeSet};
use std::collections::BTreeMap;
use std::sync::Arc;
use cadcraft_doc::{Block, Drawing, Entity, EntityKind, Handle};
@@ -102,7 +102,7 @@ fn define_from(s: &mut Session, src: &Drawing, name: &str) -> Result<String> {
// Redefining keeps the existing definition's name (block names are case-insensitive).
let name = d.blocks.keys().find(|k| k.eq_ignore_ascii_case(name)).cloned().unwrap_or_else(|| name.to_string());
merge_tables(&mut d, src);
let renames = block_names(&d, src, &name);
let renames = crate::cmd::merge::block_renames(&d, src, Some(&name));
for (from, to) in &renames {
let Some(b) = src.blocks.get(from) else { continue };
let mut nb = (**b).clone();
@@ -136,13 +136,7 @@ fn define_from(s: &mut Session, src: &Drawing, name: &str) -> Result<String> {
/// Add the source's layers, linetypes and styles that this drawing doesn't have (a name that
/// exists keeps this drawing's definition).
fn merge_tables(d: &mut Drawing, src: &Drawing) {
fn merge<T: Clone>(ours: &mut Vec<T>, theirs: &[T], name: fn(&T) -> &str) {
for t in theirs {
if !ours.iter().any(|o| name(o).eq_ignore_ascii_case(name(t))) {
ours.push(t.clone());
}
}
}
use crate::cmd::merge::merge_named as merge;
merge(&mut d.layers, &src.layers, |l| &l.name);
merge(&mut d.linetypes, &src.linetypes, |l| &l.name);
merge(&mut d.text_styles, &src.text_styles, |t| &t.name);
@@ -152,35 +146,6 @@ fn merge_tables(d: &mut Drawing, src: &Drawing) {
merge(&mut d.mline_styles, &src.mline_styles, |t| &t.name);
}
/// Which source blocks to copy, and under which name: new names keep theirs, generated
/// (anonymous `*…`) blocks that collide get a fresh generated name, and a named block this drawing
/// already has (or the block being defined) keeps this drawing's definition (not copied).
fn block_names(d: &Drawing, src: &Drawing, defining: &str) -> BTreeMap<String, String> {
let mut out = BTreeMap::new();
let mut taken: BTreeSet<String> = d.blocks.keys().map(|k| k.to_ascii_uppercase()).collect();
taken.insert(defining.to_ascii_uppercase());
for (name, b) in &src.blocks {
let upper = name.to_ascii_uppercase();
if upper.starts_with("*MODEL_SPACE") || upper.starts_with("*PAPER_SPACE") {
continue;
}
let to = if !taken.contains(&upper) {
name.clone()
} else if b.anonymous || name.starts_with('*') {
let prefix: String = name.chars().take_while(|c| !c.is_ascii_digit()).collect();
match (1..=1_000_000).map(|i| format!("{prefix}{i}")).find(|n| !taken.contains(&n.to_ascii_uppercase())) {
Some(n) => n,
None => continue,
}
} else {
continue;
};
taken.insert(to.to_ascii_uppercase());
out.insert(name.clone(), to);
}
out
}
/// A copy of a source entity for this drawing: a fresh handle, block references renamed, and
/// links to other source objects (associative dimension points) dropped.
fn adopt(d: &mut Drawing, src: &Drawing, e: &Entity, renames: &BTreeMap<String, String>) -> Entity {
crates/engine/src/cmd/blocks/xref/load.rs
+24 -11
View File
@@ -10,6 +10,7 @@
//! loaded further up (a circular reference) is not loaded again. Every failure leaves the xref's
//! block empty with a status saying why; nothing here stops the host drawing from opening.
use std::collections::HashSet;
use std::path::{Path, PathBuf};
use std::sync::Arc;
@@ -339,14 +340,15 @@ fn load_in(d: &mut Drawing, name: &str, host: Option<&Path>, chosen: bool, cx: &
let (status, detail) = match fetch(&path, host, chosen, cx, depth) {
Ok((x, file)) => match import(d, &key, &x, cx) {
Ok(()) => (XrefStatus::Loaded, file.to_string_lossy().into_owned()),
Err(e) => (XrefStatus::Unresolved, e),
Err(e) => {
// Whatever made it in before the failure goes again. A fetch that failed imported
// nothing, so it doesn't pay for a second scan of the block table.
clear(d, &key);
(XrefStatus::Unresolved, e)
}
},
Err(e) => e,
};
if status != XrefStatus::Loaded {
// Whatever made it in before the failure goes again.
clear(d, &key);
}
set_status(d, &key, status);
(status, detail)
}
@@ -390,9 +392,15 @@ fn import(d: &mut Drawing, key: &str, x: &Drawing, cx: &mut Ctx) -> Result<(), S
return Err(format!("more than {MAX_XREF_OBJECTS} objects"));
}
let dep = |sym: Sym, n: &str| (!shared(sym, n)).then(|| format!("{key}|{n}"));
// The host's names per table, looked up in a set built once: both drawings are hostile input,
// so a scan per imported record would be quadratic.
fn names<T>(all: &[T], name: fn(&T) -> &str) -> HashSet<String> {
all.iter().map(|t| name(t).to_ascii_uppercase()).collect()
}
let mut have = names(&d.layers, |l| &l.name);
for l in &x.layers {
if let Some(n) = dep(Sym::Layer, &l.name)
&& d.layer(&n).is_none()
&& have.insert(n.to_ascii_uppercase())
{
let mut l = l.clone();
l.name = n;
@@ -400,39 +408,44 @@ fn import(d: &mut Drawing, key: &str, x: &Drawing, cx: &mut Ctx) -> Result<(), S
d.layers.push(l);
}
}
let mut have = names(&d.linetypes, |t| &t.name);
for t in &x.linetypes {
if let Some(n) = dep(Sym::Linetype, &t.name)
&& d.linetype(&n).is_none()
&& have.insert(n.to_ascii_uppercase())
{
d.linetypes.push(cadcraft_doc::Linetype { name: n, ..t.clone() });
}
}
let mut have = names(&d.text_styles, |t| &t.name);
for t in &x.text_styles {
if let Some(n) = dep(Sym::Style, &t.name)
&& d.text_style(&n).is_none()
&& have.insert(n.to_ascii_uppercase())
{
d.text_styles.push(cadcraft_doc::TextStyle { name: n, ..t.clone() });
}
}
let mut have = names(&d.dim_styles, |t| &t.name);
for t in &x.dim_styles {
if let Some(n) = dep(Sym::DimStyle, &t.name)
&& d.dim_style(&n).is_none()
&& have.insert(n.to_ascii_uppercase())
{
let text_style = dep(Sym::Style, &t.text_style).unwrap_or_else(|| t.text_style.clone());
d.dim_styles.push(cadcraft_doc::DimStyle { name: n, text_style, ..t.clone() });
}
}
let mut have = names(&d.mleader_styles, |t| &t.name);
for t in &x.mleader_styles {
if let Some(n) = dep(Sym::MLeaderStyle, &t.name)
&& !d.mleader_styles.iter().any(|o| o.name.eq_ignore_ascii_case(&n))
&& have.insert(n.to_ascii_uppercase())
{
let text_style = dep(Sym::Style, &t.text_style).unwrap_or_else(|| t.text_style.clone());
d.mleader_styles.push(cadcraft_doc::MLeaderStyle { name: n, text_style, ..t.clone() });
}
}
let mut have = names(&d.table_styles, |t| &t.name);
for t in &x.table_styles {
if let Some(n) = dep(Sym::TableStyle, &t.name)
&& !d.table_styles.iter().any(|o| o.name.eq_ignore_ascii_case(&n))
&& have.insert(n.to_ascii_uppercase())
{
d.table_styles.push(cadcraft_doc::TableStyle { name: n, ..t.clone() });
}
crates/engine/src/cmd/clipboard.rs
+3 -37
View File
@@ -10,7 +10,7 @@
//! The clipboard can also travel as DXF text ([`system_text`], [`load_system_text`]), so a front
//! end can put it on the system clipboard and paste it into another CADCraft window.
use std::collections::{BTreeMap, BTreeSet, HashSet};
use std::collections::{BTreeMap, HashSet};
use std::sync::Arc;
use cadcraft_doc::{Block, Drawing, Entity, EntityKind, HVal, Handle};
@@ -279,20 +279,14 @@ fn defs_for(src: &Drawing, ents: &[Entity]) -> Drawing {
/// Add `defs`' table records and blocks that `d` doesn't have. Returns the blocks copied, from
/// their name in `defs` to their name in `d`.
fn merge(d: &mut Drawing, defs: &Drawing) -> BTreeMap<String, String> {
fn add<T: Clone>(ours: &mut Vec<T>, theirs: &[T], name: fn(&T) -> &str) {
for t in theirs {
if !ours.iter().any(|o| name(o).eq_ignore_ascii_case(name(t))) {
ours.push(t.clone());
}
}
}
use crate::cmd::merge::merge_named as add;
add(&mut d.layers, &defs.layers, |l| &l.name);
add(&mut d.linetypes, &defs.linetypes, |l| &l.name);
add(&mut d.text_styles, &defs.text_styles, |t| &t.name);
add(&mut d.dim_styles, &defs.dim_styles, |t| &t.name);
add(&mut d.mleader_styles, &defs.mleader_styles, |t| &t.name);
add(&mut d.table_styles, &defs.table_styles, |t| &t.name);
let renames = block_names(d, defs);
let renames = crate::cmd::merge::block_renames(d, defs, None);
for (from, to) in &renames {
let Some(b) = defs.blocks.get(from) else { continue };
let mut nb = (**b).clone();
@@ -308,34 +302,6 @@ fn merge(d: &mut Drawing, defs: &Drawing) -> BTreeMap<String, String> {
renames
}
/// Which blocks of `defs` to copy, and under which name: a new name keeps its own, a generated
/// (anonymous `*…`) block whose name is taken gets a fresh generated name, and a named block `d`
/// already has keeps `d`'s definition (not copied).
fn block_names(d: &Drawing, defs: &Drawing) -> BTreeMap<String, String> {
let mut out = BTreeMap::new();
let mut taken: BTreeSet<String> = d.blocks.keys().map(|k| up(k)).collect();
for (name, b) in &defs.blocks {
let upper = up(name);
if upper.starts_with("*MODEL_SPACE") || upper.starts_with("*PAPER_SPACE") {
continue;
}
let to = if !taken.contains(&upper) {
name.clone()
} else if b.anonymous || name.starts_with('*') {
let prefix: String = name.chars().take_while(|c| !c.is_ascii_digit()).collect();
match (1..=1_000_000).map(|i| format!("{prefix}{i}")).find(|n| !taken.contains(&up(n))) {
Some(n) => n,
None => continue,
}
} else {
continue;
};
taken.insert(up(&to));
out.insert(name.clone(), to);
}
out
}
/// A copy of a clipboard object for `d`: a fresh handle, block references renamed, and links to
/// objects of the source drawing (associative dimension points) dropped.
fn adopt(d: &mut Drawing, e: &Entity, renames: &BTreeMap<String, String>) -> Entity {
crates/engine/src/cmd/exchange.rs
+3 -41
View File
@@ -7,7 +7,7 @@
//! stays I/O-agnostic; the JSON forms never open dialogs (the UI asks for files when these
//! commands come from a menu, the tool bar or the command line).
use std::collections::{BTreeMap, BTreeSet};
use std::collections::BTreeMap;
use std::sync::Arc;
use cadcraft_color::Rgb;
@@ -166,7 +166,7 @@ fn import_drawing(s: &mut Session, src: &Drawing, at: Vec2, scale: f64, file: &s
let space = s.space();
let d = s.doc_mut()?;
let (layers, styles) = merge_tables(d, src);
let renames = block_names(d, src);
let renames = crate::cmd::merge::block_renames(d, src, None);
for (from, to) in &renames {
let Some(b) = src.blocks.get(from) else { continue };
let mut nb = (**b).clone();
@@ -213,16 +213,7 @@ fn read_file(_path: &str) -> Result<Vec<u8>> {
/// Add the source's layers, linetypes and styles that this drawing doesn't have (a name that
/// exists keeps this drawing's definition). Returns how many layers and how many other records were added.
fn merge_tables(d: &mut Drawing, src: &Drawing) -> (usize, usize) {
fn merge<T: Clone>(ours: &mut Vec<T>, theirs: &[T], name: fn(&T) -> &str) -> usize {
let mut n = 0;
for t in theirs {
if !ours.iter().any(|o| name(o).eq_ignore_ascii_case(name(t))) {
ours.push(t.clone());
n += 1;
}
}
n
}
use crate::cmd::merge::merge_named as merge;
let layers = merge(&mut d.layers, &src.layers, |l| &l.name);
let others = merge(&mut d.linetypes, &src.linetypes, |l| &l.name)
+ merge(&mut d.text_styles, &src.text_styles, |t| &t.name)
@@ -232,35 +223,6 @@ fn merge_tables(d: &mut Drawing, src: &Drawing) -> (usize, usize) {
(layers, others)
}
/// Which source blocks to copy, and under which name: new names keep theirs, generated
/// (anonymous `*…`) blocks that collide get a fresh generated name, and a named block this drawing
/// already has keeps this drawing's definition (not copied).
fn block_names(d: &Drawing, src: &Drawing) -> BTreeMap<String, String> {
let mut out = BTreeMap::new();
let mut taken: BTreeSet<String> = d.blocks.keys().map(|k| k.to_ascii_uppercase()).collect();
for (name, b) in &src.blocks {
let upper = name.to_ascii_uppercase();
if upper.starts_with("*MODEL_SPACE") || upper.starts_with("*PAPER_SPACE") {
continue;
}
let anonymous = b.anonymous || name.starts_with('*');
let to = if !taken.contains(&upper) {
name.clone()
} else if anonymous {
let prefix: String = name.chars().take_while(|c| !c.is_ascii_digit()).collect();
match (1..=1_000_000).map(|i| format!("{prefix}{i}")).find(|n| !taken.contains(&n.to_ascii_uppercase())) {
Some(n) => n,
None => continue,
}
} else {
continue;
};
taken.insert(to.to_ascii_uppercase());
out.insert(name.clone(), to);
}
out
}
/// A copy of a source entity for this drawing: a fresh handle, block references renamed, and
/// links to other source objects (associative dimension points) dropped.
fn adopt(d: &mut Drawing, e: &Entity, renames: &BTreeMap<String, String>) -> Entity {
crates/engine/src/cmd/file/compare/export.rs
+7 -11
View File
@@ -20,7 +20,7 @@ use std::path::{Path, PathBuf};
use std::sync::Arc;
use cadcraft_doc::color::{Color, Rgb};
use cadcraft_doc::{Block, CellStyle, DimStyle, Drawing, Entity, EntityKind, EntityStore, Group, Handle, MAX_BLOCK_DEPTH, TableStyle};
use cadcraft_doc::{Block, CellStyle, DimStyle, Drawing, Entity, EntityKind, EntityStore, Group, Handle, Layer, MAX_BLOCK_DEPTH, TableStyle};
use serde_json::{Value, json};
use super::super::super::{CommandSpec, bad};
@@ -197,6 +197,7 @@ fn snapshot(cur: &Drawing, c: &Comparison, folders: &Folders) -> Snapshot {
d.skipped_records.clear();
d.handseed = d.handseed.max(other.handseed);
use crate::cmd::merge::merge_named as merge;
merge(&mut d.layers, &other.layers, |t| &t.name);
merge(&mut d.linetypes, &other.linetypes, |t| &t.name);
merge(&mut d.text_styles, &other.text_styles, |t| &t.name);
@@ -235,22 +236,17 @@ fn snapshot(cur: &Drawing, c: &Comparison, folders: &Folders) -> Snapshot {
Snapshot { drawing: d, current, compared, counts: (here.len(), there.len()), truncated: diff.truncated, blocks_left_out, uncolored }
}
/// Add the entries of `from` whose names `to` lacks (compared without letter case).
fn merge<T: Clone>(to: &mut Vec<T>, from: &[T], name: impl Fn(&T) -> &String) {
for t in from {
if !to.iter().any(|x| name(x).eq_ignore_ascii_case(name(t))) {
to.push(t.clone());
}
}
}
/// Add copies of `objs` to model space with new handles, their block references renamed by
/// `renames` and their files rebased on `folder`, each painted in its comparison color, and
/// group them as `name` (or `name_2`… when that group name is taken). Returns the group's name.
fn add_group(d: &mut Drawing, name: &str, objs: &[(&Entity, Color)], renames: &Renames, folder: Option<&Path>, tinted: &mut Tinted) -> String {
let mut members = Vec::new();
// The layers this drawing has, looked up once per object rather than scanned.
let mut layers: HashSet<String> = d.layers.iter().map(|l| l.name.to_ascii_uppercase()).collect();
for (e, color) in objs {
d.ensure_layer(&e.common.layer);
if layers.insert(e.common.layer.to_ascii_uppercase()) {
d.layers.push(Layer::new(&e.common.layer));
}
let h = d.new_handle();
let mut copy = Entity { handle: h, ..(*e).clone() };
detach(&mut copy.kind);
crates/engine/src/cmd/merge.rs
+99
View File
@@ -0,0 +1,99 @@
//! Bringing another drawing's symbol tables and blocks into this one (INSERT of a file, IMPORT,
//! paste, COMPARE export).
//!
//! Both drawings are hostile input, so the work is linear in their sizes: names are looked up in a
//! set built once per table, and a generated block name continues from the last one allocated for
//! its prefix instead of probing every taken number again (CWE-407).
use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use cadcraft_doc::Drawing;
/// Add the entries of `theirs` whose names `ours` lacks (compared without letter case; a name that
/// exists keeps `ours`' definition). Returns how many were added.
pub(crate) fn merge_named<T: Clone>(ours: &mut Vec<T>, theirs: &[T], name: fn(&T) -> &str) -> usize {
let mut have: HashSet<String> = ours.iter().map(|o| name(o).to_ascii_uppercase()).collect();
let mut added = 0;
for t in theirs {
if have.insert(name(t).to_ascii_uppercase()) {
ours.push(t.clone());
added += 1;
}
}
added
}
/// Which blocks of `src` to copy into `d`, and under which name: a new name keeps its own, a
/// generated (anonymous `*…`) block whose name is taken gets a fresh generated name, and a named
/// block `d` already has (or `defining`, the block being defined from `src`) keeps `d`'s
/// definition (not copied).
pub(crate) fn block_renames(d: &Drawing, src: &Drawing, defining: Option<&str>) -> BTreeMap<String, String> {
let mut out = BTreeMap::new();
let mut taken: BTreeSet<String> = d.blocks.keys().map(|k| k.to_ascii_uppercase()).collect();
taken.extend(defining.map(str::to_ascii_uppercase));
// The next number to try per generated prefix: numbers below it are taken.
let mut next: HashMap<String, usize> = HashMap::new();
for (name, b) in &src.blocks {
let upper = name.to_ascii_uppercase();
if upper.starts_with("*MODEL_SPACE") || upper.starts_with("*PAPER_SPACE") {
continue;
}
let to = if !taken.contains(&upper) {
name.clone()
} else if b.anonymous || name.starts_with('*') {
let prefix: String = name.chars().take_while(|c| !c.is_ascii_digit()).collect();
let cursor = next.entry(prefix.to_ascii_uppercase()).or_insert(1);
match (*cursor..=1_000_000).map(|i| (i, format!("{prefix}{i}"))).find(|(_, n)| !taken.contains(&n.to_ascii_uppercase())) {
Some((i, n)) => {
*cursor = i + 1;
n
}
None => continue,
}
} else {
continue;
};
taken.insert(to.to_ascii_uppercase());
out.insert(name.clone(), to);
}
out
}
#[cfg(test)]
mod tests {
use std::sync::Arc;
use cadcraft_doc::{Block, Layer};
use super::*;
#[test]
fn merge_keeps_ours_and_adds_the_rest_without_regard_to_case() {
let mut ours = vec![Layer::new("Walls")];
let theirs = vec![Layer::new("WALLS"), Layer::new("Doors"), Layer::new("doors")];
assert_eq!(merge_named(&mut ours, &theirs, |l| &l.name), 1);
assert_eq!(ours.iter().map(|l| l.name.as_str()).collect::<Vec<_>>(), ["Walls", "Doors"]);
}
/// Many colliding anonymous blocks get consecutive names in one pass, not a rescan each.
#[test]
fn anonymous_collisions_take_consecutive_numbers() {
let mut d = Drawing::new_imperial();
let mut src = Drawing::new_imperial();
for i in 1..=5000 {
let mut b = Block::new(&format!("*U{i}"));
b.anonymous = true;
d.blocks.insert(b.name.clone(), Arc::new(b.clone()));
src.blocks.insert(b.name.clone(), Arc::new(b));
}
let started = std::time::Instant::now();
let renames = block_renames(&d, &src, None);
assert!(started.elapsed() < std::time::Duration::from_secs(2));
assert_eq!(renames.len(), 5000);
assert_eq!(renames.get("*U1").map(String::as_str), Some("*U5001"));
let distinct: HashSet<&String> = renames.values().collect();
assert_eq!(distinct.len(), 5000);
// Every new name is the next free number: nothing below 5001 is free, nothing above 10000 is needed.
assert!(renames.values().all(|n| n.strip_prefix("*U").and_then(|i| i.parse::<u32>().ok()).is_some_and(|i| (5001..=10000).contains(&i))));
}
}