Merge pull request #121 from CypherNaught-0x/fix/fillet-keeps-dimensions

Fillet and chamfer keep the dimensions of the corner they replace on a virtual sharp corner

crates/engine/src/cmd/sketch_corner_tests.rs

@@ -96,3 +96,90 @@ fn fillet_and_chamfer_corners() {
     assert!(solved(&s));
     assert!((area(&s) - (a0 - 8.0)).abs() < 1e-6, "{}", area(&s));
 }
+
+/// The ids of the constraints driven by the model parameter `name`.
+fn driven_by(s: &Session, name: &str) -> Vec<String> {
+    sk(s).constraints.iter().filter(|c| c.param.as_deref() == Some(name)).map(|c| c.id.clone()).collect()
+}
+
+/// The spout of a basin: a vertical riser and a horizontal top, the top's length dimensioned
+/// to the corner and the corner's height dimensioned from the origin. Filleting the corner used
+/// to drop the height and leave the length on the shortened straight part, so the top came out
+/// a fillet radius too long. Like Fusion, the corner now stays as a virtual sharp corner (a point
+/// on both lines' extensions) and the dimensions measure to it.
+#[test]
+fn a_fillet_keeps_the_dimensions_of_its_corner_on_a_virtual_sharp() {
+    let mut s = new_sketch();
+    run(&mut s, "sketch.line", json!({"points": [[0, 0], [0, 100], [-80, 100]], "ids": ["riser", "top"]}));
+    run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "riser", "mode": "vertical"}));
+    run(&mut s, "sketch.constraint.horizontal_vertical", json!({"line": "top", "mode": "horizontal"}));
+    let height = run(&mut s, "sketch.dimension", json!({"entities": ["origin", "riser.end"], "type": "vertical", "value": 100}));
+    let height = height["param"].as_str().unwrap().to_string();
+    let len = run(&mut s, "sketch.dimension", json!({"entities": ["top"], "value": 80}));
+    let len = len["param"].as_str().unwrap().to_string();
+    let (height_ids, len_ids) = (driven_by(&s, &height), driven_by(&s, &len));
+    let f = run(&mut s, "sketch.fillet", json!({"point": "riser.end", "radius": 20}));
+    assert!(solved(&s), "{f}");
+    // Both dimensions are kept (same ids) and reported as moved to the virtual corner.
+    assert_eq!(driven_by(&s, &height), height_ids);
+    assert_eq!(driven_by(&s, &len), len_ids);
+    let corner = f["virtual_corners"][0].as_str().unwrap_or_default().to_string();
+    assert!(!corner.is_empty(), "{f}");
+    let moved: Vec<String> = f["moved_to_virtual_corner"].as_array().unwrap().iter().filter_map(|v| v.as_str().map(str::to_string)).collect();
+    for id in height_ids.iter().chain(&len_ids) {
+        assert!(moved.contains(id), "{id}: {f}");
+    }
+    // The virtual corner sits where the corner was, and the part keeps its size.
+    let k = sk(&s);
+    let vc = k.point(k.point_index(&corner).unwrap()).unwrap();
+    assert!(vc.dist(solvecraft_geom::Vec2::new(0.0, 100.0)) < 1e-6, "{vc:?}");
+    let top_end = k.point(k.resolve_point("top.end").unwrap()).unwrap();
+    assert!(top_end.dist(solvecraft_geom::Vec2::new(-80.0, 100.0)) < 1e-6, "{top_end:?}");
+    let (lo, hi) = k.bounds().unwrap();
+    assert!((hi.x - lo.x - 80.0).abs() < 1e-6 && (hi.y - lo.y - 100.0).abs() < 1e-6, "{lo:?} {hi:?}");
+    // It is drawn with extension lines from both lines' ends to it.
+    let vcs = k.virtual_corners();
+    assert_eq!(vcs.len(), 1, "{vcs:?}");
+    assert_eq!(vcs[0].1.len(), 2, "{vcs:?}");
+    assert!(vcs[0].1.iter().all(|(a, b)| a.dist(vc) < 1e-9 && (b.dist(vc) - 20.0).abs() < 1e-6), "{vcs:?}");
+    // The dimensions still drive the part, measured to the virtual corner.
+    run(&mut s, "parameters.change", json!({"name": len, "expression": "90 mm"}));
+    run(&mut s, "parameters.change", json!({"name": height, "expression": "120 mm"}));
+    assert!(solved(&s));
+    let (lo, hi) = sk(&s).bounds().unwrap();
+    assert!((hi.x - lo.x - 90.0).abs() < 1e-6 && (hi.y - lo.y - 120.0).abs() < 1e-6, "{lo:?} {hi:?}");
+    // It is kept in the design file, and undo and redo take it away and bring it back.
+    let id = s.active_sketch.unwrap();
+    let doc = solvecraft_doc::Document::from_json(&s.doc.to_json()).unwrap();
+    assert_eq!(doc.sketch(id).unwrap(), s.doc.sketch(id).unwrap());
+    run(&mut s, "edit.undo", json!({}));
+    run(&mut s, "edit.undo", json!({}));
+    run(&mut s, "edit.undo", json!({}));
+    assert!(sk(&s).point_index(&corner).is_none());
+    run(&mut s, "edit.redo", json!({}));
+    assert!(sk(&s).point_index(&corner).is_some());
+    // A fillet without anything measuring to its corner leaves no virtual corner behind.
+    let mut s = new_sketch();
+    run(&mut s, "sketch.line", json!({"points": [[10, 10], [10, 100], [-80, 100]], "ids": ["riser", "top"]}));
+    let n = sk(&s).points.len();
+    let f = run(&mut s, "sketch.fillet", json!({"point": "riser.end", "radius": 20}));
+    assert_eq!(f["virtual_corners"], json!([]), "{f}");
+    // Corner gone, two tangent points and the arc's centre added.
+    assert_eq!(sk(&s).points.len(), n + 2);
+}
+
+/// Lines meeting almost straight or almost folded back: a fillet or chamfer with dimensions on
+/// the corner either works or refuses with an error, never a panic.
+#[test]
+fn a_fillet_of_a_nearly_straight_or_folded_corner_with_dimensions_does_not_panic() {
+    for (x, y) in [(100.0, 0.05), (100.0, -0.05), (100.0, 1e-9), (-100.0, 0.05), (-100.0, 1e-9)] {
+        let mut s = new_sketch();
+        run(&mut s, "sketch.line", json!({"points": [[0, 0], [100, 0], [100.0 + x, y]], "ids": ["a", "b"]}));
+        run(&mut s, "sketch.dimension", json!({"entities": ["a"], "value": 100}));
+        for r in [0.01, 1.0, 50.0] {
+            let _ = s.execute("sketch.fillet", &json!({"point": "a.end", "radius": r}));
+            let _ = s.execute("sketch.chamfer.equal_distance", &json!({"point": "a.end", "distance": r}));
+        }
+        assert!(sk(&s).points.iter().all(|p| p.pos.is_finite()));
+    }
+}

