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

Nothing on the load path checked a sketch's size, and every sketch in the
timeline is solved when a design opens. The solver sizes its dense matrices by
the connected component, so a 30 MB file with 200,000 chained points asked the
allocator for terabytes and the process aborted, which no panic guard can
intercept.

Reject an oversized sketch while the design is still raw JSON, so the typed
arrays are never built, and refuse a connected component with more variables
than fits in a bounded matrix: that component now reports a failed solve and
leaves its geometry alone. 10,000 chained points went from 9.6 GB and 25
seconds to a failed solve in well under a second.

Fixes #145

Co-Authored-By: Claude Opus 5 <[email protected]>
(cherry picked from commit 04abbf95c26f68c7d0a126ed9e2827049f593361)
This commit is contained in:
Matt committed 2026-10-11 08:22:11 -07:00
1 parent 0b5e15e2b9
commit 78f80143d5
5 files changed
+115 -2

No files matched your search

@@ -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)),
]
}
+55
View File
@@ -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}");
}
}
}
+1 -1
View File
@@ -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;
+2
View File
@@ -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 {
+50 -1
View File
@@ -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());
}
}