Merge pull request #7592 from dotCooCoo/fix/brush-freehand-unknown-brush

Brushes: brush.freehand refuses a brush or style it cannot use

crates/engine/src/cmd/brushsym.rs

@@ -180,7 +180,7 @@ pub fn specs() -> Vec<CommandSpec> {
             "Paintbrush",
             [],
             None,
-            "{…params as path.freehand, brush} paint a freehand stroke with a brush → {id}",
+            "{…params as path.freehand, brush?: name} paint a freehand stroke with a brush; without `brush` (or with null) the plain path is drawn; a `brush` that names no brush in the document, or a `style` other than \"pencil\" or \"brush\", is an error and nothing is drawn → {id}",
             has_doc,
             brush_freehand
         ),

crates/engine/src/cmd/brushsym/brushes.rs

@@ -729,8 +729,21 @@ pub(super) fn expand_brush(s: &mut Session, p: &Value) -> Result<Value> {
     Ok(json!({ "ids": ids_json(&out) }))
 }
 
+/// Paints a `path.freehand` stroke with the brush `brush` names. A `brush` left out or null draws
+/// the plain path; a `brush` that is not a string or names no brush in the document, and a `style`
+/// other than "pencil" or "brush", is an error before anything is drawn.
 pub(super) fn brush_freehand(s: &mut Session, p: &Value) -> Result<Value> {
-    let name = str_param(p, "brush").map(str::to_string).filter(|n| s.doc().is_ok_and(|st| brush::find(&st.doc, n).is_some()));
+    const C: &str = "brush.freehand";
+    let name = match crate::cmd::text_param(p, "brush", C)? {
+        None => None,
+        Some(n) if brush::find(&s.doc()?.doc, n).is_some() => Some(n.to_string()),
+        Some(n) => return Err(bad(C, format!("no brush named `{n}`"))),
+    };
+    match p.get("style") {
+        None | Some(Value::Null) => {}
+        Some(v) if matches!(v.as_str(), Some("pencil" | "brush")) => {}
+        Some(v) => return Err(bad(C, format!("`style` must be \"pencil\" or \"brush\", not {v}"))),
+    }
     let spec = find_command("path.freehand").ok_or_else(|| EngineError::UnknownCommand("path.freehand".into()))?;
     let r = crate::cmd::run_checked(s, spec, p)?;
     let (Some(name), Some(id)) = (name, id_param(&r, "id")) else { return Ok(r) };

crates/engine/src/tests/brushsym.rs

@@ -963,6 +963,69 @@ fn brush_freehand_paints_with_a_brush_in_one_step() {
     assert_eq!(brush_of(&s, id).as_deref(), Some("Charcoal"));
 }
 
+/// `brush.freehand` refuses a `brush` that is not a string or names no brush in the document,
+/// and a `style` other than "pencil" or "brush", before it draws: the document, the selection
+/// and the undo history stay as they were. A `brush` left out or null draws the plain path.
+#[test]
+fn brush_freehand_refuses_a_brush_or_style_it_cannot_use() {
+    let mut s = session();
+    let a = rect(&mut s, 0.0, 0.0, 10.0, 10.0);
+    select(&mut s, &[a]);
+    let points = json!([[10, 10], [60, 40], [120, 10]]);
+    for (extra, words) in [
+        (json!({"brush": "Nope"}), "no brush named `Nope`"),
+        (json!({"brush": ""}), "no brush named ``"),
+        (json!({"brush": 5}), "`brush` must be a string"),
+        (json!({"brush": {"name": "Charcoal"}}), "`brush` must be a string"),
+        (json!({"brush": true}), "`brush` must be a string"),
+        (json!({"brush": "Charcoal", "style": "marker"}), "`style` must be \"pencil\" or \"brush\", not \"marker\""),
+        (json!({"brush": "Charcoal", "style": 1}), "`style` must be \"pencil\" or \"brush\", not 1"),
+    ] {
+        let mut p = extra.clone();
+        p["points"] = points.clone();
+        let (before, undo) = (s.doc().unwrap().doc.clone(), s.doc().unwrap().history.undo.len());
+        let e = s.execute("brush.freehand", &p).expect_err(&format!("{extra} is refused"));
+        assert!(e.to_string().contains(words), "{extra}: {e}");
+        assert!(*s.doc().unwrap().doc == *before, "{extra} leaves the document");
+        assert_eq!(s.doc().unwrap().selection.objects, [a], "{extra} leaves the selection");
+        assert_eq!(s.doc().unwrap().history.undo.len(), undo, "{extra} records no undo step");
+    }
+    for extra in [json!({}), json!({"brush": null, "style": null})] {
+        let mut p = extra.clone();
+        p["points"] = points.clone();
+        let id = NodeId(s.execute("brush.freehand", &p).unwrap()["id"].as_u64().unwrap());
+        assert_eq!(brush_of(&s, id), None, "{extra} draws the plain path");
+    }
+    for style in ["pencil", "brush"] {
+        let r = s.execute("brush.freehand", &json!({"points": points, "style": style, "brush": "Charcoal"})).unwrap();
+        assert!(r["id"].is_u64(), "{style}: {r}");
+    }
+}
+
+/// The Paintbrush skips a brush deleted after it was chosen: a deleted tool brush gives way to
+/// the document's current brush, and with neither the stroke is drawn without a brush.
+#[test]
+fn paintbrush_tool_skips_a_deleted_brush() {
+    let mut s = session();
+    s.execute("brush.setCurrent", &json!({"name": "Arrow"})).unwrap();
+    let v = ViewInfo::default();
+    s.select_tool("paintbrush", v).unwrap();
+    s.set_tool_option("brush", &json!("Dots"));
+    s.execute("brush.delete", &json!({"name": "Dots"})).unwrap();
+    let stroke = |s: &mut Session, y: f64| {
+        for (k, x) in [(PointerKind::Down, 10.0), (PointerKind::Drag, 50.0), (PointerKind::Drag, 90.0), (PointerKind::Up, 130.0)] {
+            s.pointer(&PointerEvent::new(k, x, y), v).unwrap();
+        }
+        s.doc().unwrap().selection.objects[0]
+    };
+    let id = stroke(&mut s, 10.0);
+    assert_eq!(brush_of(&s, id).as_deref(), Some("Arrow"));
+    s.execute("brush.delete", &json!({"name": "Arrow"})).unwrap();
+    assert_eq!(s.doc().unwrap().doc.unknown.get("currentBrush"), None);
+    let id = stroke(&mut s, 110.0);
+    assert_eq!(brush_of(&s, id), None);
+}
+
 #[test]
 fn paintbrush_tool_uses_the_current_brush() {
     let mut s = session();

crates/tools/src/symbolism.rs

@@ -238,10 +238,15 @@ impl Default for BrushPaintTool {
 }
 
 impl BrushPaintTool {
+    /// The tool's brush, else the document's current brush, skipping a name the document's brush
+    /// library no longer has (a deleted brush), so the stroke is drawn without it.
     fn brush_name(&self, cx: &ToolContext) -> Option<String> {
-        self.brush.clone().or_else(|| cx.doc.unknown.get("currentBrush").and_then(Value::as_str).map(str::to_string))
+        [self.brush.clone(), vectorcraft_brush::current(cx.doc)].into_iter().flatten().find(|n| vectorcraft_brush::find(cx.doc, n).is_some())
     }
     fn map(&self, cx: &ToolContext, acts: Vec<Action>) -> Vec<Action> {
+        if !acts.iter().any(|a| matches!(a, Action::Preview(c, _) if c == "path.freehand")) {
+            return acts;
+        }
         let Some(b) = self.brush_name(cx) else { return acts };
         acts.into_iter()
             .map(|a| match a {
@@ -403,4 +408,23 @@ mod tests {
         t.set_option("brush", &json!("Dots"));
         assert_eq!(t.options()["brush"], "Dots");
     }
+
+    #[test]
+    fn paintbrush_skips_a_brush_the_document_does_not_have() {
+        let (mut d, _) = doc_with_rect();
+        let (s, p) = (Selection::default(), paint());
+        let drag = |d: &vectorcraft_doc::Document, t: &mut BrushPaintTool| {
+            let cx = cx(d, &s, &p);
+            t.pointer(&cx, &PointerEvent::new(PointerKind::Down, 0.0, 0.0));
+            t.pointer(&cx, &PointerEvent::new(PointerKind::Drag, 30.0, 0.0)).swap_remove(1)
+        };
+        d.unknown.insert("currentBrush".into(), json!("Charcoal"));
+        let mut t = BrushPaintTool::default();
+        t.set_option("brush", &json!("Deleted"));
+        let a = drag(&d, &mut t);
+        assert!(matches!(&a, Action::Preview(c, v) if c == "brush.freehand" && v["brush"] == "Charcoal"), "the current brush: {a:?}");
+        d.unknown.insert("currentBrush".into(), json!("Also deleted"));
+        let a = drag(&d, &mut BrushPaintTool::default());
+        assert!(matches!(&a, Action::Preview(c, v) if c == "path.freehand" && v.get("brush").is_none()), "no brush: {a:?}");
+    }
 }

docs/mcp.md

@@ -3151,7 +3151,12 @@ already walks Image Trace. Short agent entry points:
   is unused, and offers Remove Strokes or Cancel when one of them is in use.
 - Brushes: `brush.list`, `brush.get {name}`, `brush.apply {name, ids?}`, `brush.new {type, name?, params?, ids?}`,
   `brush.setCurrent {name}`, `brush.options {name, params?, newName?}`, `brush.delete {name} | {names}`,
-  `brush.duplicate {name, newName?} | {names}`, `brush.freehand`, `object.expandBrush {ids?}`. A document or brush
+  `brush.duplicate {name, newName?} | {names}`, `brush.freehand {…path.freehand params, brush?}`,
+  `object.expandBrush {ids?}`. `brush.freehand` draws the plain path when `brush` is left out or null. It refuses a
+  `brush` that is not a string with "`brush` must be a string", a name that no brush in the document has (an empty
+  name included) with "no brush named `<name>`", and a `style` other than "pencil" or "brush"; the document does not
+  change and no undo step is recorded. The Paintbrush skips a tool brush or current brush that has been deleted and
+  paints with the other, or draws the plain path when neither is in the document. A document or brush
   library file whose brushes repeat a name is read with the first brush of that name as stored and each later one
   numbered with the first "`<name>` 2", "`<name>` 3"… the file does not have; strokes of the name keep the first
   brush. `brush.new`,

docs/target-app-parity.md

@@ -1174,6 +1174,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur
     In the Pencil and Paintbrush Tool Options, Edit selected paths within 0 px turns editing off, so a stroke that starts exactly on an end of a selected open path draws a new path; Close paths when ends are within 0 px turns closing off, so a stroke that ends exactly where it started stays open.
     The Pencil's and the Paintbrush's Fidelity is in screen pixels, the same at any zoom: their Tool Options give it in px, from 0.01 px on the logarithmic slider to 20 px and up to 100 px in the field, and the tool sends `path.freehand` the value in points at the current zoom (#5729). The Smooth tool's and the Blob Brush's Fidelity stays in points.
     `path.freehand` and `brush.freehand` take a Fidelity of up to 100 / 0.0313 pt (about 3194.89 pt), the 100 px field maximum at the lowest zoom (3.13 %), and reject a value outside their range with an error, so a Pencil or Paintbrush stroke drawn zoomed out keeps the Fidelity set in pixels (engine tests `pencil_fit_keeps_its_anchors_on_a_stroke_drawn_zoomed_out` and `pencil_takes_fidelity_within_its_range_and_rejects_the_rest`).
+    `brush.freehand` refuses a `brush` that names no brush in the document, or is not a string, and a `style` other than "pencil" or "brush", with an error and no change or undo step, instead of drawing the plain path; the Paintbrush skips a chosen brush that has since been deleted (engine tests `brush_freehand_refuses_a_brush_or_style_it_cannot_use` and `paintbrush_tool_skips_a_deleted_brush`).
     Each drag of the Smooth tool smooths the stretch of a selected path in its reach along its length before refitting it within Fidelity, so a wobble wider than Fidelity is smoothed out and another drag over the same place smooths it further, as [the help page](https://helpx.adobe.com/illustrator/using/tool-techniques/smooth-tool.html) describes (`path.smoothRegion`). The smoothing spread (a third of the reach) and the rule that each refit stays within twice the distance the smoothing moved the path, so repeated drags do not drift from it, are VectorCraft's own. A drag over the whole of a closed path smooths it across its first anchor too, so a bump there is smoothed like any other and no corner is left; Object › Path › Smooth smooths a closed path across its first anchor the same way.
     The Smooth tool shows the Not Allowed cursor while no path is selected that it can act on (an editable path, not a guide, with three or more anchors on one of its subpaths or two anchors joined by a curve), and the release of a drag then sends no command and changes nothing; [the help page](https://helpx.adobe.com/illustrator/using/tool-techniques/smooth-tool.html) has the path selected first. A selected path anywhere on the canvas gives the Smooth cursor wherever the pointer is. The Pencil with Alt key toggles to Smooth Tool on and the Blob Brush show Not Allowed the same way when Alt gives them the Smooth tool; the Paintbrush keeps its closing cursor, since its release with Alt closes the stroke. Use Precise Cursors leaves Not Allowed as it is. This cursor rule is VectorCraft's own (tools `smooth_shows_not_allowed_without_a_selected_path_to_smooth`, `borrowed_smooth_shows_not_allowed_without_a_selected_path_to_smooth`, `smoothing_release_sends_nothing_without_a_selected_path_to_smooth`; engine `use_precise_cursors_keeps_the_smooth_tools_not_allowed_pointer`).
     When a Smooth tool drag reaches the first or last anchor of an open path, that anchor stays where it is and the curve there leaves or arrives along the smoothed path within the reach, so a jagged first or last segment no longer sets the end's direction; an end where the smoothed path turns back by 90° or more keeps its segment's direction, and where the refitted stretch ends inside the path the curve keeps the direction of the part outside the drag. This rule is VectorCraft's own (`tests/draw2.rs`, `smoothing_an_open_path_turns_its_jagged_ends_with_the_path` and `a_smooth_drag_turns_only_the_end_it_reaches`).