From c4171ebb7ac4e2025ee86debaf23a2f50ad3e5ce Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Tue, 3 Nov 2026 11:06:21 +0000 Subject: [PATCH] fix(io): DXF import looks up linetype, style and entity layer names in a set LTYPE records scanned every linetype read so far, STYLE records every text style, and each entity's layer went through Drawing::ensure_layer's scan of the layer table, so a DXF with many distinct names read in quadratic time. These now use indexes built once per pass, as LAYER records already did; first-LTYPE-wins and last-STYLE-wins are unchanged. --- crates/io/src/dxf_read/document.rs | 23 +++++++++++++++++++++-- crates/io/src/dxf_read/tables.rs | 13 ++++++++++--- crates/io/src/tests.rs | 22 ++++++++++++++++++++++ 3 files changed, 53 insertions(+), 5 deletions(-) diff --git a/crates/io/src/dxf_read/document.rs b/crates/io/src/dxf_read/document.rs index 6f61e3415..ef35212a8 100644 --- a/crates/io/src/dxf_read/document.rs +++ b/crates/io/src/dxf_read/document.rs @@ -178,14 +178,32 @@ pub(super) fn setup_layouts(d: &mut Drawing, layout_objs: &mut [LayoutDef], rx: (paper_block_for, first_layout) } +/// The drawing's layer names (lower-cased), so adding the layer an entity names is a set lookup +/// rather than a scan of the layer table per entity: many distinct names made that quadratic +/// (CWE-407). +struct LayerSet(std::collections::HashSet); + +impl LayerSet { + fn new(d: &Drawing) -> Self { + Self(d.layers.iter().map(|l| l.name.to_ascii_lowercase()).collect()) + } + /// [`Drawing::ensure_layer`] through the set. + fn ensure(&mut self, d: &mut Drawing, name: &str) { + if self.0.insert(name.to_ascii_lowercase()) { + d.layers.push(cadcraft_doc::Layer::new(name)); + } + } +} + /// Place the BLOCKS entities: model and paper space blocks go to their space, the rest become /// block definitions. pub(super) fn place_blocks(d: &mut Drawing, blocks: Vec, rx: &mut Rx, paper_block_for: &HashMap, first_layout: &str) { + let mut layers = LayerSet::new(d); for (name, base, description, ents, xref) in blocks { let up = name.to_ascii_uppercase(); if up == "*MODEL_SPACE" { for (_, _, e) in ents { - d.ensure_layer(&e.common.layer); + layers.ensure(d, &e.common.layer); d.model.push(e); } continue; @@ -241,8 +259,9 @@ pub(super) fn place_entities( paper_block_for: &HashMap, first_layout: &str, ) { + let mut layers = LayerSet::new(d); for (owner, paper, e) in entities { - d.ensure_layer(&e.common.layer); + layers.ensure(d, &e.common.layer); let owner_name = owner.and_then(|o| rx.brs.get(&o.trim().to_ascii_uppercase()).cloned()).map(|n| n.to_ascii_uppercase()); let to_paper = paper || owner_name.as_deref().is_some_and(|n| n.starts_with("*PAPER_SPACE")); if to_paper { diff --git a/crates/io/src/dxf_read/tables.rs b/crates/io/src/dxf_read/tables.rs index c914ea9f5..f05a7307e 100644 --- a/crates/io/src/dxf_read/tables.rs +++ b/crates/io/src/dxf_read/tables.rs @@ -110,6 +110,9 @@ pub(super) fn tables(tags: &[Tag], d: &mut Drawing, rx: &mut Rx) { // appends in O(1) instead of scanning the growing layer list — otherwise many distinct layer // names make import quadratic (CWE-407). let mut layer_index: HashMap = d.layers.iter().enumerate().map(|(i, l)| (l.name.to_ascii_lowercase(), i)).collect(); + // The same for linetypes (first definition wins) and text styles (last definition wins). + let mut linetype_names: std::collections::HashSet = d.linetypes.iter().map(|l| l.name.to_ascii_lowercase()).collect(); + let mut style_index: HashMap = d.text_styles.iter().enumerate().map(|(i, s)| (s.name.to_ascii_lowercase(), i)).collect(); for (kind, tg) in recs { let t = T(&tg); match kind.as_str() { @@ -159,9 +162,10 @@ pub(super) fn tables(tags: &[Tag], d: &mut Drawing, rx: &mut Rx) { } "LTYPE" => { let name = t.s(2).unwrap_or_default(); - if name.is_empty() || d.linetype(&name).is_some() || d.linetypes.len() >= MAX_OBJECTS { + if name.is_empty() || linetype_names.contains(&name.to_ascii_lowercase()) || d.linetypes.len() >= MAX_OBJECTS { continue; } + linetype_names.insert(name.to_ascii_lowercase()); let pattern = ltype_pattern(&tg, <ype_styles); d.linetypes.push(Linetype { name, description: t.s(3).unwrap_or_default(), pattern }); } @@ -184,9 +188,12 @@ pub(super) fn tables(tags: &[Tag], d: &mut Drawing, rx: &mut Rx) { vertical: t.i(70).unwrap_or(0) & 4 != 0, annotative: annotative(&tg), }; - match d.text_styles.iter_mut().find(|s| s.name.eq_ignore_ascii_case(&name)) { + match style_index.get(&name.to_ascii_lowercase()).and_then(|&i| d.text_styles.get_mut(i)) { Some(x) => *x = st, - None => d.text_styles.push(st), + None => { + style_index.insert(name.to_ascii_lowercase(), d.text_styles.len()); + d.text_styles.push(st); + } } } // The first *ACTIVE viewport holds the current snap/grid settings. diff --git a/crates/io/src/tests.rs b/crates/io/src/tests.rs index 46536f24e..f9353481e 100644 --- a/crates/io/src/tests.rs +++ b/crates/io/src/tests.rs @@ -337,6 +337,28 @@ fn entity_transparency_roundtrips_as_group_440() { assert_eq!(d.model.iter().map(|e| e.common.transparency).collect::>(), [Transparency::ByLayer, Transparency::Percent(90)]); } +/// Many distinct layer, linetype and style names (in the tables and on entities) read in linear +/// time, with the duplicate rules kept: first LTYPE wins, last STYLE wins, layers case-insensitive. +#[test] +fn many_distinct_symbol_names_read_quickly() { + const N: usize = 40_000; + let mut t = String::from("0\nSECTION\n2\nTABLES\n"); + for i in 0..N { + t.push_str(&format!("0\nLTYPE\n2\nLT{i}\n3\nfirst\n0\nSTYLE\n2\nST{i}\n40\n1\n")); + } + t.push_str("0\nLTYPE\n2\nlt0\n3\nsecond\n0\nSTYLE\n2\nst0\n40\n2\n0\nENDSEC\n0\nSECTION\n2\nENTITIES\n"); + for i in 0..N { + t.push_str(&format!("0\nLINE\n8\nL{i}\n10\n0\n20\n0\n11\n1\n21\n1\n")); + } + t.push_str("0\nLINE\n8\nl0\n10\n0\n20\n0\n11\n1\n21\n1\n0\nENDSEC\n0\nEOF\n"); + let started = std::time::Instant::now(); + let d = read(t.as_bytes(), "many.dxf").unwrap(); + assert!(started.elapsed() < std::time::Duration::from_secs(20), "took {:?}", started.elapsed()); + assert!(d.layers.len() > N && d.layers.len() < N + 3, "{}", d.layers.len()); + assert_eq!(d.linetype("LT0").map(|l| l.description.as_str()), Some("first")); + assert_eq!(d.text_style("ST0").map(|s| s.height), Some(2.0)); +} + #[test] fn hostile_dxf_does_not_panic() { for t in [