BREAK opens a closed polyline instead of refusing it (fixes #1173) (#1191)

crates/engine/src/cmd/modify.rs

@@ -1478,8 +1478,8 @@ pub(crate) fn break_entity(s: &mut Session, h: Handle, p1: Vec2, p2: Vec2) -> Re
             }
             v
         }
-        EntityKind::LwPolyline(p) if !p.closed => {
-            let pl = Polyline { vertices: p.vertices.clone(), closed: false };
+        EntityKind::LwPolyline(p) => {
+            let pl = Polyline { vertices: p.vertices.clone(), closed: p.closed };
             let segs = pl.segments();
             let total: f64 = segs.iter().map(Segment::len).sum();
             let lp = |q: Vec2| {
@@ -1496,17 +1496,28 @@ pub(crate) fn break_entity(s: &mut Session, h: Handle, p1: Vec2, p2: Vec2) -> Re
                 best.1
             };
             let (mut t1, mut t2) = (lp(p1), lp(p2));
-            if t1 > t2 {
-                std::mem::swap(&mut t1, &mut t2);
+            if p.closed {
+                if (t1 - t2).abs() < 1e-9 {
+                    return Err(EngineError::Other("Cannot break a closed polyline at a single point.".into()));
+                }
+                // Like a circle: the stretch from the first point to the second, along the
+                // vertices, goes; what is left runs from the second point round to the first.
+                let doubled: Vec<Segment> = segs.iter().chain(segs.iter()).copied().collect();
+                let end = if t1 > t2 { t1 } else { t1 + total };
+                vec![lwpoly(sub_polyline(&doubled, t2, end), false)]
+            } else {
+                if t1 > t2 {
+                    std::mem::swap(&mut t1, &mut t2);
+                }
+                let mut v = Vec::new();
+                if t1 > 1e-12 {
+                    v.push(lwpoly(sub_polyline(&segs, 0.0, t1), false));
+                }
+                if t2 < total - 1e-12 {
+                    v.push(lwpoly(sub_polyline(&segs, t2, total), false));
+                }
+                v
             }
-            let mut v = Vec::new();
-            if t1 > 1e-12 {
-                v.push(lwpoly(sub_polyline(&segs, 0.0, t1), false));
-            }
-            if t2 < total - 1e-12 {
-                v.push(lwpoly(sub_polyline(&segs, t2, total), false));
-            }
-            v
         }
         _ => return Err(EngineError::Other("Cannot break that object.".into())),
     };

crates/engine/tests/break_closed_polyline.rs

@@ -0,0 +1,51 @@
+//! BREAK on a closed polyline removes the stretch from the first to the second point along the
+//! vertex direction and leaves one open polyline (#1173).
+
+use cadcraft_doc::{EntityKind, LwPolyline};
+use cadcraft_engine::Session;
+use serde_json::{Value, json};
+
+fn polys(s: &Session) -> Vec<LwPolyline> {
+    s.doc()
+        .unwrap()
+        .model
+        .iter()
+        .filter_map(|e| match &e.kind {
+            EntityKind::LwPolyline(p) => Some(p.clone()),
+            _ => None,
+        })
+        .collect()
+}
+
+fn pts(p: &LwPolyline) -> Vec<(f64, f64)> {
+    p.vertices.iter().map(|v| (v.p.x, v.p.y)).collect()
+}
+
+fn closed_rect(s: &mut Session) -> Value {
+    s.execute("pline", &json!({ "points": [[0, 0], [100, 0], [100, 60], [0, 60]], "closed": true })).unwrap()
+}
+
+#[test]
+fn break_opens_a_closed_polyline_at_the_command_line() {
+    let mut s = Session::new();
+    s.cmdline("PLINE 0,0 100,0 100,60 0,60 C").unwrap();
+    s.cmdline("BREAK 50,0 100,30").unwrap();
+    assert!(s.running.is_none(), "{}", s.prompt_text());
+    let ps = polys(&s);
+    assert_eq!(ps.len(), 1);
+    assert!(!ps[0].closed);
+    assert_eq!(pts(&ps[0]), vec![(100.0, 30.0), (100.0, 60.0), (0.0, 60.0), (0.0, 0.0), (50.0, 0.0)]);
+}
+
+#[test]
+fn break_json_on_a_closed_polyline_follows_the_order_of_the_points() {
+    let mut s = Session::new();
+    let r = closed_rect(&mut s);
+    // Second point before the first along the vertices: the long way round goes, the short
+    // stretch between them stays.
+    s.execute("break", &json!({ "handle": r["handle"], "p1": [100, 30], "p2": [50, 0] })).unwrap();
+    let ps = polys(&s);
+    assert_eq!(ps.len(), 1);
+    assert!(!ps[0].closed);
+    assert_eq!(pts(&ps[0]), vec![(50.0, 0.0), (100.0, 0.0), (100.0, 30.0)]);
+}

docs/geometry-parity.md

@@ -84,7 +84,7 @@ constraints 10%.
 | EXTEND | every open curve | line, arc, polyline, elliptical arc, open spline (runs on along its end tangent, `modify2/curve_edits.rs`) |
 | FILLET | lines, arcs, circles, polylines, ellipses, splines, xlines, rays; Trim/No trim | lines, arcs, circles (TTR circle), polyline mode, Trim/No trim, Multiple; elliptical arcs and splines with any of these (tangent circle from sampled offsets refined by Newton's method, `geom/src/fillet.rs`; arcs trimmed or extended round the ellipse, splines split at the touch point); two straight segments of one polyline (adjacent, or with one between that the corner replaces), a polyline's straight end segment with a line (the line joins it); not polyline arc segments, xlines or rays |
 | CHAMFER | lines, polylines, xlines, rays | lines, Distance/Angle, whole polylines (Polyline option), polyline segments as for FILLET (`modify2/fillet_pline.rs`); refuses arcs, ellipses and splines with a message, as AutoCAD; not xlines or rays |
-| BREAK | every curve | line, arc, circle, polyline |
+| BREAK | every curve | line, arc, circle, polyline; a closed polyline loses the stretch from the first to the second point along its vertices and is left open (#1173) |
 | JOIN | lines, arcs, polylines, ellipses, splines, helixes | line, arc, polyline into a polyline; elliptical arcs on one ellipse into a larger arc or the full ellipse (gaps close); splines with lines, arcs, polylines and elliptical arcs into one exact NURBS (degree elevation, rational arcs; `geom/src/nurbs.rs`); no helixes |
 | LENGTHEN | lines, arcs, polylines, elliptical arcs, splines | line, arc, polyline; elliptical arc (arc length along the ellipse); open spline (cut along the curve, or run on along its end tangent) |