From 2a1aca81f058a878bcd6f2170a406efe2f76c4b9 Mon Sep 17 00:00:00 2001 From: el-j Date: Sat, 24 Oct 2026 00:27:05 +0200 Subject: [PATCH 01/18] M2.1: allow Pen tool and path.join to continue, close, and join compound paths (#3274, #3276, #3379, #3380) --- crates/engine/src/cmd/object.rs | 11 +- crates/engine/src/cmd/path.rs | 634 +++++++++++++++--- crates/engine/src/cmd/select.rs | 6 +- crates/engine/src/tests_draw2.rs | 552 ++++++++++++++- crates/tools/src/pen.rs | 295 +++++--- .../src/i18n/untranslated_messages.txt | 7 + 6 files changed, 1286 insertions(+), 219 deletions(-) diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 93cdfd74a..cdc43c51f 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -883,6 +883,10 @@ fn show_all(s: &mut Session, _: &Value) -> Result { Ok(json!({ "count": ids.len() })) } +/// A closed contour smaller than this (in square document units) has no usable winding direction, +/// so Make Compound Path leaves its orientation alone rather than flipping it on noise. +const MIN_WINDING_AREA: f64 = 1e-6; + fn compound_make(s: &mut Session, _: &Value) -> Result { let ids = selected_roots(s)?; let Some(backmost) = ids.first().copied() else { return Err(EngineError::Other("nothing selected".into())) }; @@ -904,7 +908,12 @@ fn compound_make(s: &mut Session, _: &Value) -> Result { // The paths in front run against the backmost one, so where they overlap it they cut holes // under the non-zero fill rule (#1197). A compound path's paths turn together, by its // first: its own holes (a letter's counters) stay holes. - let direction = |n: &Node| n.path_data().and_then(|pd| pd.subpaths.first()).map_or(0.0, |sp| sp.signed_area().signum()); + let direction = |n: &Node| { + n.path_data() + .and_then(|pd| pd.subpaths.first()) + .filter(|sp| sp.closed && sp.signed_area().abs() > MIN_WINDING_AREA) + .map_or(0.0, |sp| sp.signed_area().signum()) + }; let first = |unit: &[Arc]| unit.first().map_or(0.0, |p| direction(p)); let back = units.first().map_or(0.0, |u| first(u)); for (i, unit) in units.into_iter().enumerate() { diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index f55378a34..94667d776 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -3,7 +3,7 @@ use std::collections::BTreeSet; use serde_json::{Value, json}; -use vectorcraft_doc::{NodeId, NodeKind, Scaling}; +use vectorcraft_doc::{Document, Node, NodeId, NodeKind, Scaling}; use vectorcraft_geom::{Anchor, AnchorKind, PathData, Point, SubPath, Vec2}; use super::create::anchor_from_json; @@ -19,11 +19,19 @@ pub fn specs() -> Vec { "Add Anchor (Pen)", [], None, - "{id, x, y, in?: [x,y], out?: [x,y]} append to the end of the last open subpath", + "{id, x, y, in?: [x,y], out?: [x,y], subpath?: usize (the open subpath to extend, 0 included; an invalid or closed one is an error; default: the last open subpath)} append to the end of an open subpath", has_doc, append_anchor ), - cmd!("path.close", "Close Path", [], None, "{id, in?: [x,y] (handle into the first anchor), independent?: bool}", has_doc, close), + cmd!( + "path.close", + "Close Path", + [], + None, + "{id, subpath?: usize (the open subpath to close, 0 included; default: the last open one), in?: [x,y] (handle into the first anchor), independent?: bool}", + has_doc, + close + ), cmd!( "path.moveAnchors", "Move Anchors", @@ -65,7 +73,7 @@ pub fn specs() -> Vec { "Reverse Path Direction", ["Object", "Path"], None, - "{ids?|id?, reversed?: bool (true: every subpath runs counter-clockwise on screen, false: clockwise, the Attributes panel's Reverse Path Direction On / Off; omitted: flip each path)} the paths in ids or the selection (groups and compound paths: the paths inside), as one undo step → {changed: paths}", + "{ids?|id?, subpath?: usize (reverse only this subpath of each path, 0 included; default: every subpath), reversed?: bool (true: every subpath runs counter-clockwise on screen, false: clockwise, the Attributes panel's Reverse Path Direction On / Off; omitted: flip each path; with subpath, the direction is not read: that subpath is flipped)} the paths in ids or the selection (groups and compound paths: the paths inside), as one undo step → {changed: paths}", has_doc, reverse ), @@ -74,7 +82,7 @@ pub fn specs() -> Vec { "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, ...] (default: the selected paths; a compound path ID expands to its open member paths), subpaths?: [usize, usize] (which subpaths to join), ends?: [\"first\"|\"last\", \"first\"|\"last\"] (which end of each path to join, for two paths/subpaths; default without ids: direct-selected ends if any, else the nearest pair; default with ids: the nearest pair), to?: \"corner\"|\"smooth\" (the joined anchor: corner retracts both handles, smooth pulls handles in line with the neighbours)} join two or more open paths/subpaths 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", has_selection, join ), @@ -141,23 +149,42 @@ pub(super) fn path_mut(d: &mut vectorcraft_doc::Document, id: NodeId) -> Result< fn append_anchor(s: &mut Session, p: &Value) -> Result { let id = id_param(p, "id").ok_or_else(|| bad("path.appendAnchor", "missing id"))?; let a = anchor_from_json(p, "path.appendAnchor")?; + let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize); s.edit("Pen", |d, sel| { let path = path_mut(d, id)?; - match path.subpaths.last_mut() { - Some(sp) if !sp.closed => sp.anchors.push(a), - _ => path.subpaths.push(SubPath::new(vec![a], false)), - } - sel.set([id]); - let last = path.subpaths.len() - 1; - let ai = path.subpaths[last].anchors.len() - 1; - sel.anchors.insert(id, BTreeSet::from([(last, ai)])); + let si = match subpath { + // An explicit subpath is honoured, 0 included, or refused: never swapped for another. + Some(si) => match path.subpaths.get_mut(si).filter(|sp| !sp.closed) { + Some(sp) => { + sp.anchors.push(a); + si + } + None => return Err(bad("path.appendAnchor", format!("subpath {si} is not an open subpath"))), + }, + None => match path.subpaths.iter_mut().enumerate().rev().find(|(_, sp)| !sp.closed) { + Some((si, sp)) => { + sp.anchors.push(a); + si + } + None => { + path.subpaths.push(SubPath::new(vec![a], false)); + path.subpaths.len() - 1 + } + }, + }; + let ai = path.subpaths[si].anchors.len() - 1; + let top = compound_or_self(d, id); + sel.set([top]); + sel.anchors.insert(id, BTreeSet::from([(si, ai)])); Ok(()) })?; ok() } +/// Close the active open subpath of `id`, optionally setting the incoming handle of the first anchor. fn close(s: &mut Session, p: &Value) -> Result { let id = id_param(p, "id").ok_or_else(|| bad("path.close", "missing id"))?; + let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize); let h_in = match p.get("in").filter(|h| !h.is_null()) { None => None, Some(h) => Some(finite_numbers(h).map(|[x, y]| Point::new(x, y)).ok_or_else(|| bad("path.close", "`in` must be [x, y]"))?), @@ -165,7 +192,10 @@ fn close(s: &mut Session, p: &Value) -> Result { let independent = switch_or(p, "independent", false, "path.close")?; s.edit("Close Path", |d, sel| { let path = path_mut(d, id)?; - let sp = path.subpaths.iter_mut().rev().find(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?; + let sp = match subpath { + Some(si) => path.subpaths.get_mut(si).filter(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?, + None => path.subpaths.iter_mut().rev().find(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?, + }; sp.closed = true; if let (Some(h), Some(first)) = (h_in, sp.anchors.first_mut()) { first.h_in = h; @@ -174,7 +204,8 @@ fn close(s: &mut Session, p: &Value) -> Result { first.kind = AnchorKind::Smooth; } } - sel.set([id]); + let top = compound_or_self(d, id); + sel.set([top]); Ok(()) })?; ok() @@ -288,9 +319,41 @@ fn set_anchors(s: &mut Session, p: &Value) -> Result { ok() } +/// The compound path enclosing `id`, or `id` itself if it is not inside a compound path. +fn compound_or_self(d: &Document, id: NodeId) -> NodeId { + d.parent_of(id).filter(|p| matches!(d.node(*p).map(|n| &n.kind), Some(NodeKind::Compound { .. }))).unwrap_or(id) +} + +/// The open member paths of a compound path, or its first child if none are open. +fn open_members(n: &Node) -> Vec { + let NodeKind::Compound { children, .. } = &n.kind else { return vec![] }; + let open: Vec = + children.iter().filter(|c| c.path_data().is_some_and(|pd| pd.subpaths.iter().any(|sp| !sp.closed))).map(|c| c.id).collect(); + if open.is_empty() { children.first().map(|c| vec![c.id]).unwrap_or_default() } else { open } +} + +/// The selected paths to join or average, resolving compound paths to their open member paths. fn selected_paths(s: &Session) -> Result> { let st = s.doc()?; - Ok(st.selection.objects.iter().copied().filter(|id| matches!(st.doc.node(*id).map(|n| &n.kind), Some(NodeKind::Path { .. }))).collect()) + let mut out = vec![]; + for &id in &st.selection.objects { + let Some(n) = st.doc.node(id) else { continue }; + match &n.kind { + NodeKind::Path { .. } => out.push(id), + NodeKind::Compound { .. } => { + let open = open_members(n); + let with_anchors: Vec = + open.iter().filter(|id| st.selection.anchors.get(id).is_some_and(|s| !s.is_empty())).copied().collect(); + if !with_anchors.is_empty() { + out.extend(with_anchors); + } else { + out.extend(open); + } + } + _ => {} + } + } + Ok(out) } /// The paths in the subtrees of `ids` (in groups and compound paths too), each once. @@ -309,6 +372,7 @@ pub(super) fn paths_in(d: &vectorcraft_doc::Document, ids: &[NodeId]) -> Vec Result { const C: &str = "path.reverse"; let want = p.get("reversed").map(|v| v.as_bool().ok_or_else(|| bad(C, "`reversed` must be true or false"))).transpose()?; + let subpath = p.get("subpath").and_then(Value::as_u64).map(|u| u as usize); let ids = paths_in(&s.doc()?.doc, &targets(s, p)?); if ids.is_empty() { return Err(bad(C, "select paths")); @@ -316,18 +380,22 @@ fn reverse(s: &mut Session, p: &Value) -> Result { let changed = super::overprint::edit_counted(s, "Reverse Path Direction", |d| { let mut changed = 0; for id in &ids { - // The subpaths to flip: all, or those not yet running the wanted way. - let flip: Vec = d - .node(*id) - .and_then(|n| n.path_data()) - .map_or(vec![], |pd| pd.subpaths.iter().map(|sp| want.is_none_or(|ccw| (sp.signed_area() < 0.0) != ccw)).collect()); - if flip.contains(&true) { - for (sp, f) in path_mut(d, *id)?.subpaths.iter_mut().zip(flip) { - if f { - sp.reverse(); + let path = path_mut(d, *id)?; + if let Some(si) = subpath { + if let Some(sp) = path.subpaths.get_mut(si) { + sp.reverse(); + changed += 1; + } + } else { + let flip: Vec = path.subpaths.iter().map(|sp| want.is_none_or(|ccw| (sp.signed_area() < 0.0) != ccw)).collect(); + if flip.contains(&true) { + for (sp, f) in path.subpaths.iter_mut().zip(flip) { + if f { + sp.reverse(); + } } + changed += 1; } - changed += 1; } } Ok(changed) @@ -386,6 +454,70 @@ fn merge_coincident(sp: &mut SubPath, picked: impl Fn(usize) -> bool, join_kind: before - sp.anchors.len() } +/// Close subpath `si` of path `id`, merging coincident endpoints if present. +fn close_subpath_in_node(d: &mut Document, id: NodeId, si: usize, join_kind: Option) -> Result<()> { + let path = path_mut(d, id)?; + let sp = path.subpaths.get_mut(si).filter(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?; + if sp.anchors.len() > 2 + && end_points(sp).is_some_and(|(f, l)| f.distance(l) < 1e-6) + && let Some(last) = sp.anchors.pop() + && let Some(first) = sp.anchors.first_mut() + { + first.h_in = last.h_in; + sp.closed = true; + if let Some(smooth) = join_kind { + join_point(sp, 0, smooth); + } + } + sp.closed = true; + Ok(()) +} + +/// Join subpaths `sa` and `sb` in the same path node `id`. +fn join_subpaths_in_node(d: &mut Document, id: NodeId, (sa, sb): (usize, usize), (rx, ry): (bool, bool), join_kind: Option) -> Result<()> { + if sa == sb { + return close_subpath_in_node(d, id, sa, join_kind); + } + let path = path_mut(d, id)?; + let mut x = path.subpaths.get(sa).cloned().ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let mut y = path.subpaths.get(sb).cloned().ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + if x.closed || y.closed { + return Err(EngineError::Other("join needs open paths".into())); + } + 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 sa > sb { + path.subpaths.remove(sb); + if let Some(sub) = path.subpaths.get_mut(sa - 1) { + *sub = x; + } + } else { + if let Some(sub) = path.subpaths.get_mut(sa) { + *sub = x; + } + path.subpaths.remove(sb); + } + Ok(()) +} + +/// Join open paths into a single path, or close an open path, preserving compound path structure. fn join(s: &mut Session, p: &Value) -> Result { const C: &str = "path.join"; let join_kind = match p.get("to").and_then(Value::as_str) { @@ -394,19 +526,148 @@ fn join(s: &mut Session, p: &Value) -> Result { Some("corner") => Some(false), Some(_) => return Err(bad(C, "`to` must be \"corner\" or \"smooth\"")), }; - let ids = match ids_param(p, "ids") { - Some(ids) => { - let st = s.doc()?; - if let Some(id) = ids.iter().find(|id| !matches!(st.doc.node(**id).map(|n| &n.kind), Some(NodeKind::Path { .. }))) { - return Err(bad(C, format!("{} isn't a path", id.0))); + let subpaths = match p.get("subpaths") { + None => None, + Some(v) => match v.as_array().and_then(|a| { + if a.len() == 2 + && let (Some(sa), Some(sb)) = (a[0].as_u64(), a[1].as_u64()) + { + Some((sa as usize, sb as usize)) + } else { + None } - ids - } - None => selected_paths(s)?, + }) { + Some(pair) => Some(pair), + None => return Err(bad(C, "`subpaths` must be two integer indices")), + }, }; + // The ends to join, as (reverse the first path, reverse the second): the first path's end + // must come last, the second's first. + let ends = match p.get("ends") { + None => None, + Some(e) => match e.as_array().map(|a| a.iter().map(Value::as_str).collect::>()).as_deref() { + Some([Some(a @ ("first" | "last")), Some(b @ ("first" | "last"))]) => Some((*a == "first", *b == "last")), + _ => return Err(bad(C, "ends must be two of \"first\" and \"last\"")), + }, + }; + + if let Some(ids) = ids_param(p, "ids") { + if ends.is_some() && ids.len() > 2 { + return Err(bad(C, "`ends` applies to joining exactly two paths")); + } + if ids.len() == 2 && ids[0] == ids[1] { + let id = ids[0]; + let (sa, sb) = subpaths.ok_or_else(|| bad(C, "subpaths required when joining the same path"))?; + let (rx, ry) = ends.unwrap_or((false, false)); + s.edit("Join", |d, sel| { + join_subpaths_in_node(d, id, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, id)]); + Ok(()) + })?; + return ok(); + } + if ids.len() == 2 + && let Some((sa, sb)) = subpaths + { + let (a, b) = (ids[0], ids[1]); + let (rx, ry) = ends.unwrap_or((false, false)); + s.edit("Join", |d, sel| { + join_two(d, a, b, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, a)]); + Ok(()) + })?; + return ok(); + } + + let st = s.doc()?; + let mut resolved = vec![]; + for &id in &ids { + let Some(n) = st.doc.node(id) else { + return Err(bad(C, format!("{} isn't a path", id.0))); + }; + match &n.kind { + NodeKind::Path { .. } => resolved.push(id), + NodeKind::Compound { .. } => { + let open = open_members(n); + if ids.len() > 1 && open.len() != 1 { + return Err(bad(C, format!("compound {} must contain exactly one open path to join", id.0))); + } + resolved.extend(open); + } + _ => return Err(bad(C, format!("{} isn't a path", id.0))), + } + } + match resolved.as_slice() { + [one] => { + let id = *one; + let si = match subpaths { + Some((si, _)) => si, + None => { + let pd = s.doc()?.doc.node(id).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(id))?; + pd.subpaths + .iter() + .position(|sp| !sp.closed && !sp.anchors.is_empty()) + .ok_or_else(|| EngineError::Other("path is already closed".into()))? + } + }; + s.edit("Join", |d, sel| { + close_subpath_in_node(d, id, si, join_kind)?; + sel.set([compound_or_self(d, id)]); + Ok(()) + })?; + } + [a, b, ..] => { + if resolved.iter().collect::>().len() != resolved.len() { + return Err(bad(C, "join two different paths")); + } + let (a, b) = (*a, *b); + if ends.is_some() && resolved.len() > 2 { + return Err(bad(C, "`ends` applies to joining exactly two paths")); + } + if let Some((rx, ry)) = ends { + let st = s.doc()?; + let pa = st.doc.node(a).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(a))?; + let pb = st.doc.node(b).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(b))?; + let sa = match subpaths { + Some((s, _)) => s, + None => pa.subpaths.iter().position(|s| !s.closed).ok_or_else(|| EngineError::Other("join needs open paths".into()))?, + }; + let sb = match subpaths { + Some((_, s)) => s, + None => pb.subpaths.iter().position(|s| !s.closed).ok_or_else(|| EngineError::Other("join needs open paths".into()))?, + }; + s.edit("Join", |d, sel| { + join_two(d, a, b, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, a)]); + Ok(()) + })?; + return ok(); + } + let st = s.doc()?; + let mut open_subs = Vec::with_capacity(resolved.len()); + for &id in &resolved { + let pd = st.doc.node(id).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(id))?; + let si = pd + .subpaths + .iter() + .position(|sp| !sp.closed && !sp.anchors.is_empty()) + .ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + open_subs.push((id, si)); + } + s.edit("Join", |d, sel| { + let top = join_nearest(d, open_subs, join_kind)?; + sel.set([top]); + Ok(()) + })?; + } + [] => return Err(EngineError::Other("select paths to join".into())), + } + return ok(); + } + // Direct-selected anchors lying on one another along a path (two corners brought together by // Average) become one anchor; the paths themselves are joined only when there are none. - if p.get("ids").is_none() { + { let st = s.doc()?; let mut merged = vec![]; for (id, picked) in &st.selection.anchors { @@ -423,96 +684,253 @@ fn join(s: &mut Session, p: &Value) -> Result { for (id, path) in merged { *path_mut(d, id)? = path; } - // The anchor indices changed: the paths stay selected, whole. sel.set(objects); Ok(()) })?; return ok(); } } - // The ends to join, as (reverse the first path, reverse the second): the first path's end - // must come last, the second's first. - let ends = match p.get("ends") { - None => None, - Some(e) => match e.as_array().map(|a| a.iter().map(Value::as_str).collect::>()).as_deref() { - Some([Some(a @ ("first" | "last")), Some(b @ ("first" | "last"))]) => Some((*a == "first", *b == "last")), - _ => return Err(bad(C, "ends must be two of \"first\" and \"last\"")), - }, - }; - match ids.as_slice() { - [one] => { - let id = *one; - s.edit("Join", |d, _| { - let path = path_mut(d, id)?; - let sp = path.subpaths.iter_mut().find(|s| !s.closed).ok_or_else(|| EngineError::Other("path is already closed".into()))?; - // Merge coincident end points. - if sp.anchors.len() > 2 - && end_points(sp).is_some_and(|(f, l)| f.distance(l) < 1e-6) - && let Some(last) = sp.anchors.pop() - && let Some(first) = sp.anchors.first_mut() - { - first.h_in = last.h_in; - // Closed first, so the merged anchor smooths between both its neighbours. - sp.closed = true; - if let Some(smooth) = join_kind { - join_point(sp, 0, smooth); + + let direct_ends: Vec<(NodeId, usize, &'static str)> = { + let st = s.doc()?; + let mut ends = vec![]; + for (&id, picked) in &st.selection.anchors { + if let Some(pd) = st.doc.node(id).and_then(|n| n.path_data()) { + for (si, sp) in pd.subpaths.iter().enumerate() { + if sp.closed || sp.anchors.is_empty() { + continue; + } + let last_idx = sp.anchors.len() - 1; + let f = picked.contains(&(si, 0)); + let l = picked.contains(&(si, last_idx)); + if f && l && last_idx > 0 { + ends.push((id, si, "both")); + } else if f { + ends.push((id, si, "first")); + } else if l { + ends.push((id, si, "last")); } } - sp.closed = true; - Ok(()) - })?; - } - [a, b, ..] => { - let (a, b) = (*a, *b); - if a == b { - return Err(bad(C, "join two different paths")); } - 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)]; - 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() - 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; + } + ends + }; + + // More than two selected ends can't name one join: refuse it instead of joining everything. + // A single end (left selected by the Pen) falls through to joining the selected paths. + if direct_ends.len() > 2 { + return Err(bad(C, "select exactly two path ends to join")); + } + + if direct_ends.len() == 1 && direct_ends[0].2 == "both" { + let (id, si, _) = direct_ends[0]; + s.edit("Join", |d, sel| { + close_subpath_in_node(d, id, si, join_kind)?; + sel.set([compound_or_self(d, id)]); + Ok(()) + })?; + return ok(); + } + + if direct_ends.len() == 2 { + let (ida, sa, ea) = direct_ends[0]; + let (idb, sb, eb) = direct_ends[1]; + let (rx, ry) = match (ea, eb) { + ("first", "first") => (true, false), + ("first", "last") => (true, true), + ("last", "first") => (false, false), + ("last", "last") => (false, true), + _ => { + let st = s.doc()?; + let pa = st.doc.node(ida).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(ida))?; + let pb = st.doc.node(idb).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(idb))?; + let (fa, la) = pa.subpaths.get(sa).and_then(end_points).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let (fb, lb) = pb.subpaths.get(sb).and_then(end_points).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let mut best: Option<(bool, bool, f64)> = None; + let cands = [ + (la.distance(fb), false, false, (ea == "last" || ea == "both") && (eb == "first" || eb == "both")), + (la.distance(lb), false, true, (ea == "last" || ea == "both") && (eb == "last" || eb == "both")), + (fa.distance(fb), true, false, (ea == "first" || ea == "both") && (eb == "first" || eb == "both")), + (fa.distance(lb), true, true, (ea == "first" || ea == "both") && (eb == "last" || eb == "both")), + ]; + for (dist, rx_c, ry_c, valid) in cands { + if valid && best.as_ref().is_none_or(|b| dist < b.2) { + best = Some((rx_c, ry_c, dist)); } } - x.anchors.extend(y.anchors); - if let Some(smooth) = join_kind { - join_point(&mut x, junction, smooth); + match best { + Some((rx, ry, _)) => (rx, ry), + None => (ea == "first", eb == "last"), } - pa.subpaths[sa] = x; - d.remove(b)?; - sel.set([a]); + } + }; + if ida == idb { + if sa == sb { + s.edit("Join", |d, sel| { + close_subpath_in_node(d, ida, sa, join_kind)?; + sel.set([compound_or_self(d, ida)]); + Ok(()) + })?; + } else { + s.edit("Join", |d, sel| { + join_subpaths_in_node(d, ida, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, ida)]); + Ok(()) + })?; + } + return ok(); + } else { + s.edit("Join", |d, sel| { + join_two(d, ida, idb, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, ida)]); Ok(()) })?; + return ok(); } - [] => return Err(EngineError::Other("select paths to join".into())), } + + let ids = selected_paths(s)?; + if ids.is_empty() { + return Err(EngineError::Other("select paths to join".into())); + } + let mut open_subs: Vec<(NodeId, usize)> = vec![]; + { + let st = s.doc()?; + for &id in &ids { + if let Some(pd) = st.doc.node(id).and_then(|n| n.path_data()) { + for (si, sp) in pd.subpaths.iter().enumerate() { + if !sp.closed && !sp.anchors.is_empty() { + open_subs.push((id, si)); + } + } + } + } + } + if open_subs.is_empty() { + if ids.len() == 1 { + return Err(EngineError::Other("path is already closed".into())); + } + return Err(EngineError::Other("select paths to join".into())); + } + if open_subs.len() == 1 { + let (id, si) = open_subs[0]; + s.edit("Join", |d, sel| { + close_subpath_in_node(d, id, si, join_kind)?; + sel.set([compound_or_self(d, id)]); + Ok(()) + })?; + return ok(); + } + + s.edit("Join", |d, sel| { + let top = join_nearest(d, open_subs, join_kind)?; + sel.set([top]); + Ok(()) + })?; ok() } +/// Join open subpaths, always the two whose ends lie nearest each other, until one is left. +/// `open_subs` lists `(node, subpath)` pairs. Returns the node holding the result (or its compound). +fn join_nearest(d: &mut Document, mut open_subs: Vec<(NodeId, usize)>, join_kind: Option) -> Result { + let err = || EngineError::Other("join needs open paths".into()); + while open_subs.len() > 1 { + let mut ends_info = Vec::with_capacity(open_subs.len()); + for &(id, si) in &open_subs { + let p = d.node(id).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(id))?; + let sp = p.subpaths.get(si).ok_or_else(err)?; + ends_info.push(end_points(sp).ok_or_else(err)?); + } + let mut best: Option<(usize, usize, bool, bool, f64)> = None; + for (i, &(fa, la)) in ends_info.iter().enumerate() { + for (j, &(fb, lb)) in ends_info.iter().enumerate().skip(i + 1) { + let cands = + [(la.distance(fb), false, false), (la.distance(lb), false, true), (fa.distance(fb), true, false), (fa.distance(lb), true, true)]; + for (dist, rx, ry) in cands { + if best.as_ref().is_none_or(|b| dist < b.4) { + best = Some((i, j, rx, ry, dist)); + } + } + } + } + let Some((i, j, rx, ry, _)) = best else { return Err(err()) }; + let ((ida, sa), (idb, sb)) = (open_subs[i], open_subs[j]); + if ida == idb { + join_subpaths_in_node(d, ida, (sa, sb), (rx, ry), join_kind)?; + // Subpath `sb` is gone and `sa` took the merged result; later subpaths shift down. + open_subs.remove(j); + if sa > sb { + open_subs[i].1 -= 1; + } + for (k, item) in open_subs.iter_mut().enumerate() { + if k != i && item.0 == ida && item.1 > sb { + item.1 -= 1; + } + } + } else { + // `join_two` removes `b` only when `sb` was its last subpath; otherwise `b` keeps its + // other subpaths, which stay in `open_subs` with their indexes shifted down. + join_two(d, ida, idb, (sa, sb), (rx, ry), join_kind)?; + open_subs.remove(j); + for item in &mut open_subs { + if item.0 == idb && item.1 > sb { + item.1 -= 1; + } + } + } + } + let first = open_subs.first().map(|(id, _)| *id).ok_or_else(err)?; + Ok(compound_or_self(d, first)) +} + +/// Join path `b` into path `a` along the specified subpaths and orientations. +fn join_two(d: &mut Document, a: NodeId, b: NodeId, (sa, sb): (usize, usize), (rx, ry): (bool, bool), join_kind: Option) -> 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 mut x = pa.subpaths.get(sa).cloned().ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let mut y = pb.subpaths.get(sb).cloned().ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + if x.closed || y.closed || x.anchors.is_empty() || y.anchors.is_empty() { + return Err(EngineError::Other("join needs open paths".into())); + } + 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(sub) = pa.subpaths.get_mut(sa) { + *sub = x; + } + if pb.subpaths.len() <= 1 { + let parent = d.parent_of(b).filter(|p| matches!(d.node(*p).map(|n| &n.kind), Some(NodeKind::Compound { .. }))); + d.remove(b)?; + if let Some(parent) = parent + && d.node(parent).is_some_and(|n| n.children().is_some_and(|children| children.is_empty())) + { + d.remove(parent)?; + } + } else { + let pb_mut = path_mut(d, b)?; + if sb < pb_mut.subpaths.len() { + pb_mut.subpaths.remove(sb); + } + } + Ok(()) +} + pub(super) type AnchorTargets = Vec<(NodeId, Vec<(usize, usize)>)>; pub(super) fn anchor_targets(s: &Session) -> Result { @@ -624,6 +1042,7 @@ pub(crate) fn delete_anchors(s: &mut Session, _: &Value) -> Result { let cuts = refs.iter().filter_map(|&(s, seg)| (s == si).then_some(seg)).collect(); without_segments(sp, &cuts) }) + .filter(|s| s.anchors.len() >= 2) .collect(); } else { // Remove every selected anchor against the original indexes, keeping surviving @@ -635,6 +1054,7 @@ pub(crate) fn delete_anchors(s: &mut Session, _: &Value) -> Result { let gone = set.iter().filter_map(|&(s, ai)| (s == si).then_some(ai)).collect(); without_anchors(sp, &gone) }) + .filter(|s| s.anchors.len() >= 2) .collect(); } path.subpaths.retain(|s| s.anchors.len() >= 2); @@ -693,7 +1113,7 @@ fn without_anchors(sp: SubPath, removed: &BTreeSet) -> Vec { } /// Remove connections, splitting open/closed contours into their surviving open runs. -fn without_segments(sp: SubPath, removed: &BTreeSet) -> Vec { +pub(super) fn without_segments(sp: SubPath, removed: &BTreeSet) -> Vec { let cuts: BTreeSet = removed.iter().copied().filter(|&i| i < sp.segment_count()).collect(); let Some(&first) = cuts.first() else { return vec![sp] }; let n = sp.anchors.len(); diff --git a/crates/engine/src/cmd/select.rs b/crates/engine/src/cmd/select.rs index 2f39fe472..8ec804c13 100644 --- a/crates/engine/src/cmd/select.rs +++ b/crates/engine/src/cmd/select.rs @@ -471,8 +471,10 @@ fn anchors(s: &mut Session, p: &Value) -> Result { if mode == "set" { sel.clear(); } - // A path selected as a whole has every anchor selected: toggle and add start from them all. - let whole = sel.contains(id) && sel.partial(id).is_none(); + // A path selected as a whole has every anchor selected: toggle and add start from them all, + // but only when no partial anchor selection is active and only on a single object. With + // several objects selected, a click picks that anchor alone, so Join can take two picked ends. + let whole = sel.contains(id) && sel.partial(id).is_none() && sel.anchors.is_empty() && sel.objects.len() <= 1; sel.add(id); // A segment pick's endpoints aren't selected anchors: start from none of them. if sel.segments.remove(&id).is_some() { diff --git a/crates/engine/src/tests_draw2.rs b/crates/engine/src/tests_draw2.rs index cc247ee8a..d5278c566 100644 --- a/crates/engine/src/tests_draw2.rs +++ b/crates/engine/src/tests_draw2.rs @@ -1683,25 +1683,6 @@ fn direct_segment_delete_keeps_the_other_rectangle_edges_and_one_undo() { } } -/// A segment pick's endpoints are hollow: Shift-clicking or Shift-marqueeing one selects that -/// anchor, rather than toggling it off and leaving the other endpoint selected. -#[test] -fn selecting_an_anchor_after_a_segment_pick_starts_from_no_anchors() { - let mut s = session(); - let id = rect(&mut s, 100.0, 100.0, 200.0, 150.0); - let segment = json!({"id": id.0, "segments": [[0, 0]]}); - s.execute("select.segments", &segment).unwrap(); - s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 0]], "mode": "toggle"})).unwrap(); - let sel = &s.doc().unwrap().selection; - assert_eq!(sel.partial(id), Some(&[(0, 0)].into())); - assert!(sel.segments.is_empty()); - s.execute("select.segments", &segment).unwrap(); - s.execute("select.anchorsMany", &json!({"items": [{"id": id.0, "anchors": [[0, 1]]}], "mode": "add"})).unwrap(); - let sel = &s.doc().unwrap().selection; - assert_eq!(sel.partial(id), Some(&[(0, 1)].into())); - assert!(sel.segments.is_empty()); -} - #[test] fn segment_delete_preserves_curves_appearance_and_unrelated_contours() { let data = "M100 100C110 70 150 130 170 100C180 75 230 125 250 100C270 140 280 80 310 100C320 120 140 140 100 100Z M400 100L500 100L500 200Z"; @@ -2398,3 +2379,536 @@ fn open_anchor_delete_after_transform_keeps_mixed_selection_atomic() { assert_eq!(s.doc().unwrap().selection.objects, vec![id]); } } + +/// Pen tool can continue drawing an open path inside a compound path and close it. +#[test] +fn compound_path_pen_continue_and_close() { + use vectorcraft_tools::Cursor; + let (v, none) = (view(), Mods::default()); + let mut s = session(); + let r = rect(&mut s, 50.0, 50.0, 200.0, 200.0); + let p = line(&mut s, 100.0, 100.0, 200.0, 100.0); + s.execute("select.set", &json!({"ids": [r.0, p.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + s.select_tool("pen", v).unwrap(); + // Hovering over the last anchor of the open member path shows PenContinue. + assert_eq!(s.cursor(Point::new(200.0, 100.0), none, v), Cursor::PenContinue); + // Click on the last anchor to continue drawing. + gesture(&mut s, &[(200.0, 100.0)], none); + // Rubber band is active, cursor over the start of the path shows PenClose. + assert_eq!(s.cursor(Point::new(100.0, 100.0), none, v), Cursor::PenClose); + // Add an intermediate anchor. + gesture(&mut s, &[(200.0, 200.0)], none); + assert_eq!(s.cursor(Point::new(100.0, 100.0), none, v), Cursor::PenClose); + // Click on the first anchor to close the path. + gesture(&mut s, &[(100.0, 100.0)], none); + + // Verify member path is now closed inside the compound path. + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 2); + for child in ch { + let pd = child.path_data().unwrap(); + assert!(pd.is_closed(), "all member paths should be closed"); + } + // Selection should be on the compound path. + assert_eq!(s.doc().unwrap().selection.objects, vec![cid]); +} + +/// Pen tool can continue an open path inside a compound path and join another open path inside it. +#[test] +fn compound_path_pen_continue_and_join() { + use vectorcraft_tools::Cursor; + let (v, none) = (view(), Mods::default()); + let mut s = session(); + let a = line(&mut s, 100.0, 100.0, 200.0, 100.0); + let b = line(&mut s, 300.0, 100.0, 400.0, 100.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + s.select_tool("pen", v).unwrap(); + assert_eq!(s.cursor(Point::new(200.0, 100.0), none, v), Cursor::PenContinue); + // Continue from a's end. + gesture(&mut s, &[(200.0, 100.0)], none); + // Hover over b's start shows PenJoin. + assert_eq!(s.cursor(Point::new(300.0, 100.0), none, v), Cursor::PenJoin); + // Click on b's start to join them. + assert_eq!(gesture(&mut s, &[(300.0, 100.0)], none), 1); + + // The two open paths are now joined into one path inside the compound path. + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 1, "joined into one path"); + let pd = ch[0].path_data().unwrap(); + assert_eq!(pd.subpaths[0].anchors.len(), 4); + assert_eq!(s.doc().unwrap().selection.objects, vec![cid]); +} + +/// Path join (Cmd+J) works on compound paths with open paths. +#[test] +fn compound_path_join_command() { + let mut s = session(); + let a = line(&mut s, 100.0, 100.0, 200.0, 100.0); + let b = line(&mut s, 300.0, 100.0, 400.0, 100.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + // Join with compound path selected joins its 2 open paths. + s.execute("path.join", &json!({})).unwrap(); + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 1); + let pd = ch[0].path_data().unwrap(); + assert!(!pd.is_closed()); + assert_eq!(pd.subpaths[0].anchors.len(), 4); + assert_eq!(s.doc().unwrap().selection.objects, vec![cid]); + + // Joining again closes the single open path. + s.execute("path.join", &json!({})).unwrap(); + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 1); + assert!(ch[0].path_data().unwrap().is_closed()); +} + +/// Path join with direct-selected endpoints of a compound path connects those endpoints. +#[test] +fn compound_path_join_direct_selected_endpoints() { + let mut s = session(); + let a = line(&mut s, 100.0, 100.0, 200.0, 100.0); + let b = line(&mut s, 300.0, 100.0, 400.0, 100.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + let ch = s.doc().unwrap().doc.node(cid).unwrap().children().unwrap().clone(); + let ca = ch[0].id; + let cb = ch[1].id; + + // Direct select endpoints: ca's first (100, 100) and cb's start (300, 100). + s.execute("select.anchors", &json!({"id": ca.0, "anchors": [[0, 0]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": cb.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 1); + let pd = ch[0].path_data().unwrap(); + let pts: Vec<(f64, f64)> = pd.subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect(); + // ca's first was connected to cb's last. + assert_eq!(pts, [(200.0, 100.0), (100.0, 100.0), (300.0, 100.0), (400.0, 100.0)]); +} + +/// Explicit ids in path.join validate compound expansion and allow joining three or more paths. +#[test] +fn compound_path_join_explicit_ids_validation() { + let mut s = session(); + let a = line(&mut s, 100.0, 100.0, 200.0, 100.0); + let b = line(&mut s, 300.0, 100.0, 400.0, 100.0); + let c = line(&mut s, 500.0, 100.0, 600.0, 100.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + // Multi-id join with compound containing != 1 open path (has 2) errors. + let err = s.execute("path.join", &json!({"ids": [cid.0, c.0]})).unwrap_err(); + assert!(err.to_string().contains("must contain exactly one open path to join")); + + // Single compound id with 2 open paths resolves to 2 paths and succeeds. + assert!(s.execute("path.join", &json!({"ids": [cid.0]})).is_ok()); + + // Multi-id join with compound now containing 1 open path + another path succeeds. + assert!(s.execute("path.join", &json!({"ids": [cid.0, c.0]})).is_ok()); + + // Joining with explicit ends and > 2 paths errors. + let x = line(&mut s, 0.0, 0.0, 10.0, 10.0); + let y = line(&mut s, 20.0, 0.0, 30.0, 10.0); + let z = line(&mut s, 40.0, 0.0, 50.0, 10.0); + let err = s.execute("path.join", &json!({"ids": [x.0, y.0, z.0], "ends": ["first", "last"]})).unwrap_err(); + assert!(err.to_string().contains("`ends` applies to joining exactly two paths")); + + // Joining three or more paths joins them all. + s.execute("path.join", &json!({"ids": [x.0, y.0, z.0]})).unwrap(); + let d = &s.doc().unwrap().doc; + let xnode = d.node(x).unwrap(); + let pd = xnode.path_data().unwrap(); + assert_eq!(pd.subpaths[0].anchors.len(), 6); + assert!(d.node(y).is_none()); + assert!(d.node(z).is_none()); +} + +/// Joining the only member of a compound path into another path removes the empty compound parent. +#[test] +fn join_removes_empty_compound_parent() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 10.0, 0.0); + let b = line(&mut s, 12.0, 0.0, 20.0, 0.0); + // Make b into a compound path (single open path child) + s.execute("select.set", &json!({"ids": [b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + // Join a and the compound path + s.execute("select.set", &json!({"ids": [a.0, cid.0]})).unwrap(); + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + // Compound path container should be removed since its only member was joined into a. + assert!(d.node(cid).is_none()); + assert!(d.node(a).is_some()); + let pd = d.node(a).unwrap().path_data().unwrap(); + assert_eq!(pd.subpaths[0].anchors.len(), 4); +} + +/// Join with three or more open paths selected joins them all into a single path (Illustrator parity). +#[test] +fn join_three_or_more_paths() { + let mut s = session(); + // Path 1: (0, 0) -> (10, 0) + let p1 = line(&mut s, 0.0, 0.0, 10.0, 0.0); + // Path 2: (12, 0) -> (20, 0) + let p2 = line(&mut s, 12.0, 0.0, 20.0, 0.0); + // Path 3: (22, 0) -> (30, 0) + let p3 = line(&mut s, 22.0, 0.0, 30.0, 0.0); + // Path 4: (32, 0) -> (40, 0) + let p4 = line(&mut s, 32.0, 0.0, 40.0, 0.0); + + s.execute("select.set", &json!({"ids": [p1.0, p2.0, p3.0, p4.0]})).unwrap(); + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + let remaining_id = s.doc().unwrap().selection.objects[0]; + let pd = d.node(remaining_id).unwrap().path_data().unwrap(); + assert_eq!(pd.subpaths.len(), 1); + assert!(!pd.is_closed()); + assert_eq!(pd.subpaths[0].anchors.len(), 8); + let xs: Vec = pd.subpaths[0].anchors.iter().map(|a| a.p.x).collect(); + assert_eq!(xs, vec![0.0, 10.0, 12.0, 20.0, 22.0, 30.0, 32.0, 40.0]); +} + +/// Join with compound path containing three open member paths joins them all into one. +#[test] +fn compound_path_join_three_open_members() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 10.0, 0.0); + let b = line(&mut s, 15.0, 0.0, 25.0, 0.0); + let c = line(&mut s, 30.0, 0.0, 40.0, 0.0); + s.execute("select.set", &json!({"ids": [a.0, b.0, c.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + let cnode = d.node(cid).unwrap(); + let ch = cnode.children().unwrap(); + assert_eq!(ch.len(), 1); + let pd = ch[0].path_data().unwrap(); + assert!(!pd.is_closed()); + assert_eq!(pd.subpaths[0].anchors.len(), 6); + assert_eq!(s.doc().unwrap().selection.objects, vec![cid]); +} + +/// Joining one subpath of a multi-subpath node retains its other subpaths. +#[test] +fn join_multi_subpath_retains_unjoined_subpaths() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 200.0, 0.0, 300.0, 0.0); + // Add a second open subpath to b. + let second_sub = vectorcraft_geom::SubPath::polyline(&[Point::new(400.0, 50.0), Point::new(500.0, 50.0)], false); + s.edit("Add Subpath", |d, _| { + if let NodeKind::Path { path, .. } = &mut d.node_mut(b).unwrap().kind { + path.subpaths.push(second_sub); + } + Ok(()) + }) + .unwrap(); + + // Direct-select an endpoint on a and an endpoint on b's second subpath. + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 1]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[1, 0]], "mode": "add"})).unwrap(); + + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + // Node b still exists because its first subpath was not consumed. + assert!(d.node(b).is_some()); + let pb = d.node(b).unwrap().path_data().unwrap(); + assert_eq!(pb.subpaths.len(), 1); + assert_eq!(pb.subpaths[0].anchors[0].p, Point::new(200.0, 0.0)); + + // Node a now has the joined subpath. + let pa = d.node(a).unwrap().path_data().unwrap(); + assert_eq!(pa.subpaths.len(), 1); + assert_eq!(pa.subpaths[0].anchors.len(), 4); +} + +/// The Pen tool does not continue or join guide paths. +#[test] +fn pen_does_not_continue_or_join_guide_paths() { + use vectorcraft_tools::Cursor; + let (v, none) = (view(), Mods::default()); + let mut s = session(); + let g = line(&mut s, 100.0, 100.0, 200.0, 100.0); + s.execute("select.set", &json!({"ids": [g.0]})).unwrap(); + s.execute("view.guides.make", &json!({})).unwrap(); + + s.select_tool("pen", v).unwrap(); + // Hovering over the guide endpoint does not offer PenContinue. + assert_ne!(s.cursor(Point::new(200.0, 100.0), none, v), Cursor::PenContinue); +} + +/// Joining two open subpaths within the same path node merges them into one subpath. +#[test] +fn join_two_subpaths_of_same_path_node() { + let mut s = session(); + let p = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let sub2 = vectorcraft_geom::SubPath::polyline(&[Point::new(150.0, 0.0), Point::new(250.0, 0.0)], false); + s.edit("Add Subpath", |d, _| { + if let NodeKind::Path { path, .. } = &mut d.node_mut(p).unwrap().kind { + path.subpaths.push(sub2); + } + Ok(()) + }) + .unwrap(); + + // Direct-select anchor (0, 1) and anchor (1, 0) of the same node. + s.execute("select.anchors", &json!({"id": p.0, "anchors": [[0, 1], [1, 0]], "mode": "set"})).unwrap(); + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + let pd = d.node(p).unwrap().path_data().unwrap(); + assert_eq!(pd.subpaths.len(), 1); + assert!(!pd.subpaths[0].closed); + assert_eq!(pd.subpaths[0].anchors.len(), 4); + assert_eq!(pd.subpaths[0].anchors[0].p, Point::new(0.0, 0.0)); + assert_eq!(pd.subpaths[0].anchors[1].p, Point::new(100.0, 0.0)); + assert_eq!(pd.subpaths[0].anchors[2].p, Point::new(150.0, 0.0)); + assert_eq!(pd.subpaths[0].anchors[3].p, Point::new(250.0, 0.0)); +} + +/// Direct-selecting 2 open endpoints in a complex compound path with multiple open segments joins only those 2. +#[test] +fn join_complex_compound_path_direct_endpoints() { + let mut s = session(); + let c1 = line(&mut s, 0.0, 0.0, 50.0, 0.0); + let c2 = line(&mut s, 100.0, 0.0, 150.0, 0.0); + let c3 = line(&mut s, 200.0, 0.0, 250.0, 0.0); + + s.execute("select.set", &json!({"ids": [c1.0, c2.0, c3.0]})).unwrap(); + let comp = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + // Direct-select the end of c1 and the start of c3. + s.execute("select.anchors", &json!({"id": c1.0, "anchors": [[0, 1]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": c3.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + + s.execute("path.join", &json!({})).unwrap(); + + let d = &s.doc().unwrap().doc; + let comp_node = d.node(comp).unwrap(); + let children = comp_node.children().unwrap(); + // c3 was joined into c1, c2 remains untouched. Total 2 children in compound. + assert_eq!(children.len(), 2); + let joined = children.iter().find(|c| c.id == c1).unwrap().path_data().unwrap(); + assert_eq!(joined.subpaths.len(), 1); + assert_eq!(joined.subpaths[0].anchors.len(), 4); + assert_eq!(joined.subpaths[0].anchors[0].p, Point::new(0.0, 0.0)); + assert_eq!(joined.subpaths[0].anchors[1].p, Point::new(50.0, 0.0)); + assert_eq!(joined.subpaths[0].anchors[2].p, Point::new(200.0, 0.0)); + assert_eq!(joined.subpaths[0].anchors[3].p, Point::new(250.0, 0.0)); + + let intact = children.iter().find(|c| c.id == c2).unwrap().path_data().unwrap(); + assert_eq!(intact.subpaths.len(), 1); + assert_eq!(intact.subpaths[0].anchors.len(), 2); +} + +/// Continuing a complex compound path with the Pen tool begins at the exact clicked anchor and joins to another open segment. +#[test] +fn pen_continue_and_join_specific_open_subpath_in_compound() { + let v = view(); + let mut s = session(); + let c1 = line(&mut s, 10.0, 100.0, 60.0, 100.0); + let c2 = line(&mut s, 10.0, 200.0, 60.0, 200.0); + s.execute("select.set", &json!({"ids": [c1.0, c2.0]})).unwrap(); + let comp = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + + s.select_tool("pen", v).unwrap(); + // Click at the end of c1 (60, 100) to continue drawing c1. + s.pointer(&PointerEvent::new(PointerKind::Down, 60.0, 100.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, 60.0, 100.0), v).unwrap(); + + // Now click at the start of c2 (10, 200) to join c1 and c2. + s.pointer(&PointerEvent::new(PointerKind::Down, 10.0, 200.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, 10.0, 200.0), v).unwrap(); + + let d = &s.doc().unwrap().doc; + let comp_node = d.node(comp).unwrap(); + let children = comp_node.children().unwrap(); + assert_eq!(children.len(), 1); + let pd = children[0].path_data().unwrap(); + assert_eq!(pd.subpaths[0].anchors.len(), 4); + assert_eq!(pd.subpaths[0].anchors[0].p, Point::new(10.0, 100.0)); + assert_eq!(pd.subpaths[0].anchors[1].p, Point::new(60.0, 100.0)); + assert_eq!(pd.subpaths[0].anchors[2].p, Point::new(10.0, 200.0)); + assert_eq!(pd.subpaths[0].anchors[3].p, Point::new(60.0, 200.0)); +} + +#[test] +fn join_rejects_ends_with_more_than_two_paths() { + let mut s = session(); + let a = rect(&mut s, 10.0, 10.0, 20.0, 20.0); + let b = rect(&mut s, 40.0, 10.0, 20.0, 20.0); + let c = rect(&mut s, 70.0, 10.0, 20.0, 20.0); + let err = s.execute("path.join", &json!({"ids": [a.0, b.0, c.0], "ends": ["first", "last"]})).unwrap_err(); + assert!(err.to_string().contains("`ends` applies to joining exactly two paths")); +} + +#[test] +fn join_removes_empty_compound_when_last_member_joined() { + let mut s = session(); + let p1 = NodeId(s.execute("path.create", &json!({"d": "M10 10 L50 10"})).unwrap()["id"].as_u64().unwrap()); + let p2 = NodeId(s.execute("path.create", &json!({"d": "M60 10 L100 10"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.set", &json!({"ids": [p2.0]})).unwrap(); + let comp = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + assert!(matches!(s.doc().unwrap().doc.node(comp).unwrap().kind, NodeKind::Compound { .. })); + // Join p1 and compound containing p2 + s.execute("path.join", &json!({"ids": [p1.0, comp.0]})).unwrap(); + assert!(s.doc().unwrap().doc.node(comp).is_none(), "empty compound container was removed"); +} + +#[test] +fn test_join_direct_selected_two_anchors() { + for (pick_a, pick_b, exp_junction) in [(0, 0, (0.0, 105.0)), (0, 1, (0.0, 200.0)), (1, 0, (100.0, 105.0)), (1, 1, (100.0, 200.0))] { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 105.0, 0.0, 200.0, 0.0); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, pick_a]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, pick_b]], "mode": "toggle"})).unwrap(); + s.execute("path.join", &json!({})).unwrap(); + + let pd = path(&s, a); + let pts: Vec = pd.subpaths[0].anchors.iter().map(|x| x.p.x).collect(); + // Check junction: there must be consecutive points matching exp_junction + let connected = + pts.windows(2).any(|w| (w[0] == exp_junction.0 && w[1] == exp_junction.1) || (w[0] == exp_junction.1 && w[1] == exp_junction.0)); + assert!(connected, "pick_a={pick_a}, pick_b={pick_b}, exp={exp_junction:?}, got pts={pts:?}"); + } +} + +#[test] +fn test_join_direct_selected_after_whole_selection() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 105.0, 0.0, 200.0, 0.0); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + // User wants to join (0, 0) and (200, 0): pick a's anchor 0 and b's anchor 1 + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]], "mode": "toggle"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, 1]], "mode": "toggle"})).unwrap(); + s.execute("path.join", &json!({})).unwrap(); + + let pd = path(&s, a); + let pts: Vec = pd.subpaths[0].anchors.iter().map(|x| x.p.x).collect(); + let connected = pts.windows(2).any(|w| (w[0] == 0.0 && w[1] == 200.0) || (w[0] == 200.0 && w[1] == 0.0)); + assert!(connected, "expected 0.0 and 200.0 to be connected, got pts={pts:?}"); +} + +/// Review #2: the Pen sends the subpath it continues even when that is subpath 0, so the engine +/// never falls back to the last open subpath (append, close) or to every subpath (reverse). +#[test] +fn pen_continues_and_reverses_subpath_zero_of_a_path_with_two_open_subpaths() { + let v = view(); + let mut s = session(); + let id = NodeId(s.execute("path.create", &json!({"d": "M0 0 L100 0 M0 50 L100 50"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.select_tool("pen", v).unwrap(); + + // Continue from the end of subpath 0: the new anchor must land on subpath 0, not on subpath 1. + s.pointer(&PointerEvent::new(PointerKind::Down, 100.0, 0.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, 100.0, 0.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Down, 200.0, 0.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, 200.0, 0.0), v).unwrap(); + let pd = path(&s, id); + assert_eq!(pd.subpaths[0].anchors.len(), 3, "the new anchor extends subpath 0"); + assert_eq!(pd.subpaths[1].anchors.len(), 2, "subpath 1 is left alone"); + assert_eq!(pd.subpaths[0].anchors[2].p, Point::new(200.0, 0.0)); + + // Continue from the start of subpath 0: only subpath 0 reverses, the other keeps its direction. + let mut s = session(); + let id = NodeId(s.execute("path.create", &json!({"d": "M0 0 L100 0 M0 50 L100 50"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + s.select_tool("pen", v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Down, 0.0, 0.0), v).unwrap(); + s.pointer(&PointerEvent::new(PointerKind::Up, 0.0, 0.0), v).unwrap(); + let pd = path(&s, id); + assert_eq!(pd.subpaths[0].anchors[0].p, Point::new(100.0, 0.0), "subpath 0 was reversed"); + assert_eq!(pd.subpaths[1].anchors[0].p, Point::new(0.0, 50.0), "subpath 1 keeps its direction"); +} + +/// Review #5: Join closes the first open subpath, not subpath 0, so a closed subpath 0 doesn't +/// make it fail with "path is already closed". +#[test] +fn join_closes_the_open_subpath_when_subpath_zero_is_closed() { + for explicit in [false, true] { + let mut s = session(); + let id = NodeId(s.execute("path.create", &json!({"d": "M0 0 L10 0 L10 10 Z M50 0 L90 0 L90 40"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.set", &json!({"ids": [id.0]})).unwrap(); + let params = if explicit { json!({"ids": [id.0]}) } else { json!({}) }; + s.execute("path.join", ¶ms).unwrap(); + let pd = path(&s, id); + assert!(pd.subpaths[0].closed && pd.subpaths[1].closed, "both subpaths are closed now: {params}"); + assert_eq!(pd.subpaths[0].anchors.len(), 3, "the closed subpath is untouched"); + } +} + +/// Review #7: more than two selected path ends can't name one join, so it is refused rather than +/// silently joining every selected path. +#[test] +fn join_refuses_more_than_two_selected_ends() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 10.0, 0.0); + let b = line(&mut s, 20.0, 0.0, 30.0, 0.0); + let c = line(&mut s, 40.0, 0.0, 50.0, 0.0); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 0]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + s.execute("select.anchors", &json!({"id": c.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + let before = s.doc().unwrap().doc.clone(); + let err = s.execute("path.join", &json!({})).unwrap_err(); + assert!(err.to_string().contains("exactly two"), "{err}"); + assert_eq!(s.doc().unwrap().doc, before, "nothing was joined"); +} + +/// Review #7: joining a path to one subpath of a node that has other subpaths leaves that node and +/// its remaining subpaths in place, also when more paths follow. +#[test] +fn join_keeps_the_other_subpaths_of_a_joined_node() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 10.0, 0.0); + let b = NodeId(s.execute("path.create", &json!({"d": "M10 0 L20 0 M100 100 L110 100"})).unwrap()["id"].as_u64().unwrap()); + let c = line(&mut s, 20.0, 0.0, 30.0, 0.0); + s.execute("path.join", &json!({"ids": [a.0, b.0, c.0]})).unwrap(); + assert_eq!(path(&s, a).subpaths[0].anchors.len(), 4, "a, b's first subpath and c became one"); + let rest = path(&s, b); + assert_eq!(rest.subpaths.len(), 1, "b keeps its other subpath"); + assert_eq!(rest.subpaths[0].anchors[0].p, Point::new(100.0, 100.0)); + assert!(s.doc().unwrap().doc.node(c).is_none(), "c was consumed"); +} + +/// Review #7: an empty subpath is refused with an error, not an underflow. +#[test] +fn join_two_refuses_an_empty_subpath() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 10.0, 0.0); + let b = line(&mut s, 20.0, 0.0, 30.0, 0.0); + s.edit("Empty subpath", |d, _| { + if let NodeKind::Path { path, .. } = &mut d.node_mut(a).unwrap().kind { + path.subpaths[0] = vectorcraft_geom::SubPath::new(vec![], false); + } + Ok(()) + }) + .unwrap(); + assert!(s.execute("path.join", &json!({"ids": [a.0, b.0], "ends": ["last", "first"], "subpaths": [0, 0]})).is_err()); +} diff --git a/crates/tools/src/pen.rs b/crates/tools/src/pen.rs index 85168a4b2..2b7bf12e8 100644 --- a/crates/tools/src/pen.rs +++ b/crates/tools/src/pen.rs @@ -30,7 +30,7 @@ use vectorcraft_doc::{NodeId, NodeKind}; use vectorcraft_geom::{BezPath, Point, Vec2}; use crate::direct::hit_handle; -use crate::draw2::{AnchorTool, editable_paths}; +use crate::draw2::{AnchorTool, editable_paths, shown_paths}; use crate::guides::{DrawSnap, Leave}; use crate::{Action, Cursor, Mods, Overlay, PointerEvent, PointerKind, Tool, ToolContext, ToolKey}; @@ -40,6 +40,8 @@ pub struct PenTool { drawing: bool, /// ...this one, once the engine made it: another path selected meanwhile isn't drawn on. path: Option, + /// The subpath of `path` being drawn or continued. + subpath: Option, /// The anchor a press placed, while the drag shapes its handles. drag: Option, hover: Option, @@ -72,48 +74,110 @@ const DRAG_THRESHOLD: f64 = 3.0; struct Place { at: Point, path: Option, + subpath: Option, closing: bool, } -/// The open path the pen is extending: the single selected open path. -fn active_path(cx: &ToolContext) -> Option<(NodeId, Point, Point, Point)> { +/// The open path the pen is extending: the single selected open path, or the single open path of a +/// single selected compound path. +fn active_path(cx: &ToolContext) -> Option<(NodeId, usize, Point, Point, Point)> { if cx.selection.objects.len() != 1 { return None; } - let id = cx.selection.objects[0]; + let id = cx.selection.objects.first().copied()?; let n = cx.doc.node(id)?; - let NodeKind::Path { path, .. } = &n.kind else { return None }; - let sp = path.subpaths.last()?; - if sp.closed { + let path_node = match &n.kind { + NodeKind::Path { .. } => Some(n), + NodeKind::Compound { children, .. } => { + let open: Vec<&vectorcraft_doc::Node> = + children.iter().filter(|c| c.path_data().is_some_and(|pd| pd.subpaths.iter().any(|sp| !sp.closed))).map(|c| c.as_ref()).collect(); + if open.len() == 1 { open.first().copied() } else { None } + } + _ => None, + }?; + let path = path_node.path_data()?; + let open_subs: Vec<(usize, &vectorcraft_geom::SubPath)> = path.subpaths.iter().enumerate().filter(|(_, s)| !s.closed).collect(); + if open_subs.len() != 1 { return None; } + let (si, sp) = open_subs[0]; let first = sp.anchors.first()?.p; let last = sp.anchors.last()?; - Some((id, first, last.p, last.h_out)) + Some((path_node.id, si, first, last.p, last.h_out)) } -/// An end of an open path (one subpath, not `except`) under `p`, within `tol`: the path the Pen -/// continues or joins to, and whether it's the path's first anchor. Only paths that can be edited, -/// not guides. -fn open_end_at(cx: &ToolContext, p: Point, tol: f64, except: Option) -> Option<(NodeId, bool)> { - let id = vectorcraft_doc::hit::hit_test(cx.doc, p, cx.hit_options())?.leaf; - if Some(id) == except || !cx.doc.is_editable(id) { +/// Checks if an editable endpoint on path `id` lies within `tol` of point `p`, returning the endpoint info and distance. +fn endpoint_on(cx: &ToolContext, id: NodeId, p: Point, tol: f64, except: Option<(NodeId, usize)>) -> Option<((NodeId, usize, bool), f64)> { + if !cx.doc.is_editable(id) || cx.doc.node(id).is_some_and(|n| matches!(&n.kind, NodeKind::Path { guide: true, .. })) { return None; } - let NodeKind::Path { path, guide: false, .. } = &cx.doc.node(id)?.kind else { return None }; - let [sp] = path.subpaths.as_slice() else { return None }; - if sp.closed { - return None; + let pd = cx.doc.node(id).and_then(|n| n.path_data())?; + let mut best: Option<((NodeId, usize, bool), f64)> = None; + for (si, sp) in pd.subpaths.iter().enumerate() { + if Some((id, si)) == except || sp.closed || sp.anchors.is_empty() { + continue; + } + let first = sp.anchors.first().map(|a| a.p); + let Some(last) = sp.anchors.last().map(|a| a.p) else { continue }; + if let Some(end) = pick_endpoint(first, last, p, tol, Endpoint::Last) { + let pt = match end { + Endpoint::First => first.unwrap_or(last), + Endpoint::Last => last, + }; + let dist = p.distance(pt); + if best.as_ref().is_none_or(|(_, d)| dist < *d) { + best = Some(((id, si, end == Endpoint::First), dist)); + } + } } - let (first, last) = (sp.anchors.first()?.p, sp.anchors.last()?.p); - let end = pick_endpoint(Some(first), last, p, tol, Endpoint::Last)?; - Some((id, end == Endpoint::First)) + best } -/// The last anchor of `id`'s last subpath: (subpath, anchor). -fn last_anchor(cx: &ToolContext, id: NodeId) -> Option<(usize, usize)> { +/// An end of an open path (any editable path, including inside groups and compound paths, not `except`) +/// under `p`, within `tol`: the path the Pen continues or joins to, and whether it's the path's first anchor. +fn open_end_at(cx: &ToolContext, p: Point, tol: f64, except: Option<(NodeId, usize)>) -> Option<(NodeId, usize, bool)> { + let mut best: Option<((NodeId, usize, bool), f64)> = None; + // 1. Shown paths (anchors visible because object, group, or compound path is selected). + for id in shown_paths(cx) { + if let Some(cand) = endpoint_on(cx, id, p, tol, except) + && best.as_ref().is_none_or(|(_, d)| cand.1 < *d) + { + best = Some(cand); + } + } + if best.is_some() { + return best.map(|(res, _)| res); + } + // 2. Hit-tested object (clicked directly on an unselected path, group, or compound path). + if let Some(hit) = vectorcraft_doc::hit::hit_test(cx.doc, p, cx.hit_options()) { + let leaf = hit.leaf; + if cx.doc.is_editable(leaf) { + if let Some(cand) = endpoint_on(cx, leaf, p, tol, except) { + return Some(cand.0); + } + if let Some(n) = cx.doc.node(leaf) + && let NodeKind::Compound { children, .. } = &n.kind + { + for c in children { + if let Some(cand) = endpoint_on(cx, c.id, p, tol, except) + && best.as_ref().is_none_or(|(_, d)| cand.1 < *d) + { + best = Some(cand); + } + } + } + } + } + best.map(|(res, _)| res) +} + +/// The last anchor of `id`'s subpath: (subpath, anchor). +fn last_anchor(cx: &ToolContext, id: NodeId, subpath: Option) -> Option<(usize, usize)> { let pd = cx.doc.node(id)?.path_data()?; - let si = pd.subpaths.len().checked_sub(1)?; + let si = match subpath { + Some(si) if si < pd.subpaths.len() => si, + _ => pd.subpaths.len().checked_sub(1)?, + }; let ai = pd.subpaths.get(si)?.anchors.len().checked_sub(1)?; Some((si, ai)) } @@ -181,8 +245,12 @@ impl Tool for PenTool { fn pointer(&mut self, cx: &ToolContext, ev: &PointerEvent) -> Vec { let tol = cx.tol(5.0).max(cx.point_tol()); // The path a press began is the selected one by the next event. - if self.drawing && self.path.is_none() { - self.path = active_path(cx).map(|a| a.0); + if self.drawing + && self.path.is_none() + && let Some(a) = active_path(cx) + { + self.path = Some(a.0); + self.subpath = Some(a.1); } let active = self.active(cx); if self.drawing && active.is_none() && ev.kind == PointerKind::Down { @@ -190,10 +258,12 @@ impl Tool for PenTool { } // The path being drawn moves its bounds as it grows: only its anchors and segments pull. let drawn = active.map(|a| a.0); - let from = active.map(|(_, _, last, handle)| { + + let from = active.map(|(_, _, _, last, handle)| { let leave = Leave::segment(cx, last, ev.mods.shift); if handle == last { leave } else { leave.without_contact() } }); + let p = match ev.kind { PointerKind::Move => self.snap.hover(cx, ev.pos, drawn.as_slice(), from.as_ref()), PointerKind::Down => self.snap.press(cx, ev.pos, drawn.as_slice(), from.as_ref()), @@ -208,15 +278,16 @@ impl Tool for PenTool { self.last = ev.pos; self.pressed = ev.pos; self.dragged = false; - if let Some((id, first, last, _)) = active { - let end = last_anchor(cx, id); + if let Some((id, si, first, last, _)) = active { + let end = last_anchor(cx, id, Some(si)); let closing_end = end.filter(|(_, ai)| *ai > 0).map(|_| first); let picked = pick_endpoint(closing_end, last, ev.pos, tol, Endpoint::First); // Pick at the raw pointer before snapping or constraining a new point. // A one-anchor path doesn't close on itself: its anchor is the last one too. if picked == Some(Endpoint::First) { - self.drag = Some(Place { at: first, path: Some(id), closing: true }); - return vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))]; + self.drag = Some(Place { at: first, path: Some(id), subpath: Some(si), closing: true }); + let params = json!({"id": id.0, "subpath": si}); + return vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), params)]; } // Alt uses Anchor Point behavior at the terminal anchor too. if let Some(acts) = self.alt_convert(cx, ev) { @@ -228,12 +299,13 @@ impl Tool for PenTool { self.handle = Some((id, si, ai, last)); return vec![Action::Begin("Convert Anchor Point".into()), set_out_handle(id, si, ai, last)]; } - let raw_other = open_end_at(cx, ev.pos, tol, Some(id)); + let raw_other = open_end_at(cx, ev.pos, tol, Some((id, si))); if raw_other.is_none() && p != ev.pos { let snapped = pick_endpoint(closing_end, last, p, tol, Endpoint::First); if snapped == Some(Endpoint::First) { - self.drag = Some(Place { at: first, path: Some(id), closing: true }); - return vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))]; + self.drag = Some(Place { at: first, path: Some(id), subpath: Some(si), closing: true }); + let params = json!({"id": id.0, "subpath": si}); + return vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), params)]; } if snapped == Some(Endpoint::Last) && let Some((si, ai)) = end @@ -243,14 +315,17 @@ impl Tool for PenTool { } } // An end of another open path: the two become one, and the path is finished. - if let Some((other, at_first)) = raw_other.or_else(|| open_end_at(cx, p, tol, Some(id))) { + if let Some((other, other_si, at_first)) = raw_other.or_else(|| open_end_at(cx, p, tol, Some((id, si)))) { self.stop(); let end = if at_first { "first" } else { "last" }; - return vec![Action::Exec("path.join".into(), json!({"ids": [id.0, other.0], "ends": ["last", end]}))]; + let mut params = json!({"ids": [id.0, other.0], "ends": ["last", end]}); + params["subpaths"] = json!([si, other_si]); + return vec![Action::Exec("path.join".into(), params)]; } - self.drag = Some(Place { at: p, path: Some(id), closing: false }); + self.drag = Some(Place { at: p, path: Some(id), subpath: Some(si), closing: false }); self.in_h = p; - return vec![Action::Begin("Pen".into()), Action::Preview("path.appendAnchor".into(), json!({"id": id.0, "x": p.x, "y": p.y}))]; + let params = json!({"id": id.0, "x": p.x, "y": p.y, "subpath": si}); + return vec![Action::Begin("Pen".into()), Action::Preview("path.appendAnchor".into(), params)]; } if let Some(acts) = self.alt_convert(cx, ev) { return acts; @@ -261,22 +336,35 @@ impl Tool for PenTool { // A raw endpoint anywhere wins before either snapped continuation route. for at in std::iter::once(ev.pos).chain((p != ev.pos).then_some(p)) { // Continue a selected open path when clicking on one of its ends. - if let Some((id, first, last, _)) = active_path(cx) + if let Some((id, si, first, last, _)) = active_path(cx) && let Some(picked) = pick_endpoint(Some(first), last, at, tol, Endpoint::Last) { self.drawing = true; self.path = Some(id); + self.subpath = Some(si); if picked == Endpoint::First { - return vec![Action::Exec("path.reverse".into(), json!({}))]; + let p = + if cx.selection.objects.as_slice() == [id] { json!({"subpath": si}) } else { json!({"ids": [id.0], "subpath": si}) }; + return vec![Action::Exec("path.reverse".into(), p)]; } return vec![]; } // Continue any other open path from the end clicked, selecting it. - if let Some((id, at_first)) = open_end_at(cx, at, tol, None) { - (self.drawing, self.path) = (true, Some(id)); - let mut out = vec![Action::Exec("select.set".into(), json!({"ids": [id.0]}))]; + if let Some((id, si, at_first)) = open_end_at(cx, at, tol, None) { + (self.drawing, self.path, self.subpath) = (true, Some(id), Some(si)); + let top = cx + .doc + .parent_of(id) + .filter(|p| matches!(cx.doc.node(*p).map(|n| &n.kind), Some(NodeKind::Compound { .. }))) + .unwrap_or(id); + let mut out = vec![]; + if cx.selection.objects.as_slice() != [top] { + out.push(Action::Exec("select.set".into(), json!({"ids": [top.0]}))); + } if at_first { - out.push(Action::Exec("path.reverse".into(), json!({"ids": [id.0]}))); + let p = + if cx.selection.objects.as_slice() == [id] { json!({"subpath": si}) } else { json!({"ids": [id.0], "subpath": si}) }; + out.push(Action::Exec("path.reverse".into(), p)); } return out; } @@ -284,8 +372,8 @@ impl Tool for PenTool { if let Some(act) = auto_add_delete(cx, ev.pos, ev.mods, tol) { return vec![act]; } - (self.drawing, self.path) = (true, None); - self.drag = Some(Place { at: p, path: None, closing: false }); + (self.drawing, self.path, self.subpath) = (true, None, None); + self.drag = Some(Place { at: p, path: None, subpath: None, closing: false }); vec![Action::Begin("Pen".into()), Action::Preview("path.create".into(), json!({"anchors": [{"x": p.x, "y": p.y}]}))] } PointerKind::Drag => { @@ -309,13 +397,13 @@ impl Tool for PenTool { } return vec![set_out_handle(id, si, ai, h)]; } - let Some(Place { at: mut a, path, closing }) = self.drag else { return vec![] }; + let Some(Place { at: mut a, path, subpath, closing }) = self.drag else { return vec![] }; // Space held moves the anchor being placed, handles and all. if ev.mods.space && !closing { let d = ev.pos - self.last; a += d; self.in_h += d; - self.drag = Some(Place { at: a, path, closing }); + self.drag = Some(Place { at: a, path, subpath, closing }); } self.last = ev.pos; let mut out_h = ev.pos; @@ -332,19 +420,21 @@ impl Tool for PenTool { )]; }; if closing { - return vec![Action::Preview( - "path.close".into(), - json!({"id": id.0, "in": [2.0 * a.x - out_h.x, 2.0 * a.y - out_h.y], "independent": alt}), - )]; + let mut params = json!({"id": id.0, "in": [2.0 * a.x - out_h.x, 2.0 * a.y - out_h.y], "independent": alt}); + if let Some(si) = subpath { + params["subpath"] = json!(si); + } + return vec![Action::Preview("path.close".into(), params)]; } // Alt pressed mid-drag keeps the curve already shaped into the anchor: only the // outgoing handle follows. Held from the start, the incoming handle stays retracted. let in_h = if alt { self.in_h } else { a - (out_h - a) }; self.in_h = in_h; - vec![Action::Preview( - "path.appendAnchor".into(), - json!({"id": id.0, "x": a.x, "y": a.y, "in": [in_h.x, in_h.y], "out": [out_h.x, out_h.y]}), - )] + let mut params = json!({"id": id.0, "x": a.x, "y": a.y, "in": [in_h.x, in_h.y], "out": [out_h.x, out_h.y]}); + if let Some(si) = subpath { + params["subpath"] = json!(si); + } + vec![Action::Preview("path.appendAnchor".into(), params)] } PointerKind::Up => { self.snap.clear(); @@ -369,7 +459,9 @@ impl Tool for PenTool { _ => vec![], } } + /// Deactivate the pen tool, committing in-flight operations and restoring compound path selection. fn deactivate(&mut self, cx: &ToolContext) -> Vec { + let path = self.path; self.stop(); self.snap.clear(); let placing = self.drag.take().is_some(); @@ -379,6 +471,13 @@ impl Tool for PenTool { if placing || handling { acts.push(Action::Commit); } + if let Some(id) = path + && cx.selection.objects.as_slice() == [id] + && let Some(parent) = cx.doc.parent_of(id) + && cx.doc.node(parent).is_some_and(|n| matches!(n.kind, NodeKind::Compound { .. })) + { + acts.push(Action::Exec("select.set".into(), json!({"ids": [parent.0]}))); + } acts } fn overlays(&self, cx: &ToolContext) -> Vec { @@ -389,7 +488,7 @@ impl Tool for PenTool { if cx.pen_rubber_band && self.drag.is_none() && self.handle.is_none() - && let (Some((id, _, last, out)), Some(h)) = (self.active(cx), self.hover) + && let (Some((id, _, _, last, out)), Some(h)) = (self.active(cx), self.hover) { let mut bp = BezPath::new(); bp.move_to(last); @@ -413,14 +512,14 @@ impl Tool for PenTool { } let tol = cx.tol(5.0).max(cx.point_tol()); let active = self.active(cx); - if let Some((id, first, last, _)) = active { - let closing_end = last_anchor(cx, id).filter(|(_, ai)| *ai > 0).map(|_| first); + if let Some((id, si, first, last, _)) = active { + let closing_end = last_anchor(cx, id, Some(si)).filter(|(_, ai)| *ai > 0).map(|_| first); match pick_endpoint(closing_end, last, p, tol, Endpoint::First) { Some(Endpoint::First) => return Cursor::PenClose, Some(Endpoint::Last) => return Cursor::PenConvert, None => {} } - if !alt_converts(cx, p, m) && open_end_at(cx, p, tol, Some(id)).is_some() { + if !alt_converts(cx, p, m) && open_end_at(cx, p, tol, Some((id, si))).is_some() { return Cursor::PenJoin; } } @@ -435,9 +534,7 @@ impl Tool for PenTool { Some([_]) => return Cursor::PenAdd, _ => {} } - if active_path(cx).is_some_and(|(_, first, last, _)| pick_endpoint(Some(first), last, p, tol, Endpoint::Last).is_some()) - || open_end_at(cx, p, tol, None).is_some() - { + if open_end_at(cx, p, tol, None).is_some() { return Cursor::PenContinue; } match auto_add_delete(cx, p, m, tol) { @@ -454,13 +551,31 @@ impl Tool for PenTool { impl PenTool { /// The open path being drawn: the one the pen made or went on with, while it is the single /// selected open path. - fn active(&self, cx: &ToolContext) -> Option<(NodeId, Point, Point, Point)> { - active_path(cx).filter(|(id, ..)| self.drawing && self.path.is_none_or(|p| p == *id)) + fn active(&self, cx: &ToolContext) -> Option<(NodeId, usize, Point, Point, Point)> { + if self.drawing + && let Some(pid) = self.path + && (cx.selection.objects.as_slice() == [pid] + || cx.doc.parent_of(pid).is_some_and(|parent| { + cx.selection.objects.as_slice() == [parent] && cx.doc.node(parent).is_some_and(|pn| matches!(pn.kind, NodeKind::Compound { .. })) + })) + && cx.doc.is_editable(pid) + && let Some(n) = cx.doc.node(pid) + && let Some(pd) = n.path_data() + { + let (si, sp) = match self.subpath { + Some(si) => pd.subpaths.get(si).filter(|s| !s.closed).map(|s| (si, s))?, + None => pd.subpaths.iter().enumerate().rev().find(|(_, s)| !s.closed)?, + }; + let first = sp.anchors.first()?.p; + let last = sp.anchors.last()?; + return Some((pid, si, first, last.p, last.h_out)); + } + active_path(cx).filter(|(id, si, ..)| self.drawing && self.path.is_none_or(|p| p == *id) && self.subpath.is_none_or(|s| s == *si)) } /// The path is finished. fn stop(&mut self) { - (self.drawing, self.path) = (false, None); + (self.drawing, self.path, self.subpath) = (false, None, None); } /// Hand an Alt press on a selected path's handle, anchor or segment to the Anchor Point tool. @@ -550,7 +665,7 @@ mod tests { assert_eq!(sp.anchors.iter().map(|a| a.p).collect::>(), vec![first, Point::new(200.0, 300.0), last]); let mut pen = PenTool::default(); assert!(click(&mut pen, &cx, last.x, last.y, Mods::default()).is_empty()); - assert_eq!(pen.active(&cx).map(|(id, first, last, _)| (id, first, last)), Some((id, first, last))); + assert_eq!(pen.active(&cx).map(|(id, _, first, last, _)| (id, first, last)), Some((id, first, last))); assert!(!pen.busy()); let raw = Point::new(237.0, 403.0); @@ -558,8 +673,8 @@ mod tests { assert!(raw.distance(first) > tol && raw.distance(last) > tol); let from = Leave::segment(&cx, last, false); assert_eq!(DrawSnap::default().press(&cx, raw, &[id], Some(&from)), last); - assert!(open_end_at(&cx, raw, tol, Some(id)).is_none()); - assert!(open_end_at(&cx, last, tol, Some(id)).is_none()); + assert!(open_end_at(&cx, raw, tol, Some((id, 0))).is_none()); + assert!(open_end_at(&cx, last, tol, Some((id, 0))).is_none()); assert_eq!( pen.pointer(&cx, &PointerEvent::new(PointerKind::Down, raw.x, raw.y)), @@ -569,7 +684,7 @@ mod tests { assert!(pen.drag.is_none() && pen.busy()); assert_eq!(pen.pointer(&cx, &PointerEvent::new(PointerKind::Up, raw.x, raw.y)), vec![Action::Commit]); assert!(pen.handle.is_none() && !pen.busy()); - assert_eq!(pen.active(&cx).map(|(id, first, last, _)| (id, first, last)), Some((id, first, last))); + assert_eq!(pen.active(&cx).map(|(id, _, first, last, _)| (id, first, last)), Some((id, first, last))); } #[test] @@ -595,17 +710,17 @@ mod tests { }; let mut pen = PenTool::default(); assert!(click(&mut pen, &cx, last.x, last.y, Mods::default()).is_empty()); - assert_eq!(pen.active(&cx).map(|(id, first, last, _)| (id, first, last)), Some((id, first, last))); + assert_eq!(pen.active(&cx).map(|(id, _, first, last, _)| (id, first, last)), Some((id, first, last))); assert!(!pen.busy()); let tol = cx.tol(5.0).max(cx.point_tol()); assert!(raw.distance(first) > tol && raw.distance(last) > tol); - assert_eq!(open_end_at(&cx, raw, tol, Some(id)), Some((other, true))); + assert_eq!(open_end_at(&cx, raw, tol, Some((id, 0))), Some((other, 0, true))); let from = Leave::segment(&cx, last, false); assert_eq!(DrawSnap::default().press(&cx, raw, &[id], Some(&from)), last); assert_eq!( pen.pointer(&cx, &PointerEvent::new(PointerKind::Down, raw.x, raw.y)), - vec![Action::Exec("path.join".into(), json!({"ids": [id.0, other.0], "ends": ["last", "first"]}))] + vec![Action::Exec("path.join".into(), json!({"ids": [id.0, other.0], "ends": ["last", "first"], "subpaths": [0, 0]}))] ); assert!(!pen.busy() && pen.active(&cx).is_none()); assert!(pen.pointer(&cx, &PointerEvent::new(PointerKind::Up, raw.x, raw.y)).is_empty()); @@ -633,7 +748,7 @@ mod tests { assert_eq!(sp.anchors.iter().map(|a| a.p).collect::>(), vec![first, Point::new(200.0, 300.0), last]); let mut pen = PenTool::default(); assert!(click(&mut pen, &cx, last.x, last.y, Mods::default()).is_empty()); - assert_eq!(pen.active(&cx).map(|(id, first, last, _)| (id, first, last)), Some((id, first, last))); + assert_eq!(pen.active(&cx).map(|(id, _, first, last, _)| (id, first, last)), Some((id, first, last))); assert!(!pen.busy()); let raw = Point::new(63.0, 303.0); @@ -641,12 +756,12 @@ mod tests { assert!(raw.distance(first) > tol && raw.distance(last) > tol); let from = Leave::segment(&cx, last, false); assert_eq!(DrawSnap::default().press(&cx, raw, &[id], Some(&from)), first); - assert!(open_end_at(&cx, raw, tol, Some(id)).is_none()); - assert!(open_end_at(&cx, first, tol, Some(id)).is_none()); + assert!(open_end_at(&cx, raw, tol, Some((id, 0))).is_none()); + assert!(open_end_at(&cx, first, tol, Some((id, 0))).is_none()); assert_eq!( pen.pointer(&cx, &PointerEvent::new(PointerKind::Down, raw.x, raw.y)), - vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))] + vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))] ); let place = pen.drag.unwrap(); assert!(place.closing); @@ -678,17 +793,17 @@ mod tests { }; let mut pen = PenTool::default(); assert!(click(&mut pen, &cx, last.x, last.y, Mods::default()).is_empty()); - assert_eq!(pen.active(&cx).map(|(id, first, last, _)| (id, first, last)), Some((id, first, last))); + assert_eq!(pen.active(&cx).map(|(id, _, first, last, _)| (id, first, last)), Some((id, first, last))); assert!(!pen.busy()); let tol = cx.tol(5.0).max(cx.point_tol()); assert!(raw.distance(first) > tol && raw.distance(last) > tol); - assert_eq!(open_end_at(&cx, raw, tol, Some(id)), Some((other, true))); + assert_eq!(open_end_at(&cx, raw, tol, Some((id, 0))), Some((other, 0, true))); let from = Leave::segment(&cx, last, false); assert_eq!(DrawSnap::default().press(&cx, raw, &[id], Some(&from)), first); assert_eq!( pen.pointer(&cx, &PointerEvent::new(PointerKind::Down, raw.x, raw.y)), - vec![Action::Exec("path.join".into(), json!({"ids": [id.0, other.0], "ends": ["last", "first"]}))] + vec![Action::Exec("path.join".into(), json!({"ids": [id.0, other.0], "ends": ["last", "first"], "subpaths": [0, 0]}))] ); assert!(!pen.busy() && pen.active(&cx).is_none()); assert!(pen.pointer(&cx, &PointerEvent::new(PointerKind::Up, raw.x, raw.y)).is_empty()); @@ -796,7 +911,7 @@ mod tests { acts, vec![Action::Preview( "path.appendAnchor".into(), - json!({"id": id.0, "x": 400.0, "y": 400.0, "in": [350.0, 400.0], "out": [450.0, 450.0]}) + json!({"id": id.0, "x": 400.0, "y": 400.0, "in": [350.0, 400.0], "out": [450.0, 450.0], "subpath": 0}) )] ); } @@ -865,7 +980,7 @@ mod tests { assert_eq!(t.active(&cx).map(|a| a.0), Some(id)); assert_eq!( t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 13.0, 303.0)), - vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))] + vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))] ); } @@ -915,7 +1030,7 @@ mod tests { let cursor = if first_end { Cursor::PenClose } else { Cursor::PenConvert }; let actual_cursor = pen.cursor(&cx, at, mods); let expected = if first_end { - vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))] + vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))] } else { vec![Action::Begin("Convert Anchor Point".into()), set_out(id, 1, last.x, last.y)] }; @@ -952,7 +1067,7 @@ mod tests { let at = Point::new(first.x + offset / zoom, first.y); let mut pen = PenTool::default(); assert_eq!(pen.cursor(&cx, at, Mods::default()), Cursor::PenContinue); - let expected = if offset < 3.0 { vec![Action::Exec("path.reverse".into(), json!({}))] } else { vec![] }; + let expected = if offset < 3.0 { vec![Action::Exec("path.reverse".into(), json!({"subpath": 0}))] } else { vec![] }; let actual = click(&mut pen, &cx, at.x, at.y, Mods::default()); if actual != expected { failures.push((zoom, grid, offset, actual)); @@ -975,7 +1090,7 @@ mod tests { assert_eq!(drawing.cursor(&cx, at, Mods::default()), Cursor::PenClose); assert_eq!( click(&mut drawing, &cx, at.x, at.y, Mods::default()), - vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))] + vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))] ); let mut resuming = PenTool::default(); assert_eq!(resuming.cursor(&cx, at, Mods::default()), Cursor::PenContinue); @@ -1032,7 +1147,7 @@ mod tests { assert_eq!(closing.cursor(&cx, first, Mods::default()), Cursor::PenClose); assert_eq!( click(&mut closing, &cx, first.x, first.y, Mods::default()), - vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))] + vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))] ); let mut alt_pen = PenTool { drawing: true, ..PenTool::default() }; let alt = Mods { alt: true, ..Mods::default() }; @@ -1099,12 +1214,12 @@ mod tests { let p = paint(); let mut t = PenTool { drawing: true, ..PenTool::default() }; let a = t.pointer(&cx(&d, &s, &p), &PointerEvent::new(PointerKind::Down, 101.0, 301.0)); - assert_eq!(a, vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0}))]); + assert_eq!(a, vec![Action::Begin("Close Path".into()), Action::Preview("path.close".into(), json!({"id": id.0, "subpath": 0}))]); // What the tool sees once the preview applied: the path closed. d.node_mut(id).unwrap().path_data_mut().unwrap().subpaths[0].closed = true; let c = cx(&d, &s, &p); let close = |x: f64, y: f64, alt: bool| { - vec![Action::Preview("path.close".into(), json!({"id": id.0, "in": [200.0 - x, 600.0 - y], "independent": alt}))] + vec![Action::Preview("path.close".into(), json!({"id": id.0, "in": [200.0 - x, 600.0 - y], "independent": alt, "subpath": 0}))] }; for (x, y) in [(105.0, 290.0), (130.0, 260.0), (160.0, 240.0)] { assert_eq!(t.pointer(&c, &PointerEvent::new(PointerKind::Drag, x, y)), close(x, y, false), "at ({x}, {y})"); @@ -1216,10 +1331,10 @@ mod tests { let to = Point::new(150.0, 318.0); assert!(o.iter().any(|o| matches!(o, Overlay::Measure { text, .. } if text == &cx.segment_measure_label(from, to)))); let a = click(&mut t, &cx, 152.0, 318.0, Mods::default()); - assert_eq!(a[1], Action::Preview("path.appendAnchor".into(), json!({"id": id.0, "x": 150.0, "y": 318.0}))); + assert_eq!(a[1], Action::Preview("path.appendAnchor".into(), json!({"id": id.0, "x": 150.0, "y": 318.0, "subpath": 0}))); let shift = Mods { shift: true, ..Mods::default() }; let a = click(&mut t, &cx, 152.0, 310.0, shift); - assert_eq!(a[1], Action::Preview("path.appendAnchor".into(), json!({"id": id.0, "x": 150.0, "y": 300.0}))); + assert_eq!(a[1], Action::Preview("path.appendAnchor".into(), json!({"id": id.0, "x": 150.0, "y": 300.0, "subpath": 0}))); // The first anchor of a new path lands on the rect's corner. let mut t = PenTool::default(); let a = click(&mut t, &cx, 102.0, 99.0, Mods::default()); diff --git a/crates/ui-egui/src/i18n/untranslated_messages.txt b/crates/ui-egui/src/i18n/untranslated_messages.txt index e526314c8..f0c050348 100644 --- a/crates/ui-egui/src/i18n/untranslated_messages.txt +++ b/crates/ui-egui/src/i18n/untranslated_messages.txt @@ -34,6 +34,7 @@ JSON recovery transport exceeds 8 MiB; use the binary adapter PDF content is incomplete or exceeds the reader's limits; no document was imported PDF flattener: {e} PDF options: {e} +PDF security settings are not an object SVG is not UTF-8, UTF-16 or ISO 8859-1 Save writes one artboard: name one, or export several with document.export Select a dimension first @@ -73,6 +74,7 @@ Text options: {e} `direction` must be "auto", "leftToRight" or "rightToLeft" `each` is the value one design stands for, more than 0 `ecc` must be "L", "M", "Q" or "H" +`ends` applies to joining exactly two paths `event` must be one of {_1} `files` must be 1 to 1000 paths of font files the last text.findFontFiles search found `folder` must be an absolute folder path @@ -139,6 +141,7 @@ Text options: {e} `starts` must be an array of anchor indices (or nulls) `stop` must be a stop index `strokeMode` applies to strokes (stroke: true) +`subpaths` must be two integer indices `style` must be roman or japanese `symbol` must be a symbol name `target` must be "object" or "contents", not {v} @@ -386,6 +389,7 @@ command must be a shape.* command commands[{i}]: missing `command` commands[{i}]: unknown command `{id}` composer must be "singleLine" or "everyLine" +compound {_1} must contain exactly one open path to join copy needs path: the file to copy into the document's folder copy takes images only: relink placed documents where their files are copying a file isn't available on the web @@ -952,6 +956,7 @@ select area type and closed paths (at least one area type object) select artwork to add its colours select artwork to make a group of its colours select artwork to recolour, or give a colour `group` +select exactly two path ends to join select objects or give ids select objects to align to the guide select one Intertwine group @@ -1003,6 +1008,8 @@ subLines contains an invalid lane subLines is missing subLines must be a list or null subLines must contain at most {MAX_SUB_LINES} valid lanes +subpath {si} is not an open subpath +subpaths required when joining the same path swatchConflict must be "merge" or "add", not {v} text must be {characterSet?, platform?, removeLineReturns?, removeParagraphReturns?, replaceSpaces?} text transform is collapsed or unrepresentable From 11f1d5c4d6c5fcbcb8a96df2334580dc044330bf Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 24 Oct 2026 16:08:09 -0600 Subject: [PATCH 02/18] M2.1: Pen continuation in a compound path reverses only the path it continues Clicking the first anchor of an open path inside a compound path made the Pen select the compound and then reverse through the selection, which `paths_in` expands to every member path: the clicked subpath of each sibling flipped too, and under the non-zero fill rule a hole became a fill. Name the path in the reverse instead. The adjacent continuation of an already selected path is unaffected: it issues no selection change, so its selection-addressed reverse still means that path. Regressions: an engine test continues a compound member with two open subpaths at its first anchor and holds the member's other subpath and the sibling member byte-identical (anchors, handles, closed flags); a tool test pins the emitted `path.reverse` parameters to the member's id. --- crates/engine/src/tests_draw2.rs | 26 ++++++++++++++++++ crates/tools/src/pen.rs | 45 +++++++++++++++++++++++++++++--- 2 files changed, 68 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/tests_draw2.rs b/crates/engine/src/tests_draw2.rs index 3181f2155..80b3f5dc1 100644 --- a/crates/engine/src/tests_draw2.rs +++ b/crates/engine/src/tests_draw2.rs @@ -2960,3 +2960,29 @@ fn join_two_refuses_an_empty_subpath() { .unwrap(); assert!(s.execute("path.join", &json!({"ids": [a.0, b.0], "ends": ["last", "first"], "subpaths": [0, 0]})).is_err()); } + +/// Continuing a compound member at its first anchor reverses that subpath only. The Pen puts the +/// compound back in the selection before it reverses, so the reverse has to name the member path: +/// addressed through the selection it would walk the compound and flip every member's subpath. +#[test] +fn pen_continue_at_a_first_anchor_in_a_compound_reverses_only_that_subpath() { + let (v, none) = (view(), Mods::default()); + let mut s = session(); + let a = NodeId(s.execute("path.create", &json!({"d": "M100 100 L200 100 M100 150 L200 150"})).unwrap()["id"].as_u64().unwrap()); + let b = NodeId(s.execute("path.create", &json!({"d": "M300 100 L400 100 M300 150 L400 150"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.set", &json!({"ids": [a.0, b.0]})).unwrap(); + let cid = NodeId(s.execute("object.compoundPath.make", &json!({})).unwrap()["id"].as_u64().unwrap()); + // The member path itself is the whole selection, as a direct selection inside the compound + // leaves it: continuing it selects the compound again, which must not widen the reverse. + s.execute("select.set", &json!({"ids": [a.0]})).unwrap(); + let (before_a, before_b) = (path(&s, a), path(&s, b)); + + s.select_tool("pen", v).unwrap(); + gesture(&mut s, &[(100.0, 100.0)], none); + + let after_a = path(&s, a); + assert_eq!(after_a.subpaths[0].anchors.iter().map(|x| x.p).collect::>(), vec![Point::new(200.0, 100.0), Point::new(100.0, 100.0)]); + assert_eq!(after_a.subpaths[1], before_a.subpaths[1], "the member's other subpath keeps its anchors and handles"); + assert_eq!(path(&s, b), before_b, "the compound's other member keeps its anchors, handles and closed flags"); + assert_eq!(s.doc().unwrap().selection.objects, vec![cid], "the compound is selected again"); +} diff --git a/crates/tools/src/pen.rs b/crates/tools/src/pen.rs index 2b7bf12e8..dc04c370b 100644 --- a/crates/tools/src/pen.rs +++ b/crates/tools/src/pen.rs @@ -362,9 +362,10 @@ impl Tool for PenTool { out.push(Action::Exec("select.set".into(), json!({"ids": [top.0]}))); } if at_first { - let p = - if cx.selection.objects.as_slice() == [id] { json!({"subpath": si}) } else { json!({"ids": [id.0], "subpath": si}) }; - out.push(Action::Exec("path.reverse".into(), p)); + // Name the path: the `select.set` above may have just put its compound + // parent in the selection, and a reverse addressed through the + // selection would flip this subpath of every member of it. + out.push(Action::Exec("path.reverse".into(), json!({"ids": [id.0], "subpath": si}))); } return out; } @@ -1458,4 +1459,42 @@ mod tests { assert_eq!(t.cursor(&cx, empty, Mods::default()), Cursor::Pen, "the path is finished"); assert_eq!(Cursor::PenDrawing.precise(), Cursor::Crosshair); } + + /// Continuing an open path inside a compound path at its first anchor names the path it + /// reverses. The same press selects the compound, and `path.reverse` without `ids` would then + /// walk the compound and flip that subpath of every member. + #[test] + fn continuing_a_compound_member_at_its_first_anchor_reverses_that_path_by_id() { + let (mut d, _) = doc_with_rect(); + let l = d.layers[0].id; + let (a, b, compound) = (d.alloc_id(), d.alloc_id(), d.alloc_id()); + let two_subpaths = |x: f64| { + vectorcraft_geom::PathData::new(vec![ + vectorcraft_geom::SubPath::polyline(&[Point::new(x, 300.0), Point::new(x + 100.0, 300.0)], false), + vectorcraft_geom::SubPath::polyline(&[Point::new(x, 350.0), Point::new(x + 100.0, 350.0)], false), + ]) + }; + let path = |id, x| std::sync::Arc::new(vectorcraft_doc::Node::path(id, two_subpaths(x), vectorcraft_doc::Appearance::default_art())); + let mut c = vectorcraft_doc::Node::new( + compound, + NodeKind::Compound { children: vec![path(a, 10.0), path(b, 300.0)], rule: vectorcraft_geom::FillRule::NonZero }, + ); + c.appearance = vectorcraft_doc::Appearance::default_art().into(); + d.insert(Some(l), 1, c).unwrap(); + let p = paint(); + // The member path alone is the selection, as a direct selection inside the compound leaves it. + let mut s = Selection::default(); + s.set([a]); + let cx = ToolContext { smart_guides: false, snap_to_point: false, snap_to_grid: false, snap_to_pixel: false, ..cx(&d, &s, &p) }; + let mut pen = PenTool::default(); + assert_eq!( + click(&mut pen, &cx, 10.0, 300.0, Mods::default()), + vec![ + Action::Exec("select.set".into(), json!({"ids": [compound.0]})), + Action::Exec("path.reverse".into(), json!({"ids": [a.0], "subpath": 0})), + ], + "the reverse names the member path, not the compound the select.set just selected" + ); + assert_eq!((pen.path, pen.subpath), (Some(a), Some(0))); + } } From 44044d93baf10d1595ceb217a1c0a7f3fedfcd8d Mon Sep 17 00:00:00 2001 From: el-j Date: Mon, 26 Oct 2026 10:04:10 +0100 Subject: [PATCH 03/18] M2.1: honor ends on selected join and compute nearest pair for subpaths (#3274, #3276, #3379, #3380) --- crates/engine/src/cmd/path.rs | 50 ++++++++++++++++++++++++++++++-- crates/engine/src/tests_draw2.rs | 25 ++++++++++++++++ 2 files changed, 73 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index 94667d776..3684df0c3 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -408,6 +408,14 @@ fn end_points(sp: &SubPath) -> Option<(Point, Point)> { Some((sp.anchors.first()?.p, sp.anchors.last()?.p)) } +/// Orientations (reverse first, reverse second) that bring the nearest ends together. +fn nearest_ends(sp_a: &SubPath, sp_b: &SubPath) -> Option<(bool, bool)> { + let (fa, la) = end_points(sp_a)?; + let (fb, lb) = end_points(sp_b)?; + let cands = [(la.distance(fb), false, false), (la.distance(lb), false, true), (fa.distance(fb), true, false), (fa.distance(lb), true, true)]; + cands.into_iter().min_by(|p, q| p.0.total_cmp(&q.0)).map(|(_, rx, ry)| (rx, ry)) +} + /// Corner or smooth on an anchor a join leaves behind. fn join_point(sp: &mut SubPath, ai: usize, smooth: bool) { if smooth { @@ -558,7 +566,14 @@ fn join(s: &mut Session, p: &Value) -> Result { if ids.len() == 2 && ids[0] == ids[1] { let id = ids[0]; let (sa, sb) = subpaths.ok_or_else(|| bad(C, "subpaths required when joining the same path"))?; - let (rx, ry) = ends.unwrap_or((false, false)); + let st = s.doc()?; + let p = st.doc.node(id).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(id))?; + let spa = p.subpaths.get(sa).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let spb = p.subpaths.get(sb).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let (rx, ry) = match ends { + Some(e) => e, + None => nearest_ends(spa, spb).unwrap_or((false, false)), + }; s.edit("Join", |d, sel| { join_subpaths_in_node(d, id, (sa, sb), (rx, ry), join_kind)?; sel.set([compound_or_self(d, id)]); @@ -570,7 +585,15 @@ fn join(s: &mut Session, p: &Value) -> Result { && let Some((sa, sb)) = subpaths { let (a, b) = (ids[0], ids[1]); - let (rx, ry) = ends.unwrap_or((false, false)); + let st = s.doc()?; + let pa = st.doc.node(a).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(a))?; + let pb = st.doc.node(b).and_then(|n| n.path_data()).ok_or(EngineError::NoNode(b))?; + let spa = pa.subpaths.get(sa).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let spb = pb.subpaths.get(sb).ok_or_else(|| EngineError::Other("join needs open paths".into()))?; + let (rx, ry) = match ends { + Some(e) => e, + None => nearest_ends(spa, spb).unwrap_or((false, false)), + }; s.edit("Join", |d, sel| { join_two(d, a, b, (sa, sb), (rx, ry), join_kind)?; sel.set([compound_or_self(d, a)]); @@ -812,6 +835,9 @@ fn join(s: &mut Session, p: &Value) -> Result { } return Err(EngineError::Other("select paths to join".into())); } + if ends.is_some() && open_subs.len() > 2 { + return Err(bad(C, "`ends` applies to joining exactly two paths")); + } if open_subs.len() == 1 { let (id, si) = open_subs[0]; s.edit("Join", |d, sel| { @@ -821,6 +847,26 @@ fn join(s: &mut Session, p: &Value) -> Result { })?; return ok(); } + if let Some((rx, ry)) = ends + && open_subs.len() == 2 + { + let (a, sa) = open_subs[0]; + let (b, sb) = open_subs[1]; + if a == b { + s.edit("Join", |d, sel| { + join_subpaths_in_node(d, a, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, a)]); + Ok(()) + })?; + } else { + s.edit("Join", |d, sel| { + join_two(d, a, b, (sa, sb), (rx, ry), join_kind)?; + sel.set([compound_or_self(d, a)]); + Ok(()) + })?; + } + return ok(); + } s.edit("Join", |d, sel| { let top = join_nearest(d, open_subs, join_kind)?; diff --git a/crates/engine/src/tests_draw2.rs b/crates/engine/src/tests_draw2.rs index 80b3f5dc1..528f1de90 100644 --- a/crates/engine/src/tests_draw2.rs +++ b/crates/engine/src/tests_draw2.rs @@ -2169,6 +2169,31 @@ fn join_takes_the_paths_and_the_ends_to_join() { } } +/// `path.join {ends}` on selected paths honors the requested ends, not the nearest pair. +#[test] +fn join_selected_paths_honors_ends_parameter() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + s.select(|_, sel| sel.set([a, b])).unwrap(); + // The far ends: a's first to b's last (nearest pair would be a's last and b's first). + s.execute("path.join", &json!({"ends": ["first", "last"]})).unwrap(); + let pts: Vec<(f64, f64)> = path(&s, a).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect(); + assert_eq!(pts, [(100.0, 0.0), (0.0, 0.0), (200.0, 0.0), (110.0, 0.0)]); +} + +/// `path.join {ids, subpaths}` without `ends` computes the nearest pair within those subpaths. +#[test] +fn join_subpaths_without_ends_computes_nearest_pair() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 200.0, 0.0, 110.0, 0.0); + // Nearest ends between a (0..100) and b (200..110) are 100 and 110. + s.execute("path.join", &json!({"ids": [a.0, b.0], "subpaths": [0, 0]})).unwrap(); + let pts: Vec<(f64, f64)> = path(&s, a).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect(); + assert_eq!(pts, [(0.0, 0.0), (100.0, 0.0), (110.0, 0.0), (200.0, 0.0)]); +} + /// `path.join {to}` makes the joined anchor a corner or smooth point. #[test] fn join_to_corner_or_smooth_sets_the_merged_anchor() { From 9348aadb9f03aae2232ec7c99b07d14c69c7ce53 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Mon, 26 Oct 2026 22:18:44 +0000 Subject: [PATCH 04/18] feat(installer): default to a per-user ArtCraft folder --- .github/workflows/packaging-lint.yml | 19 ++ packaging/windows/INSTALLATION.md | 35 ++++ packaging/windows/check-install-policy.ps1 | 195 +++++++++++++++++++++ packaging/windows/install-policy.json | 41 +++++ packaging/windows/install-policy.wxi | 54 ++++++ packaging/windows/package.ps1 | 2 + packaging/windows/test-install-policy.ps1 | 54 ++++++ packaging/windows/vectorcraft.wxs | 60 ++----- 8 files changed, 415 insertions(+), 45 deletions(-) create mode 100644 packaging/windows/INSTALLATION.md create mode 100644 packaging/windows/check-install-policy.ps1 create mode 100644 packaging/windows/install-policy.json create mode 100644 packaging/windows/install-policy.wxi create mode 100644 packaging/windows/test-install-policy.ps1 diff --git a/.github/workflows/packaging-lint.yml b/.github/workflows/packaging-lint.yml index 386989022..c1394c403 100644 --- a/.github/workflows/packaging-lint.yml +++ b/.github/workflows/packaging-lint.yml @@ -23,3 +23,22 @@ jobs: uses: storytold/craft-common-actions/.github/workflows/packaging-lint.yml@main with: extra-checks-script: packaging/check-app-specific.sh + + windows-install-policy: + timeout-minutes: 10 + name: Windows installation policy (no app build) + runs-on: windows-latest + steps: + - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + - name: Install the pinned packaging toolchain + shell: pwsh + run: | + $tools = Join-Path $env:RUNNER_TEMP 'craft-install-policy-wix' + dotnet tool install --tool-path $tools wix --version 5.0.2 + if ($LASTEXITCODE -ne 0) { throw 'WiX installation failed.' } + $tools >> $env:GITHUB_PATH + & (Join-Path $tools 'wix.exe') extension add -g WixToolset.UI.wixext/5.0.2 WixToolset.Util.wixext/5.0.2 + if ($LASTEXITCODE -ne 0) { throw 'WiX extension installation failed.' } + - name: Build and check non-executable installer fixtures + shell: pwsh + run: ./packaging/windows/test-install-policy.ps1 -OutputDirectory (Join-Path $env:RUNNER_TEMP 'craft-install-policy-tests') diff --git a/packaging/windows/INSTALLATION.md b/packaging/windows/INSTALLATION.md new file mode 100644 index 000000000..982a5b420 --- /dev/null +++ b/packaging/windows/INSTALLATION.md @@ -0,0 +1,35 @@ +# Windows installation policy checks + +New installations default to the current account under `Programs\ArtCraft\`. +The first setup page offers installation for everyone. Scope is selected before duplicate +checks and directory costing. An update uses the existing context's remembered exact folder; +`INSTALLFOLDER` supplied explicitly is never replaced. It is a secure MSI property so the +chosen folder reaches the elevated installer. Silent machine updates pass `ALLUSERS=1`. +A cross-context copy is refused; changing context means uninstalling the old copy first. + +App registration follows the selected context. Existing optional shortcuts and file-handler +choices stay app-owned. No new cleanup operation or model/settings migration is introduced. +Packaged resources remain package-owned; separately downloaded shared models are not caches +and are outside this installer's ownership. + +## Repeatable metadata tests + +On Windows with WiX 5.0.2 and its matching UI/Util extensions: + +```powershell +./packaging/windows/test-install-policy.ps1 -OutputDirectory ./target/install-policy-tests +``` + +The script builds x64, x86 and arm64 packages with deliberately non-executable payloads. It +uses the actual package source and compiled database. A safe Windows Installer session checks +24 scope/registration/maintenance cases, remembered and explicit custom folders, sequencing, +upgrade identity and the original registration inventory recorded in `install-policy.json`. +The release packaging script runs the compiled checker before signing. Packaging CI runs the +small fixture builds without compiling the Rust application. + +Never install or distribute the `NOT-INSTALLABLE.msi` fixtures as an app. Passing these checks +does not prove real installation, elevation, legacy-version upgrades, registry writes, optional +shortcut/menu behaviour, cancellation, rollback or uninstall. Those still require the real +payload in an authorised clean Windows test environment. Cross-architecture metadata is not +execution evidence on those architectures. Existing open installer/association PRs must be +reconciled before publication. Nothing here changes macOS/Linux native packaging conventions. diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 new file mode 100644 index 000000000..dd2895643 --- /dev/null +++ b/packaging/windows/check-install-policy.ps1 @@ -0,0 +1,195 @@ +<# +.SYNOPSIS + Check the compiled installation policy without installing any product. +.DESCRIPTION + Uses Windows Installer's safe session (OpenPackage option 1). Only property-setting custom + actions are exercised. No AppSearch, install, repair, removal or file-writing action runs. +#> +param([Parameter(Mandatory = $true)] [string] $MsiPath) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +$policy = Get-Content -Raw -LiteralPath (Join-Path $PSScriptRoot 'install-policy.json') | ConvertFrom-Json +function Read-Rows($Database, [string] $Sql, [int] $Columns) { + $view = $Database.OpenView($Sql) + try { + [void]$view.Execute() + while ($null -ne ($record = $view.Fetch())) { + try { + $values = @(for ($i = 1; $i -le $Columns; $i++) { + $record.GetType().InvokeMember('StringData', 'GetProperty', $null, $record, @($i)) + }) + ,$values + } finally { [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($record) } + } + } finally { + [void]$view.Close() + [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($view) + } +} +function Set-SessionProperty($Session, [string] $Name, [string] $Value) { + [void]$Session.GetType().InvokeMember('Property', 'SetProperty', $null, $Session, @($Name, $Value)) +} +function Get-SessionProperty($Session, [string] $Name) { + $Session.GetType().InvokeMember('Property', 'GetProperty', $null, $Session, @($Name)) +} +$installer = $null; $database = $null; $session = $null +try { + $installer = New-Object -ComObject WindowsInstaller.Installer + $file = (Resolve-Path -LiteralPath $MsiPath).Path + $database = $installer.OpenDatabase($file, 0) + $session = $installer.OpenPackage($file, 1) + $properties = @{} + foreach ($row in (Read-Rows $database 'SELECT `Property`, `Value` FROM `Property`' 2)) { $properties[$row[0]] = $row[1] } + $secureProperties = @($properties['SecureCustomProperties'] -split ';') + if ('INSTALLFOLDER' -notin $secureProperties) { + throw 'The selected installation folder must survive elevation (secure INSTALLFOLDER).' + } + if ($policy.mode -eq 'perUser') { + if ($properties.ContainsKey('ALLUSERS')) { throw 'Launcher must remain per-user; machine self-update is not part of this change.' } + if ($properties['UpgradeCode'].Trim('{}') -ne $policy.upgradeCode) { throw 'Launcher upgrade identity changed.' } + $dirs = @{} + foreach ($row in (Read-Rows $database 'SELECT `Directory`, `Directory_Parent`, `DefaultDir` FROM `Directory`' 3)) { $dirs[$row[0]] = @($row[1],$row[2]) } + if ($dirs['INSTALLFOLDER'][0] -ne 'CraftSuiteFolder' -or $dirs['CraftSuiteFolder'][1] -ne 'ArtCraft' -or + $dirs['CraftSuiteFolder'][0] -ne 'LocalProgramsFolder' -or $dirs['LocalProgramsFolder'][0] -ne 'LocalAppDataFolder') { + throw 'Launcher new-install folder must be under the per-user ArtCraft suite root.' + } + foreach ($explicit in @('', 'F:\Chosen exact launcher')) { + Set-SessionProperty $session 'PREVIOUSINSTALLFOLDER' 'E:\Existing exact launcher' + Set-SessionProperty $session 'INSTALLFOLDER' $explicit + $row = @(Read-Rows $database 'SELECT `Condition` FROM `InstallUISequence` WHERE `Action` = ''CraftRestoreLauncherFolder''' 1)[0] + if ($session.EvaluateCondition($row[0]) -eq 1) { + if ($session.DoAction('CraftRestoreLauncherFolder') -ne 1) { throw 'Launcher restore action failed.' } + } + $expected = if ($explicit) { $explicit } else { 'E:\Existing exact launcher' } + if ((Get-SessionProperty $session 'INSTALLFOLDER') -ne $expected) { throw 'Launcher custom or existing path was lost.' } + } + $rows = @(Read-Rows $database 'SELECT `Root`, `Key`, `Name`, `Value` FROM `Registry`' 4) + if (@($rows | Where-Object { $_[0] -ne '1' }).Count) { throw 'Launcher registration must stay under the current account.' } + foreach ($entry in $policy.registry) { + if (@($rows | Where-Object { $_[0] -eq $entry[0] -and $_[1] -eq $entry[1] -and $_[2] -eq $entry[2] -and $_[3] -eq $entry[3] }).Count -ne 1) { + throw 'Launcher registration or shortcut preference changed unexpectedly.' + } + } + Write-Output 'PASS ArtCraft Launcher: per-user scope, suite folder, remembered/custom paths and existing registration. No installation performed.' + return + } + if ($properties['ALLUSERS'] -ne '2' -or $properties['MSIINSTALLPERUSER'] -ne '1') { + throw 'Policy regression: new packages must default to the per-user dual-scope context.' + } + if ($properties['UpgradeCode'].Trim('{}') -ne $policy.upgradeCode) { throw 'The existing upgrade identity must not change.' } + $directories = @{} + foreach ($row in (Read-Rows $database 'SELECT `Directory`, `Directory_Parent`, `DefaultDir` FROM `Directory`' 3)) { + $directories[$row[0]] = @($row[1], $row[2]) + } + if ($directories['INSTALLFOLDER'][0] -ne 'CraftSuiteFolder' -or + $directories['CraftSuiteFolder'][1] -ne 'ArtCraft') { throw 'Expected the app under the ArtCraft suite folder.' } + $programs = $directories['CraftSuiteFolder'][0] + if ($programs -eq 'ProgramFiles6432Folder') { $programs = $directories[$programs][0] } + if ($programs -notin 'ProgramFilesFolder', 'ProgramFiles64Folder') { + throw 'Use Windows Installer native folder redirection, not a hard-coded profile path.' + } + $guards = @(Read-Rows $database 'SELECT `Condition`, `Description` FROM `LaunchCondition`' 2 | + Where-Object { $_[0] -match 'CRAFT_(USER|MACHINE)_DIR' }) + if ($guards.Count -ne 2) { throw 'Expected both duplicate-installation guards.' } + $cases = 0 + foreach ($scope in @('', '2', '1')) { + foreach ($user in @('', 'D:\Existing user app')) { + foreach ($machine in @('', 'E:\Existing machine app')) { + foreach ($installed in @('', '1')) { + Set-SessionProperty $session 'ALLUSERS' $scope + Set-SessionProperty $session 'Installed' $installed + Set-SessionProperty $session 'CRAFT_USER_DIR' $user + Set-SessionProperty $session 'CRAFT_MACHINE_DIR' $machine + $allowed = $true + foreach ($guard in $guards) { + $value = $session.EvaluateCondition($guard[0]) + if ($value -notin 0, 1) { throw 'Invalid duplicate guard.' } + if ($value -eq 0) { $allowed = $false } + } + $opposite = if ($scope -eq '1') { $user } else { $machine } + $expected = [bool]$installed -or -not [bool]$opposite + if ($allowed -ne $expected) { throw "Scope regression: ALLUSERS='$scope', user=$([bool]$user), machine=$([bool]$machine), maintenance=$([bool]$installed)." } + $cases++ + } + } + } + } + # Execute the actual authored type-51 actions in a safe session, not copied resolver logic. + foreach ($scope in @('', '2', '1')) { + foreach ($explicit in @('', 'F:\Chosen exact app')) { + Set-SessionProperty $session 'ALLUSERS' $scope + Set-SessionProperty $session 'INSTALLFOLDER' $explicit + Set-SessionProperty $session 'CRAFT_USER_DIR' 'D:\Existing user app' + Set-SessionProperty $session 'CRAFT_MACHINE_DIR' 'E:\Existing machine app' + $action = if ($scope -eq '1') { 'CraftRestoreMachineFolder' } else { 'CraftRestoreUserFolder' } + if (-not $explicit) { + if ($session.DoAction($action) -ne 1) { throw 'Remembered-folder property action failed.' } + } + $expected = if ($explicit) { $explicit } elseif ($scope -eq '1') { 'E:\Existing machine app' } else { 'D:\Existing user app' } + if ((Get-SessionProperty $session 'INSTALLFOLDER') -ne $expected) { throw 'Custom or remembered folder was not preserved.' } + } + } + $ui = @{} + foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallUISequence`' 2)) { $ui[$row[0]] = [int]$row[1] } + if (-not ($ui['AppSearch'] -lt $ui['CraftScopeDlg'] -and $ui['CraftScopeDlg'] -lt $ui['LaunchConditions'] -and + $ui['CraftScopeDlg'] -lt $ui['CraftRestoreUserFolder'] -and $ui['CraftRestoreUserFolder'] -lt $ui['CostInitialize'])) { + throw 'Scope must be chosen after discovery and before duplicate checks, folder restoration and costing.' + } + $execute = @{} + foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallExecuteSequence`' 2)) { $execute[$row[0]] = [int]$row[1] } + foreach ($action in @('CraftRestoreUserFolder','CraftRestoreMachineFolder')) { + if (-not ($execute['AppSearch'] -lt $execute[$action] -and $execute[$action] -lt $execute['CostInitialize'])) { + throw 'Silent updates must restore the folder before directory costing.' + } + } + # Switching the first-page choice must reset BOTH properties, including a back-and-forth change. + $events = @(Read-Rows $database 'SELECT `Event`, `Argument`, `Condition`, `Ordering` FROM `ControlEvent` WHERE `Dialog_` = ''CraftScopeDlg'' AND `Control_` = ''Next'' ORDER BY `Ordering`' 4) + foreach ($choice in @('user','machine','user')) { + Set-SessionProperty $session 'CRAFT_SCOPE' $choice + foreach ($event in $events) { + if ($event[0] -notmatch '^\[([^\]]+)\]$') { continue } + $property = $Matches[1] + if ($session.EvaluateCondition($event[2]) -eq 1) { + Set-SessionProperty $session $property $(if ($event[1] -eq '{}') { '' } else { $event[1] }) + } + } + $allUsers = Get-SessionProperty $session 'ALLUSERS' + $perUser = Get-SessionProperty $session 'MSIINSTALLPERUSER' + if (($choice -eq 'machine' -and ($allUsers -ne '1' -or $perUser)) -or + ($choice -eq 'user' -and ($allUsers -ne '2' -or $perUser -ne '1'))) { + throw 'Scope selector does not reset the requested installation context.' + } + } + $actions = @(Read-Rows $database 'SELECT `Action`, `Source`, `Target`, `Type` FROM `CustomAction`' 4) + $savedOptions = @($actions | Where-Object { $_[0] -match '^CraftLoadOption[0-9]+M$' }) + foreach ($option in $savedOptions) { + if ([int]$option[3] -ne 51) { throw 'Saved-option test may run only property-setting actions.' } + $number = [regex]::Match($option[0], '[0-9]+').Value + foreach ($scope in @('2','1')) { + Set-SessionProperty $session 'ALLUSERS' $scope + Set-SessionProperty $session "CRAFT_OPTION_${number}_M" 'saved-machine-choice' + Set-SessionProperty $session "CRAFT_OPTION_${number}_U" 'saved-user-choice' + Set-SessionProperty $session $option[1] '' + $action = "CraftLoadOption$number" + $(if ($scope -eq '1') { 'M' } else { 'U' }) + if ($session.DoAction($action) -ne 1) { throw 'Saved-option property action failed.' } + $expected = if ($scope -eq '1') { 'saved-machine-choice' } else { 'saved-user-choice' } + if ((Get-SessionProperty $session $option[1]) -ne $expected) { throw 'Saved installer choices leaked across contexts.' } + } + } + $registry = @(Read-Rows $database 'SELECT `Root`, `Key`, `Name`, `Value` FROM `Registry`' 4) + if (@($registry | Where-Object { $_[0] -in '1', '2' }).Count) { throw 'Registration must follow the selected context, not force HKCU or HKLM.' } + if (@($registry | Where-Object { $_[0] -eq '0' -and $_[1] -match '^Software\\Classes(\\|$)' }).Count) { + throw 'HKCR keys must be relative to Classes.' + } + foreach ($entry in $policy.registry) { + $found = @($registry | Where-Object { $_[0] -eq $entry[0] -and $_[1] -eq $entry[1] -and $_[2] -eq $entry[2] -and $_[3] -eq $entry[3] }) + if ($found.Count -ne 1) { throw "Lost or changed registration: $($entry[1]) / $($entry[2])" } + } + $installDir = @($registry | Where-Object { $_[0] -eq '-1' -and $_[1] -eq "Software\$($policy.name)" -and $_[2] -eq 'InstallDir' -and $_[3] -eq '[INSTALLFOLDER]' }) + if ($installDir.Count -ne 1) { throw 'A context-sensitive installation-folder record is required.' } + Write-Output "PASS $($policy.name): $cases scope cases, folder preservation, action sequencing and registry contract. No installation performed." +} finally { + foreach ($com in @($session, $database, $installer)) { + if ($null -ne $com) { [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($com) } + } +} diff --git a/packaging/windows/install-policy.json b/packaging/windows/install-policy.json new file mode 100644 index 000000000..bb54866ff --- /dev/null +++ b/packaging/windows/install-policy.json @@ -0,0 +1,41 @@ +{ + "id": "vectorcraft", + "name": "VectorCraft", + "upgradeCode": "601C6437-08AE-4645-ADCC-27F036C2EA2F", + "sources": [ + "vectorcraft.wxs" + ], + "icon": "assets/app-icon/vectorcraft.ico", + "localization": [ + "vectorcraft.en-us.wxl" + ], + "extensions": [ + "UI", + "Util" + ], + "payloads": [ + "vectorcraft-cli.exe", + "vectorcraft.exe" + ], + "registry": [ + [ + "-1", + "Software\\Microsoft\\Windows\\CurrentVersion\\App Paths\\vectorcraft.exe", + "", + "[#VectorcraftExe]" + ], + [ + "-1", + "Software\\VectorCraft", + "InstallDir", + "[INSTALLFOLDER]" + ], + [ + "-1", + "Software\\VectorCraft", + "DesktopShortcut", + "#1" + ] + ], + "mode": "dualScope" +} diff --git a/packaging/windows/install-policy.wxi b/packaging/windows/install-policy.wxi new file mode 100644 index 000000000..c5a9124d6 --- /dev/null +++ b/packaging/windows/install-policy.wxi @@ -0,0 +1,54 @@ + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + + diff --git a/packaging/windows/package.ps1 b/packaging/windows/package.ps1 index 3e97b129e..795480d0b 100644 --- a/packaging/windows/package.ps1 +++ b/packaging/windows/package.ps1 @@ -155,6 +155,8 @@ Invoke-Native 'wix build' { -d "DocsDir=$Docs" -d "BannerBmp=$Banner" -d "DialogBmp=$Dialog" ` -o $Msi } +# Check the compiled scope, path and registration contract before signing. +& (Join-Path $PSScriptRoot 'check-install-policy.ps1') -MsiPath $Msi # wix writes its debug symbols (.wixpdb) next to the MSI; keep them out of the release assets. Remove-Item -Force -ErrorAction SilentlyContinue ([IO.Path]::ChangeExtension($Msi, '.wixpdb')) & (Join-Path $PSScriptRoot 'sign.ps1') $Msi diff --git a/packaging/windows/test-install-policy.ps1 b/packaging/windows/test-install-policy.ps1 new file mode 100644 index 000000000..21199ca7f --- /dev/null +++ b/packaging/windows/test-install-policy.ps1 @@ -0,0 +1,54 @@ +<# +.SYNOPSIS + Build non-executable installer metadata fixtures; never install them. +#> +param( + [Parameter(Mandatory = $true)] [string] $OutputDirectory, + [ValidateSet('x64','x86','arm64')] [string[]] $Architectures = @('x64','x86','arm64') +) +$ErrorActionPreference = 'Stop' +Set-StrictMode -Version Latest +$root = (Resolve-Path (Join-Path $PSScriptRoot '..\..')).Path +$policy = Get-Content -Raw -LiteralPath (Join-Path $PSScriptRoot 'install-policy.json') | ConvertFrom-Json +[void](New-Item -ItemType Directory -Force -Path $OutputDirectory) +$output = (Resolve-Path -LiteralPath $OutputDirectory).Path +Add-Type -AssemblyName System.Drawing +foreach ($arch in $Architectures) { + $stage = Join-Path $output $arch + [void](New-Item -ItemType Directory -Force -Path $stage) + foreach ($name in $policy.payloads) { + $destination = Join-Path $stage $name + [void](New-Item -ItemType Directory -Force -Path ([IO.Path]::GetDirectoryName($destination))) + Set-Content -LiteralPath $destination -Encoding Ascii -Value 'NON-EXECUTABLE MSI METADATA TEST FIXTURE' + } + foreach ($folder in @('docs','models','notices')) { + $path = Join-Path $stage $folder + [void](New-Item -ItemType Directory -Force -Path $path) + Set-Content -LiteralPath (Join-Path $path "$folder-fixture.txt") -Value 'Metadata test fixture, not a user file or model.' + } + foreach ($model in @('metadata-one.rten','metadata-two.rten')) { + Set-Content -LiteralPath (Join-Path (Join-Path $stage 'models') $model) -Encoding Ascii -Value 'NON-EXECUTABLE MODEL METADATA FIXTURE' + } + foreach ($spec in @(@('banner.bmp',493,58),@('dialog.bmp',493,312))) { + $bitmap = [Drawing.Bitmap]::new($spec[1],$spec[2],[Drawing.Imaging.PixelFormat]::Format24bppRgb) + try { $bitmap.Save((Join-Path $stage $spec[0]),[Drawing.Imaging.ImageFormat]::Bmp) } + finally { $bitmap.Dispose() } + } + Set-Content -LiteralPath (Join-Path $stage 'licence.rtf') -Encoding Ascii -Value '{\rtf1 Metadata test fixture.}' + $sources = @($policy.sources | ForEach-Object { Join-Path $PSScriptRoot $_ }) + $msi = Join-Path $stage "$($policy.id)-NOT-INSTALLABLE.msi" + $extensions = @($policy.extensions | ForEach-Object { '-ext'; "WixToolset.$_.wixext" }) + & wix build @sources -arch $arch -wx @extensions ` + -culture en-US -d 'Version=0.0.1' -d 'DisplayVersion=0.0.1-metadata-test' ` + -d "BinDir=$stage" -d "IconPath=$(Join-Path $root $policy.icon)" ` + -d "DocsDir=$(Join-Path $stage 'docs')" -d "ModelsDir=$(Join-Path $stage 'models')" ` + -d "AppNoticesDir=$(Join-Path $stage 'notices')" -d 'HasCraftFontLicences=1' -d 'HasCli=1' ` + -d "BannerBmp=$(Join-Path $stage 'banner.bmp')" -d "DialogBmp=$(Join-Path $stage 'dialog.bmp')" ` + -d "UiBannerBmp=$(Join-Path $stage 'banner.bmp')" -d "UiDialogBmp=$(Join-Path $stage 'dialog.bmp')" ` + -d "LicenseRtf=$(Join-Path $stage 'licence.rtf')" -o $msi @($policy.localization | ForEach-Object { '-loc'; Join-Path $PSScriptRoot $_ }) + if ($LASTEXITCODE -ne 0) { throw "WiX validation failed for $arch (exit $LASTEXITCODE)." } + $shell = (Get-Process -Id $PID).Path + & $shell -NoProfile -ExecutionPolicy Bypass -File (Join-Path $PSScriptRoot 'check-install-policy.ps1') -MsiPath $msi + if ($LASTEXITCODE -ne 0) { throw "Compiled installer policy failed for $arch (exit $LASTEXITCODE)." } + Write-Output "PASS $arch metadata-only package. This is not an executable installer." +} diff --git a/packaging/windows/vectorcraft.wxs b/packaging/windows/vectorcraft.wxs index 679d11dbb..933f019ef 100644 --- a/packaging/windows/vectorcraft.wxs +++ b/packaging/windows/vectorcraft.wxs @@ -1,34 +1,11 @@ - - - + + + - + @@ -38,15 +15,14 @@ - + + + - - - @@ -65,8 +41,7 @@ - + @@ -84,12 +59,10 @@ - + - + @@ -115,13 +88,11 @@ - + - - + + @@ -133,9 +104,8 @@ app. A per-machine install puts it on the common desktop. `wix msi validate` reports ICE38, ICE43 and ICE57 for it: they take a desktop shortcut for per-user data, which it isn't here. --> - - + + From 921426bf562fe742da19032397f9cc4a2dc407b7 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 10:50:17 +0000 Subject: [PATCH 05/18] fix(installer): address scope and ARM64 review feedback --- .github/workflows/windows-arm64.yml | 6 +- packaging/windows/check-install-policy.ps1 | 66 +++++++++++++++++++++- packaging/windows/install-policy.wxi | 6 +- 3 files changed, 72 insertions(+), 6 deletions(-) diff --git a/.github/workflows/windows-arm64.yml b/.github/workflows/windows-arm64.yml index 2dc19466c..b58801380 100644 --- a/.github/workflows/windows-arm64.yml +++ b/.github/workflows/windows-arm64.yml @@ -27,8 +27,10 @@ permissions: jobs: windows-arm64: - # Intentionally tracked at @main (not SHA-pinned) so org-wide CI improvements, security fixes, and build optimizations propagate without PR churn across all craftapp repos. - uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@main + # Immutable supporting workflow revision until the new install contract lands upstream. + uses: Jeremy8776/craft-common-actions/.github/workflows/windows-arm64.yml@f4666d5cbf35f9bdeda24f1f88c7fb5c6adf5276 with: app-name: vectorcraft app-dir: VectorCraft + install-relative-path: Programs/ArtCraft/VectorCraft + install-scope: perUser diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 index dd2895643..00b683d70 100644 --- a/packaging/windows/check-install-policy.ps1 +++ b/packaging/windows/check-install-policy.ps1 @@ -131,9 +131,12 @@ try { } $ui = @{} foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallUISequence`' 2)) { $ui[$row[0]] = [int]$row[1] } - if (-not ($ui['AppSearch'] -lt $ui['CraftScopeDlg'] -and $ui['CraftScopeDlg'] -lt $ui['LaunchConditions'] -and + if (-not ($ui['AppSearch'] -lt $ui['CraftPreferMachineScope'] -and + $ui['CraftPreferMachineScope'] -lt $ui['CraftScopeDlg'] -and + $ui['CraftScopeDlg'] -lt $ui['FindRelatedProducts'] -and + $ui['FindRelatedProducts'] -lt $ui['LaunchConditions'] -and $ui['CraftScopeDlg'] -lt $ui['CraftRestoreUserFolder'] -and $ui['CraftRestoreUserFolder'] -lt $ui['CostInitialize'])) { - throw 'Scope must be chosen after discovery and before duplicate checks, folder restoration and costing.' + throw 'Scope must be chosen after AppSearch and before FindRelatedProducts, launch checks and costing.' } $execute = @{} foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallExecuteSequence`' 2)) { $execute[$row[0]] = [int]$row[1] } @@ -144,7 +147,18 @@ try { } # Switching the first-page choice must reset BOTH properties, including a back-and-forth change. $events = @(Read-Rows $database 'SELECT `Event`, `Argument`, `Condition`, `Ordering` FROM `ControlEvent` WHERE `Dialog_` = ''CraftScopeDlg'' AND `Control_` = ''Next'' ORDER BY `Ordering`' 4) - foreach ($choice in @('user','machine','user')) { + # Model the full-UI upgrade entry with an existing machine-wide copy. Run + # the actual preference action, then the compiled Next events before discovery. + Set-SessionProperty $session 'ALLUSERS' '2' + Set-SessionProperty $session 'CRAFT_SCOPE' 'user' + Set-SessionProperty $session 'CRAFT_USER_DIR' '' + Set-SessionProperty $session 'CRAFT_MACHINE_DIR' 'E:\Existing machine app' + $preference = @(Read-Rows $database 'SELECT `Condition` FROM `InstallUISequence` WHERE `Action` = ''CraftPreferMachineScope''' 1)[0] + if ($session.EvaluateCondition($preference[0]) -ne 1 -or $session.DoAction('CraftPreferMachineScope') -ne 1 -or + (Get-SessionProperty $session 'CRAFT_SCOPE') -ne 'machine') { + throw 'Full-UI machine upgrades must initially select the existing machine context.' + } + foreach ($choice in @('machine','user','machine','user')) { Set-SessionProperty $session 'CRAFT_SCOPE' $choice foreach ($event in $events) { if ($event[0] -notmatch '^\[([^\]]+)\]$') { continue } @@ -176,6 +190,52 @@ try { if ((Get-SessionProperty $session $option[1]) -ne $expected) { throw 'Saved installer choices leaked across contexts.' } } } + # Follow the compiled control conditions, not the source's intended appearance. + $controls = @(Read-Rows $database 'SELECT `Dialog_`, `Control`, `Type`, `Attributes`, `Text` FROM `Control`' 5) + $conditions = @() + if (@(Read-Rows $database 'SELECT `Name` FROM `_Tables` WHERE `Name` = ''ControlCondition''' 1).Count) { + $conditions = @(Read-Rows $database 'SELECT `Dialog_`, `Control_`, `Action`, `Condition` FROM `ControlCondition`' 4) + } + function Test-ControlVisible($Control) { + $visible = ([int]$Control[3] -band 1) -ne 0 + foreach ($condition in $conditions) { + if ($condition[0] -eq $Control[0] -and $condition[1] -eq $Control[1] -and $session.EvaluateCondition($condition[3]) -eq 1) { + if ($condition[2] -eq 'Show') { $visible = $true } + if ($condition[2] -eq 'Hide') { $visible = $false } + } + } + return $visible + } + foreach ($button in @($controls | Where-Object { $_[0] -in 'InstallWelcome','InstallMaintenance','DesktopShortcutDlg' -and ([int]$_[3] -band 8388608) -ne 0 })) { + $plain = @($controls | Where-Object { $_[0] -eq $button[0] -and $_[1] -eq ($button[1] + 'User') }) + if ($plain.Count -ne 1 -or ([int]$plain[0][3] -band 8388608)) { throw 'Scope-aware buttons need an unshielded per-user counterpart.' } + foreach ($scope in @('', '2', '1')) { + Set-SessionProperty $session 'ALLUSERS' $scope + if ((Test-ControlVisible $button) -ne ($scope -eq '1') -or (Test-ControlVisible $plain[0]) -ne ($scope -ne '1')) { + throw 'The elevation shield must follow the selected scope.' + } + } + $dialog = $button[0]; $shieldId = $button[1]; $plainId = $plain[0][1] + $shieldEvents = @(Read-Rows $database "SELECT ``Event``, ``Argument``, ``Condition``, ``Ordering`` FROM ``ControlEvent`` WHERE ``Dialog_`` = '$dialog' AND ``Control_`` = '$shieldId' ORDER BY ``Ordering``" 4 | ForEach-Object { $_ -join '|' }) + $plainEvents = @(Read-Rows $database "SELECT ``Event``, ``Argument``, ``Condition``, ``Ordering`` FROM ``ControlEvent`` WHERE ``Dialog_`` = '$dialog' AND ``Control_`` = '$plainId' ORDER BY ``Ordering``" 4 | ForEach-Object { $_ -join '|' }) + if (($shieldEvents -join "`n") -ne ($plainEvents -join "`n")) { throw 'Per-user and machine buttons must perform the same operation.' } + } + foreach ($scope in @('', '2', '1')) { + Set-SessionProperty $session 'ALLUSERS' $scope + foreach ($text in @($controls | Where-Object { $_[4] -like '*for all users*' })) { + if ((Test-ControlVisible $text) -ne ($scope -eq '1')) { throw 'The welcome-page scope text must follow the selected scope.' } + } + } + if ($policy.id -in 'deckcraft','filmcraft','soundcraft') { + foreach ($route in @(@('WelcomeDlg','Next','InstallDirDlg'),@('InstallDirDlg','Back','WelcomeDlg'))) { + $dialog = $route[0]; $control = $route[1] + $routes = @(Read-Rows $database "SELECT ``Argument``, ``Condition`` FROM ``ControlEvent`` WHERE ``Dialog_`` = '$dialog' AND ``Control_`` = '$control' AND ``Event`` = 'NewDialog' ORDER BY ``Ordering``" 2) + Set-SessionProperty $session 'Installed' '' + $destination = '' + foreach ($event in $routes) { if ($session.EvaluateCondition($event[1]) -eq 1) { $destination = $event[0] } } + if ($destination -ne $route[2]) { throw 'Skipping the licence page must work in both navigation directions.' } + } + } $registry = @(Read-Rows $database 'SELECT `Root`, `Key`, `Name`, `Value` FROM `Registry`' 4) if (@($registry | Where-Object { $_[0] -in '1', '2' }).Count) { throw 'Registration must follow the selected context, not force HKCU or HKLM.' } if (@($registry | Where-Object { $_[0] -eq '0' -and $_[1] -match '^Software\\Classes(\\|$)' }).Count) { diff --git a/packaging/windows/install-policy.wxi b/packaging/windows/install-policy.wxi index c5a9124d6..dcfb7727b 100644 --- a/packaging/windows/install-policy.wxi +++ b/packaging/windows/install-policy.wxi @@ -50,5 +50,9 @@ - + + + + + From 85250cd220aea6f66ef3cac2f0771503054b6e9c Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 11:46:32 +0000 Subject: [PATCH 06/18] fix(installer): harden install-policy checks and packaging-lint workflow --- .github/workflows/packaging-lint.yml | 9 +++++++-- packaging/windows/check-install-policy.ps1 | 13 ++++++++++--- packaging/windows/test-install-policy.ps1 | 2 +- 3 files changed, 18 insertions(+), 6 deletions(-) diff --git a/.github/workflows/packaging-lint.yml b/.github/workflows/packaging-lint.yml index c1394c403..da85536c4 100644 --- a/.github/workflows/packaging-lint.yml +++ b/.github/workflows/packaging-lint.yml @@ -30,6 +30,8 @@ jobs: runs-on: windows-latest steps: - uses: actions/checkout@3d3c42e5aac5ba805825da76410c181273ba90b1 # v7.0.1 + with: + persist-credentials: false - name: Install the pinned packaging toolchain shell: pwsh run: | @@ -37,8 +39,11 @@ jobs: dotnet tool install --tool-path $tools wix --version 5.0.2 if ($LASTEXITCODE -ne 0) { throw 'WiX installation failed.' } $tools >> $env:GITHUB_PATH - & (Join-Path $tools 'wix.exe') extension add -g WixToolset.UI.wixext/5.0.2 WixToolset.Util.wixext/5.0.2 - if ($LASTEXITCODE -ne 0) { throw 'WiX extension installation failed.' } + $policy = Get-Content -Raw -LiteralPath packaging/windows/install-policy.json | ConvertFrom-Json + foreach ($extension in @($policy.extensions)) { + & (Join-Path $tools 'wix.exe') extension add -g "WixToolset.$extension.wixext/5.0.2" + if ($LASTEXITCODE -ne 0) { throw "WiX extension $extension installation failed." } + } - name: Build and check non-executable installer fixtures shell: pwsh run: ./packaging/windows/test-install-policy.ps1 -OutputDirectory (Join-Path $env:RUNNER_TEMP 'craft-install-policy-tests') diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 index 00b683d70..451295a2e 100644 --- a/packaging/windows/check-install-policy.ps1 +++ b/packaging/windows/check-install-policy.ps1 @@ -122,9 +122,11 @@ try { Set-SessionProperty $session 'CRAFT_USER_DIR' 'D:\Existing user app' Set-SessionProperty $session 'CRAFT_MACHINE_DIR' 'E:\Existing machine app' $action = if ($scope -eq '1') { 'CraftRestoreMachineFolder' } else { 'CraftRestoreUserFolder' } - if (-not $explicit) { - if ($session.DoAction($action) -ne 1) { throw 'Remembered-folder property action failed.' } - } + # Evaluate the compiled condition, so the test exercises it instead of calling the action directly. + $restore = @(Read-Rows $database ('SELECT `Condition` FROM `InstallExecuteSequence` WHERE `Action` = ''{0}''' -f $action) 1)[0] + $selected = $session.EvaluateCondition($restore[0]) -eq 1 + if ($selected -eq [bool]$explicit) { throw "$action must run only when no folder was supplied (condition: $($restore[0]))." } + if ($selected -and $session.DoAction($action) -ne 1) { throw 'Remembered-folder property action failed.' } $expected = if ($explicit) { $explicit } elseif ($scope -eq '1') { 'E:\Existing machine app' } else { 'D:\Existing user app' } if ((Get-SessionProperty $session 'INSTALLFOLDER') -ne $expected) { throw 'Custom or remembered folder was not preserved.' } } @@ -145,6 +147,11 @@ try { throw 'Silent updates must restore the folder before directory costing.' } } + foreach ($resolver in @('CraftUFromLegacyExe','CraftMFromLegacyExe')) { + if (-not ($execute['AppSearch'] -lt $execute[$resolver] -and $execute[$resolver] -lt $execute['LaunchConditions'])) { + throw 'Both legacy folder resolvers must run after AppSearch and before the launch checks.' + } + } # Switching the first-page choice must reset BOTH properties, including a back-and-forth change. $events = @(Read-Rows $database 'SELECT `Event`, `Argument`, `Condition`, `Ordering` FROM `ControlEvent` WHERE `Dialog_` = ''CraftScopeDlg'' AND `Control_` = ''Next'' ORDER BY `Ordering`' 4) # Model the full-UI upgrade entry with an existing machine-wide copy. Run diff --git a/packaging/windows/test-install-policy.ps1 b/packaging/windows/test-install-policy.ps1 index 21199ca7f..d455d19d1 100644 --- a/packaging/windows/test-install-policy.ps1 +++ b/packaging/windows/test-install-policy.ps1 @@ -38,7 +38,7 @@ foreach ($arch in $Architectures) { $sources = @($policy.sources | ForEach-Object { Join-Path $PSScriptRoot $_ }) $msi = Join-Path $stage "$($policy.id)-NOT-INSTALLABLE.msi" $extensions = @($policy.extensions | ForEach-Object { '-ext'; "WixToolset.$_.wixext" }) - & wix build @sources -arch $arch -wx @extensions ` + & wix build @sources -arch $arch @extensions ` -culture en-US -d 'Version=0.0.1' -d 'DisplayVersion=0.0.1-metadata-test' ` -d "BinDir=$stage" -d "IconPath=$(Join-Path $root $policy.icon)" ` -d "DocsDir=$(Join-Path $stage 'docs')" -d "ModelsDir=$(Join-Path $stage 'models')" ` From 094b228aea9a6b61dbb4fef33f8055a15e7541db Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 12:09:04 +0000 Subject: [PATCH 07/18] docs(installer): describe the per-user default install location and scope --- docs/mcp.md | 8 ++++---- docs/releasing.md | 7 +++++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index cc620fcb6..38ea50b6e 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -58,13 +58,13 @@ The release packages ship `vectorcraft-cli` alongside the desktop app, so no bui | Install | CLI | |---|---| -| Windows (MSI) | `C:\Program Files\VectorCraft\vectorcraft-cli.exe` by default (wherever you installed it otherwise), not on `PATH` | +| Windows (MSI) | `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft\vectorcraft-cli.exe` by default (`C:\Program Files\ArtCraft\VectorCraft\vectorcraft-cli.exe` for an all-users install, or wherever you installed it otherwise), not on `PATH` | | Linux (deb, rpm) | `/usr/bin/vectorcraft-cli` | | macOS | the separate `vectorcraft-cli--macos-.zip` release asset (the `.app` holds only the desktop app) | ```sh -# Windows, default install folder -claude mcp add vectorcraft -- "C:\Program Files\VectorCraft\vectorcraft-cli.exe" mcp +# Windows (PowerShell), default install folder +claude mcp add vectorcraft -- "$env:LOCALAPPDATA\Programs\ArtCraft\VectorCraft\vectorcraft-cli.exe" mcp # Linux, or macOS with the CLI unzipped onto PATH claude mcp add vectorcraft -- vectorcraft-cli mcp ``` @@ -75,7 +75,7 @@ For a live session, start the installed app with the control port before the MCP # macOS: quit VectorCraft first, since `open -a` passes no arguments to an app that is already running open -a VectorCraft --args --control 7979 # Windows (PowerShell), default install folder -& "C:\Program Files\VectorCraft\vectorcraft.exe" --control 7979 +& "$env:LOCALAPPDATA\Programs\ArtCraft\VectorCraft\vectorcraft.exe" --control 7979 # Linux (deb, rpm) vectorcraft --control 7979 ``` diff --git a/docs/releasing.md b/docs/releasing.md index f3bcac657..ded0fa362 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -128,11 +128,14 @@ to the target triple, so host build scripts aren't affected. inside the signed bundle), for an AppImage next to the `.AppImage` file (`$APPIMAGE`; the mounted executable's own folder is read-only). Flatpak isn't supported: `/app` is read-only and the sandbox has no folder "beside the app", so a Flatpak install always uses its own config folder. -- `vectorcraft.wxs` (WiX) is a per-machine install into Program Files with a Start Menu shortcut +- `vectorcraft.wxs` (WiX) is a dual-scope package (per-user by default into + `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft`, per-machine into `Program Files\ArtCraft\VectorCraft`; + (see `packaging/windows/INSTALLATION.md`)) with a Start Menu shortcut and an App Paths entry (Win+R `vectorcraft`). Same-version upgrades are allowed, so release candidates replace each other. - Double-clicking the MSI opens a setup wizard (WixUI_InstallDir without the licence page): - Welcome, install folder (remembered for upgrades in `HKLM\Software\VectorCraft\InstallDir`), + a scope page ("Only for me" or "Everyone on this computer"), Welcome, install folder (remembered for + upgrades in `HKLM` or `HKCU\Software\VectorCraft\InstallDir` according to scope), Shortcuts (a ticked "Create a desktop shortcut" box), Ready, a progress page, and a Finish page that says where to find the app, with a ticked "Launch VectorCraft" box. The pages name the full version (`vectorcraft.en-us.wxl`). `package.ps1` draws the wizard's banner and side bitmaps from From 005f5b4d398d17b6a2e6adc6895068a9c6c41055 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 12:25:21 +0000 Subject: [PATCH 08/18] docs(installer): note that the 32-bit all-users folder is Program Files (x86); tidy wording --- docs/mcp.md | 2 +- docs/releasing.md | 4 ++-- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/docs/mcp.md b/docs/mcp.md index 38ea50b6e..2bef0d6fb 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -58,7 +58,7 @@ The release packages ship `vectorcraft-cli` alongside the desktop app, so no bui | Install | CLI | |---|---| -| Windows (MSI) | `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft\vectorcraft-cli.exe` by default (`C:\Program Files\ArtCraft\VectorCraft\vectorcraft-cli.exe` for an all-users install, or wherever you installed it otherwise), not on `PATH` | +| Windows (MSI) | `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft\vectorcraft-cli.exe` by default (`C:\Program Files\ArtCraft\VectorCraft\vectorcraft-cli.exe` for an all-users install, under `Program Files (x86)` instead for the 32-bit build, or wherever you installed it otherwise), not on `PATH` | | Linux (deb, rpm) | `/usr/bin/vectorcraft-cli` | | macOS | the separate `vectorcraft-cli--macos-.zip` release asset (the `.app` holds only the desktop app) | diff --git a/docs/releasing.md b/docs/releasing.md index ded0fa362..39d9b338a 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -129,8 +129,8 @@ to the target triple, so host build scripts aren't affected. executable's own folder is read-only). Flatpak isn't supported: `/app` is read-only and the sandbox has no folder "beside the app", so a Flatpak install always uses its own config folder. - `vectorcraft.wxs` (WiX) is a dual-scope package (per-user by default into - `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft`, per-machine into `Program Files\ArtCraft\VectorCraft`; - (see `packaging/windows/INSTALLATION.md`)) with a Start Menu shortcut + `%LOCALAPPDATA%\Programs\ArtCraft\VectorCraft`, per-machine into `Program Files\ArtCraft\VectorCraft` (`Program Files (x86)` for the 32-bit build), + see `packaging/windows/INSTALLATION.md`) with a Start Menu shortcut and an App Paths entry (Win+R `vectorcraft`). Same-version upgrades are allowed, so release candidates replace each other. - Double-clicking the MSI opens a setup wizard (WixUI_InstallDir without the licence page): From 2298383f559a94ab154b7611a9ea59c4af16a664 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 12:26:56 +0000 Subject: [PATCH 09/18] docs(installer): say that silent installs are per-user and when the scope page appears --- docs/releasing.md | 3 ++- 1 file changed, 2 insertions(+), 1 deletion(-) diff --git a/docs/releasing.md b/docs/releasing.md index 39d9b338a..d97722cd9 100644 --- a/docs/releasing.md +++ b/docs/releasing.md @@ -133,7 +133,8 @@ to the target triple, so host build scripts aren't affected. see `packaging/windows/INSTALLATION.md`) with a Start Menu shortcut and an App Paths entry (Win+R `vectorcraft`). Same-version upgrades are allowed, so release candidates replace each other. -- Double-clicking the MSI opens a setup wizard (WixUI_InstallDir without the licence page): +- Double-clicking the MSI opens a setup wizard (WixUI_InstallDir without the licence page; the scope page appears only for a new full-UI install, not for + maintenance, uninstall or reduced and silent runs): a scope page ("Only for me" or "Everyone on this computer"), Welcome, install folder (remembered for upgrades in `HKLM` or `HKCU\Software\VectorCraft\InstallDir` according to scope), Shortcuts (a ticked "Create a desktop shortcut" box), Ready, a progress page, and a Finish page From d5aeea632e600c8f7304803b804a8754032c83c6 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 15:32:39 +0000 Subject: [PATCH 10/18] fix(installer): resolve the wizard scope and folder correctly Real msiexec runs (per-user and all-users, silent and wizard) found two defects that the metadata fixtures cannot see: - "Only for me" published ALLUSERS=2, so FindRelatedProducts treated the run as per-machine and skipped the existing per-user copy. An interactive upgrade left two registered products and uninstall left files behind. The page now leaves ALLUSERS unset for a per-user choice. - Windows Installer resolves Program Files once, for the context it started in, so choosing "Everyone" in the wizard installed into the user profile with a machine-wide registration. The wizard now sets the new-install folder itself after the choice (Program Files, Program Files (x86) for 32-bit, or the per-user Programs folder). Silent installs are unchanged. check-install-policy.ps1 now fails if either regresses. --- packaging/windows/check-install-policy.ps1 | 24 +++++++++++++++++++++- packaging/windows/install-policy.wxi | 15 +++++++++++++- 2 files changed, 37 insertions(+), 2 deletions(-) diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 index 451295a2e..a4a14bc64 100644 --- a/packaging/windows/check-install-policy.ps1 +++ b/packaging/windows/check-install-policy.ps1 @@ -140,6 +140,28 @@ try { $ui['CraftScopeDlg'] -lt $ui['CraftRestoreUserFolder'] -and $ui['CraftRestoreUserFolder'] -lt $ui['CostInitialize'])) { throw 'Scope must be chosen after AppSearch and before FindRelatedProducts, launch checks and costing.' } + # Windows Installer resolves Program Files for the context it started in, before the wizard's scope choice. + # The wizard therefore sets the new-install folder itself, and only in the wizard. + $defaults = @(Read-Rows $database 'SELECT `Action`, `Sequence`, `Condition` FROM `InstallUISequence`' 3 | Where-Object { $_[0] -like 'CraftDefault*' }) + if ($defaults.Count -lt 2) { throw 'The wizard must set the new-install folder after the scope page; Program Files is resolved before the choice.' } + foreach ($row in $defaults) { + if (-not ($ui['CraftScopeDlg'] -lt [int]$row[1] -and [int]$row[1] -lt $ui['CostInitialize'])) { throw "$($row[0]) must run after the scope page and before costing." } + } + if (@(Read-Rows $database 'SELECT `Action` FROM `InstallExecuteSequence`' 1 | Where-Object { $_[0] -like 'CraftDefault*' }).Count) { + throw 'Silent installs must not use the wizard-only default folder actions.' + } + foreach ($scope in @('1', '')) { + Set-SessionProperty $session 'ALLUSERS' $scope + Set-SessionProperty $session 'INSTALLFOLDER' '' + Set-SessionProperty $session 'CRAFT_MACHINE_DIR' '' + Set-SessionProperty $session 'CRAFT_USER_DIR' '' + foreach ($row in @($defaults | Sort-Object { [int]$_[1] })) { + if ($session.EvaluateCondition($row[2]) -eq 1 -and $session.DoAction($row[0]) -ne 1) { throw "$($row[0]) failed." } + } + $folder = Get-SessionProperty $session 'INSTALLFOLDER' + $wanted = if ($scope -eq '1') { '^[A-Za-z]:\\Program Files( \(x86\))?\\ArtCraft\\[^\\]+\\$' } else { '^' + [regex]::Escape($env:LOCALAPPDATA) + '\\Programs\\ArtCraft\\[^\\]+\\$' } + if ($folder -notmatch $wanted) { throw "Wizard default folder for ALLUSERS='$scope' is '$folder'." } + } $execute = @{} foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallExecuteSequence`' 2)) { $execute[$row[0]] = [int]$row[1] } foreach ($action in @('CraftRestoreUserFolder','CraftRestoreMachineFolder')) { @@ -177,7 +199,7 @@ try { $allUsers = Get-SessionProperty $session 'ALLUSERS' $perUser = Get-SessionProperty $session 'MSIINSTALLPERUSER' if (($choice -eq 'machine' -and ($allUsers -ne '1' -or $perUser)) -or - ($choice -eq 'user' -and ($allUsers -ne '2' -or $perUser -ne '1'))) { + ($choice -eq 'user' -and ($allUsers -or $perUser -ne '1'))) { throw 'Scope selector does not reset the requested installation context.' } } diff --git a/packaging/windows/install-policy.wxi b/packaging/windows/install-policy.wxi index dcfb7727b..3b4d4ac2e 100644 --- a/packaging/windows/install-policy.wxi +++ b/packaging/windows/install-policy.wxi @@ -1,6 +1,9 @@ + @@ -25,6 +28,16 @@ + + + + + + + + + @@ -42,7 +55,7 @@ - + From 1ffc04506c24899046db48c63e25ff650066a404 Mon Sep 17 00:00:00 2001 From: skylar Date: Tue, 27 Oct 2026 11:11:36 -0600 Subject: [PATCH 11/18] fix(engine): honor explicit ends when joining direct selections --- crates/engine/src/cmd/path.rs | 5 +- crates/engine/src/tests/draw2.rs | 105 +++++++++++++++++++++++++++++++ 2 files changed, 109 insertions(+), 1 deletion(-) diff --git a/crates/engine/src/cmd/path.rs b/crates/engine/src/cmd/path.rs index 3684df0c3..f56abc8f3 100644 --- a/crates/engine/src/cmd/path.rs +++ b/crates/engine/src/cmd/path.rs @@ -714,7 +714,10 @@ fn join(s: &mut Session, p: &Value) -> Result { } } - let direct_ends: Vec<(NodeId, usize, &'static str)> = { + // An explicit `ends` names the ends to join: the direct selection only decides when it is absent. + let direct_ends: Vec<(NodeId, usize, &'static str)> = if ends.is_some() { + vec![] + } else { let st = s.doc()?; let mut ends = vec![]; for (&id, picked) in &st.selection.anchors { diff --git a/crates/engine/src/tests/draw2.rs b/crates/engine/src/tests/draw2.rs index ee05c9ef7..7bb0a3edf 100644 --- a/crates/engine/src/tests/draw2.rs +++ b/crates/engine/src/tests/draw2.rs @@ -2394,6 +2394,111 @@ fn join_selected_paths_honors_ends_parameter() { assert_eq!(pts, [(100.0, 0.0), (0.0, 0.0), (200.0, 0.0), (110.0, 0.0)]); } +/// An explicit `ends` wins over a direct-selected end pair when `ids` is omitted, as on `main`. +#[test] +fn join_explicit_ends_win_over_direct_selected_end_points() { + let pts = |s: &Session, id: NodeId| path(s, id).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect::>(); + // (picked anchor of a, picked anchor of b, the ends asked for, the result they ask for) + let cases = [ + // Picks say a's last to b's first (also the nearest pair); the ends ask for the far ends. + (1, 0, ["first", "last"], [(100.0, 0.0), (0.0, 0.0), (200.0, 0.0), (110.0, 0.0)]), + // Picks say a's first to b's first; the ends ask for a's last to b's first. + (0, 0, ["last", "first"], [(0.0, 0.0), (100.0, 0.0), (110.0, 0.0), (200.0, 0.0)]), + ]; + for (pick_a, pick_b, ends, want) in cases { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, pick_a]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, pick_b]], "mode": "add"})).unwrap(); + let sel_before = s.doc().unwrap().selection.anchors.clone(); + s.execute("path.join", &json!({"ends": ends})).unwrap(); + assert_eq!(pts(&s, a), want, "ends {ends:?} with a:{pick_a} b:{pick_b} picked"); + assert!(s.doc().unwrap().doc.node(b).is_none(), "b was joined into a"); + // One undo step puts both lines, and the selection they had, back. + s.execute("edit.undo", &json!({})).unwrap(); + assert_eq!(pts(&s, a), [(0.0, 0.0), (100.0, 0.0)]); + assert_eq!(pts(&s, b), [(110.0, 0.0), (200.0, 0.0)]); + assert_eq!(s.doc().unwrap().selection.objects, vec![a, b], "the selection came back"); + assert_eq!(s.doc().unwrap().selection.anchors, sel_before, "with its direct-selected anchors"); + } +} + +/// The ends asked for apply to the selection order: with b selected first, both lines reverse and b keeps its id. +#[test] +fn join_explicit_ends_follow_the_selection_order() { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + // In selection order the picks (b's first, a's last) match the ends asked for; the head reads the + // pick map in id order (a before b), so it joins the other pair, which this pins. + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, 0]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 1]], "mode": "add"})).unwrap(); + s.execute("path.join", &json!({"ends": ["first", "last"]})).unwrap(); + let pts: Vec<(f64, f64)> = path(&s, b).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect(); + assert_eq!(pts, [(200.0, 0.0), (110.0, 0.0), (100.0, 0.0), (0.0, 0.0)], "b keeps its id and runs 200 to 0"); + assert!(s.doc().unwrap().doc.node(a).is_none(), "a was joined into b"); +} + +/// Two open subpaths of one path take the ends asked for, not the picked pair. +#[test] +fn join_explicit_ends_on_two_subpaths_of_one_path() { + let mut s = session(); + let id = NodeId(s.execute("path.create", &json!({"d": "M0 0 L100 0 M200 0 L110 0"})).unwrap()["id"].as_u64().unwrap()); + s.execute("select.anchors", &json!({"id": id.0, "anchors": [[0, 1], [1, 0]], "mode": "set"})).unwrap(); + // The picks say subpath 0's last to subpath 1's first; the ends ask for subpath 1's last. + s.execute("path.join", &json!({"ends": ["last", "last"]})).unwrap(); + let pd = path(&s, id); + assert_eq!(pd.subpaths.len(), 1, "the two subpaths became one"); + let pts: Vec<(f64, f64)> = pd.subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect(); + assert_eq!(pts, [(0.0, 0.0), (100.0, 0.0), (110.0, 0.0), (200.0, 0.0)]); +} + +/// `ends` names two paths: three open subpaths are refused, with and without a picked pair, and nothing changes. +#[test] +fn join_explicit_ends_refuses_more_than_two_open_subpaths() { + for picks in [false, true] { + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + let c = line(&mut s, 210.0, 0.0, 300.0, 0.0); + s.select(|_, sel| sel.set([a, b, c])).unwrap(); + if picks { + // A picked pair cannot narrow a three-path request: two paths is what `ends` names. + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, 1]], "mode": "add"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, 0]], "mode": "add"})).unwrap(); + } + let before = s.doc().unwrap().doc.clone(); + let sel_before = s.doc().unwrap().selection.objects.clone(); + let err = s.execute("path.join", &json!({"ends": ["first", "last"]})).unwrap_err(); + assert!(err.to_string().contains("exactly two paths"), "{err}"); + assert_eq!(s.doc().unwrap().doc, before, "nothing was joined"); + assert_eq!(s.doc().unwrap().selection.objects, sel_before, "the selection is unchanged"); + } +} + +/// Controls: explicit `ids` take `ends` over the picks, and without `ends` the picks still decide. +#[test] +fn join_direct_selected_end_points_controls() { + let pts = |s: &Session, id: NodeId| path(s, id).subpaths[0].anchors.iter().map(|x| (x.p.x, x.p.y)).collect::>(); + let pick = |s: &mut Session, a: NodeId, b: NodeId, pa: usize, pb: usize| { + s.execute("select.anchors", &json!({"id": a.0, "anchors": [[0, pa]], "mode": "set"})).unwrap(); + s.execute("select.anchors", &json!({"id": b.0, "anchors": [[0, pb]], "mode": "add"})).unwrap(); + }; + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + pick(&mut s, a, b, 1, 0); + s.execute("path.join", &json!({"ids": [a.0, b.0], "ends": ["first", "last"]})).unwrap(); + assert_eq!(pts(&s, a), [(100.0, 0.0), (0.0, 0.0), (200.0, 0.0), (110.0, 0.0)], "ids: the ends win"); + let mut s = session(); + let a = line(&mut s, 0.0, 0.0, 100.0, 0.0); + let b = line(&mut s, 110.0, 0.0, 200.0, 0.0); + pick(&mut s, a, b, 0, 0); + s.execute("path.join", &json!({})).unwrap(); + assert_eq!(pts(&s, a), [(100.0, 0.0), (0.0, 0.0), (110.0, 0.0), (200.0, 0.0)], "no ends: the picks decide"); +} + /// Without `ends`, the end points picked with the direct selection are the ones joined. #[test] fn join_uses_the_selected_end_points() { From 1440538ab692e092867c62d5899efda669047700 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 19:08:52 +0000 Subject: [PATCH 12/18] fix(installer): check the exact architecture-specific machine folder The pre-signing check matched any folder named Program Files, so it rejected a valid MSI when Windows resolves Program Files elsewhere, accepted the 64-bit root for an x86 package and accepted a different app name. Build the expected folder from the package architecture's resolved root and the policy name. --- packaging/windows/check-install-policy.ps1 | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 index a4a14bc64..cc9416e9c 100644 --- a/packaging/windows/check-install-policy.ps1 +++ b/packaging/windows/check-install-policy.ps1 @@ -32,6 +32,12 @@ function Set-SessionProperty($Session, [string] $Name, [string] $Value) { function Get-SessionProperty($Session, [string] $Name) { $Session.GetType().InvokeMember('Property', 'GetProperty', $null, $Session, @($Name)) } +function Get-PackageTemplate($Database) { + # Summary information property 7 is the platform: "Intel;1033", "x64;1033" or "Arm64;1033". + $summary = $Database.GetType().InvokeMember('SummaryInformation', 'GetProperty', $null, $Database, @(0)) + try { [string]$summary.GetType().InvokeMember('Property', 'GetProperty', $null, $summary, @(7)) } + finally { [void][Runtime.InteropServices.Marshal]::FinalReleaseComObject($summary) } +} $installer = $null; $database = $null; $session = $null try { $installer = New-Object -ComObject WindowsInstaller.Installer @@ -150,6 +156,12 @@ try { if (@(Read-Rows $database 'SELECT `Action` FROM `InstallExecuteSequence`' 1 | Where-Object { $_[0] -like 'CraftDefault*' }).Count) { throw 'Silent installs must not use the wizard-only default folder actions.' } + # The machine default comes from the environment of the package architecture, not from a folder name. + # x86 packages use Program Files (x86) on 64-bit Windows; x64 and Arm64 packages use the 64-bit root. + $machineRoot = if ((Get-PackageTemplate $database) -match '^Intel') { + if (Get-SessionProperty $session 'VersionNT64') { ${env:ProgramFiles(x86)} } else { $env:ProgramFiles } + } elseif ($env:ProgramW6432) { $env:ProgramW6432 } else { $env:ProgramFiles } + $machineFolder = (Join-Path $machineRoot "ArtCraft\$($policy.name)") + '\' foreach ($scope in @('1', '')) { Set-SessionProperty $session 'ALLUSERS' $scope Set-SessionProperty $session 'INSTALLFOLDER' '' @@ -159,8 +171,8 @@ try { if ($session.EvaluateCondition($row[2]) -eq 1 -and $session.DoAction($row[0]) -ne 1) { throw "$($row[0]) failed." } } $folder = Get-SessionProperty $session 'INSTALLFOLDER' - $wanted = if ($scope -eq '1') { '^[A-Za-z]:\\Program Files( \(x86\))?\\ArtCraft\\[^\\]+\\$' } else { '^' + [regex]::Escape($env:LOCALAPPDATA) + '\\Programs\\ArtCraft\\[^\\]+\\$' } - if ($folder -notmatch $wanted) { throw "Wizard default folder for ALLUSERS='$scope' is '$folder'." } + $wanted = if ($scope -eq '1') { $machineFolder } else { (Join-Path $env:LOCALAPPDATA "Programs\ArtCraft\$($policy.name)") + '\' } + if ($folder -ine $wanted) { throw "Wizard default folder for ALLUSERS='$scope' is '$folder', expected '$wanted'." } } $execute = @{} foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallExecuteSequence`' 2)) { $execute[$row[0]] = [int]$row[1] } From 28b5b9249c4757c97212fe4596818a3943ae2277 Mon Sep 17 00:00:00 2001 From: aiongg Date: Tue, 27 Oct 2026 20:52:02 +0000 Subject: [PATCH 13/18] ci: use storytold/craft-common-actions at the merged installer-policy commit --- .github/workflows/windows-arm64.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/windows-arm64.yml b/.github/workflows/windows-arm64.yml index b58801380..7fc9b5925 100644 --- a/.github/workflows/windows-arm64.yml +++ b/.github/workflows/windows-arm64.yml @@ -28,7 +28,7 @@ permissions: jobs: windows-arm64: # Immutable supporting workflow revision until the new install contract lands upstream. - uses: Jeremy8776/craft-common-actions/.github/workflows/windows-arm64.yml@f4666d5cbf35f9bdeda24f1f88c7fb5c6adf5276 + uses: storytold/craft-common-actions/.github/workflows/windows-arm64.yml@8bb4e5ab95443171104ad88113fe1ef9b0e5ce3f with: app-name: vectorcraft app-dir: VectorCraft From ddf9845a27373995d6fac07b23bbe9899630fac3 Mon Sep 17 00:00:00 2001 From: Jeremy Date: Tue, 27 Oct 2026 21:13:29 +0000 Subject: [PATCH 14/18] test(installer): cover the compiled folder searches, resolver chains and wizard folder replay The policy check now reads the compiled AppSearch and RegLocator rows for the remembered-folder properties (registry root, key, value name and type for both scopes) and runs the DIR, FOLDER, PATH and legacy exe actions in sequence order against seeded values, so their precedence and the preset-folder guard cannot regress. It also replays the wizard folder actions in sequence order (a remembered folder beats the new-install default, a typed folder beats both) and requires the machine restore action between the scope page and costing. --- packaging/windows/check-install-policy.ps1 | 76 +++++++++++++++++++++- 1 file changed, 75 insertions(+), 1 deletion(-) diff --git a/packaging/windows/check-install-policy.ps1 b/packaging/windows/check-install-policy.ps1 index cc9416e9c..5463c0083 100644 --- a/packaging/windows/check-install-policy.ps1 +++ b/packaging/windows/check-install-policy.ps1 @@ -143,7 +143,8 @@ try { $ui['CraftPreferMachineScope'] -lt $ui['CraftScopeDlg'] -and $ui['CraftScopeDlg'] -lt $ui['FindRelatedProducts'] -and $ui['FindRelatedProducts'] -lt $ui['LaunchConditions'] -and - $ui['CraftScopeDlg'] -lt $ui['CraftRestoreUserFolder'] -and $ui['CraftRestoreUserFolder'] -lt $ui['CostInitialize'])) { + $ui['CraftScopeDlg'] -lt $ui['CraftRestoreUserFolder'] -and $ui['CraftRestoreUserFolder'] -lt $ui['CostInitialize'] -and + $ui['CraftScopeDlg'] -lt $ui['CraftRestoreMachineFolder'] -and $ui['CraftRestoreMachineFolder'] -lt $ui['CostInitialize'])) { throw 'Scope must be chosen after AppSearch and before FindRelatedProducts, launch checks and costing.' } # Windows Installer resolves Program Files for the context it started in, before the wizard's scope choice. @@ -174,6 +175,28 @@ try { $wanted = if ($scope -eq '1') { $machineFolder } else { (Join-Path $env:LOCALAPPDATA "Programs\ArtCraft\$($policy.name)") + '\' } if ($folder -ine $wanted) { throw "Wizard default folder for ALLUSERS='$scope' is '$folder', expected '$wanted'." } } + # Replay the wizard's compiled folder actions in sequence order, as the full UI runs them. A remembered + # folder must win over the new-install default, and a folder the user typed must survive both. + $uiFolderActions = @(Read-Rows $database 'SELECT `Action`, `Sequence`, `Condition` FROM `InstallUISequence`' 3 | + Where-Object { $_[0] -match '^Craft(Restore|Default)' } | Sort-Object { [int]$_[1] }) + $userDefault = (Join-Path $env:LOCALAPPDATA "Programs\ArtCraft\$($policy.name)") + '\' + foreach ($case in @( + @('1', 'D:\Remembered user app', 'E:\Remembered machine app', '', 'E:\Remembered machine app'), + @('', 'D:\Remembered user app', 'E:\Remembered machine app', '', 'D:\Remembered user app'), + @('1', '', '', '', $machineFolder), + @('', '', '', '', $userDefault), + @('1', 'D:\Remembered user app', 'E:\Remembered machine app', 'F:\Chosen exact app', 'F:\Chosen exact app'))) { + Set-SessionProperty $session 'Installed' '' + Set-SessionProperty $session 'ALLUSERS' $case[0] + Set-SessionProperty $session 'CRAFT_USER_DIR' $case[1] + Set-SessionProperty $session 'CRAFT_MACHINE_DIR' $case[2] + Set-SessionProperty $session 'INSTALLFOLDER' $case[3] + foreach ($row in $uiFolderActions) { + if ($session.EvaluateCondition($row[2]) -eq 1 -and $session.DoAction($row[0]) -ne 1) { throw "$($row[0]) failed." } + } + $folder = Get-SessionProperty $session 'INSTALLFOLDER' + if ($folder -ine $case[4]) { throw "Full-UI folder for ALLUSERS='$($case[0])' is '$folder', expected '$($case[4])'." } + } $execute = @{} foreach ($row in (Read-Rows $database 'SELECT `Action`, `Sequence` FROM `InstallExecuteSequence`' 2)) { $execute[$row[0]] = [int]$row[1] } foreach ($action in @('CraftRestoreUserFolder','CraftRestoreMachineFolder')) { @@ -186,6 +209,57 @@ try { throw 'Both legacy folder resolvers must run after AppSearch and before the launch checks.' } } + # AppSearch itself never runs in this safe session, so check the compiled searches and then run the + # property actions that turn the search results into the remembered folder, once per scope. + $appSearch = @{} + foreach ($row in (Read-Rows $database 'SELECT `Property`, `Signature_` FROM `AppSearch`' 2)) { $appSearch[$row[0]] = $row[1] } + $locators = @{} + foreach ($row in (Read-Rows $database 'SELECT `Signature_`, `Root`, `Key`, `Name`, `Type` FROM `RegLocator`' 5)) { $locators[$row[0]] = $row } + $appPaths = '^Software\\Microsoft\\Windows\\CurrentVersion\\App Paths\\[^\\]+\.exe$' + # Suffix, how to match the key, key, value name, locator type (2 = raw value, 1 = file; the 64-bit flag is masked off). + $searchContract = @( + @('DIR', 'exact', "Software\$($policy.name)", 'InstallDir', '2'), + @('FOLDER', 'exact', "Software\$($policy.name)", 'InstallFolder', '2'), + @('PATH', 'regex', $appPaths, 'Path', '2'), + @('EXEFOLDER', 'regex', $appPaths, '', '1'), + @('EXE', 'regex', $appPaths, '', '1')) + $resolverCases = @( + @{ name = 'every record, DIR wins'; dir = 'D:\Dir\'; folder = 'D:\Folder\'; path = 'D:\Path\'; exeFolder = 'D:\Exe\'; exe = 'D:\Exe\app.exe'; existing = ''; expected = 'D:\Dir\' }, + @{ name = 'FOLDER wins without DIR'; dir = ''; folder = 'D:\Folder\'; path = 'D:\Path\'; exeFolder = 'D:\Exe\'; exe = 'D:\Exe\app.exe'; existing = ''; expected = 'D:\Folder\' }, + @{ name = 'PATH wins without DIR, FOLDER'; dir = ''; folder = ''; path = 'D:\Path\'; exeFolder = 'D:\Exe\'; exe = 'D:\Exe\app.exe'; existing = ''; expected = 'D:\Path\' }, + @{ name = 'legacy exe folder is last'; dir = ''; folder = ''; path = ''; exeFolder = 'D:\Exe\'; exe = 'D:\Exe\app.exe'; existing = ''; expected = 'D:\Exe\' }, + @{ name = 'legacy folder needs the exe'; dir = ''; folder = ''; path = ''; exeFolder = 'D:\Exe\'; exe = ''; existing = ''; expected = '' }, + @{ name = 'a preset folder is kept'; dir = 'D:\Dir\'; folder = 'D:\Folder\'; path = 'D:\Path\'; exeFolder = 'D:\Exe\'; exe = 'D:\Exe\app.exe'; existing = 'P:\Preset\'; expected = 'P:\Preset\' }) + foreach ($scopeSpec in @(@('U', '1', 'CRAFT_USER_DIR'), @('M', '2', 'CRAFT_MACHINE_DIR'))) { + $letter = $scopeSpec[0]; $target = $scopeSpec[2] + foreach ($item in $searchContract) { + $property = "CRAFT_${letter}_$($item[0])" + $locator = if ($appSearch.ContainsKey($property)) { $locators[$appSearch[$property]] } else { $null } + $keyOk = $null -ne $locator -and $(if ($item[1] -eq 'exact') { $locator[2] -eq $item[2] } else { $locator[2] -match $item[2] }) + if (-not $keyOk -or $locator[1] -ne $scopeSpec[1] -or $locator[3] -ne $item[3] -or ([int]$locator[4] -band 3) -ne [int]$item[4]) { + throw "Compiled search for $property is missing or points at the wrong registry location." + } + } + $chain = @(Read-Rows $database 'SELECT `Action`, `Sequence`, `Condition` FROM `InstallExecuteSequence`' 3 | + Where-Object { $_[0] -match "^Craft${letter}From" } | Sort-Object { [int]$_[1] }) + $order = @($chain | ForEach-Object { $_[0] }) -join ',' + if ($order -ne "Craft${letter}FromDIR,Craft${letter}FromFOLDER,Craft${letter}FromPATH,Craft${letter}FromLegacyExe") { + throw "Remembered-folder resolvers for scope $letter run in the wrong order: $order" + } + foreach ($case in $resolverCases) { + Set-SessionProperty $session $target $case.existing + Set-SessionProperty $session "CRAFT_${letter}_DIR" $case.dir + Set-SessionProperty $session "CRAFT_${letter}_FOLDER" $case.folder + Set-SessionProperty $session "CRAFT_${letter}_PATH" $case.path + Set-SessionProperty $session "CRAFT_${letter}_EXEFOLDER" $case.exeFolder + Set-SessionProperty $session "CRAFT_${letter}_EXE" $case.exe + foreach ($row in $chain) { + if ($session.EvaluateCondition($row[2]) -eq 1 -and $session.DoAction($row[0]) -ne 1) { throw "$($row[0]) failed." } + } + $got = Get-SessionProperty $session $target + if ($got -ne $case.expected) { throw "Resolver chain for scope $letter, case '$($case.name)': got '$got', expected '$($case.expected)'." } + } + } # Switching the first-page choice must reset BOTH properties, including a back-and-forth change. $events = @(Read-Rows $database 'SELECT `Event`, `Argument`, `Condition`, `Ordering` FROM `ControlEvent` WHERE `Dialog_` = ''CraftScopeDlg'' AND `Control_` = ''Next'' ORDER BY `Ordering`' 4) # Model the full-UI upgrade entry with an existing machine-wide copy. Run From 9997ff021503e4f323f084fb7e64618d209d0126 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Tue, 27 Oct 2026 21:09:33 -0400 Subject: [PATCH 15/18] ci: run the printps tests under their moved path and fail when none run (#4305) The module layout moved tests_printps to tests::printps, so the print workflow's filter matched nothing and the step passed with 0 tests. --- .github/workflows/print.yml | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/.github/workflows/print.yml b/.github/workflows/print.yml index 2ee35f176..400a81944 100644 --- a/.github/workflows/print.yml +++ b/.github/workflows/print.yml @@ -32,4 +32,8 @@ jobs: - run: cargo clippy --locked -p vectorcraft --all-targets -- -D warnings - run: cargo test --locked -p vectorcraft - name: Check pending submission and queue refusal feedback - run: cargo test --locked -p vectorcraft-ui-egui tests_printps + shell: bash + # A filter that matches nothing still passes, so fail unless some test ran. + run: | + cargo test --locked -p vectorcraft-ui-egui --lib -- tests::printps | tee printps.log + grep -Eq 'test result: ok\. [1-9][0-9]* passed' printps.log || { echo "::error::tests::printps matched no tests"; exit 1; } From 68b68a30acad9b3d8a72802b35f43b82e2bd7d3a Mon Sep 17 00:00:00 2001 From: Brandon Thomas Date: Wed, 28 Oct 2026 12:09:18 -0400 Subject: [PATCH 16/18] Release 0.18.0 Co-Authored-By: Claude Opus 5.5 --- Cargo.lock | 56 +++++++++++++++++++++++++++--------------------------- Cargo.toml | 2 +- 2 files changed, 29 insertions(+), 29 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index d329c26a7..ef60276c1 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -6036,7 +6036,7 @@ dependencies = [ [[package]] name = "vectorcraft" -version = "0.17.0" +version = "0.18.0" dependencies = [ "arboard", "block2 0.6.2", @@ -6077,7 +6077,7 @@ dependencies = [ [[package]] name = "vectorcraft-affinity" -version = "0.17.0" +version = "0.18.0" dependencies = [ "crc32fast", "flate2", @@ -6088,7 +6088,7 @@ dependencies = [ [[package]] name = "vectorcraft-brush" -version = "0.17.0" +version = "0.18.0" dependencies = [ "kurbo 0.13.1", "serde", @@ -6100,7 +6100,7 @@ dependencies = [ [[package]] name = "vectorcraft-cad" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo 0.13.1", @@ -6116,7 +6116,7 @@ dependencies = [ [[package]] name = "vectorcraft-cli" -version = "0.17.0" +version = "0.18.0" dependencies = [ "log", "objc2-core-foundation", @@ -6138,7 +6138,7 @@ dependencies = [ [[package]] name = "vectorcraft-color" -version = "0.17.0" +version = "0.18.0" dependencies = [ "kurbo 0.13.1", "moxcms", @@ -6150,7 +6150,7 @@ dependencies = [ [[package]] name = "vectorcraft-doc" -version = "0.17.0" +version = "0.18.0" dependencies = [ "chrono", "hayro-ccitt", @@ -6169,7 +6169,7 @@ dependencies = [ [[package]] name = "vectorcraft-effects" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo 0.13.1", @@ -6185,7 +6185,7 @@ dependencies = [ [[package]] name = "vectorcraft-engine" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-docpath", "crc32fast", @@ -6224,7 +6224,7 @@ dependencies = [ [[package]] name = "vectorcraft-eps" -version = "0.17.0" +version = "0.18.0" dependencies = [ "fax", "flate2", @@ -6248,7 +6248,7 @@ dependencies = [ [[package]] name = "vectorcraft-format" -version = "0.17.0" +version = "0.18.0" dependencies = [ "flate2", "proptest", @@ -6263,7 +6263,7 @@ dependencies = [ [[package]] name = "vectorcraft-geom" -version = "0.17.0" +version = "0.18.0" dependencies = [ "kurbo 0.13.1", "proptest", @@ -6275,7 +6275,7 @@ dependencies = [ [[package]] name = "vectorcraft-mcp" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "log", @@ -6294,7 +6294,7 @@ dependencies = [ [[package]] name = "vectorcraft-metafile" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo 0.13.1", @@ -6308,7 +6308,7 @@ dependencies = [ [[package]] name = "vectorcraft-pathops" -version = "0.17.0" +version = "0.18.0" dependencies = [ "kurbo 0.13.1", "linesweeper", @@ -6320,7 +6320,7 @@ dependencies = [ [[package]] name = "vectorcraft-pdf" -version = "0.17.0" +version = "0.18.0" dependencies = [ "aes", "chrono", @@ -6351,7 +6351,7 @@ dependencies = [ [[package]] name = "vectorcraft-plugins" -version = "0.17.0" +version = "0.18.0" dependencies = [ "serde", "serde_json", @@ -6365,7 +6365,7 @@ dependencies = [ [[package]] name = "vectorcraft-render" -version = "0.17.0" +version = "0.18.0" dependencies = [ "flate2", "gif", @@ -6394,14 +6394,14 @@ dependencies = [ [[package]] name = "vectorcraft-scriptcheck" -version = "0.17.0" +version = "0.18.0" dependencies = [ "proptest", ] [[package]] name = "vectorcraft-svg" -version = "0.17.0" +version = "0.18.0" dependencies = [ "craft-docpath", "flate2", @@ -6426,7 +6426,7 @@ dependencies = [ [[package]] name = "vectorcraft-testkit" -version = "0.17.0" +version = "0.18.0" dependencies = [ "fax", "image", @@ -6444,7 +6444,7 @@ dependencies = [ [[package]] name = "vectorcraft-text" -version = "0.17.0" +version = "0.18.0" dependencies = [ "harfrust 0.13.3", "icu_segmenter", @@ -6465,7 +6465,7 @@ dependencies = [ [[package]] name = "vectorcraft-three-d" -version = "0.17.0" +version = "0.18.0" dependencies = [ "vectorcraft-geom", "vectorcraft-pathops", @@ -6473,7 +6473,7 @@ dependencies = [ [[package]] name = "vectorcraft-tools" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo 0.13.1", @@ -6489,7 +6489,7 @@ dependencies = [ [[package]] name = "vectorcraft-trace" -version = "0.17.0" +version = "0.18.0" dependencies = [ "image", "kurbo 0.13.1", @@ -6501,7 +6501,7 @@ dependencies = [ [[package]] name = "vectorcraft-ui-egui" -version = "0.17.0" +version = "0.18.0" dependencies = [ "bytemuck", "craft-keymap", @@ -6539,7 +6539,7 @@ dependencies = [ [[package]] name = "vectorcraft-web" -version = "0.17.0" +version = "0.18.0" dependencies = [ "eframe", "egui", @@ -7670,7 +7670,7 @@ checksum = "9440ea3e5aeabb0ac63af70daf835274065238cdd0cec83418f417eae38bacee" [[package]] name = "xtask" -version = "0.17.0" +version = "0.18.0" dependencies = [ "serde_json", ] diff --git a/Cargo.toml b/Cargo.toml index 644bffe9d..d75f62eee 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -4,7 +4,7 @@ members = ["crates/*", "apps/*", "xtask"] default-members = ["crates/*", "apps/vectorcraft", "apps/vectorcraft-cli", "xtask"] [workspace.package] -version = "0.17.0" +version = "0.18.0" edition = "2024" license = "MIT OR Apache-2.0" rust-version = "1.95" From ed960ef1f1d70e5b0e4cf83f03f551fe86cad8e5 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 28 Oct 2026 13:07:32 -0400 Subject: [PATCH 17/18] Grant network access so update checks can reach GitHub (#4482) --- .../linux/flatpak/ai.storyteller.vectorcraft.bundle.yml | 6 +++--- packaging/linux/flatpak/ai.storyteller.vectorcraft.yml | 2 ++ 2 files changed, 5 insertions(+), 3 deletions(-) diff --git a/packaging/linux/flatpak/ai.storyteller.vectorcraft.bundle.yml b/packaging/linux/flatpak/ai.storyteller.vectorcraft.bundle.yml index 687445a55..94aa4f392 100644 --- a/packaging/linux/flatpak/ai.storyteller.vectorcraft.bundle.yml +++ b/packaging/linux/flatpak/ai.storyteller.vectorcraft.bundle.yml @@ -24,6 +24,8 @@ finish-args: - --socket=fallback-x11 # GPU access for the wgpu renderer (Vulkan, or GL as a fallback). - --device=dri + # Network for the update check against GitHub releases. + - --share=network # Cursor theme: without the cursor-shape protocol (e.g. GNOME 46) winit loads the theme itself. # The sandbox sets no XCURSOR_PATH and ships no cursor themes, so the pointer would turn invisible # over the window. The host's themes are mounted under /run/host (#2450). @@ -36,9 +38,7 @@ finish-args: # to a sandbox: it exposes /usr/share/fonts and ~/.local/share/fonts, and nothing else. Without # this the font list holds none of the activated families, while native apps see them. - --filesystem=~/.fonts:ro - # Deliberately not granted: network (the optional --control server listens on 127.0.0.1 in - # the sandbox; `flatpak override --user --share=network ai.storyteller.vectorcraft` exposes it - # to host agents) and home/host filesystem access beyond the read-only folders above. + # Deliberately not granted: home/host filesystem access beyond the read-only folders above. modules: - name: vectorcraft buildsystem: simple diff --git a/packaging/linux/flatpak/ai.storyteller.vectorcraft.yml b/packaging/linux/flatpak/ai.storyteller.vectorcraft.yml index c2a00eab1..4d740a8d9 100644 --- a/packaging/linux/flatpak/ai.storyteller.vectorcraft.yml +++ b/packaging/linux/flatpak/ai.storyteller.vectorcraft.yml @@ -22,6 +22,8 @@ finish-args: - --socket=wayland - --socket=fallback-x11 - --device=dri + # Network for the update check against GitHub releases. + - --share=network # Cursor theme: without the cursor-shape protocol (e.g. GNOME 46) winit loads the theme itself. # The sandbox sets no XCURSOR_PATH and ships no cursor themes, so the pointer would turn invisible # over the window. The host's themes are mounted under /run/host (#2450). From 56804f6d7e961a7d6bfb8e3d46c168b15e1feee2 Mon Sep 17 00:00:00 2001 From: "Dylan M. Taylor" Date: Wed, 28 Oct 2026 15:48:05 -0400 Subject: [PATCH 18/18] packaging: honor CARGO_BUILD_TARGET in FreeBSD package.sh (#4487) The shared release workflow cross-compiles FreeBSD aarch64 from an x86_64 VM by setting CARGO_BUILD_TARGET. package.sh ignored it: it took the arch from uname -m (the host, x86_64), read binaries from target/release, and ran the cross-built binary to print its version. Pass --target, read from target//release, take the arch from the target, and skip the version run when cross-built. --- packaging/freebsd/package.sh | 24 +++++++++++++++++------- 1 file changed, 17 insertions(+), 7 deletions(-) diff --git a/packaging/freebsd/package.sh b/packaging/freebsd/package.sh index dffc56571..56e71d25a 100755 --- a/packaging/freebsd/package.sh +++ b/packaging/freebsd/package.sh @@ -1,8 +1,8 @@ #!/usr/bin/env bash # Build and package VectorCraft for FreeBSD: # -# $DIST/vectorcraft--freebsd-x86_64.tar.gz a /usr/local-style tree: -# vectorcraft--freebsd-x86_64/{bin, share/applications, share/icons, share/mime, +# $DIST/vectorcraft--freebsd-.tar.gz a /usr/local-style tree: +# vectorcraft--freebsd-/{bin, share/applications, share/icons, share/mime, # share/metainfo, share/doc/vectorcraft} # # Install by copying the tree's contents into /usr/local, e.g. @@ -29,18 +29,27 @@ done # FreeBSD calls x86_64 "amd64"; release names use the Rust/Linux spelling. case "$(uname -m)" in - amd64 | x86_64) ARCH=x86_64 ;; - arm64 | aarch64) ARCH=aarch64 ;; + amd64 | x86_64) HOST_ARCH=x86_64 ;; + arm64 | aarch64) HOST_ARCH=aarch64 ;; *) echo "unsupported architecture $(uname -m)" >&2; exit 2 ;; esac +# The release workflow cross-compiles aarch64 on x86_64 by setting CARGO_BUILD_TARGET; the arch follows the target. +ARCH="$HOST_ARCH" +BIN="$CARGO_TARGET_DIR/release" +if [[ -n "${CARGO_BUILD_TARGET:-}" ]]; then + case "$CARGO_BUILD_TARGET" in + aarch64-unknown-freebsd) ARCH=aarch64 ;; + *) ARCH=x86_64 ;; + esac + BIN="$CARGO_TARGET_DIR/$CARGO_BUILD_TARGET/release" +fi BASENAME="vectorcraft-$VERSION-freebsd-$ARCH" echo "==> VectorCraft $VERSION for FreeBSD $ARCH" if [[ "$SKIP_BUILD" = 0 ]]; then - (cd "$ROOT" && cargo build --release --locked -p vectorcraft -p vectorcraft-cli) + (cd "$ROOT" && cargo build --release --locked -p vectorcraft -p vectorcraft-cli ${CARGO_BUILD_TARGET:+--target "$CARGO_BUILD_TARGET"}) fi -BIN="$CARGO_TARGET_DIR/release" WORK="$CARGO_TARGET_DIR/freebsd-package" STAGE="$WORK/$BASENAME" rm -rf "$WORK" @@ -65,6 +74,7 @@ done tar -C "$WORK" -czf "$DIST/$BASENAME.tar.gz" "$BASENAME" echo "wrote $DIST/$BASENAME.tar.gz" -"$STAGE/bin/vectorcraft-cli" --version +# A cross-built binary can't run on this host. +if [[ "$ARCH" = "$HOST_ARCH" ]]; then "$STAGE/bin/vectorcraft-cli" --version; fi echo "==> done" ls -lh "$DIST"