Effects: a pasted SVG filter avoids brush effect names and is refused with no free name

Follows #6592. Paste now also skips the names that effects in the
document's brush art use when it picks a free name for a pasted SVG
filter. When no free name keeps the filter within the 64 KB limit (a
one-letter name at the limit with every free letter and digit taken),
the paste is refused instead of keeping the name of the document's
filter. Clipboard::import_into and import_text_styles return a Result,
and their callers pass the error on before the document changes.

crates/engine/src/cmd/brushlib.rs

@@ -496,7 +496,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     // nothing to apply is no edit, and a click that only applies brushes the document has is
     // named for that.
     let mut probe = (*st.doc).clone();
-    let imported = clip.import_into(&mut probe, &choices, false);
+    let imported = clip.import_into(&mut probe, &choices, false)?;
     let doc_names: Vec<String> = picked.iter().map(|n| target(&imported.renamed, n)).collect();
     let paths = if apply { brush_targets(&st.doc, &roots(s, p)?) } else { vec![] };
     let paint = stroke_paint(s);

crates/engine/src/cmd/clipboard/resources.rs

@@ -28,7 +28,7 @@ use vectorcraft_doc::{
 use super::super::brushsym::{set_natural_size, stored_natural_size};
 
 use super::super::patterncmds::add_pattern;
-use super::super::{Result, bad, unique_name};
+use super::super::{EngineError, Result, bad, unique_name};
 use super::Clipboard;
 
 /// A resource kind objects refer to (graphic styles by id, the others by name).
@@ -1055,8 +1055,9 @@ impl Clipboard {
     }
 
     /// Add the resources the objects use to `d` (inside the paste's undo step) and work out how
-    /// pasted copies relink to them.
-    pub(crate) fn import_into(&self, d: &mut Document, choices: &SwatchChoices, same_doc: bool) -> Imported {
+    /// pasted copies relink to them. An error, with `d` unchanged, when a pasted SVG filter finds no
+    /// free name ([`Self::filter_links`]).
+    pub(crate) fn import_into(&self, d: &mut Document, choices: &SwatchChoices, same_doc: bool) -> Result<Imported> {
         self.import_used(d, &self.used(), choices, same_doc, false)
     }
 
@@ -1065,9 +1066,9 @@ impl Clipboard {
     /// each style, symbol, pattern and brush comes after the styles, symbols, patterns and brushes it uses
     /// ([`use_order`]), so that it is compared and relinked with their names here, and each loop
     /// of definitions that use each other is matched on its own ([`loop_groups`]).
-    fn import_used(&self, d: &mut Document, used: &Used, choices: &SwatchChoices, same_doc: bool, in_use_order: bool) -> Imported {
+    fn import_used(&self, d: &mut Document, used: &Used, choices: &SwatchChoices, same_doc: bool, in_use_order: bool) -> Result<Imported> {
         let mut out = Imported::default();
-        out.relink.filters = self.filter_links(d, same_doc);
+        out.relink.filters = self.filter_links(d, same_doc)?;
         self.import_images(d, used, same_doc, &mut out);
         self.import_swatches(d, used, choices, same_doc, &mut out);
         // Brushes whose art shows no symbols or patterns, before the symbols and patterns whose art
@@ -1230,7 +1231,7 @@ impl Clipboard {
             && out.relink.styles.is_empty()
             && out.relink.colors.is_empty()
             && used.styles.is_empty());
-        out
+        Ok(out)
     }
 
     /// How the pasted SVG filter effects relink ([`Relink::filters`]). Each names the document's
@@ -1238,8 +1239,8 @@ impl Clipboard {
     /// filter when pasted back into the document it came from, and elsewhere keeps its own under a
     /// name no filter or effect of the document gives, so editing or deleting the document's filter leaves it alone. When that name would
     /// make the filter too long to draw, the free name is a shorter start of its name ("Shif"), else
-    /// a free letter or digit; the filter's id is always its name.
-    fn filter_links(&self, d: &Document, same_doc: bool) -> BTreeMap<(String, String), (String, String)> {
+    /// a free letter or digit; the filter's id is always its name. An error when no free name fits.
+    fn filter_links(&self, d: &Document, same_doc: bool) -> Result<BTreeMap<(String, String), (String, String)>> {
         let mut found = BTreeSet::new();
         for n in &self.nodes {
             node_filters(n, &mut found);
@@ -1272,6 +1273,9 @@ impl Clipboard {
                     for root in d.art_roots() {
                         node_filter_names(root, &mut taken);
                     }
+                    for a in vectorcraft_brush::library(d).iter().flat_map(Brush::art) {
+                        node_filter_names(a, &mut taken);
+                    }
                     for g in &d.graphic_styles {
                         appearance_filter_names(&g.appearance, &mut taken);
                     }
@@ -1288,13 +1292,17 @@ impl Clipboard {
                         .filter(|x| vectorcraft_render::effects::svg_filter::check(x).is_ok_and(|id| id == new))?;
                     Some((new, drawn))
                 });
-                let Some((new, drawn)) = renamed else { continue };
+                let Some((new, drawn)) = renamed else {
+                    return Err(EngineError::Other(format!(
+                        "the pasted SVG filter {name:?} draws differently from the document's filter of that name, and no free name fits it"
+                    )));
+                };
                 taken.insert(new.clone());
                 (new, drawn)
             };
             links.insert((name, xml), to);
         }
-        links
+        Ok(links)
     }
 
     /// Image blobs: a different blob under a key the document uses gets a new key.
@@ -1409,13 +1417,13 @@ pub(crate) struct LoadedStyle {
 /// a pattern added under a free name and a pattern follows a style renamed on the way in. A style
 /// `d` has under its name, or a numbered copy of it ("Body 2"), that is the same once relinked is
 /// used; any other is added under a free name. → each style of `src` placed, in order, character
-/// styles first.
-pub(crate) fn import_text_styles(src: &Document, d: &mut Document, chars: bool, paras: bool) -> Vec<LoadedStyle> {
+/// styles first. An error, as [`Clipboard::import_into`] gives.
+pub(crate) fn import_text_styles(src: &Document, d: &mut Document, chars: bool, paras: bool) -> Result<Vec<LoadedStyle>> {
     let library = OnceCell::new();
     let used = Used::of_text_styles(&Defs::of(src, &library), chars, paras);
     let clip = Clipboard::with_used(src, &used, &library, vec![]);
     let before = [&d.char_styles, &d.para_styles].map(|l| l.iter().map(|s| s.name.clone()).collect::<BTreeSet<_>>());
-    let out = clip.import_used(d, &used, &SwatchChoices::default(), false, true);
+    let out = clip.import_used(d, &used, &SwatchChoices::default(), false, true)?;
     let mut placed: Vec<LoadedStyle> = vec![];
     for (para, r, list) in [(false, Res::CharStyle, &clip.char_styles), (true, Res::ParaStyle, &clip.para_styles)] {
         for s in list {
@@ -1425,7 +1433,7 @@ pub(crate) fn import_text_styles(src: &Document, d: &mut Document, chars: bool,
             placed.push(LoadedStyle { para, name, added });
         }
     }
-    placed
+    Ok(placed)
 }
 
 /// Swatch `w` in `d` ([`existing`]), else added under a free name (swatches and colour groups

crates/engine/src/cmd/edit.rs

@@ -1332,7 +1332,7 @@ fn paste_clip(s: &mut Session, p: &Value, mode: PasteMode, clip: &Clipboard, cho
         vec![Affine::IDENTITY]
     };
     let (ids, imported, artboards, into) = s.edit(mode.label(), |d, sel| {
-        let imported = clip.import_into(d, choices, same_doc);
+        let imported = clip.import_into(d, choices, same_doc)?;
         // Draw Inside: the clip group of the object drawn into, made on the first paste.
         let into = inside.map(|t| super::create::draw_inside_group(d, t)).transpose()?;
         let artboards: Vec<usize> = boards

crates/engine/src/cmd/library.rs

@@ -1303,7 +1303,7 @@ fn use_pattern(s: &mut Session, data: &Value, stroke: bool) -> Result<String> {
     let choices = SwatchChoices::default();
     // The pattern's name in the document, worked out on a copy: nothing to add is no edit.
     let mut probe = (*s.doc()?.doc).clone();
-    let imported = clip.import_into(&mut probe, &choices, false);
+    let imported = clip.import_into(&mut probe, &choices, false)?;
     let renamed = imported.renamed.iter().find(|r| r["kind"] == "pattern" && r["from"] == name.as_str()).and_then(|r| r["to"].as_str());
     let target = renamed.unwrap_or(&name).to_string();
     let paint = vectorcraft_doc::pattern::pattern_paint(&target);
@@ -1315,7 +1315,7 @@ fn use_pattern(s: &mut Session, data: &Value, stroke: bool) -> Result<String> {
     in_one_step(s, |s| {
         if added {
             s.edit("Add to Swatches", |d, _| {
-                clip.import_into(d, &choices, false);
+                clip.import_into(d, &choices, false)?;
                 if !has_swatch(d) {
                     let swatch = d.free_swatch_name(&target);
                     d.swatches.push(vectorcraft_color::Swatch { name: swatch, paint: paint.clone(), global: false, spot: false });

crates/engine/src/cmd/patternlib.rs

@@ -10,6 +10,7 @@ use vectorcraft_doc::pattern::pattern_paint;
 use vectorcraft_doc::{Appearance, Document, Node, NodeId, PatternDef, TileType};
 use vectorcraft_geom::{PathData, Point, Rect, SubPath, shapes};
 
+use super::Result;
 use super::clipboard::Clipboard;
 use super::clipboard::resources::SwatchChoices;
 use super::swatchlib::LibraryInfo;
@@ -84,11 +85,11 @@ pub fn swatch_source(w: &Swatch) -> Option<Arc<Document>> {
 /// Add pattern `name` of library `lib` to `d` with the resources its art uses, as pasting art
 /// painted with it does: the same pattern there already is used, and a name another pattern or
 /// swatch has gets a number → the pattern's name in `d`.
-pub(crate) fn import(d: &mut Document, lib: &Document, name: &str) -> String {
+pub(crate) fn import(d: &mut Document, lib: &Document, name: &str) -> Result<String> {
     let user = Node::path(NodeId(0), shapes::rectangle(Rect::new(0.0, 0.0, 1.0, 1.0)), Appearance::basic(pattern_paint(name), Paint::None, 0.0));
-    let imported = Clipboard::gather(lib, vec![user]).import_into(d, &SwatchChoices::default(), false);
+    let imported = Clipboard::gather(lib, vec![user]).import_into(d, &SwatchChoices::default(), false)?;
     let renamed = imported.renamed.iter().find(|r| r["kind"] == "pattern" && r["from"] == name).and_then(|r| r["to"].as_str());
-    renamed.unwrap_or(name).to_string()
+    Ok(renamed.unwrap_or(name).to_string())
 }
 
 /// The pattern of `d` that is pattern `name` of library `lib`: the same tile options and art under

crates/engine/src/cmd/swatchlib.rs

@@ -753,7 +753,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
                 if let (Paint::Pattern { pattern, .. }, Some(src)) = (&w.paint, &patterns) {
                     // The pattern comes with its swatch; a pattern the document has without one
                     // gets one.
-                    let target = patternlib::import(d, src, pattern);
+                    let target = patternlib::import(d, src, pattern)?;
                     let name = match patternlib::pattern_swatch(d, &target) {
                         Some(name) => name,
                         None => {

crates/engine/src/cmd/symbollib.rs

@@ -405,7 +405,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     };
     // Where the symbols land, worked out on a copy: nothing to add and nothing to place is no edit.
     let mut probe = (*st.doc).clone();
-    let imported = clip.import_into(&mut probe, &choices, false);
+    let imported = clip.import_into(&mut probe, &choices, false)?;
     let doc_names: Vec<String> = picked.iter().map(|n| target(&imported.renamed, n)).collect();
     let centre = st.doc.artboards.first().map(|a| a.rect.center()).unwrap_or(Point::ZERO);
     let at = if place { Some(Point::new(number_or(p, "x", centre.x, C)?, number_or(p, "y", centre.y, C)?)) } else { None };
@@ -413,7 +413,7 @@ fn add(s: &mut Session, p: &Value) -> Result<Value> {
     let mut out = json!({ "library": info.id });
     if imported.added > 0 || place {
         let placed = s.edit("Add to Symbols", |d, sel| {
-            let imported = clip.import_into(d, &choices, false);
+            let imported = clip.import_into(d, &choices, false)?;
             // Every symbol brought in (nested ones too) takes its Symbol Options along.
             for sym in &clip.symbols {
                 let to = target(&imported.renamed, &sym.name);

crates/engine/src/cmd/textstyles.rs

@@ -976,12 +976,12 @@ fn load(s: &mut Session, p: &Value, kinds: &[Kind]) -> Result<Value> {
     }
     let (chars, paras) = (kinds.contains(&Kind::Char), kinds.contains(&Kind::Para));
     let mut trial = (*s.doc()?.doc).clone();
-    let mut placed = import_text_styles(&src, &mut trial, chars, paras);
+    let mut placed = import_text_styles(&src, &mut trial, chars, paras)?;
     // An undo step only when the document changes. The step takes the document the import made,
     // unless the edit first ends a typing session, which can change the document it starts from.
     if trial != *s.doc()?.doc {
         if super::typing_in_progress(s) {
-            placed = s.edit(label, |d, _| Ok(import_text_styles(&src, d, chars, paras)))?;
+            placed = s.edit(label, |d, _| import_text_styles(&src, d, chars, paras))?;
         } else {
             s.edit(label, |d, _| {
                 *d = trial;

crates/engine/src/cmd/textstyles/tests.rs

@@ -1105,7 +1105,7 @@ fn self_referencing_pattern_is_not_added_again() {
     let different = dst.execute("shape.rectangle", &json!({"x": 200, "y": 200, "width": 30, "height": 30})).unwrap()["id"].clone();
     dst.execute("object.pattern.make", &json!({"name": "Dots", "edit": false, "ids": [different]})).unwrap();
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     assert!(dst.doc().unwrap().doc.pattern("Dots 2").is_some());
     assert!(
         dst.doc().unwrap().doc.pattern("Dots 2").unwrap().art[0]
@@ -1116,7 +1116,7 @@ fn self_referencing_pattern_is_not_added_again() {
     );
     let before = dst.doc().unwrap().doc.clone();
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     assert!(again.iter().all(|s| !s.added));
     assert_eq!(dst.doc().unwrap().doc, before);
 }
@@ -1148,13 +1148,13 @@ fn separate_self_referencing_patterns_are_not_added_again() {
         dst.execute("object.pattern.make", &json!({"name": format!("Dots{k}"), "edit": false, "ids": [different]})).unwrap();
     }
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     for k in 0..9 {
         assert!(dst.doc().unwrap().doc.pattern(&format!("Dots{k} 2")).is_some(), "{k}");
     }
     let before = dst.doc().unwrap().doc.clone();
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     assert!(again.iter().all(|s| !s.added));
     assert_eq!(dst.doc().unwrap().doc.patterns.len(), before.patterns.len());
     assert_eq!(dst.doc().unwrap().doc, before);
@@ -1162,7 +1162,7 @@ fn separate_self_referencing_patterns_are_not_added_again() {
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
     target.patterns.retain(|p| !p.name.starts_with("Dots8"));
     let left = target.patterns.len();
-    let third = super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    let third = super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     assert_eq!(dst.doc().unwrap().doc.patterns.len(), left + 1);
     assert!(third.iter().filter(|s| s.name != "Patterned8" && !s.name.starts_with("Patterned8 ")).all(|s| !s.added));
 }
@@ -1195,13 +1195,13 @@ fn a_long_loop_of_patterns_is_not_added_again() {
         dst.execute("object.pattern.make", &json!({"name": format!("Dots{k}"), "edit": false, "ids": [different]})).unwrap();
     }
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     for k in 0..9 {
         assert!(dst.doc().unwrap().doc.pattern(&format!("Dots{k} 2")).is_some(), "{k}");
     }
     let before = dst.doc().unwrap().doc.clone();
     let target = std::sync::Arc::make_mut(&mut dst.doc_mut().unwrap().doc);
-    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true);
+    let again = super::super::clipboard::resources::import_text_styles(&source, target, true, true).unwrap();
     assert!(again.iter().all(|s| !s.added));
     assert_eq!(dst.doc().unwrap().doc.patterns.len(), before.patterns.len());
     assert_eq!(dst.doc().unwrap().doc, before);

crates/engine/src/tests/svgfilter.rs

@@ -303,6 +303,48 @@ fn pasted_art_takes_no_name_the_documents_effects_use() {
     assert_eq!(names, ["Shift 2", "Shift 3"]);
 }
 
+/// A pasted filter is not renamed to a name an effect in a brush's art gives either.
+#[test]
+fn pasted_art_takes_no_name_the_documents_brushes_use() {
+    let mut s = session();
+    let a = rect(&mut s, 10.0);
+    run(&mut s, "svgFilter.define", json!({"xml": r#"<filter id="Shift 2"><feOffset dx="1" dy="1"/></filter>"#}));
+    run(&mut s, "svgFilter.apply", json!({"name": "Shift 2", "ids": [a.0]}));
+    run(&mut s, "select.set", json!({"ids": [a.0]}));
+    run(&mut s, "brush.new", json!({"type": "art", "name": "Fx"}));
+    run(&mut s, "edit.clear", json!({"ids": [a.0]}));
+    run(&mut s, "svgFilter.delete", json!({"name": "Shift 2"}));
+    run(&mut s, "svgFilter.define", json!({"xml": SHIFT}));
+    let svg = format!(
+        r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><defs>{SHIFT_MORE}</defs><rect width="30" height="30" filter="url(#Shift)"/></svg>"#
+    );
+    run(&mut s, "clipboard.importSvg", json!({"svg": svg}));
+    run(&mut s, "edit.pasteInPlace", json!({}));
+    let names: Vec<String> = filter_effects(&s).into_iter().map(|(n, _)| n).collect();
+    assert_eq!(names, ["Shift 3"]);
+}
+
+/// A pasted filter at the size limit with a one-letter name, when every free name it could take is
+/// in use, is refused rather than pasted under the name of the document's filter.
+#[test]
+fn a_paste_with_no_free_name_for_its_filter_is_refused() {
+    let mut s = session();
+    for n in ('a'..='z').chain('0'..='9') {
+        run(&mut s, "svgFilter.define", json!({"xml": format!(r#"<filter id="{n}"><feOffset dx="1" dy="1"/></filter>"#)}));
+    }
+    run(&mut s, "svgFilter.define", json!({"xml": r#"<filter id="S"><feOffset dx="3" dy="3"/></filter>"#}));
+    let (open, close) = (r#"<filter id="S"><feOffset dx="9" dy="9" result=""#, r#""/></filter>"#);
+    let big = format!("{open}{}{close}", "a".repeat(vectorcraft_doc::SvgFilter::MAX_XML - open.len() - close.len()));
+    let svg = format!(
+        r#"<svg xmlns="http://www.w3.org/2000/svg" width="100" height="100"><defs>{big}</defs><rect width="30" height="30" filter="url(#S)"/></svg>"#
+    );
+    run(&mut s, "clipboard.importSvg", json!({"svg": svg}));
+    let before = steps(&s);
+    assert!(err(&mut s, "edit.pasteInPlace", json!({})).contains("no free name"));
+    assert!(filter_effects(&s).is_empty(), "nothing pasted");
+    assert_eq!(steps(&s), before);
+}
+
 /// A pasted filter whose name holds a tab, a line feed or a carriage return keeps those characters
 /// in its id under its new name.
 #[test]

docs/mcp.md

@@ -3764,7 +3764,8 @@ what every object that uses it draws, in the edit's one undo step, and an applic
 filter. Deleting a filter that objects use leaves their effects in place, still named, drawing nothing (their `xml`
 empty) until a filter of that name is defined or imported again. A pasted effect that names one of the document's
 filters but draws another definition keeps its own filter under a free name (`Soft 2`); pasted back into the document
-it was copied from, it takes the document's filter. Opened from such an effect, Apply SVG Filter shows the
+it was copied from, it takes the document's filter. The free name is one that no filter of the document and no effect in
+its art, brushes or graphic styles uses; when no such name keeps the filter within 64 KB, the paste is refused. Opened from such an effect, Apply SVG Filter shows the
 name above the list, not selectable. `effect.apply {effect: "svg.filter", params: {xml}}` takes a filter
 directly and names it by its `id`; an `xml` that isn't one such filter is an error. `ui.svgFilterDialog {}` opens Apply
 SVG Filter (New, Edit, Delete, Import…), and from the Appearance panel it edits which filter an applied effect runs;