Merge pull request #153 from EmDecay/fix/145-sketch-size-caps

fix: hold the sketch caps at design load and bound the solver per component

apps/solvecraft-cli/tests/design_corpus.rs

@@ -161,9 +161,16 @@ fn generated() -> Vec<(String, Vec<u8>)> {
     let design = |name: &str, features: Value| {
         json!({"format": "solvecraft/11", "name": name, "units": "mm", "params": [], "next_id": 9, "features": features}).to_string().into_bytes()
     };
+    // A sketch past the caps interactive editing enforces: solving it would size dense matrices
+    // by the point count and abort on the allocation.
+    let point = json!({"id": "p1", "pos": [0.0, 0.0], "fixed": false});
+    let oversized = json!([{"id": 1, "name": "Sketch1", "suppressed": false, "type": "sketch",
+                            "plane": {"type": "origin", "name": "XY"},
+                            "sketch": {"points": vec![point; 200_000], "curves": [], "constraints": []}}]);
     vec![
         ("construction plane is its own side".into(), design("selfplane", json!([plane(1, "P1", to("P1"))]))),
         ("two construction planes refer to each other".into(), design("twoplanes", json!([plane(1, "P1", to("P2")), plane(2, "P2", to("P1"))]))),
+        ("sketch with 200,000 points".into(), design("bigsketch", oversized)),
     ]
 }
 

crates/doc/src/format.rs