crates/engine/src/cmd/sketch_modify.rs

@@ -20,7 +20,7 @@ pub static COMMANDS: &[CommandSpec] = &[
         .at("SKETCH", "MODIFY")
         .icon("sketch_fillet")
         .enabled(in_sketch)
-        .params("point: corner shared by two curves (\"l1.end\") | points: [corners] (all rounded alike: one radius, the rest equal) | a, b: two lines or arcs meeting at a corner; radius: expr"),
+        .params("point: corner shared by two curves (\"l1.end\") | points: [corners] (all rounded alike: one radius, the rest equal) | a, b: two lines or arcs meeting at a corner; radius: expr. A corner that dimensions or constraints measure to stays as a virtual sharp corner on both curves' extensions (`virtual_corners`; the moved ones in `moved_to_virtual_corner`)"),
     CommandSpec::new("sketch.chamfer.equal_distance", "Equal Distance Chamfer", chamfer_equal)
         .at("SKETCH", "MODIFY")
         .icon("sketch_chamfer")
@@ -505,8 +505,71 @@ fn reattach(sk: &mut Sketch, l: usize, corner: usize, np: usize) {
     }
 }
 
+/// What became of a rounded or cut corner's point: kept as a virtual sharp corner (its id),
+/// and the constraints and dimensions that now measure to it.
+#[derive(Default)]
+struct CornerKept {
+    virtual_corner: Option<String>,
+    moved: Vec<String>,
+}
+
+/// Keep corner point `c` of curves `ca`, `cb` (whose corner ends were just moved to `na`, `nb`)
+/// when anything measures to it: a constraint or dimension on the point, the length of either
+/// line, or it is the origin or still ends another curve. It stays as a virtual sharp corner,
+/// held on both curves' extensions (their intersection), and the line lengths become distances
+/// from the far end to it, so the part keeps its size, as in Fusion. Otherwise None: the
+/// caller removes the point.
+fn keep_corner(sk: &mut Sketch, c: usize, [(ca, na), (cb, nb)]: [(usize, usize); 2]) -> Result<Option<CornerKept>> {
+    let used = sk.curves.iter().any(|x| uses(x, c));
+    let mut moved: Vec<String> = sk.constraints.iter().filter(|k| k.kind.refers_to_point(c)).map(|k| k.id.clone()).collect();
+    for k in sk.constraints.iter_mut() {
+        let ConstraintKind::Length { l, value } = k.kind else { continue };
+        let new_end = if l == ca {
+            na
+        } else if l == cb {
+            nb
+        } else {
+            continue;
+        };
+        let Some(CurveKind::Line { a, b }) = sk.curves.get(l).map(|x| &x.kind) else { continue };
+        // Same direction as the line, so the dimension keeps its side and text place.
+        k.kind = if *a == new_end { ConstraintKind::Distance { p: c, q: *b, value } } else { ConstraintKind::Distance { p: *a, q: c, value } };
+        moved.push(k.id.clone());
+    }
+    if moved.is_empty() && !used && c != 0 {
+        return Ok(None);
+    }
+    for cu in [ca, cb] {
+        let k = ConstraintKind::PointOnCurve { p: c, c: cu };
+        if !sk.constraints.iter().any(|x| x.kind == k) {
+            add_c(sk, k)?;
+        }
+    }
+    // A point of its own now: `l1.end` names the line's new end, not the virtual corner.
+    let virtual_corner = if used || c == 0 {
+        None
+    } else {
+        let id = sk.fresh("corner");
+        if let Some(pt) = sk.points.get_mut(c) {
+            pt.id = id.clone();
+        }
+        Some(id)
+    };
+    Ok(Some(CornerKept { virtual_corner, moved }))
+}
+
 /// Points at distances `da` along a and `db` along b from the corner; lines shortened to them.
