collective
Browse / github.com/storytold/vectorcraft / patch
Fix #4156: Join joins every selected open path at once
Join (CTRL-J) should join all selected segments instead of just two per command Current Behavior: - Selecting multiple segments that can be joined and choosing Join (CTRL-J) will only join two of those segments. - This behavior is the same regardless if the segments are selected with the marquee selector or CTRL-A or shift-clicking multiple selections. Desired Behavior: - If multiple segments are selected that have overlapping nodes, choosing Join should join them all in one invocation of the Join command. - I have attached a video that shows the current behavior in VectorCraft and a similar function in Inkscape that works in one command. (note in the video I use the ma
- Status
- open
- Score
- 0.00
- Adopters
- 0 other operators
- Author
- Hassan1305 (imported from pull request #4318, open)
- Base
443b31b13df3a7ab611c2fe1b95e73898ed74fdd- Size
- +221 −54 in 2 files
- Review
- pending
- Shared
- 2026-10-28
- Id
40f6a5b3a1357b9486968bbd
How it went
No other operator has reported using it yet.
Diff
raw · crates/engine/src/cmd/path.rs crates/engine/src/tests/draw2.rs
@@ -74,7 +74,7 @@ pub fn specs() -> Vec<CommandSpec> { "Join", ["Object", "Path"], Some("Cmd+J"),- "{ids?: [path, path] (default: the selected paths), ends?: [\"first\"|\"last\", \"first\"|\"last\"] (which end of each path to join, as the Pen joins them; default: the nearest pair), to?: \"corner\"|\"smooth\" (the joined anchor: corner retracts both handles, smooth pulls handles in line with the neighbours)} join two open paths into one (the first keeps its id, selected), or close a single open path; without ids, direct-selected anchors lying on one another along a path merge into one anchor instead",+ "{ids?: [path, path, …] (default: the selected paths), ends?: [\"first\"|\"last\", \"first\"|\"last\"] (two paths only: which end of each path to join, as the Pen joins them; default: the nearest pair), to?: \"corner\"|\"smooth\" (the joined anchor: corner retracts both handles, smooth pulls handles in line with the neighbours)} join two open paths into one (the first keeps its id, selected); more than two: the open paths whose ends are closest join first, again until one is left (closed paths among them stay as they are); or close a single open path; without ids, direct-selected anchors lying on one another along a path merge into one anchor instead", has_selection, join ),@@ -386,6 +386,9 @@ fn merge_coincident(sp: &mut SubPath, picked: impl Fn(usize) -> bool, join_kind: before - sp.anchors.len() } +/// `path.join`: without `ids`, direct-selected anchors lying on one another merge first; else one+/// path closes, two paths join ([`join_pair`]), and more than two join nearest ends first+/// ([`join_nearest`]), each in one undo step. fn join(s: &mut Session, p: &Value) -> Result<Value> { const C: &str = "path.join"; let join_kind = match p.get("to").and_then(Value::as_str) {@@ -462,7 +465,7 @@ fn join(s: &mut Session, p: &Value) -> Result<Value> { Ok(()) })?; }- [a, b, ..] => {+ [a, b] => { let (a, b) = (*a, *b); if a == b { return Err(bad(C, "join two different paths"));@@ -471,67 +474,177 @@ fn join(s: &mut Session, p: &Value) -> Result<Value> { let picked_a = st.selection.anchors.get(&a).cloned().unwrap_or_default(); let picked_b = st.selection.anchors.get(&b).cloned().unwrap_or_default(); s.edit("Join", |d, sel| {- let pb = d.node(b).and_then(|n| n.path_data()).cloned().ok_or(EngineError::NoNode(b))?;- let pa = path_mut(d, a)?;- let (Some(sa), Some(sb)) = (pa.subpaths.iter().position(|s| !s.closed), pb.subpaths.iter().position(|s| !s.closed)) else {- return Err(EngineError::Other("join needs open paths".into()));- };- let mut x = pa.subpaths[sa].clone();- let mut y = pb.subpaths[sb].clone();- let (Some((xf, xl)), Some((yf, yl))) = (end_points(&x), end_points(&y)) else {- return Err(EngineError::Other("join needs open paths".into()));- };- // The ends asked for, else the closest pair.- let cands =- [(xl.distance(yf), false, false), (xl.distance(yl), false, true), (xf.distance(yf), true, false), (xf.distance(yl), true, true)];- // Direct-selected end points decide which ends meet, before the closest pair does.- let end_of = |picked: &std::collections::BTreeSet<(usize, usize)>, si: usize, len: usize| {- let last_ix = len.checked_sub(1)?;- let first = picked.contains(&(si, 0));- let last = picked.contains(&(si, last_ix));- match (first, last) {- (true, false) => Some(true),- (false, true) => Some(false),- _ => None,- }- };- let by_selection = match (end_of(&picked_a, sa, x.anchors.len()), end_of(&picked_b, sb, y.anchors.len())) {- (Some(fa), Some(fb)) => Some((fa, !fb)),- _ => None,- };- let (rx, ry) = ends- .or(by_selection)- .unwrap_or_else(|| cands.into_iter().min_by(|p, q| p.0.total_cmp(&q.0)).map_or((false, false), |(_, rx, ry)| (rx, ry)));- if rx {- x.reverse();- }- if ry {- y.reverse();- }- let junction = x.anchors.len() - 1;- if let (Some(xe), Some(ys)) = (x.anchors.last().map(|a| a.p), y.anchors.first().map(|a| a.p))- && xe.distance(ys) < 1e-6- {- let first = y.anchors.remove(0);- if let Some(l) = x.anchors.last_mut() {- l.h_out = first.h_out;- }- }- x.anchors.extend(y.anchors);- if let Some(smooth) = join_kind {- join_point(&mut x, junction, smooth);- }- pa.subpaths[sa] = x;- d.remove(b)?;+ join_pair(d, a, b, ends.or(by_selection(d, a, b, &picked_a, &picked_b)), join_kind)?; sel.set([a]); Ok(()) })?; } [] => return Err(EngineError::Other("select paths to join".into())),+ many => {+ if ends.is_some() {+ return Err(bad(C, "ends takes two paths"));+ }+ let mut seen = std::collections::BTreeSet::new();+ if let Some(id) = many.iter().find(|id| !seen.insert(**id)) {+ return Err(bad(C, format!("{} is listed twice", id.0)));+ }+ // More than two: the open paths whose ends are closest join first, again and again+ // until one path is left (closed paths in the selection are left as they are).+ let st = s.doc()?;+ let mut open: Vec<(NodeId, (Point, Point))> = many.iter().filter_map(|id| Some((*id, open_ends(&st.doc, *id)?))).collect();+ if open.len() < 2 {+ return Err(EngineError::Other("join needs open paths".into()));+ }+ s.edit("Join", |d, sel| {+ let keep = join_nearest(d, &mut open, join_kind)?;+ sel.set([keep]);+ Ok(())+ })?;+ } } ok() } +/// The ends of `id`'s first open subpath.+fn open_ends(d: &vectorcraft_doc::Document, id: NodeId) -> Option<(Point, Point)> {+ d.node(id)?.path_data()?.subpaths.iter().find(|s| !s.closed).and_then(end_points)+}++/// How far apart the nearest ends of two paths are.+fn end_gap((xf, xl): (Point, Point), (yf, yl): (Point, Point)) -> f64 {+ xl.distance(yf).min(xl.distance(yl)).min(xf.distance(yf)).min(xf.distance(yl))+}++/// Join `paths` (open paths and their ends) into one: the pair whose ends are closest first,+/// repeated until one is left, as the [Join command](https://helpx.adobe.com/illustrator/desktop/draw-shapes-and-paths/modify-paths/connect-and-join-paths.html)+/// does with more than two paths. Of each pair the one listed first keeps its id. Each path's+/// nearest other path is kept, and only redone for the paths a join touches. Returns the path left.+fn join_nearest(d: &mut vectorcraft_doc::Document, paths: &mut [(NodeId, (Point, Point))], join_kind: Option<bool>) -> Result<NodeId> {+ let n = paths.len();+ let mut alive = vec![true; n];+ let nearest_of = |paths: &[(NodeId, (Point, Point))], alive: &[bool], i: usize| -> (f64, usize) {+ let ends = paths.get(i).map(|p| p.1);+ let mut best = (f64::INFINITY, i);+ for (j, (_, e)) in paths.iter().enumerate() {+ if j != i+ && alive.get(j).copied().unwrap_or(false)+ && let Some(ends) = ends+ {+ let g = end_gap(ends, *e);+ if g < best.0 {+ best = (g, j);+ }+ }+ }+ best+ };+ let mut nearest: Vec<(f64, usize)> = (0..n).map(|i| nearest_of(paths, &alive, i)).collect();+ for _ in 1..n {+ let Some((i, &(_, j))) =+ nearest.iter().enumerate().filter(|(i, _)| alive.get(*i).copied().unwrap_or(false)).min_by(|p, q| p.1.0.total_cmp(&q.1.0))+ else {+ break;+ };+ let (keep, drop) = (i.min(j), i.max(j));+ let (Some(&(a, _)), Some(&(b, _))) = (paths.get(keep), paths.get(drop)) else { break };+ join_pair(d, a, b, None, join_kind)?;+ if let Some(f) = alive.get_mut(drop) {+ *f = false;+ }+ let ends = open_ends(d, a).ok_or_else(|| EngineError::Other("join needs open paths".into()))?;+ if let Some(p) = paths.get_mut(keep) {+ p.1 = ends;+ }+ for k in 0..n {+ if k == keep || !alive.get(k).copied().unwrap_or(false) {+ continue;+ }+ let (g, to) = nearest.get(k).copied().unwrap_or((f64::INFINITY, k));+ let redo = to == keep || to == drop;+ let gap = paths.get(k).map_or(f64::INFINITY, |p| end_gap(p.1, ends));+ if let Some(slot) = nearest.get_mut(k) {+ if redo {+ *slot = nearest_of(paths, &alive, k);+ } else if gap < g {+ *slot = (gap, keep);+ }+ }+ }+ if let Some(slot) = nearest.get_mut(keep) {+ *slot = nearest_of(paths, &alive, keep);+ }+ }+ alive.iter().position(|a| *a).and_then(|i| paths.get(i)).map(|p| p.0).ok_or_else(|| EngineError::Other("select paths to join".into()))+}++/// Which ends of `a` and `b` meet when the direct selection picked one end point of each, as+/// (reverse `a`, reverse `b`).+fn by_selection(+ d: &vectorcraft_doc::Document,+ a: NodeId,+ b: NodeId,+ picked_a: &std::collections::BTreeSet<(usize, usize)>,+ picked_b: &std::collections::BTreeSet<(usize, usize)>,+) -> Option<(bool, bool)> {+ let open = |id: NodeId| {+ let pd = d.node(id)?.path_data()?;+ let si = pd.subpaths.iter().position(|s| !s.closed)?;+ Some((si, pd.subpaths.get(si)?.anchors.len()))+ };+ let end_of = |picked: &std::collections::BTreeSet<(usize, usize)>, (si, len): (usize, usize)| {+ let last_ix = len.checked_sub(1)?;+ match (picked.contains(&(si, 0)), picked.contains(&(si, last_ix))) {+ (true, false) => Some(true),+ (false, true) => Some(false),+ _ => None,+ }+ };+ Some((end_of(picked_a, open(a)?)?, !end_of(picked_b, open(b)?)?))+}++/// Join open path `b` onto open path `a` (their first open subpaths), which keeps its id; `b` is+/// removed. `ends`: (reverse `a`, reverse `b`), so `a`'s end comes last and `b`'s first; `None`+/// for the closest pair.+fn join_pair(d: &mut vectorcraft_doc::Document, a: NodeId, b: NodeId, ends: Option<(bool, bool)>, join_kind: Option<bool>) -> Result<()> {+ let pb = d.node(b).and_then(|n| n.path_data()).cloned().ok_or(EngineError::NoNode(b))?;+ let pa = path_mut(d, a)?;+ let (Some(sa), Some(sb)) = (pa.subpaths.iter().position(|s| !s.closed), pb.subpaths.iter().position(|s| !s.closed)) else {+ return Err(EngineError::Other("join needs open paths".into()));+ };+ let (Some(mut x), Some(mut y)) = (pa.subpaths.get(sa).cloned(), pb.subpaths.get(sb).cloned()) else {+ return Err(EngineError::Other("join needs open paths".into()));+ };+ let (Some((xf, xl)), Some((yf, yl))) = (end_points(&x), end_points(&y)) else {+ return Err(EngineError::Other("join needs open paths".into()));+ };+ let cands = [(xl.distance(yf), false, false), (xl.distance(yl), false, true), (xf.distance(yf), true, false), (xf.distance(yl), true, true)];+ let (rx, ry) = ends.unwrap_or_else(|| cands.into_iter().min_by(|p, q| p.0.total_cmp(&q.0)).map_or((false, false), |(_, rx, ry)| (rx, ry)));+ if rx {+ x.reverse();+ }+ if ry {+ y.reverse();+ }+ let junction = x.anchors.len().saturating_sub(1);+ if let (Some(xe), Some(ys)) = (x.anchors.last().map(|a| a.p), y.anchors.first().map(|a| a.p))+ && xe.distance(ys) < 1e-6+ {+ let first = y.anchors.remove(0);+ if let Some(l) = x.anchors.last_mut() {+ l.h_out = first.h_out;+ }+ }+ x.anchors.extend(y.anchors);+ if let Some(smooth) = join_kind {+ join_point(&mut x, junction, smooth);+ }+ if let Some(slot) = pa.subpaths.get_mut(sa) {+ *slot = x;+ }+ d.remove(b)?;+ Ok(())+}+ pub(super) type AnchorTargets = Vec<(NodeId, Vec<(usize, usize)>)>; pub(super) fn anchor_targets(s: &Session) -> Result<AnchorTargets> {@@ -2400,6 +2400,60 @@ fn join_takes_the_paths_and_the_ends_to_join() { } } +/// #4156: with more than two paths selected, one Join joins them all, the ends closest to each+/// other first, in one undo step; a closed path among them is left as it is.+#[test]+fn join_joins_every_selected_open_path_at_once() {+ let mut s = session();+ // A triangle's three sides, each drawn on its own.+ let a = line(&mut s, 0.0, 0.0, 100.0, 0.0);+ let b = line(&mut s, 50.0, 80.0, 100.0, 0.0);+ let c = line(&mut s, 0.0, 0.0, 50.0, 80.0);+ let r = rect(&mut s, 300.0, 300.0, 10.0, 10.0);+ s.execute("select.all", &json!({})).unwrap();+ s.execute("path.join", &json!({})).unwrap();+ let left: Vec<NodeId> = paths(&s).into_iter().map(|(id, _)| id).collect();+ assert_eq!(left.len(), 2, "one joined path and the rectangle");+ assert!(left.contains(&r) && path(&s, r).subpaths[0].closed, "the closed rectangle is untouched");+ let joined = *left.iter().find(|id| **id != r).unwrap();+ assert!([a, b, c].contains(&joined));+ let mut pts: Vec<(f64, f64)> = path(&s, joined).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect();+ assert_eq!(pts.len(), 4, "the shared corners merge: {pts:?}");+ assert_eq!(pts.first(), pts.last(), "the sides run all the way round");+ pts.pop();+ pts.sort_by(|p, q| p.partial_cmp(q).unwrap());+ assert_eq!(pts, [(0.0, 0.0), (50.0, 80.0), (100.0, 0.0)]);+ assert_eq!(s.doc().unwrap().selection.objects, [joined]);+ s.execute("edit.undo", &json!({})).unwrap();+ assert_eq!(paths(&s).len(), 4, "one undo step");++ // Many segments of one polyline, drawn out of order, join into it in one go.+ let mut s = session();+ let n = 200;+ let mut order: Vec<usize> = (0..n).collect();+ // A fixed shuffle: every 7th step through the indices (7 and 200 are coprime).+ order = (0..n).map(|k| order[(k * 7) % n]).collect();+ for i in order {+ let x = i as f64 * 10.0;+ let y = if i % 2 == 0 { 0.0 } else { 5.0 };+ let y2 = if i % 2 == 0 { 5.0 } else { 0.0 };+ line(&mut s, x, y, x + 10.0, y2);+ }+ s.execute("select.all", &json!({})).unwrap();+ s.execute("path.join", &json!({})).unwrap();+ let left: Vec<NodeId> = paths(&s).into_iter().map(|(id, _)| id).collect();+ assert_eq!(left.len(), 1);+ let xs: Vec<f64> = path(&s, left[0]).subpaths[0].anchors.iter().map(|x| x.p.x).collect();+ assert_eq!(xs.len(), n + 1);+ assert!(xs.windows(2).all(|w| w[1] > w[0]) || xs.windows(2).all(|w| w[1] < w[0]), "in order along the line");++ // `ends` names two paths only.+ let mut s = session();+ let ids: Vec<u64> = (0..3).map(|i| line(&mut s, i as f64 * 20.0, 0.0, i as f64 * 20.0 + 10.0, 0.0).0).collect();+ assert!(s.execute("path.join", &json!({"ids": ids, "ends": ["first", "last"]})).is_err());+ assert!(s.execute("path.join", &json!({"ids": [ids[0], ids[1], ids[0]]})).is_err(), "a path listed twice");+}+ /// Without `ends`, the end points picked with the direct selection are the ones joined. #[test] fn join_uses_the_selected_end_points() {