From 11f1d5c4d6c5fcbcb8a96df2334580dc044330bf Mon Sep 17 00:00:00 2001 From: skylar taylor-barrick Date: Sat, 24 Oct 2026 16:08:09 -0600 Subject: [PATCH] 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))); + } }