Merge pull request #2905 from storytold/fix/2850-pencil-redraw

Pencil: a stroke that starts and ends on a selected path redraws the section between (Edit Selected Paths, #2850)

crates/engine/src/cmd/draw2.rs

@@ -26,7 +26,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Pencil",
             [],
             None,
-            "{points: [[x,y]…] or [[x,y,pressure]…] (pen pressure 0..1: the stroke keeps it, for Calligraphic brushes with Pressure variation; a point without one presses fully), fidelity?: pt (1.5), closed?, style?: \"pencil\"|\"brush\", fill?: bool, keepSelected?: bool (true: the drawn or continued path is selected; false: it is not, and the rest of the selection stays), extend?: {id, end: \"start\"|\"end\"}} fit a freehand stroke → {id}",
+            "{points: [[x,y]…] or [[x,y,pressure]…] (pen pressure 0..1: the stroke keeps it, for Calligraphic brushes with Pressure variation; a point without one presses fully), fidelity?: pt (1.5), closed?, style?: \"pencil\"|\"brush\", fill?: bool, keepSelected?: bool (true: the drawn or continued path is selected; false: it is not, and the rest of the selection stays), extend?: {id, end: \"start\"|\"end\"}, redraw?: {id, within?: pt (12)}} fit a freehand stroke → {id}. extend continues the path from that end; redraw (Edit Selected Paths) replaces the section of the path between where the stroke starts and ends, each within `within` of one subpath (the shorter way round a closed one), and draws a new path when the stroke doesn't end on it",
             has_doc,
             freehand
         ),
@@ -548,6 +548,87 @@ fn fit_freehand(pts: &[Point], tol: f64, closed: bool) -> SubPath {
     fitted.subpaths.into_iter().next().filter(|s| s.anchors.len() >= 2).unwrap_or(sp)
 }
 
+/// Pencil's Edit Selected Paths in the middle of a path: a stroke from one point of a subpath of
+/// `pd` to another, each within `within` of it, replaces the section between them (the shorter
+/// way round a closed subpath) with the stroke fitted at `tol`. The subpath's index and its new
+/// shape; None when the stroke doesn't start and end on the same subpath, or meets it once.
+fn redraw_section(pd: &PathData, pts: &[Point], tol: f64, within: f64) -> Option<(usize, SubPath)> {
+    use kurbo::ParamCurveArclen;
+    let (first, last) = (*pts.first()?, *pts.last()?);
+    let (si, _, _, _, d_a) = pd.nearest(first)?;
+    let (sj, _, _, _, d_b) = pd.nearest(last)?;
+    if si != sj || d_a > within || d_b > within || pts.len() < 2 {
+        return None;
+    }
+    let mut sp = pd.subpaths.get(si)?.clone();
+    if sp.anchors.len() < 2 {
+        return None;
+    }
+    // The anchor nearest to `q`: the one there, else a new one, and whether it was added. Each
+    // point is looked up on the subpath as it is by then: splitting a straight segment makes two
+    // new ones, which a `t` along the old one doesn't measure.
+    let place = |sp: &mut SubPath, q: Point| {
+        let (_, seg, t, _, _) = PathData::single(sp.clone()).nearest(q)?;
+        let n = sp.anchors.len();
+        Some(if t <= 1e-6 {
+            (seg % n, false)
+        } else if t >= 1.0 - 1e-6 {
+            ((seg + 1) % n, false)
+        } else {
+            (sp.insert_anchor(seg, t), true)
+        })
+    };
+    let (mut from, _) = place(&mut sp, first)?;
+    let (to, added) = place(&mut sp, last)?;
+    if added && to <= from {
+        from += 1;
+    }
+    if from == to {
+        return None;
+    }
+    let n = sp.anchors.len();
+    // The length of the run of segments from anchor i forward to anchor j.
+    let run = |i: usize, j: usize| (0..(j + n - i) % n).map(|k| sp.segment((i + k) % n).arclen(0.01)).sum::<f64>();
+    // The stroke goes from `start` to `end` and replaces the run between them; `kept` runs from
+    // `end` round to `start`, then the stroke's inner anchors follow.
+    let (start, end, reversed) = if sp.closed {
+        if run(from, to) <= run(to, from) { (from, to, false) } else { (to, from, true) }
+    } else if from < to {
+        (from, to, false)
+    } else {
+        (to, from, true)
+    };
+    let mut stroke = pts.to_vec();
+    if reversed {
+        stroke.reverse();
+    }
+    let (Some(s0), Some(s1)) = (stroke.first_mut(), sp.anchors.get(start)) else { return None };
+    *s0 = s1.p;
+    let (Some(e0), Some(e1)) = (stroke.last_mut(), sp.anchors.get(end)) else { return None };
+    *e0 = e1.p;
+    let fit = fit_freehand(&stroke, tol, false).anchors;
+    let (Some(f0), Some(f1)) = (fit.first().copied(), fit.last().copied()) else { return None };
+    let inner = fit.get(1..fit.len().saturating_sub(1)).unwrap_or_default();
+    let (before, after): (Vec<Anchor>, Vec<Anchor>) = if sp.closed {
+        ((0..=(start + n - end) % n).filter_map(|k| sp.anchors.get((end + k) % n).copied()).collect(), vec![])
+    } else {
+        (sp.anchors.get(..=start)?.to_vec(), sp.anchors.get(end..)?.to_vec())
+    };
+    let mut anchors = before;
+    if let Some(a) = anchors.last_mut() {
+        *a = Anchor::with_handles(a.p, a.h_in, f0.h_out);
+    }
+    anchors.extend_from_slice(inner);
+    let at_end = anchors.len();
+    anchors.extend(after);
+    // The anchor the stroke ends on: the first after it, or round to the first of a closed one.
+    let end_at = if sp.closed { 0 } else { at_end };
+    if let Some(a) = anchors.get_mut(end_at) {
+        *a = Anchor::with_handles(a.p, f1.h_in, a.h_out);
+    }
+    Some((si, SubPath::new(anchors, sp.closed)))
+}
+
 fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
     const C: &str = "path.freehand";
     let samples = samples_param(p, "points", C)?;
