From 9ca7d307a46e3152a1867efe2011d52170f73d21 Mon Sep 17 00:00:00 2001 From: Karl Von Richter Date: Wed, 14 Oct 2026 19:27:39 +1000 Subject: [PATCH] Offset of an irregular closed spline no longer fails its error certificate; offset fit nodes stay private (#1009) Fixes #971. At the node limit the offset certificate splits the fitted curve where it meets the exact offset midpoint, so curves with different parameter speeds certify within the same 0.002 mm tolerance. Associated offset fit nodes are hidden from display, picking, box selection and snapping until Break Association. Thanks @Karl-VonRichter. --- ROADMAP.md | 8 +- crates/engine/src/cmd/sketch_modify_tests.rs | 60 ++++++++++++ crates/engine/src/view.rs | 40 ++++++-- crates/sketch/src/freeform_offset.rs | 96 +++++++++++++++++-- crates/ui-egui/src/dialogs.rs | 28 +----- crates/ui-egui/src/drag_snap.rs | 4 +- crates/ui-egui/src/sketch_drag_ui_tests.rs | 93 ++++++++++++++++++ crates/ui-egui/src/viewport.rs | 9 +- .../sketch_offset_irregular_spline.json | 22 +++++ 9 files changed, 319 insertions(+), 41 deletions(-) create mode 100644 crates/ui-egui/tests/scenarios/sketch_offset_irregular_spline.json diff --git a/ROADMAP.md b/ROADMAP.md index 418d9375..f101971c 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -270,8 +270,12 @@ PCB / Package, Render, Animation, Simulation, Manufacture/CAM, Drawings. Sweeps along 3D sketch curves follow their polyline (smooth walls when the path is smooth). - Extend intersects exact ellipses and rational/high-degree splines without moving the line or circular support. Associative free-form offsets have a whole-span 0.002 mm normal-distance - certificate and refine within bounded node limits while retaining existing IDs. Singular, - self-intersecting and ambiguous curves refuse atomically; mixed junctions and nonsmooth + certificate and refine within bounded node limits while retaining existing IDs. + At the node limit, geometric certificate subdivisions accommodate unequal spline parameter + speeds without changing stored nodes or relaxing the distance tolerance (#971). + Associated offset fit nodes stay hidden from display, picking and snapping; dragging the + editable source updates the derived boundary live. Break Association restores point editing. + Singular, self-intersecting and ambiguous curves refuse atomically; mixed junctions and nonsmooth closed natural-spline seams remain limitations. New explicitly closed fit splines and closed offset outputs use periodic C2 interpolation, supporting repeated certified offsets; older natural splines are preserved. Clicking the starting fit point deliberately closes a new loop. diff --git a/crates/engine/src/cmd/sketch_modify_tests.rs b/crates/engine/src/cmd/sketch_modify_tests.rs index 272ec305..f1674e3d 100644 --- a/crates/engine/src/cmd/sketch_modify_tests.rs +++ b/crates/engine/src/cmd/sketch_modify_tests.rs @@ -723,6 +723,66 @@ fn offset_free_form_curves() { assert_eq!(ids(&r["curves"]).len(), 1, "{r}"); } +#[test] +fn irregular_closed_spline_offset_certificate_edits_undoes_and_reopens() { + // Public-safe reproduction of #971: the fit uses chord-length parameters, + // while its exact offset's speed varies across eight unequal source spans. + let mut s = new_sketch(); + run( + &mut s, + "sketch.spline.fit_point", + json!({ + "points":[[-17,15],[0,8],[17,13],[15,0],[5,-7],[-1,-14],[-9,-1],[-21,-1]], + "periodic":true + }), + ); + let original = sketch(&s); + let before = s.doc.to_json(); + let result = run(&mut s, "sketch.offset", json!({"curves":["s1"],"distance":2.0,"chain":true})); + let output = result["curves"][0].as_str().unwrap().to_string(); + let parameter = result["param"].as_str().unwrap().to_string(); + let point_ids = sketch(&s).curves[1].kind.point_ids(); + let verify = |s: &Session, distance: f64| { + let state = s.world_state(); + let sk = &state.sketch(1).unwrap().sketch; + assert_eq!(sk.curves[0], original.curves[0]); + assert_eq!(&sk.points[..original.points.len()], &original.points); + let index = sk.curve_index(&output).unwrap(); + assert_eq!(sk.curves[index].kind.point_ids(), point_ids); + assert!(matches!(sk.curves[index].kind, CurveKind::Spline { periodic: true, .. })); + let exact = sk.exact_segs(0).unwrap(); + let fitted = sk.exact_segs(index).unwrap(); + let mut max_error = 0.0_f64; + for curve in &fitted { + for j in 0..=8 { + let p = curve.point_at(j as f64 / 8.0); + let normal_distance = exact.iter().map(|s| s.point_at(s.closest_param(p)).dist(p)).fold(f64::INFINITY, f64::min); + max_error = max_error.max((normal_distance - distance).abs()); + } + } + assert!(max_error < solidcraft_sketch::OFFSET_TOLERANCE, "normal-distance error {max_error}"); + eprintln!("#971 offset {distance} mm: maximum sampled normal-distance error {max_error} mm"); + assert_eq!(sk.freeform_offsets.len(), 1); + assert_eq!(sk.freeform_offsets[0].divisions, 32); + sk.validate_freeform_offsets().unwrap(); + sk.validate_periodic_splines().unwrap(); + assert_eq!(state.sketch(1).unwrap().profiles.len(), 2); + }; + verify(&s, 2.0); + run(&mut s, "edit.undo", json!({})); + assert_eq!(s.doc.to_json(), before); + run(&mut s, "edit.redo", json!({})); + verify(&s, 2.0); + run(&mut s, "parameters.change", json!({"name":parameter,"expression":"3 mm"})); + verify(&s, 3.0); + run(&mut s, "edit.undo", json!({})); + verify(&s, 2.0); + let mut reopened = Session::new(solidcraft_doc::Document::from_json(&s.doc.to_json()).unwrap()); + verify(&reopened, 2.0); + run(&mut reopened, "parameters.change", json!({"name":parameter,"expression":"3 mm"})); + verify(&reopened, 3.0); +} + #[test] fn certified_offset_follows_dimension_drag_reopen_break_and_undo() { let mut s = new_sketch(); diff --git a/crates/engine/src/view.rs b/crates/engine/src/view.rs index 7ad14fbf..f87379a0 100644 --- a/crates/engine/src/view.rs +++ b/crates/engine/src/view.rs @@ -169,13 +169,41 @@ pub fn sketch_curve_visible(sk: &Sketch, i: usize) -> bool { !(c.construction && sk.view.hide_construction) } -/// Should sketch point `i` be drawn? Text outlines' points are not (they are not picked or -/// constrained individually). -pub fn sketch_point_visible(sk: &Sketch, i: usize) -> bool { - match sk.points.get(i).and_then(|p| p.link.as_deref()) { - Some(l) => sk.link(l).is_none_or(|l| l.kind != solidcraft_sketch::LinkKind::Text), - None => true, +/// Point roles shown and picked individually. Associated offsets keep their generated +/// fit nodes private until their association is broken. Build once per sketch/frame. +pub struct SketchPointVisibility<'a> { + hidden: std::collections::HashSet<&'a str>, +} + +impl<'a> SketchPointVisibility<'a> { + pub fn new(sk: &'a Sketch) -> Self { + let outputs: std::collections::HashSet<&str> = sk.freeform_offsets.iter().map(|o| o.output.as_str()).collect(); + // A repeated link ID names its first link, as Sketch::link does. + let mut seen = std::collections::HashSet::new(); + let text: std::collections::HashSet<&str> = sk + .links + .iter() + .filter(|l| seen.insert(l.id.as_str())) + .filter(|l| l.kind == solidcraft_sketch::LinkKind::Text) + .map(|l| l.id.as_str()) + .collect(); + let mut hidden: std::collections::HashSet<&str> = + sk.points.iter().filter(|p| p.link.as_deref().is_some_and(|l| text.contains(l))).map(|p| p.id.as_str()).collect(); + for curve in sk.curves.iter().filter(|c| outputs.contains(c.id.as_str())) { + hidden.extend(curve.kind.point_ids().into_iter().filter_map(|i| sk.points.get(i).map(|p| p.id.as_str()))); + } + Self { hidden } } + + pub fn takes(&self, point: &solidcraft_sketch::SPoint) -> bool { + !self.hidden.contains(point.id.as_str()) + } +} + +/// Should sketch point `i` be drawn or picked individually? Text outlines and derived +/// offset fit nodes are private. Batch callers use [`SketchPointVisibility`]. +pub fn sketch_point_visible(sk: &Sketch, i: usize) -> bool { + sk.points.get(i).is_some_and(|p| SketchPointVisibility::new(sk).takes(p)) } /// The scene for a headless render, cut by the session's section. diff --git a/crates/sketch/src/freeform_offset.rs b/crates/sketch/src/freeform_offset.rs index f20e3eed..20adc640 100644 --- a/crates/sketch/src/freeform_offset.rs +++ b/crates/sketch/src/freeform_offset.rs @@ -1,7 +1,9 @@ //! Persistent free-form offsets with a whole-span normal-distance error certificate. //! The certificate bounds the second derivative of the exact normal offset and of //! the actual cubic interpolant (natural for legacy output, periodic for new closed output). The endpoint interpolation theorem then -//! bounds their distance everywhere, rather than just at selected samples. +//! bounds their distance everywhere, rather than just at selected samples. At the +//! node limit, interior splits align the curves geometrically instead of requiring +//! identical parameter speeds; stored nodes and interpolation remain unchanged. use crate::{CurveKind, Sketch, SketchError, fit_beziers_mode}; use serde::{Deserialize, Serialize}; use solidcraft_geom::{Seg2, Vec2}; @@ -14,6 +16,11 @@ struct Fit { tolerance: f64, periodic: bool, } +#[derive(Clone, Copy)] +struct Certificate { + tolerance: f64, + align_midpoints: bool, +} type Result = std::result::Result; fn bad(message: &str) -> SketchError { SketchError::Invalid(message.into()) @@ -202,7 +209,8 @@ impl H { /// Subdivision tightens the proof, not the stored geometry. Endpoint discrepancies /// are measured against the exact rational normal, then the second derivative /// interpolation bound certifies the full intervening interval. -fn certify(h: &H, cubic: &[Vec2; 4], distance: f64, tolerance: f64, depth: u8, budget: &mut usize) -> Result<()> { +fn certify(h: &H, cubic: &[Vec2; 4], distance: f64, proof: Certificate, depth: u8, budget: &mut usize) -> Result<()> { + let tolerance = proof.tolerance; if *budget == 0 { return Err(bad("offset certificate work limit exceeded")); } @@ -220,13 +228,24 @@ fn certify(h: &H, cubic: &[Vec2; 4], distance: f64, tolerance: f64, depth: u8, b } let (left, right) = h.split(0.5); let curve = Seg2::Cubic { p0: cubic[0], p1: cubic[1], p2: cubic[2], p3: cubic[3] }; - let (a, b) = curve.split_half(); + // A chord-length fit need not have the source's parameter speed. When node + // refinement is exhausted, pair the exact offset midpoint with a point on + // the fitted cubic instead of assuming its parameter is 0.5. Each interior + // split defines an increasing, piecewise-affine correspondence. The same + // endpoint/second-derivative theorem applies on every pair of restricted + // spans, proving a whole-curve distance bound in both directions. Projection + // only chooses the split; its accuracy is checked by the certificate itself. + let split = if proof.align_midpoints { curve.closest_param(left.normal_end(true, distance)?) } else { 0.5 }; + if !split.is_finite() || !(1e-9..=1.0 - 1e-9).contains(&split) { + return Err(bad("offset spline has no interior certificate correspondence")); + } + let (a, b) = curve.split_at(split); let controls = |s: Seg2| match s { Seg2::Cubic { p0, p1, p2, p3 } => Ok([p0, p1, p2, p3]), _ => Err(bad("invalid cubic certificate")), }; - certify(&left, &controls(a)?, distance, tolerance, depth + 1, budget)?; - certify(&right, &controls(b)?, distance, tolerance, depth + 1, budget) + certify(&left, &controls(a)?, distance, proof, depth + 1, budget)?; + certify(&right, &controls(b)?, distance, proof, depth + 1, budget) } fn normal_point(seg: &Seg2, t: f64, d: f64) -> Result { @@ -345,7 +364,12 @@ fn geometry(sk: &Sketch, source: usize, forward: bool, d: f64, divisions: usize, let cubics = fit_beziers_mode(&points, periodic && closed)?; let allowance = (*work).min(65_536); let mut budget = allowance; - let result = restricted.iter().zip(&cubics).try_for_each(|(h, c)| certify(h, c, d, tolerance, 0, &mut budget)); + let mut result = + restricted.iter().zip(&cubics).try_for_each(|(h, c)| certify(h, c, d, Certificate { tolerance, align_midpoints: false }, 0, &mut budget)); + if result.is_err() && (divisions == 256 || pieces.len().saturating_mul(divisions.saturating_mul(2)) >= crate::MAX_SPLINE_POINTS) { + result = + restricted.iter().zip(&cubics).try_for_each(|(h, c)| certify(h, c, d, Certificate { tolerance, align_midpoints: true }, 0, &mut budget)); + } *work -= allowance - budget; result?; verify_simple(&points, &cubics, closed)?; @@ -567,6 +591,66 @@ impl Sketch { mod tests { use super::*; + #[test] + fn certified_offset_irregular_closed_spline_at_node_limit() { + let mut sk = Sketch::new(); + let pts = [(-17.0, 15.0), (0.0, 8.0), (17.0, 13.0), (15.0, 0.0), (5.0, -7.0), (-1.0, -14.0), (-9.0, -1.0), (-21.0, -1.0), (-17.0, 15.0)] + .into_iter() + .map(|(x, y)| sk.add_point(Vec2::new(x, y), None).unwrap()) + .collect(); + let source = sk.add_curve(CurveKind::Spline { pts, control: false, degree: 3, periodic: true }, None).unwrap(); + let original = sk.clone(); + for (forward, distance) in [(true, 2.0), (false, -2.0)] { + let mut sk = original.clone(); + let output = sk.add_freeform_offset(source, forward, distance).unwrap(); + assert_eq!(sk.freeform_offsets[0].divisions, 32); + assert_eq!(&sk.points[..original.points.len()], &original.points); + assert_eq!(sk.curves[source], original.curves[source]); + let exact = sk.exact_segs(source).unwrap(); + let fitted = sk.exact_segs(output).unwrap(); + assert_eq!(fitted.first().unwrap().start(), fitted.last().unwrap().end()); + let mut max_error = 0.0_f64; + for (i, piece) in exact.iter().enumerate() { + for j in 0..=128 { + let t = j as f64 / 128.0; + let want = piece.point_at(t) + piece.derivative(t).normalized().unwrap().perp() * 2.0; + let span = i * 32 + (j / 4).min(31); + let span = if forward { span } else { fitted.len() - 1 - span }; + let curve = fitted[span]; + max_error = max_error.max(curve.point_at(curve.closest_param(want)).dist(want)); + } + } + assert!(max_error < OFFSET_TOLERANCE, "normal-offset error {max_error}"); + for curve in &fitted { + for j in 0..=8 { + let p = curve.point_at(j as f64 / 8.0); + let distance = exact.iter().map(|s| s.point_at(s.closest_param(p)).dist(p)).fold(f64::INFINITY, f64::min); + assert!((distance - 2.0).abs() < OFFSET_TOLERANCE, "normal distance {distance}"); + } + } + let saved = serde_json::to_string(&sk).unwrap(); + let mut reopened: Sketch = serde_json::from_str(&saved).unwrap(); + reopened.refresh_offsets().unwrap(); + assert_eq!(reopened, sk); + } + } + + #[test] + fn geometric_certificate_aligns_speed_but_keeps_error_regularity_and_work_limits() { + let source = H::from_seg(&Seg2::Line { a: Vec2::ZERO, b: Vec2::X }).unwrap(); + // Analytically identical straight lines, traversed at different speeds. + let cubic = [Vec2::new(0.0, 2.0), Vec2::new(0.1, 2.0), Vec2::new(0.2, 2.0), Vec2::new(1.0, 2.0)]; + let mut budget = 65_536; + assert!(certify(&source, &cubic, 2.0, Certificate { tolerance: OFFSET_TOLERANCE, align_midpoints: false }, 0, &mut budget).is_err()); + let proof = Certificate { tolerance: OFFSET_TOLERANCE, align_midpoints: true }; + certify(&source, &cubic, 2.0, proof, 0, &mut budget).unwrap(); + assert!(certify(&source, &cubic, 2.0, proof, 0, &mut 0).is_err()); + let bowed = [cubic[0], cubic[1] + Vec2::Y, cubic[2] + Vec2::Y, cubic[3]]; + assert!(certify(&source, &bowed, 2.0, proof, 0, &mut budget).is_err()); + let stationary = H::from_seg(&Seg2::Line { a: Vec2::ZERO, b: Vec2::ZERO }).unwrap(); + assert!(certify(&stationary, &cubic, 2.0, proof, 0, &mut budget).is_err()); + } + fn ellipse() -> (Sketch, usize) { let mut sketch = Sketch::new(); let center = sketch.add_point(Vec2::ZERO, None).unwrap(); diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 7052e167..51d3f0f3 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -905,7 +905,7 @@ pub(crate) fn snaps_to_points(accept: Accept) -> bool { } /// Which of a sketch's points a face place snaps to (the Single hole's): the points of geometry -/// the sketch shows. Not a text outline's points, which are never drawn or picked on their own +/// the sketch shows. Not generated offset nodes or a text outline's points, which are never drawn or picked on their own /// (as `view::sketch_point_visible` has it), nor projected points while the sketch hides /// projected geometry (Show Projected off), whose curves aren't picked either. Hidden sketches /// offer no points at all. Show Points doesn't count: it only hides the markers of the sketch @@ -913,35 +913,17 @@ pub(crate) fn snaps_to_points(accept: Accept) -> bool { pub(crate) struct SnapPoints<'a> { /// Show Projected is off: no linked point is taken. hide_linked: bool, - /// The text sources' link ids (looked up once, however many points and links there are). - text: std::collections::HashSet<&'a str>, + shown: solidcraft_engine::view::SketchPointVisibility<'a>, } impl<'a> SnapPoints<'a> { - pub(crate) fn of(sk: &'a solidcraft_engine::sketch::Sketch) -> SnapPoints<'a> { - let hide_linked = sk.view.hide_projected; - let text = if hide_linked { - Default::default() - } else { - // An id names its first link, as `Sketch::link` finds it. - let mut seen = std::collections::HashSet::new(); - sk.links - .iter() - .filter(|l| seen.insert(l.id.as_str())) - .filter(|l| l.kind == solidcraft_engine::sketch::LinkKind::Text) - .map(|l| l.id.as_str()) - .collect() - }; - SnapPoints { hide_linked, text } + pub(crate) fn of(sk: &'a solidcraft_engine::sketch::Sketch) -> Self { + Self { hide_linked: sk.view.hide_projected, shown: solidcraft_engine::view::SketchPointVisibility::new(sk) } } /// Does a face place snap to point `p` of the sketch? pub(crate) fn takes(&self, p: &solidcraft_engine::sketch::SPoint) -> bool { - match p.link.as_deref() { - None => true, - Some(_) if self.hide_linked => false, - Some(l) => !self.text.contains(l), - } + self.shown.takes(p) && !(self.hide_linked && p.link.is_some()) } } diff --git a/crates/ui-egui/src/drag_snap.rs b/crates/ui-egui/src/drag_snap.rs index 5b3ce739..1cdc1359 100644 --- a/crates/ui-egui/src/drag_snap.rs +++ b/crates/ui-egui/src/drag_snap.rs @@ -90,7 +90,9 @@ pub fn find(sk: &Sketch, moving: &[(usize, Vec2)], exclude: &[usize], mm_px: f64 return keep(None); } let id = |i: usize| sk.points.get(i).map(|p| p.id.clone()).unwrap_or_default(); - let targets: Vec<(usize, Vec2)> = sk.points.iter().enumerate().filter(|(i, _)| !exclude.contains(i)).map(|(i, p)| (i, p.pos)).collect(); + let visible = solidcraft_engine::view::SketchPointVisibility::new(sk); + let targets: Vec<(usize, Vec2)> = + sk.points.iter().enumerate().filter(|(i, p)| !exclude.contains(i) && visible.takes(p)).map(|(i, p)| (i, p.pos)).collect(); let free_curves: Vec = (0..sk.curves.len()).filter(|&c| sk.curves.get(c).is_some_and(|cu| !exclude.iter().any(|&p| cu.kind.uses(p)))).collect(); let mut best: Option<(f64, Snap)> = None; diff --git a/crates/ui-egui/src/sketch_drag_ui_tests.rs b/crates/ui-egui/src/sketch_drag_ui_tests.rs index 85c12117..96c57de7 100644 --- a/crates/ui-egui/src/sketch_drag_ui_tests.rs +++ b/crates/ui-egui/src/sketch_drag_ui_tests.rs @@ -151,3 +151,96 @@ fn a_dragged_line_that_stops_short_stays_under_the_cursor_when_it_can_follow_aga assert!(start.dist(Vec2::new(x + 2.0, y)) < 0.1, "{start:?}"); assert!(!h.app.session.model.is_stale()); } + +#[test] +fn associated_spline_offset_hides_fit_nodes_and_follows_a_source_drag_live() { + let mut h = Harness::new(); + h.app.home.open = false; + run(&mut h, "sketch.create", json!({"plane":"XY"})); + run( + &mut h, + "sketch.spline.fit_point", + json!({ + "points":[[-17,15],[0,8],[17,13],[15,0],[5,-7],[-1,-14],[-9,-1],[-21,-1]], + "periodic":true + }), + ); + let output = h.app.run("sketch.offset", json!({"curves":["s1"],"distance":2,"chain":true})).unwrap()["curves"][0].as_str().unwrap().to_owned(); + h.call("ui.view", json!({"view":"top"})); + h.app.cam.distance = 65.0; + h.frames(3); + let sid = h.app.session.active_sketch.unwrap(); + let original = h.app.session.world_state().sketch(sid).unwrap().sketch.clone(); + let source = original.curve_index("s1").unwrap(); + let derived = original.curve_index(&output).unwrap(); + let source_points = original.curves[source].kind.point_ids(); + let derived_points = original.curves[derived].kind.point_ids(); + assert!(derived_points.len() > 100, "exercise the crowded generated fit nodes"); + for &i in &source_points { + assert!(solidcraft_engine::view::sketch_point_visible(&original, i)); + } + let snaps = crate::dialogs::SnapPoints::of(&original); + for &i in &derived_points { + assert!(!solidcraft_engine::view::sketch_point_visible(&original, i)); + assert!(!snaps.takes(&original.points[i])); + } + let p = original.points[derived_points[32]].pos; + let pos = screen(&mut h, p.x, p.y); + let proj = crate::viewport::projection(&h.app, h.app.viewport.rect.unwrap()); + let hits = crate::viewport::pick_all(&h.app, &proj, pos); + assert!(hits.iter().any(|hit| matches!(hit, crate::viewport::Hit::SketchCurve {id,..} if id == &output))); + assert!( + !hits + .iter() + .any(|hit| matches!(hit, crate::viewport::Hit::SketchPoint {id,..} if derived_points.iter().any(|i| original.points[*i].id == *id))) + ); + let snap = crate::drag_snap::find(&original, &[(source_points[1], p)], &source_points, 0.01); + assert!( + !matches!(snap.map(|s| s.target), Some(crate::drag_snap::Target::Point(id)) if derived_points.iter().any(|i| original.points[*i].id == id)) + ); + crate::drag_snap::clear(); + + let before = h.app.session.doc.to_json(); + let undo = h.app.session.undo.len(); + let a = screen(&mut h, 0.0, 8.0); + let b = screen(&mut h, 1.0, 9.0); + let c = screen(&mut h, 2.0, 10.0); + let mut path = vec![a]; + path.extend(steps(a, b, 4)); + press_and_move(&mut h, &path); + for (pos, target) in [(b, Vec2::new(1.0, 9.0)), (c, Vec2::new(2.0, 10.0))] { + h.app.synthetic.push(egui::Event::PointerMoved(pos)); + play(&mut h); + assert!(h.app.viewport.point_drag.is_some()); + assert!(h.app.session.model.is_stale(), "the drag has not been released"); + let state = h.app.session.world_state(); + let sk = &state.sketch(sid).unwrap().sketch; + assert!(sk.points[source_points[1]].pos.dist(target) < 0.1, "source follows the held pointer: {:?}", sk.points[source_points[1]].pos); + assert_ne!(sk.exact_segs(derived), original.exact_segs(derived), "the offset follows before release"); + assert_eq!(sk.freeform_offsets[0].distance, 2.0); + sk.validate_freeform_offsets().unwrap(); + let points = sk.curves[derived].kind.point_ids(); + assert!(points.iter().all(|i| !solidcraft_engine::view::sketch_point_visible(sk, *i))); + for piece in sk.exact_segs(derived).unwrap() { + for t in [0.0, 0.25, 0.5, 0.75, 1.0] { + let p = piece.point_at(t); + let distance = sk.exact_segs(source).unwrap().iter().map(|s| s.point_at(s.closest_param(p)).dist(p)).fold(f64::INFINITY, f64::min); + assert!((distance - 2.0).abs() < 0.002, "live offset normal distance {distance}"); + } + } + } + release(&mut h, c); + assert!(!h.app.session.model.is_stale()); + assert_eq!(h.app.session.undo.len(), undo + 1); + run(&mut h, "edit.undo", json!({})); + assert_eq!(h.app.session.doc.to_json(), before); + run(&mut h, "edit.redo", json!({})); + run(&mut h, "sketch.offset.break", json!({"curve":output})); + let state = h.app.session.world_state(); + let sk = &state.sketch(sid).unwrap().sketch; + assert!(sk.curves[derived].kind.point_ids().iter().all(|i| solidcraft_engine::view::sketch_point_visible(sk, *i))); + run(&mut h, "edit.undo", json!({})); + let state = h.app.session.world_state(); + let sk = &state.sketch(sid).unwrap().sketch; + assert!(sk.curves[derived].kind.point_ids().iter().all(|i| !solidcraft_engine::view::sketch_point_visible(sk, *i))); +} diff --git a/crates/ui-egui/src/viewport.rs b/crates/ui-egui/src/viewport.rs index 0ec95df8..4fe90b95 100644 --- a/crates/ui-egui/src/viewport.rs +++ b/crates/ui-egui/src/viewport.rs @@ -853,8 +853,9 @@ fn pick_hits(app: &SolveApp, proj: &Proj, pos: Pos2, other_sketch_points: bool, for ss in point_sketches.iter().filter_map(|sid| st.sketch(*sid)) { let sid = ss.feature; let shown = snapping.then(|| crate::dialogs::SnapPoints::of(&ss.sketch)); + let visible = solidcraft_engine::view::SketchPointVisibility::new(&ss.sketch); for p in &ss.sketch.points { - if shown.as_ref().is_some_and(|f| !f.takes(p)) { + if !visible.takes(p) || shown.as_ref().is_some_and(|f| !f.takes(p)) { continue; } if let Some(sp) = proj.to_screen(ss.plane.to_world(p.pos)) { @@ -2586,10 +2587,11 @@ fn box_select(app: &mut SolveApp, proj: &Proj, bx: BoxSel, add: bool) { out.push(Sel::SketchCurve { id: c.id.clone() }); } } + let visible = solidcraft_engine::view::SketchPointVisibility::new(&ss.sketch); for (i, p) in ss.sketch.points.iter().enumerate().filter(|_| !off("sketch") && !off("sketch_points")) { if accepts.is_none_or(|a| a & crate::selection::SKETCH_POINTS != 0) && i != 0 - && solidcraft_engine::view::sketch_point_visible(&ss.sketch, i) + && visible.takes(p) && bx.takes(&to2(&[ss.plane.to_world(p.pos)])) { out.push(Sel::SketchPoint { id: p.id.clone() }); @@ -2763,8 +2765,9 @@ fn overlays(app: &SolveApp, painter: &egui::Painter, proj: &Proj) { let sk = &ss.sketch; let locked = sk.locked_points(); let virtual_corners: Vec = sk.virtual_corners().into_iter().map(|(i, _)| i).collect(); + let visible = solidcraft_engine::view::SketchPointVisibility::new(sk); for (i, p) in sk.points.iter().enumerate() { - if !solidcraft_engine::view::sketch_point_visible(sk, i) { + if !visible.takes(p) { continue; } let hidden = (sk.view.hide_points && i != 0) || (sk.view.hide_projected && p.link.is_some()); diff --git a/crates/ui-egui/tests/scenarios/sketch_offset_irregular_spline.json b/crates/ui-egui/tests/scenarios/sketch_offset_irregular_spline.json new file mode 100644 index 00000000..2054f38a --- /dev/null +++ b/crates/ui-egui/tests/scenarios/sketch_offset_irregular_spline.json @@ -0,0 +1,22 @@ +[ + {"note": "#971: a 2 mm offset of this closed irregular fit spline used to exhaust its node limit with an endpoint certificate error. Preview must succeed, Cancel must preserve the source, and OK must commit the preview."}, + {"call": "engine.execute", "params": {"command": "sketch.create", "params": {"plane": "XY"}}}, + {"call": "engine.execute", "params": {"command": "sketch.spline.fit_point", "params": {"points": [[-17,15],[0,8],[17,13],[15,0],[5,-7],[-1,-14],[-9,-1],[-21,-1]], "periodic": true}}}, + {"call": "ui.view", "params": {"view": "top"}}, + {"start": "sketch.offset"}, + {"click": {"sketch": [0,8]}}, + {"until": {"count": {"name": "sketch_preview_curves", "value": 1}}}, + {"expect": {"dialog": "Offset", "sketch_curves": {"own": 1}, "errors": 0}}, + {"key": "Escape"}, + {"expect": {"dialog": null, "sketch_curves": {"own": 1}, "errors": 0}}, + {"start": "sketch.offset"}, + {"click": {"sketch": [0,8]}}, + {"until": {"count": {"name": "sketch_preview_curves", "value": 1}}}, + {"key": "Enter"}, + {"expect": {"dialog": null, "sketch_curves": {"own": 2}, "errors": 0}}, + {"expect": {"query": {"command": "sketch.inspect", "path": ["curves", 1, "periodic"], "value": true}}}, + {"call": "engine.execute", "params": {"command": "edit.undo", "params": {}}}, + {"expect": {"sketch_curves": {"own": 1}, "errors": 0}}, + {"call": "engine.execute", "params": {"command": "edit.redo", "params": {}}}, + {"expect": {"sketch_curves": {"own": 2}, "errors": 0}} +]