diff --git a/src/app/commands/draw.rs b/src/app/commands/draw.rs index 67a06ab81..b7ab4904a 100644 --- a/src/app/commands/draw.rs +++ b/src/app/commands/draw.rs @@ -2346,3 +2346,22 @@ mod region_tests { } } } + +#[cfg(test)] +mod degenerate_constraint_tests { + use crate::app::OpenCADStudio; + + /// Retaining a zero-length line's size used to hand the solver a NaN + /// Jacobian, and its SVD fallback never returned. + #[test] + fn equal_constraint_with_a_zero_length_line_returns() { + let mut app = OpenCADStudio::new_for_test(); + app.automation_op(r#"{"op":"new"}"#); + app.automation_op(r#"{"op":"run","cmd":"LINE 0,0 0,0 "}"#); + app.automation_op(r#"{"op":"run","cmd":"LINE 5,5 20,7 "}"#); + app.automation_op(r#"{"op":"select","type":"Line"}"#); + app.automation_op(r#"{"op":"run","cmd":"ECONSTRAINT"}"#); + let lines = app.automation_op(r#"{"op":"query","type":"Line","detail":"geometry"}"#); + assert_eq!(lines["ok"], true, "{lines}"); + } +} diff --git a/src/io/mod.rs b/src/io/mod.rs index fdc687529..d7be16cf8 100644 --- a/src/io/mod.rs +++ b/src/io/mod.rs @@ -2146,8 +2146,12 @@ pub(crate) fn is_entity_corrupt(e: &EntityType) -> bool { && (s.knots.iter().any(|k| !k.is_finite()) || s.knots.windows(2).any(|w| w[1] < w[0]) || (!compact_periodic_knots && s.knots.len() != n + deg + 1)); + // A single control point with no knots and no fit points cannot + // form a curve, and no knot vector exists for it. + let lone_point = n == 1 && s.knots.is_empty() && s.fit_points.is_empty(); n >= MAX_VERTS || degree_bad + || lone_point || s.control_points.iter().any(|p| !finite_vec3(p)) || knots_bad } @@ -2155,8 +2159,32 @@ pub(crate) fn is_entity_corrupt(e: &EntityType) -> bool { } } +/// Give control-point splines stored without knots a degree their control +/// points can carry, and the matching clamped knot vector. +/// +/// A degree-p curve needs at least p + 1 control points. Knot generation +/// (here, and in the DWG writer for knot-less splines) subtracts in `usize` +/// and underflows when there are fewer; in release builds the wrapped count +/// drives an unbounded allocation. Lowering the degree keeps the geometry the +/// points describe: two points are a straight segment at any degree. +fn normalize_knotless_splines(doc: &mut CadDocument) { + for entity in doc.entities_mut() { + let EntityType::Spline(spline) = entity else { + continue; + }; + let n = spline.control_points.len(); + if !spline.knots.is_empty() || n < 2 || spline.degree < 1 { + continue; + } + let degree = (spline.degree as usize).min(n - 1); + spline.degree = degree as i32; + spline.knots = acadrust::entities::Spline::generate_clamped_knots(degree, n); + } +} + pub fn purge_corrupt_entities(doc: &mut CadDocument) -> usize { use crate::par::prelude::*; + normalize_knotless_splines(doc); // Detection is pure and read-only; the per-vertex finite/extent checks on // large polylines dominate, so fan the scan out across cores. Gather // entity references in one pass, test in parallel, then remove serially @@ -2363,6 +2391,42 @@ mod corrupt_guard_tests { use acadrust::entities::{Arc, Circle, EntityType, Spline}; use acadrust::types::Vector3; + fn knotless_spline(degree: i32, points: usize) -> Spline { + let mut spline = Spline::new(); + spline.degree = degree; + spline.control_points = (0..points) + .map(|index| Vector3::new(index as f64, index as f64, 0.0)) + .collect(); + spline + } + + // Degree 3 over two control points with no knot vector: generating knots + // for it underflows, and writing it to DWG used to panic (debug) or + // allocate without bound (release). + #[test] + fn knotless_spline_with_too_few_points_is_lowered_and_saves_as_dwg() { + let mut doc = CadDocument::new(); + doc.add_entity(EntityType::Spline(knotless_spline(3, 2))); + doc.add_entity(EntityType::Spline(knotless_spline(3, 1))); + assert_eq!(purge_corrupt_entities(&mut doc), 1, "the lone point is dropped"); + let spline = doc + .entities() + .find_map(|entity| match entity { + EntityType::Spline(spline) => Some(spline.clone()), + _ => None, + }) + .expect("the two-point spline is kept"); + assert_eq!(spline.degree, 1); + assert_eq!(spline.knots, vec![0.0, 0.0, 1.0, 1.0]); + + let path = std::env::temp_dir().join(format!( + "ocs_knotless_spline_{}.dwg", + std::process::id() + )); + save_as_version(&doc, &path, acadrust::DxfVersion::AC1032).expect("save"); + let _ = std::fs::remove_file(&path); + } + // Small but finite arcs are valid records. Kernel tessellation is bounded, // so opening must retain them instead of treating their size as corruption. #[test] diff --git a/src/scene/parametric_solve.rs b/src/scene/parametric_solve.rs index 30e56a8fc..bffeb669a 100644 --- a/src/scene/parametric_solve.rs +++ b/src/scene/parametric_solve.rs @@ -1830,6 +1830,29 @@ fn retained_radius(entity: &EntityType) -> Option { } } +/// Hold the distance between two points at `length`. +/// +/// `P2PDistance` divides by the current distance, so a zero-length segment +/// gives the solver a NaN Jacobian. Keep coincident endpoints together with +/// per-axis differences instead. +fn retain_segment_length( + sys: &mut System, + a: GPoint, + b: GPoint, + current_length: f64, + length: f64, +) { + if current_length > f64::EPSILON && length > f64::EPSILON { + let target = sys.add_param(length, true); + sys.add_constraint(Rc::new(P2PDistance::new(a, b, target))); + return; + } + let dx = sys.add_param(0.0, true); + let dy = sys.add_param(0.0, true); + sys.add_constraint(Rc::new(Difference::new(a.x, b.x, dx))); + sys.add_constraint(Rc::new(Difference::new(a.y, b.y, dy))); +} + /// Visit vertices and segments in stored drawing order, including the closing edge last. fn polyline_ref_order( document: &acadrust::CadDocument, @@ -2139,8 +2162,7 @@ fn solve_scope( .get(handle) .and_then(|entity| retained_line_length(entity, None)) .unwrap_or(current_length); - let target = sys.add_param(length, true); - sys.add_constraint(Rc::new(P2PDistance::new(line.p1, line.p2, target))); + retain_segment_length(&mut sys, line.p1, line.p2, current_length, length); } EntityGeom::Polyline { points, @@ -2169,8 +2191,7 @@ fn solve_scope( .get(handle) .and_then(|entity| retained_line_length(entity, Some(index))) .unwrap_or(current_length); - let target = sys.add_param(length, true); - sys.add_constraint(Rc::new(P2PDistance::new(a, b, target))); + retain_segment_length(&mut sys, a, b, current_length, length); } } EntityGeom::Circle(circle) => { @@ -2346,8 +2367,8 @@ fn solve_scope( }) }); if directional && !retain_size { - let length = sys.add_param((x2 - x1).hypot(y2 - y1), true); - sys.add_constraint(Rc::new(P2PDistance::new(line.p1, line.p2, length))); + let length = (x2 - x1).hypot(y2 - y1); + retain_segment_length(&mut sys, line.p1, line.p2, length, length); } else if !directional { let dx = sys.add_param(x2 - x1, true); let dy = sys.add_param(y2 - y1, true);