From 20476760bc9ccb361b05c2f1189348a38b7c3211 Mon Sep 17 00:00:00 2001 From: ncic <1848265+ncic@users.noreply.github.com> Date: Sun, 25 Oct 2026 03:40:21 +0100 Subject: [PATCH] Pencil: Edit Selected Paths redraws a selected compound path's member paths MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit A selected compound path (a shape with a hole, such as one made with Object › Compound Path › Make) could not be redrawn with the Pencil or the Paintbrush: at the press, find_redraw looked only for selected objects that are paths themselves, and a compound path's paths are its children. The stroke then drew a new path, which Keep selected selected in place of the compound path, so every later stroke near the shape drew a new path too: redrawing seemed to stop working. - find_redraw takes a selected compound path's member paths as well, the nearest one within Edit selected paths within of the press. - path.freehand's redraw keeps the compound path selected (or, with Keep selected off, takes it out of the selection) when the redrawn path is one of its members, so the next stroke redraws it too. Test: pencil_redraws_a_selected_compound_path redraws the outer path and then the hole of a selected compound path; it fails without this change (the first stroke draws a new path). cargo xtask ci: every step passes except one test in `test`, vectorcraft-pdf tests_import_fidelity::text_becomes_point_type, which fails the same way on unmodified main on this machine; it is unrelated to this change. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/cmd/draw2.rs | 7 +++-- crates/engine/src/tests_draw2.rs | 49 +++++++++++++++++++++++++++++++ crates/tools/src/draw2/gesture.rs | 24 +++++++++++++-- 3 files changed, 75 insertions(+), 5 deletions(-) diff --git a/crates/engine/src/cmd/draw2.rs b/crates/engine/src/cmd/draw2.rs index ee3e474cd..8a8e7b8b1 100644 --- a/crates/engine/src/cmd/draw2.rs +++ b/crates/engine/src/cmd/draw2.rs @@ -843,10 +843,13 @@ fn freehand(s: &mut Session, p: &Value) -> Result { if let Some((si, sp)) = redraw_section(&pd, &pts, fit_tol, within) { s.edit(if brush { "Paintbrush" } else { "Pencil" }, |d, sel| { *path_mut(d, id)?.subpaths.get_mut(si).ok_or_else(|| bad(C, "no such subpath"))? = sp; + // A member path of a compound path is redrawn in it: the compound path is what is + // selected. + let whole = d.parent_of(id).filter(|p| d.node(*p).is_some_and(|n| matches!(n.kind, NodeKind::Compound { .. }))).unwrap_or(id); if keep_selected { - sel.set([id]); + sel.set([whole]); } else { - sel.remove(id); + sel.remove(whole); } Ok(()) })?; diff --git a/crates/engine/src/tests_draw2.rs b/crates/engine/src/tests_draw2.rs index cfa37b917..f712891d5 100644 --- a/crates/engine/src/tests_draw2.rs +++ b/crates/engine/src/tests_draw2.rs @@ -2722,3 +2722,52 @@ fn pencil_option_takes_a_hundredth_and_smooth_keeps_its_floor() { assert_eq!(s.tool_options_of(tool)["fidelity"].as_f64(), Some(if tool == "smooth" { 0.05 } else { 0.02 }), "{tool}"); } } + +/// A selected compound path (a shape with a hole) is overdrawn as its paths are: a Pencil stroke +/// starting and ending on its outer path, or on its hole, redraws that member path, draws no new +/// path, and leaves the compound path selected, so the next stroke redraws it too. +#[test] +fn pencil_redraws_a_selected_compound_path() { + let mut s = session(); + s.execute("select.none", &json!({})).unwrap(); + let ring = |cx: f64, rx: f64, ry: f64| -> Vec { + (0..72) + .map(|i| { + let a = f64::from(i * 5).to_radians(); + json!([cx + rx * a.cos(), 300.0 + ry * a.sin()]) + }) + .collect() + }; + let outer = s.execute("path.freehand", &json!({"points": ring(400.0, 150.0, 90.0), "closed": true})).unwrap()["id"].as_u64().unwrap(); + let hole = s.execute("path.freehand", &json!({"points": ring(380.0, 60.0, 30.0), "closed": true})).unwrap()["id"].as_u64().unwrap(); + s.execute("select.set", &json!({"ids": [outer, hole]})).unwrap(); + s.execute("object.compoundPath.make", &json!({})).unwrap(); + let compound = s.doc().unwrap().selection.objects.clone(); + assert_eq!(compound.len(), 1); + let members = |s: &Session| -> Vec { + s.doc().unwrap().doc.node(compound[0]).unwrap().children().unwrap().iter().map(|c| c.path_data().unwrap().clone()).collect() + }; + let before = members(&s); + let v = view(); + s.select_tool("pencil", v).unwrap(); + let count = |s: &Session| s.doc().unwrap().doc.layers[0].children().map_or(0, |c| c.len()); + let stroke = |s: &mut Session, pts: &[(f64, f64)]| { + s.pointer(&PointerEvent::new(PointerKind::Down, pts[0].0, pts[0].1), v).unwrap(); + for &(x, y) in &pts[1..] { + s.pointer(&PointerEvent::new(PointerKind::Drag, x, y), v).unwrap(); + } + let l = pts[pts.len() - 1]; + s.pointer(&PointerEvent::new(PointerKind::Up, l.0, l.1), v).unwrap(); + }; + // Over the outer path's right side, bowing outward: from (550, 300) round to (520, 360). + stroke(&mut s, &[(550.0, 300.0), (575.0, 315.0), (580.0, 335.0), (560.0, 355.0), (520.0, 360.0)]); + assert_eq!(count(&s), 1, "no new path"); + assert_eq!(s.doc().unwrap().selection.objects, compound, "the compound path stays selected"); + let after = members(&s); + assert!(after[0] != before[0] && after[1] == before[1], "the outer path is redrawn, the hole left alone"); + // Then over the hole's left side: the compound path is still the one redrawn. + stroke(&mut s, &[(320.0, 300.0), (310.0, 290.0), (312.0, 280.0), (330.0, 274.0)]); + assert_eq!(count(&s), 1, "no new path"); + assert_eq!(s.doc().unwrap().selection.objects, compound); + assert!(members(&s)[1] != before[1], "the hole is redrawn"); +} diff --git a/crates/tools/src/draw2/gesture.rs b/crates/tools/src/draw2/gesture.rs index 9228523dc..2bf687510 100644 --- a/crates/tools/src/draw2/gesture.rs +++ b/crates/tools/src/draw2/gesture.rs @@ -15,7 +15,7 @@ //! vertical line through the press point. use serde_json::{Value, json}; -use vectorcraft_doc::NodeId; +use vectorcraft_doc::{NodeId, NodeKind}; use vectorcraft_geom::Point; use super::{FEEDBACK, eraser_area, points_json, polyline}; @@ -88,6 +88,17 @@ pub struct GestureTool { snap: DrawSnap, } +/// The paths a stroke can redraw for selected object `id`: the path itself, or a compound path's +/// member paths. +fn redraw_targets(cx: &ToolContext, id: NodeId) -> Vec { + let Some(n) = cx.doc.node(id) else { return vec![] }; + match &n.kind { + NodeKind::Path { .. } => vec![id], + NodeKind::Compound { children, .. } => children.iter().filter(|c| c.path_data().is_some()).map(|c| c.id).collect(), + _ => vec![], + } +} + impl GestureTool { pub fn new(id: &str) -> Self { let id: &'static str = match id { @@ -269,9 +280,16 @@ impl GestureTool { if self.edit_within <= 0.0 || !tol.is_finite() { return None; } + // A selected compound path (a shape with a hole) redraws the member path the stroke starts + // on, the nearest of them: a compound path has no path of its own. cx.selection.objects.iter().find_map(|id| { - let pd = cx.doc.node(*id).and_then(|n| n.path_data()).filter(|_| cx.doc.is_editable(*id))?; - pd.nearest(p).filter(|n| n.4 <= tol).map(|_| (*id, tol)) + redraw_targets(cx, *id) + .into_iter() + .filter(|m| cx.doc.is_editable(*m)) + .filter_map(|m| Some((m, cx.doc.node(m)?.path_data()?.nearest(p)?.4))) + .filter(|(_, d)| *d <= tol) + .min_by(|a, b| a.1.total_cmp(&b.1)) + .map(|(m, _)| (m, tol)) }) }