@@ -630,6 +711,28 @@ fn freehand(s: &mut Session, p: &Value) -> Result<Value> {
         })?;
         return Ok(json!({ "id": id.0 }));
     }
+    if let Some(rd) = p.get("redraw").filter(|v| !v.is_null()) {
+        let id = id_param(rd, "id").ok_or_else(|| bad(C, "redraw needs id"))?;
+        let within = f64_or(rd, "within", 12.0);
+        if !within.is_finite() || within < 0.0 {
+            return Err(bad(C, "redraw within must be a finite distance of 0 or more"));
+        }
+        let (pd, _) = path_of(&s.doc()?.doc, id)?;
+        // shortcut: a stroke pressure profile is left as it was, measured along the old path; fit
+        // it to the new length if Calligraphic pressure strokes get redrawn.
+        if let Some((si, sp)) = redraw_section(&pd, &pts, 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;
+                if keep_selected {
+                    sel.set([id]);
+                } else {
+                    sel.remove(id);
+                }
+                Ok(())
+            })?;
+            return Ok(json!({ "id": id.0 }));
+        }
+    }
     if pts.len() < 2 {
         return Err(bad(C, "need at least two points"));
     }

crates/engine/src/tests_draw2.rs

@@ -130,6 +130,62 @@ fn pencil_continues_selected_open_path() {
     assert_eq!(p.subpaths[0].anchors[0].p, Point::new(100.0, 100.0));
 }
 
