diff --git a/crates/brush/src/lib.rs b/crates/brush/src/lib.rs index f25615c589..080462d0b6 100644 --- a/crates/brush/src/lib.rs +++ b/crates/brush/src/lib.rs @@ -762,9 +762,40 @@ pub fn library(doc: &Document) -> Vec { } /// Parse a stored library value (malformed entries are skipped, values kept within their -/// ranges; see [`Brush::from_json`]). +/// ranges; see [`Brush::from_json`]). A name that repeats is kept by its first brush, and each +/// later brush of that name is numbered ([`number_repeated_names`]). pub fn parse_library(v: &serde_json::Value) -> Vec { - v.as_array().map(|a| a.iter().filter_map(|b| Brush::from_json(b.clone()).ok()).collect()).unwrap_or_default() + let mut lib: Vec = v.as_array().map(|a| a.iter().filter_map(|b| Brush::from_json(b.clone()).ok()).collect()).unwrap_or_default(); + number_repeated_names(&mut lib); + lib +} + +/// Renames the second and later brushes of each repeated name in `lib` to the first "Name 2", +/// "Name 3"… that no brush in `lib` has. The first brush of a name keeps it, so a stroke that names +/// it draws the brush [`find`] gave it before. +fn number_repeated_names(lib: &mut [Brush]) { + let mut seen = std::collections::HashSet::new(); + if lib.iter().all(|b| seen.insert(b.name.as_str())) { + return; + } + let mut taken: std::collections::HashSet = lib.iter().map(|b| b.name.clone()).collect(); + let mut kept = std::collections::HashSet::new(); + // The next number to try for each repeated name, so the brushes of one name try each number + // once between them. + let mut next: std::collections::HashMap = std::collections::HashMap::new(); + for b in lib.iter_mut() { + if kept.insert(b.name.clone()) { + continue; + } + let from = next.entry(b.name.clone()).or_insert(2); + // `taken` is finite, so one of the numbers is free. + if let Some((i, name)) = (*from..).map(|i| (i, format!("{} {i}", b.name))).find(|(_, n)| !taken.contains(n)) { + *from = i.saturating_add(1); + taken.insert(name.clone()); + kept.insert(name.clone()); + b.name = name; + } + } } /// Store `lib` as the document's brush library. diff --git a/crates/brush/src/tests.rs b/crates/brush/src/tests.rs index affd2fa6b7..2d5156d162 100644 --- a/crates/brush/src/tests.rs +++ b/crates/brush/src/tests.rs @@ -945,6 +945,34 @@ fn library_defaults_store_and_find() { assert_eq!(unique_name(&lib, "Arrow"), "Arrow 2"); } +/// A stored library that repeats a name reads with the first brush of that name as stored and each +/// later one numbered, past every name the library already has; strokes of the name keep drawing +/// the first brush. +#[test] +fn repeated_brush_names_are_numbered_when_read() { + let mut d = Document::new(100.0, 100.0); + let named = |name: &str, size: f64| Brush { name: name.into(), ..calli(0.0, 100.0, size) }; + store(&mut d, &[named("Nib", 1.0), named("Nib 2", 2.0), named("Nib", 3.0), named("Other", 4.0), named("Nib", 5.0), named("Nib 2", 6.0)]); + let lib = library(&d); + let names: Vec<&str> = lib.iter().map(|b| b.name.as_str()).collect(); + assert_eq!(names, ["Nib", "Nib 2", "Nib 3", "Other", "Nib 4", "Nib 2 2"]); + let size = |b: &Brush| if let BrushKind::Calligraphic(c) = &b.kind { c.size } else { 0.0 }; + assert_eq!(lib.iter().map(size).collect::>(), [1.0, 2.0, 3.0, 4.0, 5.0, 6.0]); + assert_eq!(find(&d, "Nib").map(|b| size(&b)), Some(1.0)); + assert_eq!(find(&d, "Nib 3").map(|b| size(&b)), Some(3.0)); + // Reading again gives the same names, and the numbered library stored again reads as stored. + assert_eq!(library(&d), lib); + store(&mut d, &lib); + assert_eq!(library(&d), lib); + // Many brushes of one name take the numbers in order, past the numbered names the file has. + let many: Vec = (0..5000).map(|i| named(if i == 1 { "Nib 3" } else { "Nib" }, 1.0)).collect(); + store(&mut d, &many); + let names: Vec = library(&d).into_iter().map(|b| b.name).collect(); + assert_eq!(names[..5], ["Nib", "Nib 3", "Nib 2", "Nib 4", "Nib 5"]); + assert_eq!(names.last().map(String::as_str), Some("Nib 5000")); + assert_eq!(names.iter().collect::>().len(), names.len()); +} + #[test] fn brushes_serde_round_trip() { let lib = defaults().to_vec(); diff --git a/crates/engine/src/cmd/docinfo.rs b/crates/engine/src/cmd/docinfo.rs index a9fb07bdf8..f1bd5ceb2c 100644 --- a/crates/engine/src/cmd/docinfo.rs +++ b/crates/engine/src/cmd/docinfo.rs @@ -671,7 +671,7 @@ fn info(s: &mut Session, p: &Value) -> Result { fonts.extend(super::fonts::art_texts_of(d).into_iter().flat_map(|(_, t, _)| t.runs.iter().map(|r| super::fonts::font_of(&r.style)))); } let font_names: Vec = fonts.iter().map(super::fonts::font_label).collect(); - // Each brush the art uses with its type, found by name in the Brushes panel as the renderer finds it (the last of two + // Each brush the art uses with its type, found by name in the Brushes panel as the renderer finds it (the first of two // brushes with one name); a brush the panel does not hold draws as a plain stroke and has no type. A row names the type // as the New Brush dialog does (`Art Brush`), and the panel translates these values when it shows them. let kinds: HashMap<&str, &vectorcraft_brush::BrushKind> = library.iter().map(|b| (b.name.as_str(), &b.kind)).collect(); @@ -948,7 +948,7 @@ mod tests { let d = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); d.node_mut(NodeId(missing)).unwrap().appearance.stroke_mut().unwrap().paint = Paint::None; assert_eq!(s.execute("document.info", &json!({"category": "brushes"})).unwrap()["brushes"], json!([])); - // Two panel brushes with one name: the type is the one the renderer draws with, the last one's. + // Two panel brushes with one name: the type is the one the renderer draws with, the first one's. let twin = square(&mut s, 140.0); s.execute("brush.apply", &json!({"ids": [twin], "name": "Charcoal"})).unwrap(); let d = std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc); @@ -958,7 +958,7 @@ mod tests { lib.extend(kinds.into_iter().map(|kind| vectorcraft_brush::Brush { name: "Twin".into(), kind })); vectorcraft_brush::store(d, &lib); d.node_mut(NodeId(twin)).unwrap().appearance.stroke_mut().unwrap().brush = Some("Twin".into()); - assert_eq!(s.execute("document.info", &json!({"category": "brushes"})).unwrap()["brushes"], json!([{"name": "Twin", "type": "scatter"}])); + assert_eq!(s.execute("document.info", &json!({"category": "brushes"})).unwrap()["brushes"], json!([{"name": "Twin", "type": "art"}])); // Type: a brushed stroke on drawn characters is listed, one on spaces alone is not. let blank = s.execute("text.create", &json!({"x": 10, "y": 60, "text": " "})).unwrap()["id"].as_u64().unwrap(); let word = s.execute("text.create", &json!({"x": 10, "y": 90, "text": "Hi"})).unwrap()["id"].as_u64().unwrap(); diff --git a/crates/engine/src/tests/brushlib.rs b/crates/engine/src/tests/brushlib.rs index 2d363d18cf..98a577abed 100644 --- a/crates/engine/src/tests/brushlib.rs +++ b/crates/engine/src/tests/brushlib.rs @@ -721,3 +721,24 @@ fn other_documents_brushes_load_and_unreadable_files_are_errors() { let bytes = vectorcraft_format::base64_decode(r["dataBase64"].as_str().unwrap()).unwrap(); assert_eq!(vectorcraft_brush::library(&vectorcraft_format::load(&bytes).unwrap()).len(), 1); } + +/// A library file whose brushes repeat a name loads with the later brush numbered, and each one +/// can be added to a document by its own name. +#[test] +fn a_library_file_that_repeats_a_brush_name_loads_with_the_later_one_numbered() { + use vectorcraft_brush::{Brush, Calligraphic}; + let nib = |size: f64| Brush { name: "Nib".into(), kind: BrushKind::Calligraphic(Calligraphic { size, ..Default::default() }) }; + let mut file = vectorcraft_doc::Document::new(100.0, 100.0); + vectorcraft_brush::store(&mut file, &[nib(1.0), nib(2.0)]); + let b64 = vectorcraft_format::base64_encode(&vectorcraft_format::save_file(&file)); + let mut s = session(); + let r = run(&mut s, "brush.loadLibrary", json!({"dataBase64": b64, "name": "Nibs.vectorcraft"})); + let library = r["library"].as_str().unwrap().to_string(); + let listed = run(&mut s, "brush.library", json!({ "library": library })); + let names: Vec<&str> = listed["brushes"].as_array().unwrap().iter().map(|b| b["name"].as_str().unwrap()).collect(); + assert_eq!(names, ["Nib", "Nib 2"]); + let r = run(&mut s, "brush.addFromLibrary", json!({"library": library, "name": "Nib 2"})); + assert_eq!(strs(&r["added"]), ["Nib 2"]); + let size = |b: Brush| if let BrushKind::Calligraphic(c) = b.kind { c.size } else { 0.0 }; + assert_eq!(size(brush(&s, "Nib 2")), 2.0); +} diff --git a/crates/engine/src/tests/brushsym.rs b/crates/engine/src/tests/brushsym.rs index 1a1fcd87b5..e961a59e81 100644 --- a/crates/engine/src/tests/brushsym.rs +++ b/crates/engine/src/tests/brushsym.rs @@ -631,6 +631,37 @@ fn brush_options_rename_duplicate_and_delete_update_strokes() { assert!(s.execute("brush.delete", &json!({"name": "5 pt. Round"})).is_err()); } +/// A document whose brushes repeat a name opens with the later brush numbered: strokes of the name +/// keep the first brush, and renaming or deleting one brush leaves the other and its strokes alone. +#[test] +fn a_repeated_brush_name_is_numbered_when_the_document_opens() { + use vectorcraft_brush::{Brush, BrushKind, Calligraphic}; + let nib = |size: f64| Brush { name: "Nib".into(), kind: BrushKind::Calligraphic(Calligraphic { size, ..Default::default() }) }; + let mut s = session(); + vectorcraft_brush::store(std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc), &[nib(1.0), nib(2.0)]); + let a = rect(&mut s, 10.0, 10.0, 50.0, 50.0); + s.execute("brush.apply", &json!({"name": "Nib", "ids": [a.0]})).unwrap(); + let saved = s.execute("document.serialize", &json!({})).unwrap()["dataBase64"].clone(); + let mut o = Session::new(); + o.execute("document.open", &json!({"name": "a.vectorcraft", "dataBase64": saved})).unwrap(); + let names = |o: &mut Session| -> Vec { + let list = o.execute("brush.list", &json!({})).unwrap(); + list["brushes"].as_array().unwrap().iter().map(|b| b["name"].as_str().unwrap().to_string()).collect() + }; + assert_eq!(names(&mut o), ["Nib", "Nib 2"]); + assert_eq!(brush_of(&o, a).as_deref(), Some("Nib")); + assert_eq!(o.execute("brush.get", &json!({"name": "Nib"})).unwrap()["size"], json!(1.0)); + assert_eq!(o.execute("brush.get", &json!({"name": "Nib 2"})).unwrap()["size"], json!(2.0)); + let b = rect(&mut o, 80.0, 10.0, 50.0, 50.0); + o.execute("brush.apply", &json!({"name": "Nib 2", "ids": [b.0]})).unwrap(); + o.execute("brush.options", &json!({"name": "Nib", "newName": "Pen"})).unwrap(); + assert_eq!(names(&mut o), ["Pen", "Nib 2"]); + assert_eq!((brush_of(&o, a).as_deref(), brush_of(&o, b).as_deref()), (Some("Pen"), Some("Nib 2"))); + o.execute("brush.delete", &json!({"name": "Nib 2"})).unwrap(); + assert_eq!(names(&mut o), ["Pen"]); + assert_eq!((brush_of(&o, a).as_deref(), brush_of(&o, b)), (Some("Pen"), None)); +} + /// `brush.move`, which the Brushes panel runs when a brush is dragged, reorders brushes among those of /// their type in one undo step and refuses to move one into another type. #[test] diff --git a/crates/eps/src/ai_editing/tests.rs b/crates/eps/src/ai_editing/tests.rs index 1321188f12..d998ff8e3d 100644 --- a/crates/eps/src/ai_editing/tests.rs +++ b/crates/eps/src/ai_editing/tests.rs @@ -8406,8 +8406,9 @@ fn brushes_the_reader_cant_read_are_left_out() { } left_out(vec![Brush { name: "A/B".into(), ..keep.clone() }], "slash"); left_out(vec![Brush { kind: BrushKind::Bristle(Bristle::default()), ..keep.clone() }], "a second brush of the name"); - // A Pattern brush named "X" is left out, so a Calligraphic brush named "X" after it comes back. A - // path with "X" is drawn with the Pattern brush, so it comes back as its drawn look. + // A Pattern brush named "X" is left out, so a Calligraphic brush named "X" after it comes back, + // as "X 2", the name the library gives it when it is read. A path with "X" is drawn with the + // Pattern brush, so it comes back as its drawn look. let mut doc = Document::new(300.0, 200.0); let x = Brush { name: "X".into(), ..keep.clone() }; vectorcraft_brush::store(&mut doc, &[Brush { name: "X".into(), kind: BrushKind::Pattern(PatternBrush::default()) }, x.clone()]); @@ -8415,7 +8416,7 @@ fn brushes_the_reader_cant_read_are_left_out() { let out = export_ai_editing(&doc, &opts()).unwrap(); assert!(out.warnings.iter().any(|w| w == writer::BRUSHES), "{:?}", out.warnings); let back = read_back(&doc); - assert_eq!(vectorcraft_brush::library(&back), [x]); + assert_eq!(vectorcraft_brush::library(&back), [Brush { name: "X 2".into(), ..x }]); let n = &back.layers[0].children().unwrap()[0]; assert!(n.appearance.stroke().is_none_or(|s| s.brush.is_none()), "not with the other brush of the name: {n:#?}"); } diff --git a/crates/render/src/tests/memory.rs b/crates/render/src/tests/memory.rs index 011f9fd7fe..d286fa7e3c 100644 --- a/crates/render/src/tests/memory.rs +++ b/crates/render/src/tests/memory.rs @@ -931,6 +931,8 @@ fn brush_art_draws_edits_of_the_symbols_and_patterns_it_shows() { painted.transform(Affine::translate((0.0, 8.0)), true); let scatter = Node::group(d.alloc_id(), vec![Arc::new(instance), Arc::new(painted)]); let mut lib = vectorcraft_brush::library(&d); + // The default library has its own `Dots`, which a stroke would draw with first. + lib.retain(|b| b.name != "Dots"); lib.push(Brush { name: "Dots".into(), kind: BrushKind::Scatter(Scatter { art: scatter, spacing: (150.0, 150.0), ..Default::default() }) }); vectorcraft_brush::store(&mut d, &lib); let mut ap = vectorcraft_doc::Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 1.0); @@ -997,6 +999,8 @@ fn brush_art_draws_renamed_symbols_and_patterns_it_shows() { painted.transform(Affine::translate((0.0, 8.0)), true); let scatter = Node::group(d.alloc_id(), vec![Arc::new(instance), Arc::new(painted)]); let mut lib = vectorcraft_brush::library(&d); + // The default library has its own `Dots`, which a stroke would draw with first. + lib.retain(|b| b.name != "Dots"); lib.push(Brush { name: "Dots".into(), kind: BrushKind::Scatter(Scatter { art: scatter, spacing: (150.0, 150.0), ..Default::default() }) }); vectorcraft_brush::store(&mut d, &lib); let mut ap = vectorcraft_doc::Appearance::basic(Paint::None, Paint::solid(Color::BLACK), 1.0); diff --git a/docs/mcp.md b/docs/mcp.md index a3431f76dc..dd92e83577 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -3029,7 +3029,10 @@ 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?}`. `brush.new`, + `brush.duplicate {name, newName?} | {names}`, `brush.freehand`, `object.expandBrush {ids?}`. 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 "`` 2", "`` 3"… the file does not have; strokes of the name keep the first + brush. `brush.new`, `brush.duplicate` and `brush.options` trim the white space around the name they are given. A blank `name` gives the type's default name ("New Calligraphic Brush" for a calligraphic brush), and a blank `newName` gives "`` copy" in `brush.duplicate` and keeps the name in `brush.options`. A Hue Shift `colorization` given to diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index f6bb99ae84..90ad104550 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -719,6 +719,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - **New Document profiles:** each built-in category's presets start documents from that category's profile, a native document holding the starting swatches, graphic styles, brushes, symbols and other assets, the ruler and Document Setup options and the print settings ([Create a custom new document profile](https://helpx.adobe.com/illustrator/desktop/add-and-import-files/start-a-new-file/create-a-custom-new-document-profile.html)). New Document's Manage Profiles pop-up opens a profile to edit and resets one to the shipped profile (`file.newProfiles`, `file.newProfiles.edit`, `file.newProfiles.reset`; `file.new {profile}`). The profiles live in VectorCraft's own `New Document Profiles` folder next to the preferences; a profile that cannot be read or is larger than 64 MiB leaves the defaults and is reported. Tests: `tests::newdoc::an_edited_profile_starts_the_documents_of_its_category`, `the_profile_follows_the_preset_access_and_reset`, `profile_errors_name_the_file_by_its_name_outside_the_read_roots`, `a_damaged_or_oversized_profile_leaves_the_defaults_and_is_reported`, `profile_names_are_checked_and_editing_needs_the_folder`, `dialogs::new_document::tests::manage_profiles_edits_and_resets_the_profiles`. - **Control bar with the Artboard tool:** while the Artboard tool is in use, the Control bar shows the active artboard's controls in place of the object controls: Preset, Portrait and Landscape, New Artboard, Delete Artboard, the Name field, Artboard Options, Rearrange All Artboards, the Transform link, Move and Scale Artwork with Artboard and, while the bar has room, the reference point with X and Y, and W and H with the Constrain Width and Height Proportions link. Each runs the command its Properties or Transform panel counterpart runs (`artboard.setProps`, `artboard.new`, `artboard.delete`, `tool.options {tool: "artboard"}`), and the reference point is the one the Transform panel uses. In an 800-point window the controls up to the Transform link fit, Move and Scale Artwork with Artboard show as their boxes alone with their names in tooltips, and the link opens the Transform panel with the artboard's position and size. Pointer and key tests in `panels/artboards.rs` cover the controls, W about the reference point, the orientation and the name. Public reference: [Using multiple artboards](https://web.archive.org/web/20210925094818/https://helpx.adobe.com/illustrator/using/using-multiple-artboards.html) (archived copy). - **Names of new and renamed artboards, brushes and patterns:** `artboard.new` and `artboard.setProps` trim the white space around the name they are given, as the Artboards panel's rename does, and a blank name gives the artboard its default name, Artboard N for the Nth artboard. `brush.new`, `brush.duplicate`, `brush.options`, `object.pattern.make`, `object.pattern.saveCopy` and `pattern.options` trim the name as the Brush Options dialog and the Pattern Options panel do, and compare the trimmed name with the names there are; a blank name gives the name each command gives when there is none, and Brush Options and Pattern Options keep the name. Tests in `tests/panelcmds.rs`, `tests/brushsym.rs` and `tests/pattern.rs` cover each command. Public references: [Using multiple artboards](https://web.archive.org/web/20210925094818/https://helpx.adobe.com/illustrator/using/using-multiple-artboards.html), [Brushes](https://web.archive.org/web/20251018101754/https://helpx.adobe.com/illustrator/using/brushes.html) and [Create and edit patterns](https://web.archive.org/web/20211031180358/https://helpx.adobe.com/illustrator/using/create-edit-patterns.html) (archived copies). +- **Repeated brush names in a file:** a document or brush library file whose brushes repeat a name opens with the first brush of that name as stored and each later one numbered, as "Name 2", "Name 3" and so on, skipping names the file already has. Strokes that name the brush keep drawing the first one, and `brush.list`, `brush.options`, `brush.delete`, `brush.library` and the Brushes panel then reach each brush by its own name. This numbering is VectorCraft's own rule. Tests in `crates/brush/src/tests.rs`, `tests/brushsym.rs` and `tests/brushlib.rs` cover it. - **Pattern names and swatch names:** `pattern.options` refuses a new name that another pattern, a swatch or a color group has. It takes a name that only the pattern's own swatches have. The reference page does not say what happens when the new name is taken; the refusal follows VectorCraft's rule that swatches and color groups share one namespace (`Document::swatch_name_taken`), which `swatch.new` and `swatch.edit` also follow. A rename gives the new name only to the pattern's swatches that had the old one; any other swatch with the old name keeps it, and art linked to that swatch stays linked. `object.pattern.make` and `object.pattern.saveCopy` number a color group's name as they number a swatch's. Tests in `tests/pattern.rs` cover each command. Public reference: [Create and edit patterns](https://web.archive.org/web/20211031180358/https://helpx.adobe.com/illustrator/using/create-edit-patterns.html) (archived copy). - **Cancel, Delete and rename leave no paint of a missing pattern:** the help page lists Save a Copy, Done and Cancel for leaving pattern editing (https://helpx.adobe.com/illustrator/using/create-edit-patterns.html). Canceling a pattern made for the edit removes it with every swatch of it, a color group's included. Canceling a new pattern, `pattern.delete` and a `pattern.options` rename reach every paint of the pattern: art outside the editing layer, opacity mask art, type runs, symbol definitions, graph designs, the series and label paints a graph keeps for its next redraw, other patterns' tiles (a copy Save a Copy made included), graphic and character styles, brush art, and the fill, stroke and appearance new art takes. Canceling a new pattern and Delete make those paints no paint, a VectorCraft rule; a rename renames them. Canceling the edit of an existing pattern puts its definition back as it was when editing started; after a rename in Pattern Options, it also gives the pattern its old name back with its swatches and paints, unless another pattern, swatch or color group has taken that name. Undo and Redo restore new art's paints with the document. Tests in `tests/pattern.rs` cover each command. - **A clip group in Properties:** with one clip group selected, the panel has Edit Clipping Path and Edit Contents at its top, under the Clip Group heading, which select the clipping path or the clipped art for the selection tools to transform alone, as Object › Clipping Mask does (: with a clip group selected, the tutorial uses Edit Clipping Path at the top of the Properties panel to select the mask for editing, and uses Edit Contents to select the masked art). Edit Contents shows only when the group clips something. Not checked against the reference app: whether Properties keeps the buttons while the clipping path or the contents is selected; the Control bar has no clip-group buttons here. Evidence: `panels::properties::tests::one_clip_group_has_edit_clipping_path_and_edit_contents_at_the_top`.