-fn cut_corner(sk: &mut Sketch, la: usize, lb: usize, c: usize, fa: usize, fb: usize, da: f64, db: f64, cmd: &str) -> Result<(usize, usize, Vec2)> {
+fn cut_corner(
+    sk: &mut Sketch,
+    la: usize,
+    lb: usize,
+    c: usize,
+    fa: usize,
+    fb: usize,
+    da: f64,
+    db: f64,
+    cmd: &str,
+) -> Result<(usize, usize, Vec2, CornerKept)> {
     let (pc, pa, pb) = (sk.point(c).unwrap_or_default(), sk.point(fa).unwrap_or_default(), sk.point(fb).unwrap_or_default());
     if !(da > 1e-9 && db > 1e-9 && da < pc.dist(pa) - 1e-9 && db < pc.dist(pb) - 1e-9) {
         return Err(bad(cmd, "too large for these lines"));
@@ -517,13 +580,14 @@ fn cut_corner(sk: &mut Sketch, la: usize, lb: usize, c: usize, fa: usize, fb: us
     let tb = sk.add_point(pc + v * db, None)?;
     reattach(sk, la, c, ta);
     reattach(sk, lb, c, tb);
-    let used = sk.curves.iter().any(|x| uses(x, c));
-    if !used && c != 0 {
+    let kept = keep_corner(sk, c, [(la, ta), (lb, tb)])?;
+    let removed = kept.is_none();
+    if removed {
         sk.remove_points(&[c]);
     }
-    // Indices may have shifted when the corner point was removed.
-    let fix = |i: usize| if !used && c != 0 && i > c { i - 1 } else { i };
-    Ok((fix(ta), fix(tb), pc))
+    // Indices shift when the corner point was removed.
+    let fix = |i: usize| if removed && i > c { i - 1 } else { i };
+    Ok((fix(ta), fix(tb), pc, kept.unwrap_or_default()))
 }
 
 fn fillet(s: &mut Session, p: &Value) -> Result<Value> {
@@ -541,9 +605,10 @@ fn fillet(s: &mut Session, p: &Value) -> Result<Value> {
             return Err(bad(cmd, "radius must be positive"));
         }
         let pname = doc.new_model_param(&r_expr, "mm");
-        let (mut arcs, mut cons) = (Vec::new(), Vec::new());
+        let (mut arcs, mut cons, mut kept) = (Vec::new(), Vec::new(), Vec::new());
         for q in &corners {
-            let (arc, mut c) = fillet_corner(sk, q, r, cmd)?;
+            let (arc, mut c, k) = fillet_corner(sk, q, r, cmd)?;
+            kept.push(k);
             // One radius for them all: the first carries the dimension, the rest are equal to it.
             match arcs.first() {
                 None => c.push(sk.add_constraint(ConstraintKind::Radius { c: arc, value: r }, Some(pname.clone()))?),
@@ -552,14 +617,27 @@ fn fillet(s: &mut Session, p: &Value) -> Result<Value> {
             cons.extend(c);
             arcs.push(arc);
         }
-        Ok((ids_of(sk, &arcs), cons, pname))
+        Ok((ids_of(sk, &arcs), cons, pname, kept_json(kept)))
     })?;
-    Ok(json!({"curves": out.0, "constraints": out.1, "param": out.2, "sketch": info}))
+    let (vc, moved) = out.3;
+    Ok(json!({"curves": out.0, "constraints": out.1, "param": out.2, "virtual_corners": vc, "moved_to_virtual_corner": moved, "sketch": info}))
+}
+
+/// The virtual corners kept and the constraints moved to them, for a response.
+fn kept_json(kept: Vec<CornerKept>) -> (Vec<String>, Vec<String>) {
+    let vcs = kept.iter().filter_map(|k| k.virtual_corner.clone()).collect();
+    let mut moved: Vec<String> = Vec::new();
+    for id in kept.into_iter().flat_map(|k| k.moved) {
+        if !moved.contains(&id) {
+            moved.push(id);
+        }
+    }
+    (vcs, moved)
 }
 
 /// Round one corner (`point` or `a`, `b`) with an arc of radius `r` tangent to both curves:
-/// the arc and the tangent constraints.
-fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize, Vec<String>)> {
+/// the arc, the tangent constraints and what became of the corner point.
+fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize, Vec<String>, CornerKept)> {
     let (la, lb, c, fa, fb) = match corner(sk, p, cmd) {
         Ok(x) => x,
         // Not two lines: a corner with an arc.
@@ -578,7 +656,7 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize
     }
     let bis = (u + v).normalized().ok_or_else(|| bad(cmd, "the lines are parallel"))?;
     let center = pc + bis * (r / half.sin());
-    let (ta, tb, _) = cut_corner(sk, la, lb, c, fa, fb, d, d, cmd)?;
+    let (ta, tb, _, kept) = cut_corner(sk, la, lb, c, fa, fb, d, d, cmd)?;
     let (qa, qb) = (sk.point(ta).unwrap_or_default(), sk.point(tb).unwrap_or_default());
     let ccw = (qa - center).cross(qb - center) > 0.0;
     let arc = if ccw {
@@ -587,13 +665,13 @@ fn fillet_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<(usize
         sk.add_arc(center, qb, qa, [None, Some(tb), Some(ta)], None)?
     };
     let cons = vec![add_c(sk, ConstraintKind::Tangent { a: la, b: arc })?, add_c(sk, ConstraintKind::Tangent { a: lb, b: arc })?];
-    Ok((arc, cons))
+    Ok((arc, cons, kept))
 }
 
 /// Fillet a corner where a line meets an arc, or two arcs meet: the arc of radius `r` touching
 /// both, nearest the corner; both curves are cut back to it. None when the corner is not of
 /// that kind.
-fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<Option<(usize, Vec<String>)>> {
+fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<Option<(usize, Vec<String>, CornerKept)>> {
     let ends = |c: usize| match sk.curves.get(c).map(|c| &c.kind) {
         Some(CurveKind::Line { a, b }) | Some(CurveKind::Arc { a, b, .. }) => Some((*a, *b)),
         _ => None,
@@ -655,14 +733,15 @@ fn round_corner(sk: &mut Sketch, p: &Value, r: f64, cmd: &str) -> Result<Option<
     let arc =
         if ccw { sk.add_arc(cen, ta, tb, [None, Some(na), Some(nb)], None)? } else { sk.add_arc(cen, tb, ta, [None, Some(nb), Some(na)], None)? };
     let cons = vec![add_c(sk, ConstraintKind::Tangent { a: ca, b: arc })?, add_c(sk, ConstraintKind::Tangent { a: cb, b: arc })?];
-    let used = sk.curves.iter().any(|x| x.kind.uses(q));
-    if !used && q != 0 {
-        let arc_id = sk.curves.get(arc).map(|c| c.id.clone()).unwrap_or_default();
-        sk.remove_points(&[q]);
-        let arc = sk.curve_index(&arc_id).unwrap_or(arc);
-        return Ok(Some((arc, cons)));
+    match keep_corner(sk, q, [(ca, na), (cb, nb)])? {
+        Some(kept) => Ok(Some((arc, cons, kept))),
+        None => {
+            let arc_id = sk.curves.get(arc).map(|c| c.id.clone()).unwrap_or_default();
+            sk.remove_points(&[q]);
+            let arc = sk.curve_index(&arc_id).unwrap_or(arc);
+            Ok(Some((arc, cons, CornerKept::default())))
+        }
     }
-    Ok(Some((arc, cons)))
 }
 
 fn chamfer(s: &mut Session, p: &Value, cmd: &str, dists: impl FnOnce(f64) -> Result<(f64, f64)>) -> Result<Value> {
@@ -672,11 +751,12 @@ fn chamfer(s: &mut Session, p: &Value, cmd: &str, dists: impl FnOnce(f64) -> Res
         let (u, v) = ((pa - pc).normalized().unwrap_or(Vec2::X), (pb - pc).normalized().unwrap_or(Vec2::Y));
         let corner_angle = u.cross(v).atan2(u.dot(v)).abs();
         let (da, db) = dists(corner_angle)?;
-        let (ta, tb, _) = cut_corner(sk, la, lb, c, fa, fb, da, db, cmd)?;
+        let (ta, tb, _, kept) = cut_corner(sk, la, lb, c, fa, fb, da, db, cmd)?;
         let l = sk.add_line_pts(ta, tb, None)?;
-        Ok(ids_of(sk, &[l]))
+        Ok((ids_of(sk, &[l]), kept_json(vec![kept])))
     })?;
-    Ok(json!({"curves": out, "sketch": info}))
+    let (curves, (vc, moved)) = out;
+    Ok(json!({"curves": curves, "virtual_corners": vc, "moved_to_virtual_corner": moved, "sketch": info}))
 }
 
 fn chamfer_equal(s: &mut Session, p: &Value) -> Result<Value> {

crates/engine/src/view.rs

@@ -95,6 +95,13 @@ pub fn sketch_lines(sk: &Sketch, plane: &Plane, active: bool, determined: &[bool
             out.push((seg.polyline(0.02).iter().map(|p| plane.to_world(*p)).collect(), col, c.construction || c.centerline));
         }
     }
+    // Virtual sharp corners (kept by fillets and chamfers): thin extension lines from the
+    // curves to the corner, like construction geometry.
+    for (_, ext) in sk.virtual_corners() {
+        for (a, b) in ext {
+            out.push((vec![plane.to_world(a), plane.to_world(b)], colors::SKETCH_CONSTRUCTION, true));
+        }
+    }
     // 3D curves are in world coordinates already.
     for w in sk.wires.iter().filter(|w| !w.fit.is_empty() || !sk.view.hide_projected) {
         // Curves drawn in a 3D sketch look like the sketch's own; included ones are references.

crates/sketch/src/model.rs

@@ -361,6 +361,10 @@ impl ConstraintKind {
             LinearDiameter { .. } => "LinearDiameterDimension",
         }
     }
+    /// Does the constraint refer to point `p` (by index)?
+    pub fn refers_to_point(&self, p: usize) -> bool {
+        self.indices().0.contains(&p)
+    }
     fn indices(&self) -> (Vec<usize>, Vec<usize>) {
         use ConstraintKind::*;
         match *self {
@@ -892,6 +896,48 @@ impl Sketch {
         out
     }
 
+    /// Virtual sharp corners (what a fillet or chamfer leaves where dimensions measured to the
+    /// corner): points used by no curve, held on two or more lines or arcs (their extensions).
+    /// Each with the extension segments from the corner to the nearest end of those curves, for
+    /// drawing.
+    pub fn virtual_corners(&self) -> Vec<(usize, Vec<(Vec2, Vec2)>)> {
+        let mut out = Vec::new();
+        for (i, p) in self.points.iter().enumerate().skip(1) {
+            if self.point_used_by_curve(i) {
+                continue;
+            }
+            let on: Vec<usize> = self
+                .constraints
+                .iter()
+                .filter_map(|c| match c.kind {
+                    ConstraintKind::PointOnCurve { p, c } if p == i => Some(c),
+                    _ => None,
+                })
+                .filter(|c| matches!(self.curves.get(*c).map(|c| &c.kind), Some(CurveKind::Line { .. } | CurveKind::Arc { .. })))
+                .collect();
+            if on.len() < 2 {
+                continue;
+            }
+            let mut ext = Vec::new();
+            for c in on {
+                let Some((a, b)) = self.curves.get(c).and_then(|c| c.kind.ends()) else { continue };
+                let (Some(pa), Some(pb)) = (self.point(a), self.point(b)) else { continue };
+                let near = if p.pos.dist(pa) <= p.pos.dist(pb) { pa } else { pb };
+                // A line only needs one when the corner is beyond its ends.
+                let inside = matches!(self.curves.get(c).map(|c| &c.kind), Some(CurveKind::Line { .. })) && {
+                    let d = pb - pa;
+                    let t = (p.pos - pa).dot(d) / d.dot(d).max(1e-300);
+                    (0.0..=1.0).contains(&t)
+                };
+                if !inside && p.pos.dist(near) > 1e-9 {
+                    ext.push((p.pos, near));
+                }
+            }
+            out.push((i, ext));
+        }
+        out
+    }
+
     /// Is curve `c` held in place: fixed or projected (linked).
     pub fn curve_locked(&self, c: usize) -> bool {
         self.curves.get(c).is_some_and(|c| c.fixed || c.link.is_some() || self.freeform_offsets.iter().any(|offset| offset.output == c.id))

crates/ui-egui/src/viewport.rs

@@ -2112,6 +2112,7 @@ fn overlays(app: &SolveApp, painter: &egui::Painter, proj: &Proj) {
     let Some(ss) = st.sketch(sid) else { return };
     let sk = &ss.sketch;
     let locked = sk.locked_points();
+    let virtual_corners: Vec<usize> = sk.virtual_corners().into_iter().map(|(i, _)| i).collect();
     for (i, p) in sk.points.iter().enumerate() {
         if !solvecraft_engine::view::sketch_point_visible(sk, i) {
             continue;
@@ -2128,6 +2129,9 @@ fn overlays(app: &SolveApp, painter: &egui::Painter, proj: &Proj) {
         };
         if i == 0 {
             painter.circle(sp, 3.5, Color32::from_rgb(240, 200, 60), Stroke::new(1.0, t.body_edge));
+        } else if virtual_corners.contains(&i) {
+            // A virtual sharp corner: an open square, it is not on any drawn curve.
+            painter.rect_stroke(Rect::from_center_size(sp, vec2(6.0, 6.0)), 0.0, Stroke::new(1.2, c), egui::StrokeKind::Middle);
         } else {
             painter.rect_filled(Rect::from_center_size(sp, vec2(5.0, 5.0)), 0.0, c);
         }