+/// Edit Selected Paths in the middle of a path (#2850): a Pencil stroke that starts and ends on a
+/// selected path replaces the section between, drawn either way; the path stays one path with
+/// its ends, and is one undo step.
+#[test]
+fn pencil_redraws_the_section_of_a_selected_path_between_its_ends() {
+    let bump = [(100.0, 101.0), (125.0, 70.0), (150.0, 60.0), (175.0, 70.0), (200.0, 101.0)];
+    for reversed in [false, true] {
+        let mut s = session();
+        let id = line(&mut s, 0.0, 100.0, 300.0, 100.0);
+        let mut pts = bump.to_vec();
+        if reversed {
+            pts.reverse();
+        }
+        let undo = undo_steps(&s);
+        drag(&mut s, "pencil", &pts, Mods::default());
+        assert_eq!(paths(&s).len(), 1, "no new object");
+        assert_eq!(undo_steps(&s), undo + 1);
+        let sp = &path(&s, id).subpaths[0];
+        assert!(!sp.closed);
+        let ps: Vec<Point> = sp.anchors.iter().map(|a| a.p).collect();
+        assert!(near(ps[0], Point::new(0.0, 100.0)) && near(*ps.last().unwrap(), Point::new(300.0, 100.0)), "{ps:?}");
+        // The section starts and ends on the line, where the stroke met it, and rises between.
+        let on = |x: f64| ps.iter().any(|p| (p.x - x).abs() < 1.5 && (p.y - 100.0).abs() < 1e-6);
+        assert!(on(100.0) && on(200.0), "{reversed}: {ps:?}");
+        let top = kurbo::Shape::bounding_box(&path(&s, id).to_bezpath()).y0;
+        assert!(top < 75.0, "the bump: top {top}");
+        assert!(ps.windows(2).all(|w| w[0].x <= w[1].x + 1.0), "in order along the line: {ps:?}");
+    }
+}
+
+/// On a closed path the stroke replaces the shorter way between where it starts and ends, and the
+/// path stays closed; a stroke that doesn't end on the path draws a new one.
+#[test]
+fn pencil_redraws_the_shorter_way_round_a_closed_path() {
+    let mut s = session();
+    let id = rect(&mut s, 0.0, 0.0, 200.0, 100.0);
+    s.execute("select.set", &json!({"ids": [id.0]})).unwrap();
+    s.execute("path.freehand", &json!({"points": [[50, 1], [80, -25], [100, -30], [120, -25], [150, 1]], "redraw": {"id": id.0, "within": 4}}))
+        .unwrap();
+    assert_eq!(paths(&s).len(), 1);
+    let sp = &path(&s, id).subpaths[0];
+    assert!(sp.closed);
+    let ps: Vec<Point> = sp.anchors.iter().map(|a| a.p).collect();
+    for corner in [(0.0, 0.0), (200.0, 0.0), (200.0, 100.0), (0.0, 100.0)] {
+        assert!(ps.iter().any(|p| near(*p, Point::new(corner.0, corner.1))), "{corner:?} kept: {ps:?}");
+    }
+    let top = kurbo::Shape::bounding_box(&path(&s, id).to_bezpath()).y0;
+    assert!(top < -20.0, "the dip: top {top}");
+    // Not ending on the path: a new path, the rectangle as it was.
+    let before = path(&s, id);
+    s.execute("path.freehand", &json!({"points": [[50, 100], [80, 150], [120, 160]], "redraw": {"id": id.0, "within": 4}})).unwrap();
+    assert_eq!(paths(&s).len(), 2);
+    assert_eq!(path(&s, id), before);
+    assert!(s.execute("path.freehand", &json!({"points": [[0, 0], [1, 1]], "redraw": {"id": id.0, "within": -1}})).is_err());
+}
+
 /// With Keep selected off, the path a stroke draws or continues is not selected afterwards, the
 /// rest of the selection stays, and the stroke is one undo step.
 #[test]

crates/tools/src/draw2/gesture.rs

