collective
Browse / github.com/storytold/cadcraft / patch
feat(engine): support joining multiple connected chains in JOIN
feat(engine): support joining multiple connected chains in JOIN ## Summary This PR improves the JOIN command so that multiple disconnected chains or independent closed polygons can be joined simultaneously in a single command execution. ### Key Changes - **Multi-chain support in \crates/engine/src/cmd/modify.rs\:** - Iteratively extracts all connected loops and chains from the pool of selected segments. - Automatically breaks each chain when a closed loop is formed, allowing multiple closed polygons to be created cleanly in a single operation. - Returns \Result<Vec<Handle>>\ with all newly created entity handles. - **JSON API & interactive prompt:**
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- engdennylsonsantos-cloud (imported from pull request #2099, open)
- Base
41d9b5e957b0719c04b9687c4f29088d77153c31- Size
- +224 −68 in 3 files
- Review
- pending
- Shared
- 2026-10-28
- Id
d642c597ca86624863aed6f1
How it went
No other operator has reported using it yet.
Diff
raw · contributors/people.toml crates/engine/src/cmd/modify.rs crates/engine/src/cmd/modify2_tests.rs
@@ -87,3 +87,8 @@ real_name = "Richard Sauer" display_name = "Richard Sauer" sync_github_name = false +[people.engdennylsonsantos-cloud]+real_name = "Dennylson Santos"+sync_github_name = true++@@ -1622,96 +1622,151 @@ fn run_breakat(s: &mut Session, p: &Value) -> Result<Value> { Ok(json!({ "handles": r.iter().map(|h| h.hex()).collect::<Vec<_>>() })) } -/// Join lines/arcs/polylines that share endpoints into one polyline (or merge collinear lines).-pub(crate) fn join(s: &mut Session, hs: &[Handle]) -> Result<Option<Handle>> {+/// Join lines/arcs/polylines that share endpoints into polylines (or merge collinear lines).+pub(crate) fn join(s: &mut Session, hs: &[Handle]) -> Result<Vec<Handle>> { if let Some(h) = super::modify2::join_curves(s, hs)? {- return Ok(Some(h));+ return Ok(vec![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<Segment> = Vec::new();- // Source entity of each segment in `segs`, so only entities that end up in the chain are removed.- let mut owners: Vec<Handle> = Vec::new();- let mut common = None;+ let mut pool: Vec<(Segment, Handle)> = Vec::new();+ let mut entity_common = std::collections::HashMap::new(); for h in hs { let Some(e) = d.entity(*h) else { continue };- common.get_or_insert_with(|| e.common.clone());+ entity_common.insert(*h, e.common.clone()); match &e.kind {- EntityKind::Line(l) => segs.push(Segment::Line(Line::new(l.a.xy(), l.b.xy()))),- EntityKind::Arc(a) => segs.push(Segment::Arc { arc: Arc::new(a.center.xy(), a.radius, a.start, a.end), ccw: true }),- EntityKind::LwPolyline(p) if !p.closed => segs.extend(Polyline { vertices: p.vertices.clone(), closed: false }.segments()),+ EntityKind::Line(l) => pool.push((Segment::Line(Line::new(l.a.xy(), l.b.xy())), *h)),+ EntityKind::Arc(a) => pool.push((Segment::Arc { arc: Arc::new(a.center.xy(), a.radius, a.start, a.end), ccw: true }, *h)),+ EntityKind::LwPolyline(p) if !p.closed => {+ for seg in (Polyline { vertices: p.vertices.clone(), closed: false }).segments() {+ pool.push((seg, *h));+ }+ } _ => {} }- owners.resize(segs.len(), *h); }- if segs.len() < 2 {+ if pool.len() < 2 { return Err(EngineError::Other("Select at least two objects to join.".into())); }- // Chain greedily from the first segment.- let mut chain = vec![segs.remove(0)];- let mut used = vec![owners.remove(0)];+ let tol = 1e-6;- loop {- let start = chain.first().map(Segment::start).unwrap_or_default();- let end = chain.last().map(Segment::end).unwrap_or_default();- let Some(i) =- segs.iter().position(|s| s.start().near(end, tol) || s.end().near(end, tol) || s.start().near(start, tol) || s.end().near(start, tol))- else {- break;- };- let s0 = segs.remove(i);- used.push(owners.remove(i));- if s0.start().near(end, tol) {- chain.push(s0);- } else if s0.end().near(end, tol) {- chain.push(s0.reversed());- } else if s0.end().near(start, tol) {- chain.insert(0, s0);- } else {- chain.insert(0, s0.reversed());+ let mut total: std::collections::HashMap<Handle, usize> = std::collections::HashMap::new();+ for (_, owner) in &pool {+ *total.entry(*owner).or_default() += 1;+ }++ let mut chains: Vec<(Vec<Segment>, Vec<Handle>)> = Vec::new();++ while !pool.is_empty() {+ let (first_seg, first_owner) = pool.remove(0);+ let mut chain = vec![first_seg];+ let mut used = vec![first_owner];++ loop {+ let start = chain.first().map(Segment::start).unwrap_or_default();+ let end = chain.last().map(Segment::end).unwrap_or_default();++ // Stop growing this chain once it closes into a loop.+ if chain.len() >= 2 && start.near(end, tol) {+ break;+ }++ let Some(i) = pool+ .iter()+ .position(|(s, _)| s.start().near(end, tol) || s.end().near(end, tol) || s.start().near(start, tol) || s.end().near(start, tol))+ else {+ break;+ };++ let (s0, owner) = pool.remove(i);+ used.push(owner);+ if s0.start().near(end, tol) {+ chain.push(s0);+ } else if s0.end().near(end, tol) {+ chain.push(s0.reversed());+ } else if s0.end().near(start, tol) {+ chain.insert(0, s0);+ } else {+ chain.insert(0, s0.reversed());+ }+ }++ let closed = chain.len() >= 2 && chain.first().zip(chain.last()).is_some_and(|(a, b)| a.start().near(b.end(), tol));+ let mut unique_owners = used.clone();+ unique_owners.sort();+ unique_owners.dedup();++ let mut used_counts: std::collections::HashMap<Handle, usize> = std::collections::HashMap::new();+ for owner in &used {+ *used_counts.entry(*owner).or_default() += 1;+ }+ let fully_consumed = used_counts.iter().all(|(owner, count)| total.get(owner) == Some(count));++ if fully_consumed && (unique_owners.len() >= 2 || (chain.len() >= 2 && closed)) {+ chains.push((chain, used)); } }- if chain.len() < 2 {++ if chains.is_empty() { return Err(EngineError::Other("Objects are not connected.".into())); }- let closed = chain.first().zip(chain.last()).is_some_and(|(a, b)| a.start().near(b.end(), tol));- let mut vs: Vec<PolyVertex> = chain- .iter()- .map(|s| PolyVertex {- p: s.start(),- bulge: match s {- Segment::Line(_) => 0.0,- Segment::Arc { arc, ccw } => cadcraft_geom::arc_to_bulge(if *ccw { arc.sweep() } else { -arc.sweep() }),- },- ..Default::default()- })- .collect();- if !closed && let Some(l) = chain.last() {- vs.push(PolyVertex::new(l.end()));- }+ let space = s.space(); let doc = s.doc_mut()?;- for h in hs.iter().filter(|h| used.contains(*h)) {- doc.remove_entity(*h);- }- let nh = doc.new_handle();- if let Some(st) = doc.space_mut(&space) {- st.push(Entity { handle: nh, common: common.unwrap_or_default(), kind: lwpoly(vs, closed) });+ let mut out = Vec::new();++ for (chain, used) in chains {+ let closed = chain.len() >= 2 && chain.first().zip(chain.last()).is_some_and(|(a, b)| a.start().near(b.end(), tol));+ let mut vs: Vec<PolyVertex> = chain+ .iter()+ .map(|s| PolyVertex {+ p: s.start(),+ bulge: match s {+ Segment::Line(_) => 0.0,+ Segment::Arc { arc, ccw } => cadcraft_geom::arc_to_bulge(if *ccw { arc.sweep() } else { -arc.sweep() }),+ },+ ..Default::default()+ })+ .collect();+ if !closed && let Some(l) = chain.last() {+ vs.push(PolyVertex::new(l.end()));+ }++ let mut unique_owners = used.clone();+ unique_owners.sort();+ unique_owners.dedup();+ for h in &unique_owners {+ doc.remove_entity(*h);+ }++ let first_owner = used.first().copied();+ let common = first_owner.and_then(|h| entity_common.get(&h).cloned()).unwrap_or_default();++ let nh = doc.new_handle();+ if let Some(st) = doc.space_mut(&space) {+ st.push(Entity { handle: nh, common, kind: lwpoly(vs, closed) });+ }+ out.push(nh); }+ s.set_selection(Vec::new());- Ok(Some(nh))+ Ok(out) } +/// Run the JOIN command with JSON arguments. fn run_join(s: &mut Session, p: &Value) -> Result<Value> { let hs = targets(s, p)?; if p.get("close").and_then(Value::as_bool) == Some(true) { let h = super::modify2::join_close(s, &hs)?;- return Ok(json!({ "handle": h.hex() }));+ return Ok(json!({ "handle": h.hex(), "handles": [h.hex()] })); }- let r = join(s, &hs)?;- Ok(json!({ "handle": r.map(|h| h.hex()) }))+ let rs = join(s, &hs)?;+ Ok(json!({+ "handle": rs.first().map(|h| h.hex()),+ "handles": rs.iter().map(|h| h.hex()).collect::<Vec<_>>()+ })) } // ---------------- interactive ----------------@@ -1795,17 +1850,25 @@ impl SelectThen { /// JOIN the selected objects and report what they became. fn join_now(&mut self, s: &mut Session) -> Result<Step> { match join(s, &self.objs) {- Ok(h) => {+ Ok(hs) => { // Objects outside the joined chain are kept, so count only the ones JOIN consumed // (and the source object an ellipse or spline join keeps). let d = s.doc()?;- let n = self.objs.iter().filter(|o| d.entity(**o).is_none() || h == Some(**o)).count();- let what = match h.and_then(|h| d.entity(h)).map(|e| &e.kind) {- Some(EntityKind::Spline(_)) => "spline",- Some(EntityKind::Ellipse(_)) => "ellipse",- _ => "polyline",- };- s.echo(format!("{n} objects joined into 1 {what}"))+ let n = self.objs.iter().filter(|o| d.entity(**o).is_none() || hs.contains(o)).count();+ let m = hs.len();+ if m == 0 {+ s.echo("0 objects joined".to_string());+ } else if m == 1 {+ let h = hs.first().copied();+ let what = match h.and_then(|h| d.entity(h)).map(|e| &e.kind) {+ Some(EntityKind::Spline(_)) => "spline",+ Some(EntityKind::Ellipse(_)) => "ellipse",+ _ => "polyline",+ };+ s.echo(format!("{n} objects joined into 1 {what}"));+ } else {+ s.echo(format!("{n} objects joined into {m} polylines"));+ } } Err(e) => s.echo(e.to_string()), }@@ -111,6 +111,94 @@ fn join_keeps_selected_objects_outside_the_chain() { assert!(d.entity(Handle::parse_hex(&c).unwrap()).is_some(), "unrelated circle stays"); } +/// Test joining multiple polygons and open polyline strips simultaneously.+#[test]+fn join_multiple_polygons_simultaneously() {+ let mut s = Session::new();+ // Square 1 (4 lines)+ let s1_1 = h(&s.execute("line", &json!({"points": [[0, 0], [10, 0]]})).unwrap());+ let s1_2 = h(&s.execute("line", &json!({"points": [[10, 0], [10, 10]]})).unwrap());+ let s1_3 = h(&s.execute("line", &json!({"points": [[10, 10], [0, 10]]})).unwrap());+ let s1_4 = h(&s.execute("line", &json!({"points": [[0, 10], [0, 0]]})).unwrap());++ // Square 2 (4 lines)+ let s2_1 = h(&s.execute("line", &json!({"points": [[20, 0], [30, 0]]})).unwrap());+ let s2_2 = h(&s.execute("line", &json!({"points": [[30, 0], [30, 10]]})).unwrap());+ let s2_3 = h(&s.execute("line", &json!({"points": [[30, 10], [20, 10]]})).unwrap());+ let s2_4 = h(&s.execute("line", &json!({"points": [[20, 10], [20, 0]]})).unwrap());++ // Open line strip (3 lines)+ let o_1 = h(&s.execute("line", &json!({"points": [[40, 0], [50, 0]]})).unwrap());+ let o_2 = h(&s.execute("line", &json!({"points": [[50, 0], [50, 5]]})).unwrap());+ let o_3 = h(&s.execute("line", &json!({"points": [[50, 5], [60, 5]]})).unwrap());++ // Unconnected line+ let far = h(&s.execute("line", &json!({"points": [[100, 100], [110, 110]]})).unwrap());++ let all_handles = [+ s1_1.clone(),+ s1_2.clone(),+ s1_3.clone(),+ s1_4.clone(),+ s2_1.clone(),+ s2_2.clone(),+ s2_3.clone(),+ s2_4.clone(),+ o_1.clone(),+ o_2.clone(),+ o_3.clone(),+ far.clone(),+ ];++ let r = s.execute("join", &json!({"handles": all_handles})).unwrap();+ let created = r["handles"].as_array().unwrap();+ assert_eq!(created.len(), 3, "expected 3 polylines: square 1, square 2, and open strip");++ let d = s.doc().unwrap();+ // 11 source lines are consumed+ for hex in [&s1_1, &s1_2, &s1_3, &s1_4, &s2_1, &s2_2, &s2_3, &s2_4, &o_1, &o_2, &o_3] {+ assert!(d.entity(Handle::parse_hex(hex).unwrap()).is_none());+ }+ // Unconnected line stays+ assert!(d.entity(Handle::parse_hex(&far).unwrap()).is_some());++ // Check closed squares and open polyline+ let mut closed_count = 0;+ let mut open_count = 0;+ for h_val in created {+ let hex = h_val.as_str().unwrap();+ let p = poly(&kind(&s, hex));+ if p.closed {+ closed_count += 1;+ assert_eq!(p.vertices.len(), 4);+ } else {+ open_count += 1;+ assert_eq!(p.vertices.len(), 4);+ }+ }+ assert_eq!(closed_count, 2);+ assert_eq!(open_count, 1);+}++/// Test that partial-owner chains (e.g. self-touching open polyline with a tail) are rejected+/// to avoid deleting the source entity and losing unconsumed segments.+#[test]+fn join_partial_owner_rejected_to_prevent_loss() {+ let mut s = Session::new();+ // Open polyline with loop (0,0 -> 10,0 -> 10,10 -> 0,0) and tail (0,0 -> 0,-10)+ let p = pline(&mut s, json!([[0, 0], [10, 0], [10, 10], [0, 0], [0, -10]]), false);+ let far = h(&s.execute("line", &json!({"points": [[100, 100], [110, 110]]})).unwrap());++ // JOIN should reject because the polyline is not fully consumed into a clean chain+ let r = s.execute("join", &json!({"handles": [p.clone(), far.clone()]}));+ assert!(r.is_err());++ // Both entities remain intact in document+ let d = s.doc().unwrap();+ assert!(d.entity(Handle::parse_hex(&p).unwrap()).is_some());+ assert!(d.entity(Handle::parse_hex(&far).unwrap()).is_some());+}+ #[test] fn pedit_command_line() { let mut s = Session::new();