From 8e0b867f6cf79e31d99c4961aa86f8c86aefe17f Mon Sep 17 00:00:00 2001 From: Atmik Shetty <109427822+atmikshetty@users.noreply.github.com> Date: Tue, 13 Oct 2026 14:33:16 +0530 Subject: [PATCH] BREAK, BREAKATPOINT and JOIN: leave objects on locked layers alone (fixes #727) (#730) BREAK and BREAKATPOINT (break_entity) and line/arc/polyline JOIN (modify::join) removed their source objects and added replacements without checking the source layer's lock, so a locked line could be cut or merged into a polyline. ERASE, MOVE, TRIM, FILLET, EXPLODE and the ellipse/spline JOIN already guard against this. Both now refuse with the same errors the sibling commands use: "The object is on a locked layer." (BREAK, BREAKATPOINT, like FILLET) and "An object is on a locked layer." (JOIN, like the ellipse/spline join). JOIN refuses the whole request when any selected object is locked, so a mixed-layer pair keeps its locked original in either order. The guards sit in the shared functions, so JSON calls with explicit handles or the selection, the interactive prompts and the command line all reach them. The interactive commands report the refusal on the command line. Tests (crates/engine/tests/break_join_locked_layers.rs, red before the fix): BREAK and BREAKATPOINT on a locked line by handle and by selection; BREAK on a locked arc and circle; JOIN of locked lines and of a locked/unlocked pair in both orders; interactive BREAK and JOIN. Each checks the drawing is unchanged while locked, and that the same edit works unlocked (BREAK also undoes and redoes). Co-authored-by: Brandon Thomas Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/modify.rs | 6 + .../engine/tests/break_join_locked_layers.rs | 135 ++++++++++++++++++ 2 files changed, 141 insertions(+) create mode 100644 crates/engine/tests/break_join_locked_layers.rs diff --git a/crates/engine/src/cmd/modify.rs b/crates/engine/src/cmd/modify.rs index 2ef051f..4f3fc6c 100644 --- a/crates/engine/src/cmd/modify.rs +++ b/crates/engine/src/cmd/modify.rs @@ -1414,6 +1414,9 @@ fn run_explode(s: &mut Session, p: &Value) -> Result { /// Break `h` between two points (or at one point when p1 == p2). pub(crate) fn break_entity(s: &mut Session, h: Handle, p1: Vec2, p2: Vec2) -> Result> { + if super::curves::is_locked(s, h) { + return Err(EngineError::Other("The object is on a locked layer.".into())); + } let e = s.doc()?.entity(h).map(|e| (**e).clone()).ok_or_else(|| EngineError::Other("no such object".into()))?; let pieces: Vec = match &e.kind { EntityKind::Line(l) => { @@ -1523,6 +1526,9 @@ pub(crate) fn join(s: &mut Session, hs: &[Handle]) -> Result> { if let Some(h) = super::modify2::join_curves(s, hs)? { return Ok(Some(h)); } + if hs.iter().any(|h| super::curves::is_locked(s, *h)) { + return Err(EngineError::Other("An object is on a locked layer.".into())); + } let d = s.doc()?; let mut segs: Vec = Vec::new(); // Source entity of each segment in `segs`, so only entities that end up in the chain are removed. diff --git a/crates/engine/tests/break_join_locked_layers.rs b/crates/engine/tests/break_join_locked_layers.rs new file mode 100644 index 0000000..0a1f6e7 --- /dev/null +++ b/crates/engine/tests/break_join_locked_layers.rs @@ -0,0 +1,135 @@ +//! BREAK, BREAKATPOINT and JOIN leave objects on locked layers alone (#727). + +use cadcraft_engine::geom::Vec2; +use cadcraft_engine::{Input, Session}; +use serde_json::{Value, json}; + +fn h(r: &Value) -> String { + r["handle"].as_str().or_else(|| r["handles"][0].as_str()).unwrap().to_string() +} + +/// Everything in the drawing a locked-layer edit could disturb. +fn snapshot(s: &Session) -> String { + let d = s.doc().unwrap(); + format!("{:?}", d.model.iter().map(|e| (e.handle, e.common.layer.clone(), e.kind.clone())).collect::>()) +} + +fn set_lock(s: &mut Session, locked: bool) { + s.execute("layer.set", &json!({"name": "0", "locked": locked})).unwrap(); +} + +fn line(s: &mut Session, a: [f64; 2], b: [f64; 2]) -> String { + h(&s.execute("line", &json!({"points": [a, b]})).unwrap()) +} + +fn lines_ab(s: &mut Session) -> (String, String) { + (line(s, [0.0, 0.0], [10.0, 0.0]), line(s, [10.0, 0.0], [20.0, 0.0])) +} + +#[test] +fn break_and_breakatpoint_refuse_a_locked_line() { + let mut s = Session::new(); + let a = line(&mut s, [0.0, 0.0], [10.0, 0.0]); + let control = line(&mut s, [0.0, 5.0], [10.0, 5.0]); + set_lock(&mut s, true); + let before = snapshot(&s); + assert!(s.execute("break", &json!({"handle": a, "p1": [3, 0], "p2": [7, 0]})).is_err()); + assert!(s.execute("breakatpoint", &json!({"handle": a, "at": [5, 0]})).is_err()); + // Selection fallback reaches the same guard. + s.execute("select", &json!({"handles": [a]})).unwrap(); + assert!(s.execute("break", &json!({"p1": [3, 0], "p2": [7, 0]})).is_err()); + assert!(s.execute("breakatpoint", &json!({"at": [5, 0]})).is_err()); + assert_eq!(snapshot(&s), before); + // Unlocked, the same edits work, and undo and redo them. + set_lock(&mut s, false); + let unlocked = snapshot(&s); + let r = s.execute("break", &json!({"handle": a, "p1": [3, 0], "p2": [7, 0]})).unwrap(); + assert_eq!(r["handles"].as_array().unwrap().len(), 2); + assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(&a).unwrap()).is_none()); + assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(&control).unwrap()).is_some()); + let broken = snapshot(&s); + s.execute("undo", &json!({})).unwrap(); + assert_eq!(snapshot(&s), unlocked); + s.execute("redo", &json!({})).unwrap(); + assert_eq!(snapshot(&s), broken); +} + +#[test] +fn break_refuses_a_locked_arc_and_circle() { + let mut s = Session::new(); + let arc = h(&s.execute("arc", &json!({"center": [0, 0], "radius": 5, "start": 0, "end": 90})).unwrap()); + let circle = h(&s.execute("circle", &json!({"center": [20, 0], "radius": 3})).unwrap()); + set_lock(&mut s, true); + let before = snapshot(&s); + assert!(s.execute("break", &json!({"handle": arc, "p1": [5, 0], "p2": [0, 5]})).is_err()); + assert!(s.execute("break", &json!({"handle": circle, "p1": [23, 0], "p2": [20, 3]})).is_err()); + assert_eq!(snapshot(&s), before); +} + +#[test] +fn join_refuses_when_any_line_is_locked() { + let mut s = Session::new(); + let (a, b) = lines_ab(&mut s); + set_lock(&mut s, true); + let before = snapshot(&s); + assert!(s.execute("join", &json!({"handles": [a, b]})).is_err()); + s.execute("select", &json!({"handles": [a, b]})).unwrap(); + assert!(s.execute("join", &json!({})).is_err()); + assert_eq!(snapshot(&s), before); + set_lock(&mut s, false); + let r = s.execute("join", &json!({"handles": [a, b]})).unwrap(); + assert!(r["handle"].is_string()); + assert_eq!(s.doc().unwrap().model.len(), 1); +} + +#[test] +fn join_keeps_a_locked_original_in_a_mixed_layer_pair() { + for locked_first in [true, false] { + let mut s = Session::new(); + s.execute("layer.new", &json!({"name": "L"})).unwrap(); + // The first line goes on the soon-locked layer 0, the second on "L" (or the reverse). + let on_l = |s: &mut Session, a: [f64; 2], b: [f64; 2]| { + s.execute("layer.current", &json!({"name": "L"})).unwrap(); + let r = line(s, a, b); + s.execute("layer.current", &json!({"name": "0"})).unwrap(); + r + }; + let (a, b) = if locked_first { + (line(&mut s, [0.0, 0.0], [10.0, 0.0]), on_l(&mut s, [10.0, 0.0], [20.0, 0.0])) + } else { + (on_l(&mut s, [0.0, 0.0], [10.0, 0.0]), line(&mut s, [10.0, 0.0], [20.0, 0.0])) + }; + let locked = if locked_first { &a } else { &b }; + set_lock(&mut s, true); + let before = snapshot(&s); + assert!(s.execute("join", &json!({"handles": [a, b]})).is_err()); + assert_eq!(snapshot(&s), before, "locked_first = {locked_first}"); + assert!(s.doc().unwrap().entity(cadcraft_engine::doc::Handle::parse_hex(locked).unwrap()).is_some()); + } +} + +#[test] +fn interactive_break_and_join_refuse_locked_lines() { + let mut s = Session::new(); + let (a, b) = lines_ab(&mut s); + set_lock(&mut s, true); + let before = snapshot(&s); + // BREAK: pick the line, then the second break point. + s.start("break").unwrap(); + s.input(Input::Point(Vec2::new(3.0, 0.0))).unwrap(); + let _ = s.input(Input::Point(Vec2::new(7.0, 0.0))); + assert!(s.log.last().is_some_and(|m| m.contains("locked layer")), "{:?}", s.log.last()); + assert_eq!(snapshot(&s), before); + // JOIN on a preselected pair reports the refusal on the command line. + s.cancel(); + s.execute("select", &json!({"handles": [a, b]})).unwrap(); + s.start("join").unwrap(); + assert!(s.log.last().is_some_and(|m| m.contains("locked layer")), "{:?}", s.log.last()); + assert_eq!(snapshot(&s), before); + // Unlocked, the interactive commands still edit. + set_lock(&mut s, false); + s.start("break").unwrap(); + s.input(Input::Point(Vec2::new(3.0, 0.0))).unwrap(); + s.input(Input::Point(Vec2::new(7.0, 0.0))).unwrap(); + assert_ne!(snapshot(&s), before); +}