refactor: split complex fillet, model, dimstyle and rectangle code into helpers

Pure extraction, no behaviour change: same order of side effects, same
early returns and error values. Clears cognitive-complexity findings on
tangent_circles, Model::build_budgeted, DimStyle::apply_fields and
run_rectang2 (S3776), and replaces a redundant closure with a method
reference in menus.rs (S1612).

crates/constraints/src/model.rs

@@ -191,16 +191,10 @@ impl Model {
         Self::build_budgeted(d, handles, &mut members_left)
     }
 
-    /// Build solver parameters for supported entities while sharing rigid-member sampling work with other models.
+    /// Add the parameters of one supported entity to `m`, and return its shape; `None` when it is
-    pub(crate) fn build_budgeted(d: &Drawing, handles: &[Handle], members_left: &mut usize) -> Model {
+    /// left out (too large, degenerate or unsupported).
-        let mut m = Model::default();
+    fn add_params(m: &mut Model, d: &Drawing, h: Handle, kind: &EntityKind, off: usize, members_left: &mut usize) -> Option<Shape> {
-        for &h in handles {
+        let shape = match kind {
-            if m.index.contains_key(&h) {
-                continue;
-            }
-            let Some(e) = d.entity(h) else { continue };
-            let off = m.x0.len();
-            let shape = match &e.kind {
             EntityKind::Line(l) => {
                 m.x0.extend([l.a.x, l.a.y, l.b.x, l.b.y]);
                 Shape::Line
@@ -215,7 +209,7 @@ impl Model {
             }
             EntityKind::LwPolyline(p) => {
                 if p.vertices.len() < 2 || off + p.vertices.len() * 2 > MAX_PARAMS {
-                        continue;
+                    return None;
                 }
                 for v in &p.vertices {
                     m.x0.extend([v.p.x, v.p.y]);
@@ -240,7 +234,7 @@ impl Model {
                 let pts = if fit { &sp.fit } else { &sp.control };
                 let n = pts.len();
                 if !(2..=MAX_SPLINE_POINTS).contains(&n) || off + n * 2 > MAX_PARAMS || (!fit && !sp.is_valid()) {
-                        continue;
+                    return None;
                 }
                 for p in pts {
                     m.x0.extend([p.x, p.y]);
@@ -263,8 +257,21 @@ impl Model {
                 m.rigid.insert(h, pts);
                 Shape::Insert { n }
             }
-                _ => continue,
+            _ => return None,
         };
+        Some(shape)
+    }
+
+    /// Build solver parameters for supported entities while sharing rigid-member sampling work with other models.
+    pub(crate) fn build_budgeted(d: &Drawing, handles: &[Handle], members_left: &mut usize) -> Model {
+        let mut m = Model::default();
+        for &h in handles {
+            if m.index.contains_key(&h) {
+                continue;
+            }
+            let Some(e) = d.entity(h) else { continue };
+            let off = m.x0.len();
+            let Some(shape) = Self::add_params(&mut m, d, h, &e.kind, off, members_left) else { continue };
             if m.x0.len() > MAX_PARAMS || m.x0.get(off..).unwrap_or(&[]).iter().any(|v| !v.is_finite()) {
                 m.x0.truncate(off);
                 continue;

crates/doc/src/tables.rs

@@ -441,24 +441,11 @@ impl DimStyle {
             .or_else(|| ["name", "annotative"].into_iter().find(|f| f.eq_ignore_ascii_case(name)))
     }
 
-    /// Set fields from a JSON object of field or DIM* variable names (numbers, booleans as
+    /// The value `v` takes for a field whose current value is `old`: coerced to the field's type where
-    /// 0/1, colours as ACI numbers or names). Returns the names that were not recognised or
+    /// the text or number can be read as it.
-    /// had unusable values.
+    fn coerce_value(old: &serde_json::Value, v: &serde_json::Value, field: &str) -> serde_json::Value {
-    pub fn apply_fields(&mut self, src: &serde_json::Map<String, serde_json::Value>) -> Vec<String> {
         use serde_json::Value;
-        let mut rejected = Vec::new();
+        match (old, v) {
-        let Ok(mut cur) = serde_json::to_value(&*self) else { return src.keys().cloned().collect() };
-        for (k, v) in src {
-            let Some(field) = DimStyle::field_name(k).filter(|f| *f != "name") else {
-                rejected.push(k.clone());
-                continue;
-            };
-            let Some(obj) = cur.as_object_mut() else { break };
-            let Some(old) = obj.get(field).cloned() else {
-                rejected.push(k.clone());
-                continue;
-            };
-            let coerced = match (&old, v) {
             (Value::Bool(_), Value::Number(n)) => Value::Bool(n.as_f64().is_some_and(|x| x != 0.0)),
             (Value::Bool(_), Value::String(t)) => Value::Bool(matches!(t.to_ascii_lowercase().as_str(), "1" | "on" | "true" | "yes")),
             (Value::Number(_), Value::String(t)) => match t.trim().parse::<f64>() {
@@ -480,7 +467,41 @@ impl DimStyle {
             }
             (Value::Object(_), Value::String(t)) => Color::parse(t).and_then(|c| serde_json::to_value(c).ok()).unwrap_or_else(|| v.clone()),
             _ => v.clone(),
+        }
+    }
+
+    /// Whether `coerced` is outside the range its `field` can take.
+    fn out_of_range(field: &str, coerced: &serde_json::Value) -> bool {
+        let x = coerced.as_f64();
+        match field {
+            "textHeight" | "tolScale" => x.is_some_and(|x| x <= 0.0),
+            "scale" | "arrowSize" | "tickSize" | "fixedExtLength" => x.is_some_and(|x| x < 0.0),
+            "fit" | "angularZeroSuppression" => x.is_some_and(|x| x > 3.0),
+            "textMove" | "tolJust" | "arcSymbol" | "textFill" => x.is_some_and(|x| x > 2.0),
+            "dimLineweight" | "extLineweight" => x.is_some_and(|x| !(-3.0..=211.0).contains(&x)),
+            "jogAngle" => x.is_some_and(|x| !(5.0..=90.0).contains(&x)),
+            _ => false,
+        }
+    }
+
+    /// Set fields from a JSON object of field or DIM* variable names (numbers, booleans as
+    /// 0/1, colours as ACI numbers or names). Returns the names that were not recognised or
+    /// had unusable values.
+    pub fn apply_fields(&mut self, src: &serde_json::Map<String, serde_json::Value>) -> Vec<String> {
+        use serde_json::Value;
+        let mut rejected = Vec::new();
+        let Ok(mut cur) = serde_json::to_value(&*self) else { return src.keys().cloned().collect() };
+        for (k, v) in src {
+            let Some(field) = DimStyle::field_name(k).filter(|f| *f != "name") else {
+                rejected.push(k.clone());
+                continue;
             };
+            let Some(obj) = cur.as_object_mut() else { break };
+            let Some(old) = obj.get(field).cloned() else {
+                rejected.push(k.clone());
+                continue;
+            };
+            let coerced = Self::coerce_value(&old, v, field);
             // Integers stored in u8 fields: round floats.
             let coerced = match (&old, &coerced) {
                 // Lineweights (i16): whole hundredths of a mm or -1/-2/-3.
@@ -502,16 +523,7 @@ impl DimStyle {
                 _ => coerced,
             };
             // Sizes that can't be negative (text height and tolerance scale: not zero either).
-            let x = coerced.as_f64();
+            let out_of_range = Self::out_of_range(field, &coerced);
-            let out_of_range = match field {
-                "textHeight" | "tolScale" => x.is_some_and(|x| x <= 0.0),
-                "scale" | "arrowSize" | "tickSize" | "fixedExtLength" => x.is_some_and(|x| x < 0.0),
-                "fit" | "angularZeroSuppression" => x.is_some_and(|x| x > 3.0),
-                "textMove" | "tolJust" | "arcSymbol" | "textFill" => x.is_some_and(|x| x > 2.0),
-                "dimLineweight" | "extLineweight" => x.is_some_and(|x| !(-3.0..=211.0).contains(&x)),
-                "jogAngle" => x.is_some_and(|x| !(5.0..=90.0).contains(&x)),
-                _ => false,
-            };
             if out_of_range {
                 rejected.push(k.clone());
                 continue;

crates/engine/src/cmd/draw2.rs

@@ -1585,25 +1585,10 @@ fn rect_kind(first: Vec2, (w, h): (f64, f64), rot: f64, fillet: f64, chamfer: (f
     k
 }
 
-/// Draws a rectangle from the JSON form: `p1` is a corner, or the centre when `center` is
+/// The width and height of a rectangle from its `dimensions`, its `p2` corner, its `area` with
-/// true, and the size comes from `p2`, `dimensions` or `area` plus `length`/`breadth`.
+/// `length` or `breadth`, or its second corner `p2`, rotated by `rot` about `a`.
-pub(crate) fn run_rectang2(s: &mut Session, p: &Value) -> Result<Value> {
+fn rect_size(p: &Value, a: Vec2, rot: f64, centered: bool) -> Result<(f64, f64)> {
-    let a = point_req("rectang", p, "p1")?;
+    Ok(if let Some(d) = p.get("dimensions").and_then(Value::as_array) {
-    // Centre mode: `p1` is the rectangle's centre, so the rectangle spans it evenly.
-    let centered = bool_or(p, "center", false);
-    let rot = f64_or(p, "rotation", 0.0).to_radians();
-    let ch = p
-        .get("chamfer")
-        .and_then(Value::as_array)
-        .map(|c| (c.first().and_then(Value::as_f64).unwrap_or(0.0), c.get(1).and_then(Value::as_f64).unwrap_or(0.0)))
-        .map(|(x, y)| (if x.is_finite() { x } else { 0.0 }, if y.is_finite() { y } else { 0.0 }))
-        .unwrap_or((0.0, 0.0));
-    if ch.0 < 0.0 || ch.1 < 0.0 {
-        return Err(bad("rectang", "`chamfer` distances must be zero or positive"));
-    }
-    let fillet = size_param("rectang", p, "fillet", true)?.unwrap_or(0.0);
-    let width = size_param("rectang", p, "width", true)?.unwrap_or(0.0);
-    let (w, h) = if let Some(d) = p.get("dimensions").and_then(Value::as_array) {
         let l = d.first().and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("rectang", "dimensions: [length, width]"))?;
         let wd = d.get(1).and_then(Value::as_f64).filter(|v| v.is_finite()).ok_or_else(|| bad("rectang", "dimensions: [length, width]"))?;
         let (sx, sy) = match point_param(p, "p2") {
@@ -1628,7 +1613,28 @@ pub(crate) fn run_rectang2(s: &mut Session, p: &Value) -> Result<Value> {
         let loc = (b - a).rotate(-rot);
         // In centre mode the picked point is one corner, mirrored across the centre.
         if centered { (2.0 * loc.x, 2.0 * loc.y) } else { (loc.x, loc.y) }
-    };
+    })
+}
+
+/// Draws a rectangle from the JSON form: `p1` is a corner, or the centre when `center` is
+/// true, and the size comes from `p2`, `dimensions` or `area` plus `length`/`breadth`.
+pub(crate) fn run_rectang2(s: &mut Session, p: &Value) -> Result<Value> {
+    let a = point_req("rectang", p, "p1")?;
+    // Centre mode: `p1` is the rectangle's centre, so the rectangle spans it evenly.
+    let centered = bool_or(p, "center", false);
+    let rot = f64_or(p, "rotation", 0.0).to_radians();
+    let ch = p
+        .get("chamfer")
+        .and_then(Value::as_array)
+        .map(|c| (c.first().and_then(Value::as_f64).unwrap_or(0.0), c.get(1).and_then(Value::as_f64).unwrap_or(0.0)))
+        .map(|(x, y)| (if x.is_finite() { x } else { 0.0 }, if y.is_finite() { y } else { 0.0 }))
+        .unwrap_or((0.0, 0.0));
+    if ch.0 < 0.0 || ch.1 < 0.0 {
+        return Err(bad("rectang", "`chamfer` distances must be zero or positive"));
+    }
+    let fillet = size_param("rectang", p, "fillet", true)?.unwrap_or(0.0);
+    let width = size_param("rectang", p, "width", true)?.unwrap_or(0.0);
+    let (w, h) = rect_size(p, a, rot, centered)?;
     if w.abs() < 1e-12 || h.abs() < 1e-12 || !w.is_finite() || !h.is_finite() {
         return Err(bad("rectang", "rectangle has no area"));
     }

crates/geom/src/fillet.rs

@@ -236,6 +236,46 @@ pub fn tangent_circles(c1: &TanCurve, c2: &TanCurve, r: f64, near: &[Vec2]) -> V
     if !(r.is_finite() && r >= 0.0) {
         return Vec::new();
     }
+    let Some(win) = search_window(c1, c2, near) else { return Vec::new() };
+    let size = win.width().max(win.height()).max(r).max(1e-9);
+    let win = win.expand(2.0 * r + 0.25 * size);
+    let tol = 1e-9 * size.max(1.0);
+    let (Some(r1), Some(r2)) = (c1.range(&win), c2.range(&win)) else { return Vec::new() };
+    let signs: &[(f64, f64)] = if r > 0.0 { &[(1.0, 1.0), (1.0, -1.0), (-1.0, 1.0), (-1.0, -1.0)] } else { &[(0.0, 0.0)] };
+    let mut out: Vec<CornerFit> = Vec::new();
+    let mut work = 16_000_000usize;
+    for (s1, s2) in signs {
+        let (d1, d2) = (s1 * r, s2 * r);
+        let (o1, o2) = (sample(c1, &r1, d1), sample(c2, &r2, d2));
+        let search = TanSearch { c1, r1: &r1, d1, c2, r2: &r2, d2, tol };
+        // Refine each pair before visiting the next: raw duplicate seeds in an early run must
+        // not consume all candidate slots. A separate request-wide budget bounds actual work.
+        for a in &o1 {
+            for b in &o2 {
+                if !refine_pair(&search, a, b, &mut work, &mut out) {
+                    return out;
+                }
+            }
+        }
+    }
+    out
+}
+
+/// The two curves of one tangent search for one sign choice: their ranges and offsets, and the
+/// tolerance.
+struct TanSearch<'a> {
+    c1: &'a TanCurve,
+    r1: &'a Range,
+    d1: f64,
+    c2: &'a TanCurve,
+    r2: &'a Range,
+    d2: f64,
+    tol: f64,
+}
+
+/// The window the tangent search looks in: the pick points and the extents of the curves that
+/// are not lines. `None` when it is empty or not finite.
+fn search_window(c1: &TanCurve, c2: &TanCurve, near: &[Vec2]) -> Option<Bounds2> {
     let mut win = Bounds2::from_points(near.iter().copied().filter(|p| p.is_finite()));
     for c in [c1, c2] {
         match c {
@@ -251,47 +291,34 @@ pub fn tangent_circles(c1: &TanCurve, c2: &TanCurve, r: f64, near: &[Vec2]) -> V
         }
     }
     if win.is_empty() || !(win.width().is_finite() && win.height().is_finite()) {
-        return Vec::new();
+        return None;
     }
-    let size = win.width().max(win.height()).max(r).max(1e-9);
+    Some(win)
-    let win = win.expand(2.0 * r + 0.25 * size);
+}
-    let tol = 1e-9 * size.max(1.0);
+
-    let (Some(r1), Some(r2)) = (c1.range(&win), c2.range(&win)) else { return Vec::new() };
+/// Refine the crossings of the samples `a` and `b` into circles, adding the distinct ones to
-    let signs: &[(f64, f64)] = if r > 0.0 { &[(1.0, 1.0), (1.0, -1.0), (-1.0, 1.0), (-1.0, -1.0)] } else { &[(0.0, 0.0)] };
+/// `out`. `false` when the work budget or the solution cap ends the search.
-    let mut out: Vec<CornerFit> = Vec::new();
+fn refine_pair(search: &TanSearch, a: &[(f64, Vec2)], b: &[(f64, Vec2)], work: &mut usize, out: &mut Vec<CornerFit>) -> bool {
-    let mut work = 16_000_000usize;
-    for (s1, s2) in signs {
-        let (d1, d2) = (s1 * r, s2 * r);
-        let (o1, o2) = (sample(c1, &r1, d1), sample(c2, &r2, d2));
-        // Refine each pair before visiting the next: raw duplicate seeds in an early run must
-        // not consume all candidate slots. A separate request-wide budget bounds actual work.
-        for a in &o1 {
-            for b in &o2 {
     let mut seeds = Vec::new();
-                crossings(a, b, &mut seeds, &mut work);
+    crossings(a, b, &mut seeds, work);
     seeds.sort_by(|x, y| x.0.total_cmp(&y.0).then(x.1.total_cmp(&y.1)));
     seeds.dedup_by(|x, y| (x.0 - y.0).abs() < 1e-9 && (x.1 - y.1).abs() < 1e-9);
     for seed in seeds {
         // Newton refinement has a fixed iteration cap and several curve evaluations.
-                    let Some(left) = work.checked_sub(128) else { return out };
+        let Some(left) = work.checked_sub(128) else { return false };
-                    work = left;
+        *work = left;
-                    let Some((t1, t2)) = refine(c1, &r1, d1, c2, &r2, d2, seed, tol) else { continue };
+        let TanSearch { c1, r1, d1, c2, r2, d2, tol } = *search;
+        let Some((t1, t2)) = refine(c1, r1, d1, c2, r2, d2, seed, tol) else { continue };
         let (Some(center), p1, p2) = (c1.offset(t1, d1), c1.point(t1), c2.point(t2)) else { continue };
         let near_dup = |f: &CornerFit| f.center.near(center, tol * 1e3) && f.p1.near(p1, tol * 1e3) && f.p2.near(p2, tol * 1e3);
         if !out.iter().any(near_dup) {
             out.push(CornerFit { t1, t2, p1, p2, center });
         }
         if out.len() >= MAX_SOLUTIONS {
-                        return out;
+            return false;
         }
     }
-                if work == 0 {
+    *work != 0
-                    return out;
-                }
-            }
-        }
-    }
-    out
 }
 
 #[cfg(test)]

crates/ui-egui/src/menus.rs

@@ -172,7 +172,7 @@ fn run_ui_command_inner(app: &mut CadApp, id: &str, params: &Value) -> Option<Re
             let pick = app.services.pick_lisp.as_ref()?;
             let dir = app.ui.last_lisp_dir.clone().unwrap_or_default();
             let Some(path) = pick(&dir) else { return Some(Ok(Value::Null)) };
-            if let Some(dir) = std::path::Path::new(&path).parent().and_then(|p| p.to_str()).filter(|d| !d.is_empty()) {
+            if let Some(dir) = std::path::Path::new(&path).parent().and_then(std::path::Path::to_str).filter(|d| !d.is_empty()) {
                 app.ui.last_lisp_dir = Some(dir.to_string());
             }
             if let Err(e) = app.session.start("appload") {