From acb6b880d5a36fda080d7c42a074766db002fcd2 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Sat, 7 Nov 2026 03:23:41 -0800 Subject: [PATCH] Links: links inside symbols, patterns and masks are saved with relative paths A save wrote the path relative to the document only for linked images and placed documents in the layers. Links inside symbols, graph designs, patterns and opacity masks kept only their absolute path, so after the document and its files moved to another folder together, those files were not found. - The save sets the relative path of every link the document holds. - On open, a link inside that art whose file was found away from its path takes the new path, as a link in the layers does, so the next save writes the right relative path. - Document::update_links_where changes only the links it picks and copies only the nodes on the way to them. Docs: docs/mcp.md (Linked images), docs/target-app-parity.md. --- crates/doc/src/links.rs | 96 ++++++++++++++++++++++++++++++++ crates/engine/src/cmd/links.rs | 56 +++++++++++++------ crates/engine/src/tests/links.rs | 92 ++++++++++++++++++++++++++++++ docs/mcp.md | 6 +- docs/target-app-parity.md | 1 + 5 files changed, 231 insertions(+), 20 deletions(-) diff --git a/crates/doc/src/links.rs b/crates/doc/src/links.rs index 781e61e68..ffa9eac0c 100644 --- a/crates/doc/src/links.rs +++ b/crates/doc/src/links.rs @@ -415,6 +415,68 @@ impl Document { } } + /// [`Self::update_links`] for the links `pick` holds for only, in one pass; only the nodes on the + /// way to one are copied. + pub fn update_links_where(&mut self, pick: impl Fn(&LinkInfo) -> bool, mut f: impl FnMut(&mut LinkInfo)) { + fn link_of(n: &Node) -> Option<&LinkInfo> { + match &n.kind { + NodeKind::Image(image) => image.link.as_ref(), + NodeKind::PlacedDocument(placed) => Some(&placed.link), + _ => None, + } + } + /// `n` updated in place, a copy of it when shared; left as it is when it holds no picked link. + fn update(n: &mut Arc, pick: &dyn Fn(&LinkInfo) -> bool, f: &mut dyn FnMut(&mut LinkInfo)) { + // Each child is updated through a second handle, so an updated child is a new node and a new + // pointer marks a change. + let mask = n.extras().mask.as_ref().and_then(|m| { + let mut art = Arc::clone(&m.art); + update(&mut art, pick, f); + (!Arc::ptr_eq(&art, &m.art)).then_some(art) + }); + let mut children = vec![]; + for (i, c) in n.children().into_iter().flatten().enumerate() { + let mut new = Arc::clone(c); + update(&mut new, pick, f); + if !Arc::ptr_eq(&new, c) { + children.push((i, new)); + } + } + let own = link_of(n).is_some_and(pick); + if !own && mask.is_none() && children.is_empty() { + return; + } + let n = Arc::make_mut(n); + let link = match &mut n.kind { + NodeKind::Image(image) => image.link.as_mut(), + NodeKind::PlacedDocument(placed) => Some(&mut placed.link), + _ => None, + }; + if let Some(link) = link.filter(|_| own) { + f(link); + } + if let (Some(art), Some(m)) = (mask, n.extras_if_any().and_then(|e| e.mask.as_mut())) { + m.art = art; + } + if let Some(slots) = n.children_mut() { + for (i, c) in children { + if let Some(slot) = slots.get_mut(i) { + *slot = c; + } + } + } + } + let roots = self + .layers + .iter_mut() + .chain(self.symbols.iter_mut().map(|s| &mut s.art)) + .chain(self.graph_designs.iter_mut().map(|g| &mut g.art)) + .chain(self.patterns.iter_mut().flat_map(|p| &mut p.art)); + for root in roots { + update(root, &pick, &mut f); + } + } + /// Change every image object (in the layers, symbol definitions and pattern swatches, opacity-mask /// art included) for which `pick` holds; only the subtrees holding one are copied. pub fn update_images(&mut self, pick: impl Fn(&ImageObject) -> bool, mut f: impl FnMut(&mut ImageObject)) { @@ -486,6 +548,40 @@ mod tests { .unwrap() } + /// `update_links_where` changes only the picked links, in a copy of the nodes on the way to them; + /// the other nodes stay shared with the document it was cloned from. + #[test] + fn update_links_where_copies_only_the_nodes_it_changes() { + let mut d = Document::new(10.0, 10.0); + image(&mut d, "a", Some(LinkInfo::new("/art/a.png"))); + image(&mut d, "b", Some(LinkInfo::new("/art/b.png"))); + image(&mut d, "c", None); + let mut copy = d.clone(); + let mut calls = 0; + copy.update_links_where( + |l| l.path == "/art/a.png", + |l| { + calls += 1; + l.relative = Some("art/a.png".into()); + }, + ); + assert_eq!(calls, 1); + let node = |d: &Document, key: &str| { + d.layers[0].children().unwrap().iter().find(|n| matches!(&n.kind, NodeKind::Image(im) if im.key == key)).unwrap().clone() + }; + let relative = |n: &Node| match &n.kind { + NodeKind::Image(im) => im.link.as_ref().and_then(|l| l.relative.clone()), + _ => None, + }; + assert_eq!((relative(&node(©, "a")), relative(&node(&d, "a"))), (Some("art/a.png".into()), None)); + assert!(Arc::ptr_eq(&node(©, "b"), &node(&d, "b")) && Arc::ptr_eq(&node(©, "c"), &node(&d, "c"))); + assert!(!Arc::ptr_eq(©.layers[0], &d.layers[0])); + // Nothing picked: nothing is copied. + let mut same = d.clone(); + same.update_links_where(|_| false, |_| {}); + assert!(Arc::ptr_eq(&same.layers[0], &d.layers[0])); + } + #[test] fn the_placeholder_is_a_png() { let png = missing_placeholder().unwrap(); diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index 6b1d45f7f..c09ac6773 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -678,6 +678,8 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p // of other keys linking to the same file share them, rather than each reading, decoding and // keeping a copy of its own. let mut read_images: BTreeMap = BTreeMap::new(); + // (the link as it was, the link now) of each file found away from its path or read again. + let mut moved: Vec<(LinkInfo, LinkInfo)> = vec![]; for g in groups(d, None) { let row = rows(std::iter::once(&g), doc_path); let unread = g.document.is_none() && g.link.hash.is_none() && g.link.size.is_none() && previewing(d, &g.key); @@ -686,8 +688,15 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p (Status::Missing, _) => out.missing.extend(row), (Status::Modified, Some(mut f)) if update || unread => { match read_for(&mut f, &g, "document.open").and_then(|read| install_read(d, &g.ids, read, unread.then_some(Preserve::Bounds))) { - Ok(()) if unread => {} - Ok(()) => out.updated.extend(row), + Ok(()) => { + // Links with no ids here were not read: they only take the path the file was found at. + if f.path != g.link.path { + moved.push((g.link.clone(), LinkInfo { path: f.path.clone(), ..g.link.clone() })); + } + if !unread { + out.updated.extend(row); + } + } Err(_) => out.missing.extend(row), } } @@ -709,6 +718,7 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p }; if link != g.link { set_links(d, &g.ids, &link); + moved.push((g.link.clone(), link)); } } } @@ -720,6 +730,7 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p let link = f.link_hashed(hash, &g.link); if link != g.link { set_links(d, &g.ids, &link); + moved.push((g.link.clone(), link)); } continue; } @@ -745,11 +756,28 @@ pub fn resolve(d: &mut Document, doc_path: Option<&str>, update: bool, low_res_p }; if link != g.link { set_links(d, &g.ids, &link); + moved.push((g.link.clone(), link)); } } (Status::Ok, None) => {} } } + // The same links in symbols, graph designs, patterns and opacity masks, which have no ids here. + if !moved.is_empty() { + let mut by_path: BTreeMap> = BTreeMap::new(); + for (old, new) in moved { + by_path.entry(old.path.clone()).or_default().push((old, new)); + } + let new = |l: &LinkInfo| by_path.get(&l.path).and_then(|m| m.iter().find(|(old, _)| old == l)).map(|(_, new)| new); + d.update_links_where( + |l| new(l).is_some(), + |l| { + if let Some(n) = new(l) { + *l = n.clone(); + } + }, + ); + } out } @@ -826,28 +854,20 @@ fn relative_path(target: &Path, base: &Path) -> Option { } /// `d` as saved to `dest`: the links' paths relative to its folder written in; `None` when they -/// are already. +/// are already. Links in the layers, symbols, graph designs, patterns and opacity masks are all set. pub fn with_relative_paths(d: &Document, dest: &str) -> Option { let dest = absolute_path(dest); let dir = Path::new(&dest).parent()?; - let mut changes = vec![]; - d.walk(|n| { - if let Some((l, _, _)) = linked(n) { - let r = relative_path(Path::new(&l.path), dir); - if r != l.relative { - changes.push((n.id, r)); - } - } - }); - if changes.is_empty() { + let relative = |l: &LinkInfo| relative_path(Path::new(&l.path), dir); + let outdated = |l: &LinkInfo| relative(l) != l.relative; + let mut changed = false; + d.visit_images(|_, im| changed |= im.link.as_ref().is_some_and(outdated)); + d.visit_placed(|_, p| changed |= outdated(&p.link)); + if !changed { return None; } let mut d = d.clone(); - for (id, r) in changes { - if let Some(l) = d.node_mut(id).and_then(link_mut) { - l.relative = r; - } - } + d.update_links_where(outdated, |l| l.relative = relative(l)); Some(d) } diff --git a/crates/engine/src/tests/links.rs b/crates/engine/src/tests/links.rs index 3fe46ad35..1eeb1ac14 100644 --- a/crates/engine/src/tests/links.rs +++ b/crates/engine/src/tests/links.rs @@ -244,6 +244,98 @@ fn the_relative_path_finds_the_file_after_the_folder_moves() { assert!(near(centre_colour(&s.doc().unwrap().doc), RED)); } +/// Links in symbols, graph designs, patterns and opacity masks are saved with their relative +/// paths, as links in the layers are: after the folder moves, each file is found and its link takes +/// the new path, which the next save writes relative to the document again. +#[test] +fn links_outside_the_layers_find_their_files_after_the_folder_moves() { + let a = Folder::new("nested-move-a"); + let b = Folder::new("nested-move-b"); + let c = Folder::new("nested-move-c"); + let names = ["layer", "symbol", "design", "pattern", "mask"]; + for (i, name) in (0u8..).zip(names) { + write(&a.file(&format!("art/{name}.png")), &png(20, 10, [20 + 40 * i, 90, 160])); + } + let mut s = session(); + place(&mut s, &a.file("art/layer.png")); + let id = place(&mut s, &a.file("art/symbol.png")); + s.execute("symbol.new", &json!({"name": "Mark", "ids": [id.0]})).unwrap(); + let id = place(&mut s, &a.file("art/design.png")); + s.execute("graph.design", &json!({"new": "Bar"})).unwrap(); + s.execute("edit.clear", &json!({"ids": [id.0]})).unwrap(); + let id = place(&mut s, &a.file("art/pattern.png")); + s.execute("object.pattern.make", &json!({"ids": [id.0], "edit": false})).unwrap(); + s.execute("edit.clear", &json!({"ids": [id.0]})).unwrap(); + let masked = super::rect(&mut s, 10.0, 10.0, 60.0, 60.0); + let mask = place(&mut s, &a.file("art/mask.png")); + s.execute("transparency.makeOpacityMask", &json!({"ids": [masked.0, mask.0]})).unwrap(); + let mut in_layers = vec![]; + s.doc().unwrap().doc.walk(|n| { + if let NodeKind::Image(im) = &n.kind { + in_layers.extend(im.link.as_ref().map(|l| l.name().to_string())); + } + }); + assert_eq!(in_layers, ["layer.png"], "only one file is shown by an object in the layers"); + // The links' paths, each image checked to show its file's pixels rather than the saved preview. + let paths = |s: &Session| { + let d = &s.doc().unwrap().doc; + let mut out = vec![]; + d.visit_images(|_, im| { + let l = im.link.as_ref().unwrap(); + assert!(!d.images[&im.key].is_proxy(), "{} shows its preview", l.name()); + out.push(PathBuf::from(&l.path)); + }); + out.sort(); + out + }; + let files = |dir: &Folder| { + let mut out: Vec = names.iter().map(|n| PathBuf::from(dir.file(&format!("art/{n}.png")))).collect(); + out.sort(); + out + }; + assert_eq!(paths(&s), files(&a)); + save(&mut s, &a.file("poster.vectorcraft")); + // Move the document with its links: the old folder is gone. + std::fs::remove_dir_all(&b.0).unwrap(); + std::fs::rename(&a.0, &b.0).unwrap(); + let r = open(&mut s, &b.file("poster.vectorcraft")); + assert_eq!((r["missingLinks"].clone(), r["modifiedLinks"].clone()), (json!([]), json!([])), "{r}"); + assert_eq!(paths(&s), files(&b), "the links follow the files"); + // Saved again and moved again. + save(&mut s, &b.file("poster.vectorcraft")); + std::fs::remove_dir_all(&c.0).unwrap(); + std::fs::rename(&b.0, &c.0).unwrap(); + let r = open(&mut s, &c.file("poster.vectorcraft")); + assert_eq!((r["missingLinks"].clone(), r["modifiedLinks"].clone()), (json!([]), json!([])), "{r}"); + assert_eq!(paths(&s), files(&c)); +} + +/// With Update Links: Automatically, a file in a symbol and in the layers that changed and moved +/// with the document is read again for the layers' image, and the symbol's link takes its new path +/// too. +#[test] +fn a_symbols_link_to_a_moved_and_modified_file_takes_its_new_path() { + let a = Folder::new("nested-modified-a"); + let b = Folder::new("nested-modified-b"); + let pic = a.file("art/photo.png"); + write(&pic, &png(20, 10, RED)); + let mut s = session(); + s.execute("prefs.set", &json!({"key": "updateLinks", "value": "automatically"})).unwrap(); + let id = place(&mut s, &pic); + s.execute("symbol.new", &json!({"name": "Mark", "ids": [id.0]})).unwrap(); + let id = place(&mut s, &pic); + save(&mut s, &a.file("poster.vectorcraft")); + write(&pic, &png(30, 30, BLUE)); + std::fs::remove_dir_all(&b.0).unwrap(); + std::fs::rename(&a.0, &b.0).unwrap(); + let r = open(&mut s, &b.file("poster.vectorcraft")); + assert_eq!((r["modifiedLinks"].clone(), r["updatedLinks"][0]["ids"].clone()), (json!([]), json!([id.0])), "{r}"); + let moved = PathBuf::from(b.file("art/photo.png")); + let mut paths = vec![]; + s.doc().unwrap().doc.visit_images(|_, im| paths.extend(im.link.as_ref().map(|l| PathBuf::from(&l.path)))); + assert_eq!(paths, [moved.clone(), moved], "the layers' link, then the symbol's"); +} + #[test] fn a_missing_link_renders_its_preview_and_relinks() { let dir = Folder::new("missing"); diff --git a/docs/mcp.md b/docs/mcp.md index a51a5f6c8..9ec05bd8d 100644 --- a/docs/mcp.md +++ b/docs/mcp.md @@ -4323,9 +4323,11 @@ is mixed with middle gray by its alpha. A placed image linked to its file (`file.place {path}`, Link on) records the file's absolute path, its size, modification time and hash, and keeps a low-resolution preview (at most 256 px a side): a `.vectorcraft` save writes -the preview instead of the pixels, plus the path relative to the saved file. `document.open` reads +the preview instead of the pixels, plus the path relative to the saved file. Links inside symbols, graph designs, +patterns and opacity masks are saved with their relative paths too. `document.open` reads the linked files again, looking for each at its path, then at its relative path and by name in the document's folder -(so a folder moved with its links still opens). Only files on this computer, or on the network share the document is +(so a folder moved with its links still opens), and each link, in the layers or inside that art, takes the path where +its file was found. Only files on this computer, or on the network share the document is on, are looked at: a path to another computer, a device or a URL counts as missing. On Windows, a document opened from a mapped drive (`Z:\…`) is on the share the drive is mapped to, and links on that share (`\\server\share\…`) are looked at. A link on a drive mapped to another share (`Y:\…`) counts as missing, from a document on this computer too. diff --git a/docs/target-app-parity.md b/docs/target-app-parity.md index 92848cc5e..fcc2e558b 100644 --- a/docs/target-app-parity.md +++ b/docs/target-app-parity.md @@ -1334,6 +1334,7 @@ Moved from the ROADMAP's "Shipped so far" on 2026-10-10. Update it when a featur - On the desktop with Enable Background Export on (Preferences › File Handling), File › Package… collects the linked files and fonts and writes the package from the document as it was at OK on the thread that writes background saves and exports, with "Packaging" in the status bar and the busy spinner until it is written; a save of unsaved changes that Package starts is written first. `file.package` over MCP and scripts replies as before. - On the desktop with Enable Background Export on, Export for Screens and the Asset Export panel's Export encode and write their files (into the folder, or one ZIP in it) from the document as it was at OK on the thread that writes background saves and exports, with "Exporting" in the status bar and the export progress dialog, which counts the files and whose Cancel stops before the next file. `document.exportForScreens` and `assets.export` over MCP and scripts reply as before. - Missing linked files can be looked for in a folder and its subfolders: Find in Folder… in the Linked File Not Found dialog and in the Links panel menu lists the files with each one's name, those with the size and contents saved with the link first, and relinks the files chosen, copying an image's file into the document's folder when asked, without replacing a file there (`links.findFiles`, `links.relink {copy}`). In the Links panel it looks for the selected linked files when any are selected. + - Linked files inside symbols, graph designs, patterns and opacity masks are saved with their paths relative to the document, as linked files in the layers are, so they are found after the document and its files move to another folder together; on open each of those links takes the path where its file was found. Evidence: `links_outside_the_layers_find_their_files_after_the_folder_moves`. - On Windows, a document opened from a mapped network drive finds its links stored with the share's own path (`\\server\share\…`), as the same document opened from the share does; links on other computers stay missing. - On Windows, a link on a drive mapped to a network share other than the document's (`Y:\…`) is missing, with the reason that the path is on another computer, as a link stored with that share's own path is; links on this computer's drives and on the document's share are followed. - In the Links panel, Shift-click selects a range of links and Ctrl/Cmd-click adds or removes one.