diff --git a/crates/engine/src/audit.rs b/crates/engine/src/audit.rs index c055bff25..5940a96c1 100644 --- a/crates/engine/src/audit.rs +++ b/crates/engine/src/audit.rs @@ -234,9 +234,10 @@ fn tables(d: &mut Drawing, out: &mut Vec) { missing("table style", "Standard", out); d.table_styles.insert(0, TableStyle::default()); } - let linetypes: Vec = d.linetypes.iter().map(|l| l.name.clone()).collect(); + // Name sets built once: both tables come from the drawing, so a scan per reference is quadratic. + let linetypes: HashSet = d.linetypes.iter().map(|l| l.name.to_ascii_lowercase()).collect(); for l in &mut d.layers { - if !linetypes.iter().any(|n| n.eq_ignore_ascii_case(&l.linetype)) { + if !linetypes.contains(&l.linetype.to_ascii_lowercase()) { out.push(issue( "missingLinetype", "tables", @@ -247,8 +248,8 @@ fn tables(d: &mut Drawing, out: &mut Vec) { l.linetype = "Continuous".into(); } } - let styles: Vec = d.text_styles.iter().map(|s| s.name.clone()).collect(); - let known = |n: &str| styles.iter().any(|s| s.eq_ignore_ascii_case(n)); + let styles: HashSet = d.text_styles.iter().map(|s| s.name.to_ascii_lowercase()).collect(); + let known = |n: &str| styles.contains(&n.to_ascii_lowercase()); for s in &mut d.dim_styles { if !known(&s.text_style) { out.push(issue( diff --git a/crates/engine/src/cmd/layer.rs b/crates/engine/src/cmd/layer.rs index c5fa89403..41c58febe 100644 --- a/crates/engine/src/cmd/layer.rs +++ b/crates/engine/src/cmd/layer.rs @@ -408,12 +408,24 @@ fn run_state_restore(s: &mut Session, p: &Value) -> Result { let name = str_param(p, "name").ok_or_else(|| bad("layerstate.restore", "`name` is required"))?; let d = s.doc_mut()?; let st = d.layer_states.iter().find(|st| st.name == name).cloned().ok_or_else(|| bad("layerstate.restore", "no such layer state"))?; - for saved in st.layers { - if let Some(l) = d.layer_mut(&saved.name) { - *l = saved; + restore_layers(d, st.layers); + ok() +} + +/// Put the saved layers back over the drawing's layers of the same name (without regard to case); +/// saved layers the drawing no longer has are skipped. Both lists can come from a file, so the +/// drawing's layers are looked up in an index rather than scanned per saved layer. +pub(crate) fn restore_layers(d: &mut cadcraft_doc::Drawing, saved: Vec) { + let mut index: std::collections::HashMap = std::collections::HashMap::with_capacity(d.layers.len()); + for (i, l) in d.layers.iter().enumerate() { + // The first of names that differ only in case, as `Drawing::layer_mut` finds it. + index.entry(l.name.to_ascii_lowercase()).or_insert(i); + } + for layer in saved { + if let Some(l) = index.get(&layer.name.to_ascii_lowercase()).and_then(|&i| d.layers.get_mut(i)) { + *l = layer; } } - ok() } fn run_state_list(s: &mut Session, _p: &Value) -> Result { @@ -461,6 +473,22 @@ mod tests { use crate::Session; + /// Restoring many saved layers, most of them missing, takes one pass and matches without case. + #[test] + fn restore_layers_matches_by_name_in_linear_time() { + let mut d = cadcraft_doc::Drawing::new_imperial(); + d.layers = (0..20_000).map(|i| cadcraft_doc::Layer::new(&format!("L{i}"))).collect(); + let mut saved: Vec = (0..20_000).map(|i| cadcraft_doc::Layer::new(&format!("missing{i}"))).collect(); + let mut back = cadcraft_doc::Layer::new("l7"); + back.frozen = true; + saved.push(back); + let started = std::time::Instant::now(); + super::restore_layers(&mut d, saved); + assert!(started.elapsed() < std::time::Duration::from_secs(2)); + assert!(d.layer("L7").is_some_and(|l| l.frozen)); + assert_eq!(d.layers.len(), 20_000); + } + #[test] fn layer_states_save_list_restore_delete() { let mut s = Session::new(); diff --git a/crates/engine/src/cmd/utility/calc.rs b/crates/engine/src/cmd/utility/calc.rs index 0c0d29b32..bcba8052a 100644 --- a/crates/engine/src/cmd/utility/calc.rs +++ b/crates/engine/src/cmd/utility/calc.rs @@ -1,6 +1,8 @@ //! The CAL / QuickCalc evaluator: numbers and points (`[x,y]`, `[d { s: &'a [u8], i: usize, vars: &'a Map, + /// `vars` by ASCII-lower-cased name (the first of names that differ only in case), so a + /// case-insensitive lookup is not a scan of the map per reference. + folded: HashMap, depth: usize, unary_depth: usize, } @@ -230,7 +235,7 @@ impl Parser<'_> { match name.as_str() { "pi" => Ok(Val::Num(std::f64::consts::PI)), "e" => Ok(Val::Num(std::f64::consts::E)), - _ => var(self.vars, &name).ok_or_else(|| format!("unknown name `{name}`")), + _ => var(self.vars, &self.folded, &name).ok_or_else(|| format!("unknown name `{name}`")), } } _ => Err("expected a number".into()), @@ -239,8 +244,8 @@ impl Parser<'_> { } /// A variable: a number, or a point as `[x, y]`. Names are case-insensitive. -fn var(vars: &Map, name: &str) -> Option { - let v = vars.get(name).or_else(|| vars.iter().find(|(k, _)| k.eq_ignore_ascii_case(name)).map(|(_, v)| v))?; +fn var(vars: &Map, folded: &HashMap, name: &str) -> Option { + let v = vars.get(name).or_else(|| folded.get(&name.to_ascii_lowercase()).copied())?; if let Some(n) = v.as_f64() { return Some(Val::Num(n)); } @@ -411,7 +416,11 @@ pub fn eval_val(expr: &str, vars: &Map) -> Result { if expr.len() > MAX_LEN { return Err("expression too long".into()); } - let mut p = Parser { s: expr.as_bytes(), i: 0, vars, depth: 0, unary_depth: 0 }; + let mut folded = HashMap::with_capacity(vars.len()); + for (k, v) in vars { + folded.entry(k.to_ascii_lowercase()).or_insert(v); + } + let mut p = Parser { s: expr.as_bytes(), i: 0, vars, folded, depth: 0, unary_depth: 0 }; let v = p.expr()?; if p.peek().is_some() { return Err(format!("unexpected `{}`", expr.get(p.i..).unwrap_or(""))); @@ -587,6 +596,21 @@ mod tests { eval_val(s, &Map::new()) } + /// Variables are found without regard to case; an exact match wins over a case variant. + /// (Names are read lower-cased, so `X` finds `x` too.) + #[test] + fn variables_ignore_case_and_prefer_exact_names() { + let mut vars = Map::new(); + for i in 0..10_000 { + vars.insert(format!("v{i}"), json!(i)); + } + vars.insert("X".into(), json!(1)); + vars.insert("x".into(), json!(2)); + assert!(matches!(eval_val("V9999 + 1", &vars), Ok(Val::Num(n)) if n == 10_000.0)); + assert!(matches!(eval_val("x", &vars), Ok(Val::Num(n)) if n == 2.0)); + assert!(eval_val("nope", &vars).is_err()); + } + #[test] fn points_and_vectors() { assert_eq!(ev("[1,2]+[3,4]*2"), Ok(Val::Pt(Vec2::new(7.0, 10.0)))); diff --git a/crates/engine/src/cmd/view/named_views.rs b/crates/engine/src/cmd/view/named_views.rs index f88d240db..c89b8cadb 100644 --- a/crates/engine/src/cmd/view/named_views.rs +++ b/crates/engine/src/cmd/view/named_views.rs @@ -185,13 +185,7 @@ fn restore(s: &mut Session, name: &str) -> Result { if let Some(state) = &v.layer_state { let d = s.doc_mut()?; match d.layer_states.iter().find(|l| l.name.eq_ignore_ascii_case(state)).cloned() { - Some(saved) => { - for layer in saved.layers { - if let Some(l) = d.layer_mut(&layer.name) { - *l = layer; - } - } - } + Some(saved) => super::super::layer::restore_layers(d, saved.layers), None => msg.push_str(&format!(" Its layer snapshot \"{state}\" is missing.")), } }