Merge pull request #4220 from ncic/pencil-redraw-compound

Pencil: Edit Selected Paths redraws a selected compound path

crates/engine/src/cmd/draw2.rs

@@ -843,10 +843,13 @@ fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
         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(())
             })?;

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<serde_json::Value> {
+        (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<PathData> {
+        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");
+}

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<NodeId> {
+    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))
         })
     }