@@ -27,6 +27,9 @@ pub struct GestureTool {
     began: bool,
     /// Pencil/Paintbrush continuing a selected open path: (path, continue from its start).
     extend: Option<(NodeId, bool)>,
+    /// Pencil/Paintbrush starting on a selected path away from its ends: (path, Edit Selected
+    /// Paths' distance in points); the stroke redraws the section it ends on (#2850).
+    redraw: Option<(NodeId, f64)>,
     /// Fit tolerance in points (Pencil/Paintbrush/Smooth "Fidelity").
     pub fidelity: f64,
     /// Brush / eraser diameter in points.
@@ -66,6 +69,7 @@ impl GestureTool {
             active: false,
             began: false,
             extend: None,
+            redraw: None,
             fidelity,
             size: 10.0,
             fill: false,
@@ -143,6 +147,18 @@ impl GestureTool {
         None
     }
 
+    /// The selected path whose outline is near `p` (Edit Selected Paths, off at 0 pixels).
+    fn find_redraw(&self, cx: &ToolContext, p: Point) -> Option<(NodeId, f64)> {
+        let tol = cx.tol(self.edit_within);
+        if self.edit_within <= 0.0 || !tol.is_finite() {
+            return None;
+        }
+        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))
+        })
+    }
+
     /// Parameters of `path.freehand` for the current stroke.
     pub fn freehand_params(&self, closed: bool) -> Value {
         let mut v = json!({
@@ -155,6 +171,8 @@ impl GestureTool {
         });
         if let Some((id, start)) = self.extend {
             v["extend"] = json!({"id": id.0, "end": if start { "start" } else { "end" }});
+        } else if let Some((id, within)) = self.redraw {
+            v["redraw"] = json!({"id": id.0, "within": within});
         }
         v
     }
@@ -211,8 +229,9 @@ impl Tool for GestureTool {
                 self.active = true;
                 self.began = false;
                 self.extend = if self.draws_path() { self.find_extend(cx, p) } else { None };
-                // A path continued starts at its end; a new one where it snaps.
-                let start = if self.snaps() && self.extend.is_none() { self.snap.press(cx, p, &[], None) } else { p };
+                self.redraw = if self.draws_path() && self.extend.is_none() { self.find_redraw(cx, p) } else { None };
+                // A path continued starts at its end, one redrawn on it; a new one where it snaps.
+                let start = if self.snaps() && self.extend.is_none() && self.redraw.is_none() { self.snap.press(cx, p, &[], None) } else { p };
                 self.clear();
                 self.push(start, ev.pressure);
                 vec![]
@@ -260,6 +279,7 @@ impl Tool for GestureTool {
                 self.began = false;
                 self.clear();
                 self.extend = None;
+                self.redraw = None;
                 out
             }
             _ => vec![],

docs/ui-parity.md

@@ -60,7 +60,7 @@ tools, never checked side by side) isn't captured row by row.
 | Curvature tool: click/drag points, double-click or Alt-click for a corner, temporary Direct Selection, point conversion | partial | circular point markers without direction handles or duplicate markers, Anchor Size and Hide Edges regression-tested (#1236, in part); curve geometry: each stretch of three points follows the circle through them (handles along its tangent, chord/(3·cos²(α/2)) long, shorter in a turn tighter than 30°), open ends curving in, the same rule for the rubber band, dragged, inserted and converted points (#1236, in part); neighbours of a point dragged on an existing path keep their tangents, and modifiers requested in #975 remain |
 | Blend spine Anchor Point conversion and handle editing (#2544) | implemented | symmetric handles, independent drag/retraction, visible-handle policy and selection/stacking covered by tool and engine regressions; Windows native canvas checks and screenshots reviewed in PR #2643 |
 | Add/Delete/Anchor Point tools; Anchor Point tool drags segments to reshape | done | #2267: a click on a handle end retracts that handle and leaves the anchor a corner with the other one |
-| Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | partial | Alt to Smooth is missing; with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679) |
+| Pencil: Fidelity, Keep selected, Alt to Smooth, edit selected paths, close by proximity | partial | Alt to Smooth is missing; Edit Selected Paths continues a selected path from an end and, from a stroke that starts and ends on a selected path, redraws the section between (the shorter way round a closed path, #2850); with Keep selected (Pencil and Paintbrush, on by default) off, the path a stroke draws or continues is not selected afterwards; a filled Pencil path keeps a stroke of None (#1804, part of #1679) |
 | Join tool scrubbing, Average, Join | done | Join merges the direct-selected anchors lying on one another along a path, as after Average, into one (#1533) |
 | Average and Join in one step, with a corner or smooth choice for the joined anchor | missing | `path.join` and `path.average` are separate commands, and Join has no corner/smooth choice; [gap 1](gaps.md) |
 | Smooth tool smooths only the stretch of path under the drag | done | #1512 |