@@ -153,6 +153,7 @@ pub fn read(s: &str) -> Result<Document> {
     validate_versioned_geometry(&v)?;
     validate_emboss_targets(&v)?;
     validate_freeform_offsets(&v)?;
+    validate_sketch_sizes(&v)?;
     crate::format_offsets::validate_raw(&v)?;
     let legacy_wires = validate_wires(&v, version)?;
     validate_body_transfers(&v)?;
@@ -213,6 +214,29 @@ pub fn read(s: &str) -> Result<Document> {
     Ok(d)
 }
 
+/// The caps interactive editing enforces, applied to the raw JSON so an oversized sketch is
+/// never deserialised. Every sketch in the timeline is solved when the design opens, and the
+/// solver sizes its dense matrices by the sketch, so a file with hundreds of thousands of
+/// points would ask the allocator for terabytes and abort before anything could report it.
+fn validate_sketch_sizes(value: &Value) -> Result<()> {
+    let Some(features) = value.get("features").and_then(Value::as_array) else { return Ok(()) };
+    let invalid = || DocError::Invalid("damaged design: sketch too large".into());
+    for feature in features {
+        if feature.get("type").and_then(Value::as_str) != Some("sketch") {
+            continue;
+        }
+        let Some(sketch) = feature.get("sketch") else { continue };
+        let len = |key: &str| sketch.get(key).and_then(Value::as_array).map_or(0, Vec::len);
+        if len("points") > solvecraft_sketch::MAX_POINTS
+            || len("curves") > solvecraft_sketch::MAX_CURVES
+            || len("constraints") > solvecraft_sketch::MAX_CONSTRAINTS
+        {
+            return Err(invalid());
+        }
+    }
+    Ok(())
+}
+
 /// Bound arrays before typed allocation; only old files with absent markers need migration.
 fn validate_wires(value: &Value, version: u32) -> Result<Vec<(usize, usize)>> {
     let invalid = || DocError::Invalid("damaged design: 3D curve arrays, ids or coordinate spaces are invalid".into());
@@ -1199,4 +1223,35 @@ mod tests {
         assert!(read(&v.to_string()).is_err(), "out-of-range numbers anywhere");
         assert!(read("{\"format\": 2}").is_err());
     }
+
+    /// Every sketch in the timeline is solved when the design opens, and the solver sizes its
+    /// dense matrices by the sketch. The caps interactive editing enforces have to hold for a
+    /// file too, before the arrays are deserialised.
+    #[test]
+    fn oversized_sketches_are_refused_before_typed_decoding() {
+        use solvecraft_sketch::{MAX_CONSTRAINTS, MAX_CURVES, MAX_POINTS, Sketch};
+
+        // A sketch right at the point cap, chained by coincident constraints.
+        let mut sketch = Sketch::new();
+        let mut previous = 0;
+        while sketch.points.len() < MAX_POINTS {
+            let i = sketch.add_point(solvecraft_geom::Vec2::new(0.001 * sketch.points.len() as f64, 0.0), None).unwrap();
+            sketch.add_constraint(solvecraft_sketch::ConstraintKind::Coincident { p: previous, q: i }, None).unwrap();
+            previous = i;
+        }
+        let mut doc = Document::new("big sketch");
+        doc.add_feature(crate::FeatureKind::Sketch { plane: crate::PlaneRef::Origin { name: "XY".into() }, sketch }, None).unwrap();
+        let at_cap: Value = serde_json::from_str(&doc.to_json()).unwrap();
+        assert!(read(&at_cap.to_string()).is_ok(), "a sketch at the caps must still load");
+
+        // One past each cap, grown at the JSON level: no typed array is ever built.
+        for (key, cap) in [("points", MAX_POINTS), ("constraints", MAX_CONSTRAINTS), ("curves", MAX_CURVES)] {
+            let mut value = at_cap.clone();
+            let slot = &mut value["features"][0]["sketch"][key];
+            let first = slot.as_array().and_then(|a| a.first()).cloned().unwrap_or(Value::Null);
+            *slot = Value::Array(vec![first; cap + 1]);
+            let error = read(&value.to_string()).unwrap_err().to_string();
+            assert!(error.contains("sketch too large"), "{key}: {error}");
+        }
+    }
 }

crates/sketch/src/lib.rs

@@ -30,7 +30,7 @@ pub use curves::{
 pub use dims::{DimFrame, DimLayout, chain_centres, default_text, dim_frame, dim_layout, encode_text};
 pub use freeform_offset::{FreeformOffset, OFFSET_TOLERANCE};
 pub use link::{Link, LinkGeom, LinkKind, LinkSource, MAX_LINK_ENTITIES};
-pub use model::{Constraint, ConstraintKind, Curve, CurveKind, SPoint, Sketch, SketchError, SketchView};
+pub use model::{Constraint, ConstraintKind, Curve, CurveKind, MAX_CONSTRAINTS, MAX_CURVES, MAX_POINTS, SPoint, Sketch, SketchError, SketchView};
 pub use offset::OffsetChain;
 pub use offset_topology::{OffsetCreated, OffsetRole, OffsetRoleFlags, OffsetRoleKind, OffsetTopology};
 pub use periodic::periodic_beziers;

crates/sketch/src/model.rs

@@ -4,6 +4,8 @@ use solvecraft_geom::{Seg2, Vec2};
 /// Hard caps that keep hostile input from exhausting memory or time.
 pub const MAX_POINTS: usize = 20_000;
 pub const MAX_CONSTRAINTS: usize = 40_000;
+/// Curves need points, so this follows the point cap.
+pub const MAX_CURVES: usize = MAX_POINTS;
 
 #[derive(Debug, Clone, PartialEq, thiserror::Error)]
 pub enum SketchError {

crates/sketch/src/solver.rs

@@ -15,6 +15,12 @@ use crate::linalg::{Mat, determined_vars, solve as lin_solve};
 use crate::model::{ConstraintKind, CurveKind, Sketch};
 
 const MAX_ITERS: usize = 500;
+/// Variables in one connected component. Each component is solved as a dense system: an n by n
+/// matrix for the Levenberg-Marquardt step, rows by n for the Jacobian. 4,000 variables is a
+/// 128 MB matrix; past that the request grows to gigabytes and an allocation failure aborts the
+/// process, which no panic guard can intercept. A component this size is reported as failed and
+/// its geometry left alone.
+const MAX_COMPONENT_VARS: usize = 4_000;
 /// Converged when every residual is below this (mm).
 const TOL: f64 = 1e-9;
 /// A solution is accepted when every residual is below this (mm).
@@ -1050,8 +1056,9 @@ pub fn solve_holding(sk: &mut Sketch, hold: &Hold) -> SolveReport {
     let mut iterations = 0;
     // Solve each component; `blocks` is consumed by index so rebuild per component.
     let all_blocks = blocks;
+    let oversized = |vars: &[usize]| vars.len() > MAX_COMPONENT_VARS;
     for (_, vars, bis) in &comps {
-        if bis.is_empty() {
+        if bis.is_empty() || oversized(vars) {
             continue;
         }
         let sub: Vec<(Block, Vec<usize>)> = bis.iter().filter_map(|&bi| all_blocks.get(bi).map(|(b, v)| (copy_block(b), v.clone()))).collect();
@@ -1064,6 +1071,10 @@ pub fn solve_holding(sk: &mut Sketch, hold: &Hold) -> SolveReport {
     let mut failing: Vec<String> = Vec::new();
     let mut det = vec![false; lay.n];
     let mut rank = 0;
+    // A component too large to solve is a failure, not an allocation.
+    if comps.iter().any(|(_, vars, bis)| !bis.is_empty() && oversized(vars)) {
+        max_residual = f64::MAX;
+    }
     let mut check = |r: &[f64], owner: &[usize], blocks: &[(Block, Vec<usize>)], failing: &mut Vec<String>| {
         max_residual = max_residual.max(max_abs(r));
         for (row, v) in r.iter().enumerate() {
@@ -1077,6 +1088,9 @@ pub fn solve_holding(sk: &mut Sketch, hold: &Hold) -> SolveReport {
         }
     };
     for (_, vars, bis) in &comps {
+        if oversized(vars) {
+            continue;
+        }
         let sub: Vec<(Block, Vec<usize>)> = bis.iter().filter_map(|&bi| all_blocks.get(bi).map(|(b, v)| (copy_block(b), v.clone()))).collect();
         if sub.is_empty() {
             continue;
@@ -1250,4 +1264,39 @@ mod typed_dimension_tests {
         sk.remove_curves(&[l]);
         assert!(sk.constraints.is_empty());
     }
+
+    /// One connected component is solved as a dense system, so its variable count sets the
+    /// allocation: 10,000 chained points are 20,000 variables and a 3.2 GB matrix. The solve
+    /// has to report failure instead of asking for the memory, because an allocation failure
+    /// aborts the process where a failed solve does not.
+    #[test]
+    fn components_too_large_to_solve_fail_instead_of_allocating() {
+        let mut sk = Sketch::new();
+        let mut previous = 0;
+        for i in 1..10_000 {
+            let p = sk.add_point(Vec2::new(0.001 * i as f64, 0.0), None).unwrap();
+            sk.add_constraint(ConstraintKind::Coincident { p: previous, q: p }, None).unwrap();
+            previous = p;
+        }
+        let before = sk.points.clone();
+        let t = std::time::Instant::now();
+        let report = solve(&mut sk);
+        assert_eq!(report.status, SolveStatus::Failed);
+        // Geometry is left as it was.
+        assert_eq!(sk.points, before);
+        assert!(t.elapsed() < std::time::Duration::from_secs(10), "took {:?}", t.elapsed());
+    }
+
+    /// A sketch whose components each stay under the cap still solves.
+    #[test]
+    fn many_small_components_still_solve() {
+        let mut sk = Sketch::new();
+        for i in 0..200 {
+            let a = sk.add_point(Vec2::new(i as f64, 0.0), None).unwrap();
+            let b = sk.add_point(Vec2::new(i as f64, 5.0), None).unwrap();
+            let l = sk.add_line_pts(a, b, None).unwrap();
+            sk.add_constraint(ConstraintKind::Vertical { l }, None).unwrap();
+        }
+        assert!(solve(&mut sk).ok());
+    }
 }