From 597f0e649f240c306b91abc0d8968347117c0a41 Mon Sep 17 00:00:00 2001 From: japbcoelho <319161831+japbcoelho@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:49:46 -0300 Subject: [PATCH 1/5] Color parsing: one scale per colour, not per component (#510) A CMYK component of 1 or less was read as a fraction even when the other components were percentages, so {"c": 0.7, "m": 0, "y": 12.2, "k": 0} (a pale cream) became C 70 % Y 12.2 % and exported as `0.7019608 0 0.12156863 0 k`. The same applied to RGB arrays. Now a colour is read as percentages (0..100, or 0..255 for RGB) when any component is above 1, and as fractions only when every component is within 0..1. Co-authored-by: Claude Opus 5.5 --- crates/engine/src/cmd/style.rs | 27 +++++++++++++++++++++++---- 1 file changed, 23 insertions(+), 4 deletions(-) diff --git a/crates/engine/src/cmd/style.rs b/crates/engine/src/cmd/style.rs index 94bd92c6..296a8d07 100644 --- a/crates/engine/src/cmd/style.rs +++ b/crates/engine/src/cmd/style.rs @@ -716,13 +716,18 @@ pub fn parse_color(v: &Value) -> Option { use designcraft_color::Color; match v { Value::String(s) => Color::from_hex(s), + // One scale per colour, not per component: {c: 0.7, y: 12.2} is C 0.7 %, Y 12.2 %. Only a colour whose + // components are all within 0..1 is read as fractions (the older form). Value::Array(a) if a.len() >= 3 => { - let f = |i: usize| a[i].as_f64().map(|x| if x > 1.0 { x / 255.0 } else { x } as f32); - Some(Color::rgb(f(0)?, f(1)?, f(2)?)) + let v = [a[0].as_f64()?, a[1].as_f64()?, a[2].as_f64()?]; + let scale = if v.iter().any(|&x| x > 1.0) { 255.0 } else { 1.0 }; + Some(Color::rgb((v[0] / scale) as f32, (v[1] / scale) as f32, (v[2] / scale) as f32)) } Value::Object(o) => { - let g = |k: &str| o.get(k).and_then(Value::as_f64).map(|x| if x > 1.0 { x / 100.0 } else { x } as f32); - Some(Color::cmyk(g("c")?, g("m")?, g("y")?, g("k")?)) + let g = |k: &str| o.get(k).and_then(Value::as_f64); + let v = [g("c")?, g("m")?, g("y")?, g("k")?]; + let scale = if v.iter().any(|&x| x > 1.0) { 100.0 } else { 1.0 }; + Some(Color::cmyk((v[0] / scale) as f32, (v[1] / scale) as f32, (v[2] / scale) as f32, (v[3] / scale) as f32)) } _ => None, } @@ -1040,6 +1045,20 @@ fn rename_in(it: &mut designcraft_doc::Item, from: &str, to: &str) { mod color_tests { use super::*; + #[test] + fn percent_components_at_most_one_stay_percent() { + use designcraft_color::Color; + // {"c": 0.7, ..., "y": 12.2} is a pale cream (C 0.7 %, Y 12.2 %), not C 70 %. + let Some(Color::Cmyk { c, m, y, k }) = parse_color(&json!({"c": 0.7, "m": 0, "y": 12.2, "k": 0})) else { panic!("not CMYK") }; + assert!((c - 0.007).abs() < 1e-6 && m == 0.0 && (y - 0.122).abs() < 1e-6 && k == 0.0, "got {c} {m} {y} {k}"); + // Fractions (every component within 0..1) keep working. + let Some(Color::Cmyk { c, k, .. }) = parse_color(&json!({"c": 0.5, "m": 0, "y": 0, "k": 1})) else { panic!("not CMYK") }; + assert!((c - 0.5).abs() < 1e-6 && (k - 1.0).abs() < 1e-6); + // RGB arrays follow the same rule: [1, 128, 255] is 0..255. + let Some(Color::Rgb { r, g, b }) = parse_color(&json!([1, 128, 255])) else { panic!("not RGB") }; + assert!((r - 1.0 / 255.0).abs() < 1e-6 && (g - 128.0 / 255.0).abs() < 1e-6 && (b - 1.0).abs() < 1e-6); + } + #[test] fn style_groups_rename_every_use() { let mut s = Session::new(); From ca1a8a12bceda169f55f700ae3fbfe034c03c728 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 11 Oct 2026 20:49:56 -0400 Subject: [PATCH 2/5] Place: an image placed into a frame leaves Auto-Fit off (#528) * Place: an image placed into a frame fills it once and leaves Auto-Fit off * Place: the fitting test names its fitting, since a placed image no longer auto-fits --- crates/engine/src/cmd/file.rs | 6 +++--- crates/engine/src/cmd/object.rs | 4 ++-- crates/engine/src/tests.rs | 10 +++++++++- 3 files changed, 14 insertions(+), 6 deletions(-) diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 3a2885ce..a0c98f64 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -351,14 +351,14 @@ fn file_place(s: &mut Session, p: &Value) -> Result { let id = if let Some(fid) = target_frame { let it = d.item_mut(fid).ok_or(designcraft_doc::DocError::NoItem(fid))?; let r = it.inner_bounds(); - // Fill frame proportionally. + // Fill the frame proportionally once. Auto-Fit stays off, so resizing the frame crops. let k = (r.width() / nw).max(r.height() / nh); let (gw, gh) = (nw * k, nh * k); it.content = Content::Graphic(Graphic { asset: aid, size: (nw, nh), xf: Affine::translate((r.x0 + (r.width() - gw) / 2.0, r.y0 + (r.height() - gh) / 2.0)) * Affine::scale(k), - auto_fit: designcraft_doc::Fitting::FillProportionally, + auto_fit: Default::default(), fit_align: 4, crop: [0.0; 4], }); @@ -473,7 +473,7 @@ fn place_drop(s: &mut Session, p: &Value) -> Result { asset: aid, size: (nw, nh), xf: Affine::translate((r.x0 + (r.width() - nw * k) / 2.0, r.y0 + (r.height() - nh * k) / 2.0)) * Affine::scale(k), - auto_fit: designcraft_doc::Fitting::FillProportionally, + auto_fit: Default::default(), fit_align: 4, crop: [0.0; 4], }); diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index 17c44c27..c4f2562a 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -2463,8 +2463,8 @@ mod fitting_tests { s.execute("object.fittingOptions", &json!({"ids": [f], "fitting": "fitProportionally", "align": 0})).unwrap(); let c = content(&s); assert!((c.x0 - 100.0).abs() < 1e-6 && (c.y0 - 100.0).abs() < 1e-6 && (c.width() - 200.0).abs() < 1e-6, "{c:?}"); - // Crop 10 pt on every side: the content spreads past the frame. - s.execute("object.fittingOptions", &json!({"ids": [f], "crop": 10, "align": 4})).unwrap(); + // Crop 10 pt on every side and fit again: the content spreads past the frame. + s.execute("object.fittingOptions", &json!({"ids": [f], "fitting": "fitProportionally", "crop": 10, "align": 4})).unwrap(); let c = content(&s); assert!((c.width() - 220.0).abs() < 1e-6, "{c:?}"); // Auto-fit: resizing the frame refits the content. diff --git a/crates/engine/src/tests.rs b/crates/engine/src/tests.rs index c1032bcc..17d1ed09 100644 --- a/crates/engine/src/tests.rs +++ b/crates/engine/src/tests.rs @@ -284,7 +284,15 @@ fn place_gun_click_drag_and_into_frame() { let f = s.execute("frame.create", &json!({"rect": [300, 300, 400, 400], "content": "graphic"})).unwrap()["id"].as_u64().unwrap(); s.execute("place.load", &json!({"base64": b64})).unwrap(); s.execute("place.drop", &json!({"frame": f})).unwrap(); - assert!(matches!(s.doc().unwrap().doc.item(designcraft_doc::ItemId(f)).unwrap().content, designcraft_doc::Content::Graphic(_))); + let graphic_xf = |s: &Session| match &s.doc().unwrap().doc.item(designcraft_doc::ItemId(f)).unwrap().content { + designcraft_doc::Content::Graphic(g) => g.xf, + _ => panic!("not a graphic"), + }; + let placed = graphic_xf(&s); + // Dragging the top edge down crops the top only: the image stays where it is. + s.execute("selection.set", &json!({"ids": [f]})).unwrap(); + s.execute("transform.resize", &json!({"from": [300, 300, 400, 400], "to": [300, 340, 400, 400]})).unwrap(); + assert_eq!(graphic_xf(&s), placed); } #[test] From 223dedfe5a56b35dfd4ba9f2643d96c0f7b5d005 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:23:58 -0400 Subject: [PATCH 3/5] feat(assets): deduplicate files and omit unused assets on save Share duplicate file data while keeping distinct link paths independent, and leave unused assets out of saved documents. Validation: all six cargo xtask ci gates passed on the merge candidate; 1,170 native tests passed and all 15 WASM packages checked. Independent regression checks cover updates, relinking, embedding, undo and save/reopen. Task: #420 --- crates/doc/src/assets.rs | 351 ++++++++++++++++++++++++ crates/doc/src/lib.rs | 2 + crates/engine/src/cmd/datamerge/fill.rs | 41 ++- crates/engine/src/cmd/file.rs | 12 +- crates/engine/src/cmd/links.rs | 117 ++++++++ crates/engine/src/cmd/mathexpr.rs | 15 +- crates/engine/src/cmd/media.rs | 9 +- crates/engine/src/cmd/object.rs | 20 +- crates/engine/src/cmd/package.rs | 6 +- crates/engine/src/cmd/table.rs | 16 +- crates/format/src/lib.rs | 153 +++++++++-- crates/idml/src/import.rs | 41 ++- crates/idml/src/tests.rs | 1 + crates/idml/src/tests/assets.rs | 110 ++++++++ 14 files changed, 812 insertions(+), 82 deletions(-) create mode 100644 crates/doc/src/assets.rs create mode 100644 crates/idml/src/tests/assets.rs diff --git a/crates/doc/src/assets.rs b/crates/doc/src/assets.rs new file mode 100644 index 00000000..24ba5c24 --- /dev/null +++ b/crates/doc/src/assets.rs @@ -0,0 +1,351 @@ +//! Placed files (assets): one asset per distinct file, the assets a document uses, and the +//! compaction a save applies (duplicates merged, unused assets left out). +//! +//! An asset holds what is shared by every placement of a file (bytes, link, type, the PDF page +//! shown); each placement's frame data (transform, crop, fitting) lives on its [`Graphic`]. + +use std::collections::{BTreeMap, BTreeSet, HashMap}; +use std::sync::Arc; + +use crate::story::Story; +use crate::{Asset, AssetId, Content, Document, Item}; + +/// Nesting limit for groups, anchored objects, table cells and notes. A document nested deeper +/// is treated as using every asset, so nothing it references is merged away or pruned. +const MAX_DEPTH: usize = 64; + +impl Asset { + /// Whether `other` holds the same file: the same type, page, link and bytes. Files at + /// different paths stay apart even with identical bytes, since each can be updated or + /// relinked on its own. Assets without bytes (a link that couldn't be read) are the same only + /// when they link to the same path. + pub fn same_file(&self, other: &Asset) -> bool { + if self.mime != other.mime || self.page != other.page { + return false; + } + if self.link != other.link { + return false; + } + match (self.data.is_empty(), other.data.is_empty()) { + (false, false) => self.same_bytes(other), + (true, true) => self.link.is_some(), + _ => false, + } + } + + fn same_bytes(&self, other: &Asset) -> bool { + Arc::ptr_eq(&self.data, &other.data) || (self.data.len() == other.data.len() && *self.data == *other.data) + } +} + +/// The asset in `assets` holding the same file as `asset`. +pub fn find_same_asset(assets: &BTreeMap>, asset: &Asset) -> Option { + assets.values().find(|a| a.same_file(asset)).map(|a| a.id) +} + +/// Add `asset` to `assets` unless they already hold the same file; returns the id placements +/// should reference. A new asset with the bytes of an existing one shares its buffer. +pub fn insert_asset(assets: &mut BTreeMap>, mut asset: Asset) -> AssetId { + if let Some(id) = find_same_asset(assets, &asset) { + return id; + } + if !asset.data.is_empty() + && let Some(a) = assets.values().find(|a| a.same_bytes(&asset)) + { + asset.data = Arc::clone(&a.data); + } + let id = asset.id; + assets.insert(id, Arc::new(asset)); + id +} + +/// Calls `f` with every asset reference in `it` and its children; false when nesting exceeded +/// [`MAX_DEPTH`] (some references weren't visited). +fn item_refs(it: &Item, f: &mut impl FnMut(AssetId), depth: usize) -> bool { + if depth > MAX_DEPTH { + return false; + } + if let Content::Graphic(g) = &it.content { + f(g.asset); + } + if let Some(p) = it.media.as_ref().and_then(|m| m.poster) { + f(p); + } + it.children().iter().all(|c| item_refs(c, f, depth + 1)) +} + +/// Asset references in a story's anchored objects, graphic cells and notes (recursively). +fn story_refs(st: &Story, f: &mut impl FnMut(AssetId), depth: usize) -> bool { + if depth > MAX_DEPTH { + return false; + } + let mut complete = st.objects.iter().all(|o| item_refs(&o.item, f, depth + 1)); + for t in st.tables.values() { + for c in &t.cells { + if let Some(g) = &c.graphic { + f(g.asset); + } + complete &= story_refs(&c.text, f, depth + 1); + } + } + for n in st.notes.iter().chain(&st.endnotes) { + complete &= story_refs(&n.text, f, depth + 1); + } + complete +} + +/// Every asset reference in a story (its anchored objects, graphic cells and notes). +pub fn story_asset_refs(st: &Story) -> Vec { + let mut out = Vec::new(); + story_refs(st, &mut |a| out.push(a), 0); + out +} + +/// Point a story's references to a key of `map` at its value. +pub fn remap_story_assets(st: &mut Story, map: &HashMap) { + if !map.is_empty() { + remap_story(st, map, 0); + } +} + +fn item_uses(it: &Item, map: &HashMap) -> bool { + let mut hit = false; + item_refs(it, &mut |a| hit |= map.contains_key(&a), 0); + hit +} + +fn story_uses(st: &Story, map: &HashMap) -> bool { + let mut hit = false; + story_refs(st, &mut |a| hit |= map.contains_key(&a), 0); + hit +} + +fn remap_id(id: &mut AssetId, map: &HashMap) { + if let Some(n) = map.get(id) { + *id = *n; + } +} + +/// Rewrites the references in `it`, copying only the shared items that change. +fn remap_item(it: &mut Arc, map: &HashMap, depth: usize) { + if depth > MAX_DEPTH || !item_uses(it, map) { + return; + } + remap_item_in(Arc::make_mut(it), map, depth); +} + +fn remap_item_in(it: &mut Item, map: &HashMap, depth: usize) { + if let Content::Graphic(g) = &mut it.content { + remap_id(&mut g.asset, map); + } + if let Some(p) = it.media.as_mut().and_then(|m| m.poster.as_mut()) { + remap_id(p, map); + } + if let Some(kids) = it.children_mut() { + for k in kids { + remap_item(k, map, depth + 1); + } + } +} + +fn remap_story(st: &mut Story, map: &HashMap, depth: usize) { + if depth > MAX_DEPTH { + return; + } + for o in &mut st.objects { + if item_uses(&o.item, map) { + remap_item_in(&mut Arc::make_mut(o).item, map, depth + 1); + } + } + for t in st.tables.values_mut() { + if !t.cells.iter().any(|c| c.graphic.as_ref().is_some_and(|g| map.contains_key(&g.asset)) || story_uses(&c.text, map)) { + continue; + } + for c in &mut Arc::make_mut(t).cells { + if let Some(g) = &mut c.graphic { + remap_id(&mut g.asset, map); + } + remap_story(&mut c.text, map, depth + 1); + } + } + for n in st.notes.iter_mut().chain(st.endnotes.iter_mut()) { + if story_uses(&n.text, map) { + remap_story(&mut Arc::make_mut(n).text, map, depth + 1); + } + } +} + +impl Document { + /// Add a placed file, reusing the asset of an identical file already in the document; + /// returns the id to reference. + pub fn add_asset(&mut self, asset: Asset) -> AssetId { + insert_asset(&mut self.assets, asset) + } + + /// Copy asset `id` of `src` into this document (or reuse the identical asset it already + /// has); returns the id to reference here, `None` when `src` has no such asset. + pub fn adopt_asset(&mut self, src: &Document, id: AssetId) -> Option { + let a = src.assets.get(&id)?; + if let Some(same) = find_same_asset(&self.assets, a) { + return Some(same); + } + let fresh = AssetId(self.alloc()); + Some(self.add_asset(Asset { id: fresh, ..(**a).clone() })) + } + + /// Calls `f` with every asset reference: graphics on document and parent spreads (in + /// groups and multi-state objects too), media posters, anchored and inline objects, graphic + /// table cells and notes. False when nesting exceeded the depth limit. + fn asset_refs(&self, f: &mut impl FnMut(AssetId)) -> bool { + let mut complete = true; + for sp in self.spreads.iter().chain(&self.parents) { + for it in &sp.items { + complete &= item_refs(it, f, 0); + } + } + for st in self.stories.values() { + complete &= story_refs(st, f, 0); + } + complete + } + + /// The assets something in the document references. When the document nests deeper than + /// the traversal limit, every asset counts as used. + pub fn used_assets(&self) -> BTreeSet { + let mut used = BTreeSet::new(); + if !self.asset_refs(&mut |a| { + used.insert(a); + }) { + return self.assets.keys().copied().collect(); + } + used + } + + /// Point every reference to a key of `map` at its value. + pub fn remap_assets(&mut self, map: &HashMap) { + if map.is_empty() { + return; + } + for sp in self.spreads.iter_mut().chain(self.parents.iter_mut()) { + if sp.items.iter().any(|it| item_uses(it, map)) { + for it in &mut Arc::make_mut(sp).items { + remap_item(it, map, 0); + } + } + } + for st in self.stories.values_mut() { + if story_uses(st, map) { + remap_story(Arc::make_mut(st), map, 0); + } + } + } + + /// Merge assets holding the same file into the first of them (lowest id), rewriting the + /// references; returns how many assets were removed. + pub fn merge_duplicate_assets(&mut self) -> usize { + if !self.asset_refs(&mut |_| {}) { + return 0; + } + let mut reps: HashMap<(usize, &str, u32), Vec<&Arc>> = HashMap::new(); + let mut map = HashMap::new(); + for a in self.assets.values() { + let bucket = reps.entry((a.data.len(), a.mime.as_str(), a.page)).or_default(); + match bucket.iter().find(|r| r.same_file(a)) { + Some(r) => { + map.insert(a.id, r.id); + } + None => bucket.push(a), + } + } + self.remap_assets(&map); + self.assets.retain(|id, _| !map.contains_key(id)); + map.len() + } + + /// Merge duplicate assets and drop the ones nothing references (what a saved file holds). + pub fn compact_assets(&mut self) { + self.merge_duplicate_assets(); + let used = self.used_assets(); + self.assets.retain(|id, _| used.contains(id)); + } +} + +#[cfg(test)] +mod tests { + use super::*; + use crate::{Graphic, ItemId, LayerId, Shape, StoryId}; + use designcraft_geom::{Affine, Rect, shapes}; + + fn asset(id: u64, data: &[u8]) -> Asset { + Asset { id: AssetId(id), name: "p.png".into(), mime: "image/png".into(), link: None, data: Arc::new(data.to_vec()), pixels: None, page: 0 } + } + + fn graphic_item(id: u64, asset: u64, crop: f64) -> Item { + let mut it = Item::new(ItemId(id), LayerId(1), Shape::Rectangle, shapes::rectangle(Rect::new(0.0, 0.0, 10.0, 10.0))); + it.content = Content::Graphic(Graphic { + asset: AssetId(asset), + size: (10.0, 10.0), + xf: Affine::translate((crop, 0.0)), + auto_fit: Default::default(), + fit_align: 4, + crop: [crop; 4], + }); + it + } + + #[test] + fn identical_files_share_one_asset() { + let mut d = Document::new(&Default::default()); + let a = d.add_asset(asset(1, b"same")); + let b = d.add_asset(asset(2, b"same")); + let c = d.add_asset(asset(3, b"diff")); + let p = d.add_asset(Asset { page: 1, ..asset(4, b"same") }); + assert_eq!(a, b); + assert_ne!(a, c); + assert_ne!(a, p); + assert_eq!(d.assets.len(), 3); + // Unreadable links match by path only. + let m1 = d.add_asset(Asset { link: Some("/m/1.png".into()), ..asset(5, b"") }); + let m2 = d.add_asset(Asset { link: Some("/m/2.png".into()), ..asset(6, b"") }); + let m3 = d.add_asset(asset(7, b"")); + let m4 = d.add_asset(asset(8, b"")); + assert_ne!(m1, m2); + assert_ne!(m3, m4); + } + + #[test] + fn files_at_different_paths_stay_separate_assets() { + let mut d = Document::new(&Default::default()); + let linked = |id, path: &str| Asset { link: Some(path.into()), ..asset(id, b"same") }; + let embedded = d.add_asset(asset(1, b"same")); + let a = d.add_asset(linked(2, "/p/a.png")); + let b = d.add_asset(linked(3, "/q/b.png")); + assert_eq!(d.add_asset(linked(4, "/p/a.png")), a, "the same path placed again"); + assert_eq!(d.assets.len(), 3); + assert!(embedded != a && a != b && embedded != b); + assert_eq!(d.assets[&embedded].link, None); + assert_eq!(d.assets[&b].link.as_deref(), Some("/q/b.png")); + assert!(Arc::ptr_eq(&d.assets[&a].data, &d.assets[&b].data), "identical bytes share one buffer"); + } + + #[test] + fn compaction_merges_duplicates_and_drops_unused_assets() { + let mut d = Document::new(&Default::default()); + for (id, data) in [(1, b"one"), (2, b"one"), (3, b"two"), (4, b"old")] { + d.assets.insert(AssetId(id), Arc::new(asset(id, data))); + } + let sp = Arc::make_mut(d.spreads.first_mut().unwrap()); + sp.items.push(Arc::new(graphic_item(10, 1, 1.0))); + sp.items.push(Arc::new(graphic_item(11, 2, 2.0))); + // A graphic cell and an anchored object in a story. + let mut st = Story::new(StoryId(20)); + st.objects.push(Arc::new(crate::AnchoredObject::new(graphic_item(12, 3, 3.0), Default::default()))); + d.stories.insert(StoryId(20), Arc::new(st)); + assert_eq!(d.used_assets(), [1, 2, 3].map(AssetId).into_iter().collect()); + + d.compact_assets(); + assert_eq!(d.assets.keys().copied().collect::>(), vec![AssetId(1), AssetId(3)]); + let crops: Vec<(AssetId, f64)> = d.spreads[0].items.iter().filter_map(|i| i.graphic().map(|g| (g.asset, g.crop[0]))).collect(); + assert_eq!(crops, vec![(AssetId(1), 1.0), (AssetId(1), 2.0)]); + } +} diff --git a/crates/doc/src/lib.rs b/crates/doc/src/lib.rs index c4b69e86..2c90d8b9 100644 --- a/crates/doc/src/lib.rs +++ b/crates/doc/src/lib.rs @@ -12,6 +12,7 @@ pub mod anchored; pub mod arabic; pub mod arrow; +pub mod assets; pub mod attrs; pub mod build; pub mod cjk; @@ -36,6 +37,7 @@ use std::collections::BTreeMap; use std::sync::Arc; pub use anchored::{AnchorPosition, AnchoredObject, OBJECT_MARK}; +pub use assets::{find_same_asset, insert_asset, remap_story_assets, story_asset_refs}; pub use attrs::*; pub use datamerge::{ DataField, DataFieldKind, DataMerge, DataSource, Delimiter, Fingerprint, MergeOptions, Placeholder, PlaceholderAnchor, PlaceholderRole, diff --git a/crates/engine/src/cmd/datamerge/fill.rs b/crates/engine/src/cmd/datamerge/fill.rs index deb0948b..89dd0c74 100644 --- a/crates/engine/src/cmd/datamerge/fill.rs +++ b/crates/engine/src/cmd/datamerge/fill.rs @@ -75,6 +75,11 @@ fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document let new_id = ItemId(dst.alloc()); maps.items.insert(old, new_id); it.id = new_id; + if let Some(m) = it.media.as_mut() + && let Some(p) = m.poster + { + m.poster = dst.adopt_asset(src, p).or(Some(p)); + } match &mut it.content { Content::Text(tf) => { let new_sid = if let Some(n) = maps.stories.get(&tf.story) { @@ -83,6 +88,15 @@ fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document let n = StoryId(dst.alloc()); let mut st = src.story(tf.story).cloned().unwrap_or_else(|| Story::new(n)); st.id = n; + let mut assets = HashMap::new(); + for a in designcraft_doc::story_asset_refs(&st) { + if let Some(na) = dst.adopt_asset(src, a) + && na != a + { + assets.insert(a, na); + } + } + designcraft_doc::remap_story_assets(&mut st, &assets); st.frames.clear(); dst.stories.insert(n, Arc::new(st)); maps.stories.insert(tf.story, n); @@ -94,8 +108,8 @@ fn renumber(dst: &mut designcraft_doc::Document, src: &designcraft_doc::Document } } Content::Graphic(g) => { - if let Some(a) = src.assets.get(&g.asset) { - dst.assets.entry(g.asset).or_insert_with(|| a.clone()); + if let Some(a) = dst.adopt_asset(src, g.asset) { + g.asset = a; } } Content::Group { items } => { @@ -355,21 +369,18 @@ fn place_image( }; let (nw, nh) = (f64::from(pw), f64::from(ph)); let mode = fitting_of(&options.fitting); - let aid = AssetId(doc.alloc()); + let fresh = AssetId(doc.alloc()); let name = path.file_name().map(|n| n.to_string_lossy().into_owned()).unwrap_or_else(|| field.to_string()); let link = options.link_images.then(|| path.to_string_lossy().into_owned()); - doc.assets.insert( - aid, - Arc::new(Asset { - id: aid, - name, - mime: designcraft_render::image_mime(&bytes).to_string(), - link, - data: Arc::new(bytes), - pixels: Some((pw, ph)), - page: 0, - }), - ); + let aid = doc.add_asset(Asset { + id: fresh, + name, + mime: designcraft_render::image_mime(&bytes).to_string(), + link, + data: Arc::new(bytes), + pixels: Some((pw, ph)), + page: 0, + }); let mut g = Graphic { asset: aid, size: (nw, nh), xf: Affine::translate((inner.x0, inner.y0)), auto_fit: mode, fit_align: 0, crop: [0.0; 4] }; if let Some(xf) = g.fitted(inner, mode) { g.xf = xf; diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index a0c98f64..97ded38f 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -337,9 +337,9 @@ fn file_place(s: &mut Session, p: &Value) -> Result { let py = p.get("y").and_then(Value::as_f64); let want_w = p.get("width").and_then(Value::as_f64); s.edit(|d, sel| { - let aid = AssetId(d.alloc()); + let fresh = AssetId(d.alloc()); let mime = designcraft_render::image_mime(&bytes).to_string(); - d.assets.insert(aid, Arc::new(Asset { page: pdf_page, id: aid, name, mime, link, data: Arc::new(bytes), pixels: Some((pw, ph)) })); + let aid = d.add_asset(Asset { page: pdf_page, id: fresh, name, mime, link, data: Arc::new(bytes), pixels: Some((pw, ph)) }); // The width the whole page (or image) gets; the height follows the proportions. let w = match want_w { Some(w) => w, @@ -443,13 +443,9 @@ fn place_load(s: &mut Session, p: &Value) -> Result { let (pw, ph) = designcraft_render::image_size(&bytes).ok_or_else(|| bad("place.load", "unsupported or corrupt image"))?; let natural = designcraft_render::natural_size(&bytes, 0).unwrap_or((pw as f64, ph as f64)); let aid = s.edit(|d, _| { - let aid = AssetId(d.alloc()); + let fresh = AssetId(d.alloc()); let mime = designcraft_render::image_mime(&bytes).to_string(); - d.assets.insert( - aid, - Arc::new(Asset { page: 0, id: aid, name: name.clone(), mime, link: link.clone(), data: Arc::new(bytes), pixels: Some((pw, ph)) }), - ); - Ok(aid) + Ok(d.add_asset(Asset { page: 0, id: fresh, name: name.clone(), mime, link: link.clone(), data: Arc::new(bytes), pixels: Some((pw, ph)) })) })?; s.loaded = Some((aid, natural)); s.set_tool("placeGun"); diff --git a/crates/engine/src/cmd/links.rs b/crates/engine/src/cmd/links.rs index c05c6dad..73de3ea7 100644 --- a/crates/engine/src/cmd/links.rs +++ b/crates/engine/src/cmd/links.rs @@ -112,8 +112,10 @@ pub fn status(a: &designcraft_doc::Asset) -> &'static str { } fn list(d: &Document) -> Vec { + let used = d.used_assets(); d.assets .values() + .filter(|a| used.contains(&a.id)) .map(|a| { let uses: Vec = uses(d, a.id) .into_iter() @@ -311,3 +313,118 @@ mod relink_folder_tests { let _ = std::fs::remove_dir_all(&dir); } } + +#[cfg(test)] +mod shared_asset_tests { + use super::*; + + fn png(w: u32, h: u32) -> Vec { + designcraft_render::Rendered { width: w, height: h, pixels: vec![200u8; (w * h * 4) as usize] }.to_png() + } + + #[test] + fn placements_of_one_file_share_an_asset_and_saves_drop_replaced_ones() { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + let a = super::super::file::base64_encode(&png(10, 10)); + s.execute("file.place", &json!({"base64": a, "name": "a.png", "x": 72, "y": 72})).unwrap(); + s.execute("edit.deselectAll", &json!({})).unwrap(); + s.execute("file.place", &json!({"base64": a, "name": "a.png", "x": 200, "y": 72})).unwrap(); + let l = s.execute("links.list", &json!({})).unwrap(); + assert_eq!(l.as_array().unwrap().len(), 1); + assert_eq!(l[0]["uses"].as_array().unwrap().len(), 2); + assert_eq!(s.doc().unwrap().doc.assets.len(), 1); + + // A new image into a frame: the old asset stays in memory (undo) but not in the file. + let f = s.execute("frame.create", &json!({"rect": [100, 300, 200, 400]})).unwrap()["id"].as_u64().unwrap(); + let b = super::super::file::base64_encode(&png(20, 10)); + let c = super::super::file::base64_encode(&png(30, 10)); + s.execute("file.place", &json!({"base64": b, "name": "b.png", "frame": f})).unwrap(); + s.execute("file.place", &json!({"base64": c, "name": "c.png", "frame": f})).unwrap(); + let d = &s.doc().unwrap().doc; + assert_eq!(d.assets.len(), 3); + assert_eq!(s.execute("links.list", &json!({})).unwrap().as_array().unwrap().len(), 2); + let back = designcraft_format::load(&designcraft_format::save(&s.doc().unwrap().doc).unwrap()).unwrap(); + let mut names: Vec<&str> = back.assets.values().map(|a| a.name.as_str()).collect(); + names.sort(); + assert_eq!(names, ["a.png", "c.png"]); + } + + #[test] + fn files_with_identical_bytes_at_different_paths_keep_their_own_links() { + let dir = std::env::temp_dir().join(format!("dc-same-bytes-{}", std::process::id())); + let _ = std::fs::remove_dir_all(&dir); + std::fs::create_dir_all(&dir).unwrap(); + let (a, b) = (dir.join("a.png"), dir.join("b.png")); + std::fs::write(&a, png(10, 10)).unwrap(); + std::fs::write(&b, png(10, 10)).unwrap(); + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + for (path, x) in [(&a, 72), (&b, 200)] { + s.execute("edit.deselectAll", &json!({})).unwrap(); + s.execute("file.place", &json!({"path": path.to_string_lossy(), "x": x, "y": 72})).unwrap(); + } + assert_eq!(s.doc().unwrap().doc.assets.len(), 2); + let rows = |s: &mut Session| -> Vec<(String, Value)> { + let l = s.execute("links.list", &json!({})).unwrap(); + l.as_array().unwrap().iter().map(|r| (r["path"].as_str().unwrap().to_string(), r["pixels"].clone())).collect() + }; + let paths = |r: &[(String, Value)]| r.iter().map(|(p, _)| p.clone()).collect::>(); + let before = rows(&mut s); + assert_eq!(paths(&before), [a.to_string_lossy(), b.to_string_lossy()]); + + // b.png changes on disk: updating it leaves a.png's placement alone. + std::fs::write(&b, png(20, 10)).unwrap(); + assert_eq!(s.execute("links.update", &json!({})).unwrap()["updated"], 1); + let after = rows(&mut s); + assert_eq!(paths(&after), paths(&before)); + assert_eq!((after[0].1.clone(), after[1].1.clone()), (json!([10, 10]), json!([20, 10]))); + let _ = std::fs::remove_dir_all(&dir); + } + + #[test] + fn pasted_graphics_reuse_the_identical_asset_of_the_target() { + use designcraft_doc::build::NewDocument; + use designcraft_doc::{Asset, Graphic, Item, ItemId, Shape, SpreadRef}; + let asset = |id: u64, data: Vec| Asset { + id: AssetId(id), + name: "p.png".into(), + mime: "image/png".into(), + link: None, + data: Arc::new(data), + pixels: None, + page: 0, + }; + let mut src = Document::new(&NewDocument::default()); + let mut dst = Document::new(&NewDocument::default()); + let (pa, pb) = (png(4, 4), png(8, 4)); + // The source's asset id is taken in the target by a different file. + let sid = AssetId(src.alloc()); + src.assets.insert(sid, Arc::new(asset(sid.0, pa.clone()))); + dst.assets.insert(sid, Arc::new(asset(sid.0, pb))); + dst.alloc(); + let same = AssetId(dst.alloc()); + assert_ne!(same, sid); + dst.assets.insert(same, Arc::new(asset(same.0, pa))); + let iid = ItemId(src.alloc()); + let mut it = Item::new( + iid, + src.default_layer(), + Shape::Rectangle, + designcraft_geom::shapes::rectangle(designcraft_geom::Rect::new(0.0, 0.0, 9.0, 9.0)), + ); + it.content = Content::Graphic(Graphic { + asset: sid, + size: (4.0, 4.0), + xf: designcraft_geom::Affine::IDENTITY, + auto_fit: Default::default(), + fit_align: 4, + crop: [1.0; 4], + }); + src.insert_item(SpreadRef::Doc(0), it, None).unwrap(); + let ids = super::super::object::duplicate_from(&mut dst, &src, &[iid], SpreadRef::Doc(0), designcraft_geom::Vec2::ZERO).unwrap(); + let g = dst.item(ids[0]).and_then(|i| i.graphic()).unwrap(); + assert_eq!((g.asset, g.crop), (same, [1.0; 4])); + assert_eq!(dst.assets.len(), 2); + } +} diff --git a/crates/engine/src/cmd/mathexpr.rs b/crates/engine/src/cmd/mathexpr.rs index 791a0d70..ec90d52e 100644 --- a/crates/engine/src/cmd/mathexpr.rs +++ b/crates/engine/src/cmd/mathexpr.rs @@ -13,11 +13,16 @@ use crate::{Result, Session}; fn math_item(d: &mut designcraft_doc::Document, latex: &str, size: f64, lid: designcraft_doc::LayerId, at: (f64, f64)) -> Item { let (svg, (w, h)) = crate::math::to_svg(latex, size); - let aid = AssetId(d.alloc()); - d.assets.insert( - aid, - Arc::new(Asset { page: 0, id: aid, name: "math.svg".into(), mime: "image/svg+xml".into(), link: None, data: Arc::new(svg), pixels: None }), - ); + let fresh = AssetId(d.alloc()); + let aid = d.add_asset(Asset { + page: 0, + id: fresh, + name: "math.svg".into(), + mime: "image/svg+xml".into(), + link: None, + data: Arc::new(svg), + pixels: None, + }); let id = ItemId(d.alloc()); let r = Rect::new(at.0, at.1, at.0 + w, at.1 + h); let mut it = Item::new(id, lid, Shape::Rectangle, shapes::rectangle(r)); diff --git a/crates/engine/src/cmd/media.rs b/crates/engine/src/cmd/media.rs index b8008958..b6e4cd4b 100644 --- a/crates/engine/src/cmd/media.rs +++ b/crates/engine/src/cmd/media.rs @@ -25,8 +25,8 @@ pub(crate) fn place_media(s: &mut Session, p: &Value, name: String, mime: &str, let (px, py) = (p.get("x").and_then(Value::as_f64), p.get("y").and_then(Value::as_f64)); let mime = mime.to_string(); s.edit(|d, sel| { - let aid = AssetId(d.alloc()); - d.assets.insert(aid, Arc::new(Asset { page: 0, id: aid, name, mime, link, data: Arc::new(bytes), pixels: None })); + let fresh = AssetId(d.alloc()); + let aid = d.add_asset(Asset { page: 0, id: fresh, name, mime, link, data: Arc::new(bytes), pixels: None }); let id = match target { Some(fid) => { let it = d.item_mut(fid).ok_or(designcraft_doc::DocError::NoItem(fid))?; @@ -90,10 +90,9 @@ pub fn specs() -> Vec { s.edit(|d, _| { let poster_id = match poster { Some(Some((bytes, name, link, px))) => { - let aid = AssetId(d.alloc()); + let fresh = AssetId(d.alloc()); let mime = designcraft_render::image_mime(&bytes).to_string(); - d.assets.insert(aid, Arc::new(Asset { page: 0, id: aid, name, mime, link, data: Arc::new(bytes), pixels: Some(px) })); - Some(Some(aid)) + Some(Some(d.add_asset(Asset { page: 0, id: fresh, name, mime, link, data: Arc::new(bytes), pixels: Some(px) }))) } Some(None) => Some(None), None => None, diff --git a/crates/engine/src/cmd/object.rs b/crates/engine/src/cmd/object.rs index c4f2562a..ca405a18 100644 --- a/crates/engine/src/cmd/object.rs +++ b/crates/engine/src/cmd/object.rs @@ -900,6 +900,11 @@ pub fn duplicate_from(dst: &mut Document, src: &Document, ids: &[ItemId], to: Sp fn renumber(dst: &mut Document, src: &Document, it: &mut Item, stories: &mut HashMap) { it.id = ItemId(dst.alloc()); let my_id = it.id; + if let Some(m) = it.media.as_mut() + && let Some(p) = m.poster + { + m.poster = dst.adopt_asset(src, p).or(Some(p)); + } match &mut it.content { Content::Text(tf) => { let new_sid = match stories.get(&tf.story) { @@ -908,6 +913,15 @@ fn renumber(dst: &mut Document, src: &Document, it: &mut Item, stories: &mut Has let n = StoryId(dst.alloc()); let mut st = src.story(tf.story).cloned().unwrap_or_else(|| Story::new(n)); st.id = n; + let mut assets = HashMap::new(); + for a in designcraft_doc::story_asset_refs(&st) { + if let Some(na) = dst.adopt_asset(src, a) + && na != a + { + assets.insert(a, na); + } + } + designcraft_doc::remap_story_assets(&mut st, &assets); st.frames.clear(); dst.stories.insert(n, Arc::new(st)); stories.insert(tf.story, n); @@ -920,10 +934,8 @@ fn renumber(dst: &mut Document, src: &Document, it: &mut Item, stories: &mut Has } } Content::Graphic(g) => { - if let Some(a) = src.assets.get(&g.asset) - && !dst.assets.contains_key(&g.asset) - { - dst.assets.insert(g.asset, a.clone()); + if let Some(a) = dst.adopt_asset(src, g.asset) { + g.asset = a; } } Content::Group { items } => { diff --git a/crates/engine/src/cmd/package.rs b/crates/engine/src/cmd/package.rs index 46866751..25f09537 100644 --- a/crates/engine/src/cmd/package.rs +++ b/crates/engine/src/cmd/package.rs @@ -285,13 +285,13 @@ fn write_links(d: &Document, root: &OutputRoot, dir: &Path, only: Option<&[Asset let mut out = d.clone(); let mut taken = Vec::new(); let mut written = Vec::new(); - let mut ids: Vec = d.assets.keys().copied().collect(); - ids.sort_by_key(|i| i.0); + // Only files something places; the saved document leaves the others out. + let ids = d.used_assets(); for id in ids { if only.is_some_and(|o| !o.contains(&id)) { continue; } - let a = &d.assets[&id]; + let Some(a) = d.assets.get(&id) else { continue }; let name = link_name(a.link.as_deref().unwrap_or(&a.name), id, &mut taken); let path = dir.join(&name); root.write(&path, a.data.as_slice())?; diff --git a/crates/engine/src/cmd/table.rs b/crates/engine/src/cmd/table.rs index 9929c1c7..c2313170 100644 --- a/crates/engine/src/cmd/table.rs +++ b/crates/engine/src/cmd/table.rs @@ -1402,20 +1402,10 @@ fn place_graphic(s: &mut Session, p: &Value) -> Result { let (pw, ph) = designcraft_render::image_size(&bytes).ok_or_else(|| bad(ID, "unsupported or corrupt image"))?; let fill = p.get("fit").and_then(Value::as_str) == Some("fill"); s.edit(|d, sel| { - let aid = designcraft_doc::AssetId(d.alloc()); + let fresh = designcraft_doc::AssetId(d.alloc()); let mime = designcraft_render::image_mime(&bytes).to_string(); - d.assets.insert( - aid, - std::sync::Arc::new(designcraft_doc::Asset { - page: 0, - id: aid, - name, - mime, - link, - data: std::sync::Arc::new(bytes), - pixels: Some((pw, ph)), - }), - ); + let aid = + d.add_asset(designcraft_doc::Asset { page: 0, id: fresh, name, mime, link, data: std::sync::Arc::new(bytes), pixels: Some((pw, ph)) }); let st = d.story_mut(g.story).ok_or_else(|| bad(ID, "no story"))?; let t = st.table_mut(g.table).ok_or_else(|| bad(ID, "no table"))?; let (r, c) = t.owner(g.range.r0, g.range.c0); diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index f80ad0a6..408bb3cc 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -2,14 +2,15 @@ //! //! - `mimetype` (stored first, uncompressed): `application/vnd.designcraft+zip` //! - `document.json`: the [`Document`] (pretty JSON, documented by the serde model in `designcraft-doc`) -//! - `assets/.`: embedded placed files, byte-for-byte +//! - `assets/.`: embedded placed files, byte-for-byte: one per distinct file, and only +//! the ones the document references (see [`Document::compact_assets`]) //! - `meta.json`: format version and generator //! //! Older single-file JSON documents (assets as base64 in `assetData`) still open. #![deny(clippy::unwrap_used, clippy::expect_used, clippy::panic, clippy::unimplemented, clippy::todo, clippy::unreachable)] #![forbid(unsafe_code)] -use std::collections::HashSet; +use std::collections::{HashMap, HashSet}; use std::io::{Cursor, Read, Write}; use std::sync::Arc; @@ -42,8 +43,12 @@ fn ext_for(mime: &str) -> &'static str { } } -/// Serialize a document to `.designcraft` bytes. +/// Serialize a document to `.designcraft` bytes. The file holds each distinct placed file once +/// and leaves out assets nothing references; `doc` itself keeps them (undo can bring them back). pub fn save(doc: &Document) -> Result, FormatError> { + let mut compact = doc.clone(); + compact.compact_assets(); + let doc = &compact; let io = |e: zip::result::ZipError| FormatError::Io(e.to_string()); let mut buf = Cursor::new(Vec::new()); { @@ -100,13 +105,26 @@ fn load_zip(bytes: &[u8]) -> Result { upgrade(&mut v); let mut doc: Document = serde_json::from_value(v).map_err(|e| FormatError::NotOurs(e.to_string()))?; let names: Vec = z.file_names().map(str::to_string).collect(); + // Files saved with one copy per placement: identical copies share one buffer as they are + // read, so memory holds each distinct file once. + let mut loaded: HashMap>>> = HashMap::new(); for name in names { let Some(stem) = name.strip_prefix("assets/") else { continue }; let Some(id) = stem.split('.').next().and_then(|s| s.parse::().ok()) else { continue }; if let (Some(data), Some(a)) = (read(&mut z, &name), doc.assets.get_mut(&AssetId(id))) { - Arc::make_mut(a).data = Arc::new(data); + let same = loaded.entry(data.len()).or_default(); + let data = match same.iter().find(|d| ***d == data) { + Some(d) => d.clone(), + None => { + let d = Arc::new(data); + same.push(d.clone()); + d + } + }; + Arc::make_mut(a).data = data; } } + doc.merge_duplicate_assets(); doc.check().map_err(|e| FormatError::NotOurs(e.to_string()))?; Ok(doc) } @@ -124,6 +142,7 @@ fn load_legacy_json(bytes: &[u8]) -> Result { } } } + doc.merge_duplicate_assets(); doc.check().map_err(|e| FormatError::NotOurs(e.to_string()))?; Ok(doc) } @@ -205,24 +224,67 @@ mod tests { use designcraft_doc::geom::Rect; use designcraft_doc::{Asset, ParaFormat, SpreadRef}; + fn png_asset(d: &mut Document, data: &[u8]) -> AssetId { + let aid = AssetId(d.alloc()); + let a = Asset { + page: 0, + id: aid, + name: "x.png".into(), + mime: "image/png".into(), + link: None, + data: Arc::new(data.to_vec()), + pixels: Some((1, 1)), + }; + d.assets.insert(aid, Arc::new(a)); + aid + } + + fn place(d: &mut Document, asset: AssetId, crop: f64) -> designcraft_doc::ItemId { + use designcraft_doc::geom::{Affine, shapes}; + use designcraft_doc::{Content, Graphic, Item, ItemId, Shape}; + let id = ItemId(d.alloc()); + let mut it = Item::new(id, d.default_layer(), Shape::Rectangle, shapes::rectangle(Rect::new(0.0, 0.0, 50.0, 50.0))); + it.content = Content::Graphic(Graphic { + asset, + size: (1.0, 1.0), + xf: Affine::translate((crop, crop)), + auto_fit: Default::default(), + fit_align: 4, + crop: [crop; 4], + }); + d.insert_item(SpreadRef::Doc(0), it, None).unwrap(); + id + } + + fn asset_entries(bytes: &[u8]) -> usize { + let z = zip::ZipArchive::new(Cursor::new(bytes)).unwrap(); + z.file_names().filter(|n| n.starts_with("assets/")).count() + } + + /// Zip `d` the way files were saved with one asset per placement (nothing merged or pruned). + fn save_uncompacted(d: &Document) -> Vec { + let mut buf = Cursor::new(Vec::new()); + let mut z = zip::ZipWriter::new(&mut buf); + let opts = zip::write::SimpleFileOptions::default(); + z.start_file("mimetype", opts).unwrap(); + z.write_all(MIME.as_bytes()).unwrap(); + z.start_file("document.json", opts).unwrap(); + z.write_all(&serde_json::to_vec(d).unwrap()).unwrap(); + for (id, a) in &d.assets { + z.start_file(format!("assets/{}.png", id.0), opts).unwrap(); + z.write_all(&a.data).unwrap(); + } + z.finish().unwrap(); + buf.into_inner() + } + #[test] fn roundtrip_with_assets() { let mut d = Document::new(&NewDocument { pages: 3, ..Default::default() }); let lid = d.default_layer(); d.add_text_frame(SpreadRef::Doc(0), Rect::new(10.0, 10.0, 200.0, 200.0), lid, "héllo\nworld", ParaFormat::default()).unwrap(); - let aid = AssetId(d.alloc()); - d.assets.insert( - aid, - Arc::new(Asset { - page: 0, - id: aid, - name: "x.png".into(), - mime: "image/png".into(), - link: None, - data: Arc::new(vec![1, 2, 3, 4]), - pixels: Some((1, 1)), - }), - ); + let aid = png_asset(&mut d, &[1, 2, 3, 4]); + place(&mut d, aid, 0.0); let bytes = save(&d).unwrap(); assert!(bytes.starts_with(b"PK")); let back = load(&bytes).unwrap(); @@ -231,6 +293,63 @@ mod tests { assert_eq!(back.stories, d.stories); } + #[test] + fn saves_hold_each_used_file_once() { + let mut d = Document::new(&NewDocument::default()); + let first = png_asset(&mut d, &[7; 64]); + let copy = png_asset(&mut d, &[7; 64]); + let replaced = png_asset(&mut d, &[9; 64]); + let a = place(&mut d, first, 1.0); + let b = place(&mut d, copy, 2.0); + assert_eq!(d.assets.len(), 3); + + // Duplicates merge into the first asset, the unreferenced one is left out, and every + // placement keeps its own crop and transform. + let bytes = save(&d).unwrap(); + assert_eq!(asset_entries(&bytes), 1); + assert_eq!(d.assets.len(), 3, "the open document keeps its assets"); + let back = load(&bytes).unwrap(); + assert_eq!(back.assets.keys().copied().collect::>(), vec![first]); + assert!(!back.assets.contains_key(&replaced)); + for (id, crop) in [(a, 1.0), (b, 2.0)] { + let g = back.item(id).and_then(|i| i.graphic()).unwrap(); + assert_eq!((g.asset, g.crop, g.xf), (first, [crop; 4], d.item(id).unwrap().graphic().unwrap().xf)); + } + } + + #[test] + fn files_with_identical_bytes_at_different_paths_keep_their_paths() { + let mut d = Document::new(&NewDocument::default()); + let ids: Vec = (0..3).map(|_| png_asset(&mut d, &[3; 32])).collect(); + for (id, link) in ids.iter().zip(["/p/a.png", "/q/b.png"]) { + if let Some(a) = d.assets.get_mut(id) { + Arc::make_mut(a).link = Some(link.into()); + } + } + let items: Vec<_> = ids.iter().enumerate().map(|(k, id)| place(&mut d, *id, k as f64)).collect(); + for bytes in [save(&d).unwrap(), save_uncompacted(&d)] { + let back = load(&bytes).unwrap(); + assert_eq!(back.assets.len(), 3); + let links: Vec> = items + .iter() + .map(|i| back.item(*i).and_then(|i| i.graphic()).and_then(|g| back.assets.get(&g.asset)).unwrap().link.as_deref()) + .collect(); + assert_eq!(links, [Some("/p/a.png"), Some("/q/b.png"), None]); + } + } + + #[test] + fn older_files_with_one_asset_per_placement_open_with_one() { + let mut d = Document::new(&NewDocument::default()); + let ids: Vec = (0..3).map(|_| png_asset(&mut d, &[5; 32])).collect(); + for (k, id) in ids.iter().enumerate() { + place(&mut d, *id, k as f64); + } + let back = load(&save_uncompacted(&d)).unwrap(); + assert_eq!(back.assets.len(), 1); + assert_eq!(asset_entries(&save(&back).unwrap()), 1); + } + /// The object of the item `id` in a document's JSON. fn item_json(v: &mut Value, id: u64) -> Option<&mut Value> { if v.get("id").and_then(Value::as_u64) == Some(id) && v.get("content").is_some() { diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index d85d9ee0..13877526 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -132,6 +132,8 @@ struct Importer<'r> { page_index: HashMap, item_ids: HashMap, assets: BTreeMap>, + /// Assets read from a link path, so later placements of that path reuse them unread. + linked_assets: HashMap, ctx: ItemCtx, sections: Vec
, footnote_options: designcraft_doc::FootnoteOptions, @@ -217,6 +219,7 @@ impl<'r> Importer<'r> { page_index: HashMap::new(), item_ids: HashMap::new(), assets: BTreeMap::new(), + linked_assets: HashMap::new(), ctx: ItemCtx { threads: Vec::new() }, sections: Vec::new(), footnote_options: Default::default(), @@ -2205,12 +2208,29 @@ impl<'r> Importer<'r> { let size = ((r - l).abs(), (b - t).abs()); let link = g.find("Link"); let uri = link.and_then(|k| k.get("LinkResourceURI")).map(uri_to_path); + let graphic = |asset| { + Content::Graphic(designcraft_doc::Graphic { + asset, + size, + xf: gxf * Affine::translate((l, t)), + auto_fit: Default::default(), + fit_align: 4, + crop: [0.0; 4], + }) + }; let mut data = g.prop_el("Contents").map(|c| base64_decode(&c.text_content())).unwrap_or_default(); + let mut read_from = None; if data.is_empty() && let Some(p) = &uri - && let Some(d) = (self.read_link)(p) { - data = d; + // Every placement of a linked file shares one asset; read the file once. + if let Some(id) = self.linked_assets.get(p) { + return graphic(*id); + } + if let Some(d) = (self.read_link)(p) { + data = d; + read_from = Some(p.clone()); + } } let (mime, px) = sniff_image(&data); let mime = mime.map(str::to_string).unwrap_or_else(|| match link.and_then(|k| k.get("LinkResourceFormat")).unwrap_or("") { @@ -2226,17 +2246,14 @@ impl<'r> Importer<'r> { }); let name = uri.as_deref().and_then(|p| p.rsplit(['/', '\\']).next()).filter(|n| !n.is_empty()).map(str::to_string).unwrap_or_else(|| "image".into()); - let id = AssetId(self.alloc()); + let fresh = AssetId(self.alloc()); let link_path = uri.filter(|p| p.contains('/') || p.contains('\\')); - self.assets.insert(id, Arc::new(Asset { page: 0, id, name, mime, link: link_path, data: Arc::new(data), pixels })); - Content::Graphic(designcraft_doc::Graphic { - asset: id, - size, - xf: gxf * Affine::translate((l, t)), - auto_fit: Default::default(), - fit_align: 4, - crop: [0.0; 4], - }) + let id = + designcraft_doc::insert_asset(&mut self.assets, Asset { page: 0, id: fresh, name, mime, link: link_path, data: Arc::new(data), pixels }); + if let Some(p) = read_from { + self.linked_assets.insert(p, id); + } + graphic(id) } fn link_threads(&mut self) { diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 7b747587..4c781700 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -7,6 +7,7 @@ use designcraft_geom::{Rect, shapes}; use super::*; +mod assets; mod decorations; fn zip_files(files: &[(&str, &str)]) -> Vec { diff --git a/crates/idml/src/tests/assets.rs b/crates/idml/src/tests/assets.rs new file mode 100644 index 00000000..dd276144 --- /dev/null +++ b/crates/idml/src/tests/assets.rs @@ -0,0 +1,110 @@ +//! Placed files: every placement of one file shares an asset. + +use std::cell::Cell; + +use designcraft_doc::{AssetId, Content}; + +use super::*; + +const PHOTO: &str = "file:/photos/wedding%20day/couple.jpg"; + +/// A graphic frame at `x` showing `image` (an `` element) with `crop` on every side. +fn frame(n: usize, x: f64, crop: f64, image: &str) -> String { + format!( + r#" + + + + + {image} + "# + ) +} + +fn image(n: usize, uri: &str, contents: Option<&[u8]>) -> String { + let contents = contents.map(|c| format!("", base64_encode(c))).unwrap_or_default(); + format!( + r#" + {contents} + + "# + ) +} + +fn package(frames: &[String]) -> Vec { + let spread = format!( + r#" + + + + {} + +"#, + frames.join("\n") + ); + zip_files(&[ + ("designmap.xml", DESIGNMAP), + ("Resources/Graphic.xml", GRAPHIC), + ("Resources/Styles.xml", STYLES), + ("Resources/Preferences.xml", PREFS), + ("MasterSpreads/MasterSpread_m1.xml", MASTER), + ("Spreads/Spread_sp1.xml", &spread), + ("Stories/Story_s1.xml", STORY), + ]) +} + +/// (asset, crop, transform) of each graphic, left to right. +fn placements(d: &Document) -> Vec<(AssetId, [f64; 4], designcraft_geom::Affine)> { + let mut out: Vec<(f64, AssetId, [f64; 4], designcraft_geom::Affine)> = d.spreads[0] + .items + .iter() + .filter_map(|i| match &i.content { + Content::Graphic(g) => Some((i.xf.translation().x, g.asset, g.crop, g.xf)), + _ => None, + }) + .collect(); + out.sort_by(|a, b| a.0.total_cmp(&b.0)); + out.into_iter().map(|(_, a, c, x)| (a, c, x)).collect() +} + +#[test] +fn one_file_placed_several_times_imports_as_one_asset() { + let bytes = vec![42u8; 300]; + let reads = Cell::new(0); + let reader = |p: &str| { + reads.set(reads.get() + 1); + p.ends_with("couple.jpg").then(|| bytes.clone()) + }; + // Three placements linking the file, and one embedding the same bytes under another path: + // that one keeps its own asset and path. + let frames = vec![ + frame(1, 0.0, 1.0, &image(1, PHOTO, None)), + frame(2, 100.0, 2.0, &image(2, PHOTO, None)), + frame(3, 200.0, 3.0, &image(3, PHOTO, None)), + frame(4, 300.0, 4.0, &image(4, "file:/elsewhere/copy.jpg", Some(&bytes))), + ]; + let d = import_idml_with(&package(&frames), &reader).unwrap(); + assert_eq!(reads.get(), 1, "the linked file is read once"); + assert_eq!(d.assets.len(), 2); + let p = placements(&d); + assert_eq!(p.iter().map(|(_, c, _)| c[0]).collect::>(), vec![1.0, 2.0, 3.0, 4.0]); + let link = |id| d.assets.get(id).and_then(|a| a.link.as_deref()); + let links: Vec<_> = p.iter().map(|(id, _, _)| link(id)).collect(); + assert_eq!(links[..3], [Some("/photos/wedding day/couple.jpg"); 3]); + assert_eq!(links[3], Some("/elsewhere/copy.jpg")); + assert!(p[..3].iter().all(|(id, _, _)| *id == p[0].0)); + + // Exported linked or embedded, the round trip keeps the two assets and each placement's crop + // and transform. + for embed_images in [false, true] { + let out = export_idml_with(&d, &ExportOptions { embed_images }); + let back = import_idml_with(&out, &reader).unwrap(); + assert_eq!(back.assets.len(), 2, "embed_images: {embed_images}"); + let q = placements(&back); + assert_eq!(q.len(), 4); + for ((_, c0, x0), (_, c1, x1)) in p.iter().zip(&q) { + assert_eq!(c0, c1); + assert!(x0.as_coeffs().iter().zip(x1.as_coeffs()).all(|(a, b)| (a - b).abs() < 1e-6), "{x0:?} {x1:?}"); + } + } +} From 95c8c9cc5bc28605530e67e9d34b8cdd6331d8cc Mon Sep 17 00:00:00 2001 From: moppeeer Date: Mon, 12 Oct 2026 03:32:49 +0200 Subject: [PATCH 4/5] fix(hyphenation): honor capitalized-word settings for uppercase words Honor the capitalized-word hyphenation setting for uppercase words and retain the current roadmap entries. Validation: all six cargo xtask ci gates passed on the current main merge candidate; 1,170 native tests passed and all 15 WASM packages checked. Task: #538 Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com> --- ROADMAP.md | 1 + crates/compose/src/hyphen/mod.rs | 10 ++++++---- 2 files changed, 7 insertions(+), 4 deletions(-) diff --git a/ROADMAP.md b/ROADMAP.md index 82a42cf1..ec8eb092 100644 --- a/ROADMAP.md +++ b/ROADMAP.md @@ -129,6 +129,7 @@ list) · [roadmap.md](docs/roadmap.md) (milestones, plan) · [parity-checklist.m Newest first. +- **2026-10-12** · Words in capitals hyphenate like capitalized words (FERRUM COM-MODI), as in InDesign; Hyphenate Capitalized Words off keeps them whole. No readiness percentage change. - **2026-10-11 (printing):** `file.printers` and `file.print` use craft-libs/craft-print for CUPS and Windows. DesignCraft retains PDF export, page selection, spreads, marks and bleed; dry runs create no temporary PDF or print job. Windows selects matching or default driver paper and shrinks oversized sheets without enlarging smaller ones; queue duplex defaults are preserved. Headless tests exercise exported bytes, job settings, dry runs and refusals; shared native Windows CI passes, including paused-queue submission and cleanup. PostScript, production print controls and physical output validation remain open. No readiness percentage change. - **2026-10-10 (UI.4–UI.5):** Panel tabs can move into other groups, split the dock, float, resize and return to their saved locations through shared docking transactions. Layouts persist in custom workspaces; UI and control commands share the same validated action path. Shared pane geometry and widget-state theme application retain all five palettes and local fonts. Split panes send keyboard input only to the focused canvas and scope contextual toolbar IDs per pane. No readiness percentage change. diff --git a/crates/compose/src/hyphen/mod.rs b/crates/compose/src/hyphen/mod.rs index e21ade03..e3f4a43b 100644 --- a/crates/compose/src/hyphen/mod.rs +++ b/crates/compose/src/hyphen/mod.rs @@ -233,9 +233,8 @@ pub fn hyphen_points_in(word: &str, lim: &Limits, lang: Lang) -> Vec { if letters < lim.min_word.max(2) { return false; } - if letters > 1 && run.iter().filter(|c| c.is_alphabetic()).all(|c| c.is_uppercase()) { - return false; - } + // Words in capitals (FERRUM COMMODI) hyphenate like capitalized words, as InDesign's: Hyphenate + // Capitalized Words off keeps both, acronyms included, whole. if !lim.capitalized && run[0].is_uppercase() { return false; } @@ -333,7 +332,10 @@ mod tests { #[test] fn case_apostrophes_and_punctuation() { assert_eq!(h("Typography"), "Ty-pog-ra-phy"); - assert_eq!(h("TYPOGRAPHY"), "TYPOGRAPHY"); + assert_eq!(h("TYPOGRAPHY"), "TY-POG-RA-PHY"); + assert_eq!(h("COMMODI"), "COM-MO-DI"); + let whole = Limits { capitalized: false, ..Limits::default() }; + assert_eq!(hyphenate_word("TYPOGRAPHY", &whole), "TYPOGRAPHY", "Hyphenate Capitalized Words off"); assert_eq!(h("typography,"), "ty-pog-ra-phy,"); assert_eq!(h("“typography”"), "“ty-pog-ra-phy”"); assert_eq!(h("typography's"), "ty-pog-ra-phy's"); From f70f93de441df0879ca56adf4ec89f2eda77c323 Mon Sep 17 00:00:00 2001 From: aiongg <67543387+aiongg@users.noreply.github.com> Date: Sun, 11 Oct 2026 21:39:34 -0400 Subject: [PATCH 5/5] fix(idml): report missing parts and show import warnings Match IDML part names leniently, report missing parts, and show import warnings when opening a file. Cover the malformed URI crash with a regression test. Validation: all six cargo xtask ci gates passed on the current main merge candidate; 1,176 native tests passed and all 15 WASM packages checked. Hosted FreeBSD tests and Windows ARM64 packaging and install/run checks passed. Task: #356 Co-authored-by: skylartaylor <26588250+skylartaylor@users.noreply.github.com> --- Cargo.lock | 1 + crates/engine/src/cmd/file.rs | 2 +- crates/engine/src/cmd/interchange.rs | 17 +++-- crates/idml/src/import.rs | 100 ++++++++++++++++++------- crates/idml/src/lib.rs | 2 +- crates/idml/src/tests.rs | 54 ++++++++++++++ crates/ui-egui/Cargo.toml | 1 + crates/ui-egui/src/lib.rs | 108 +++++++++++++++++++++++++++ 8 files changed, 253 insertions(+), 32 deletions(-) diff --git a/Cargo.lock b/Cargo.lock index 7e5ebb85..05729755 100644 --- a/Cargo.lock +++ b/Cargo.lock @@ -1160,6 +1160,7 @@ dependencies = [ "unicode-bidi", "unicode-bidi-mirroring", "unicode-segmentation", + "zip", ] [[package]] diff --git a/crates/engine/src/cmd/file.rs b/crates/engine/src/cmd/file.rs index 97ded38f..ed126eee 100644 --- a/crates/engine/src/cmd/file.rs +++ b/crates/engine/src/cmd/file.rs @@ -18,7 +18,7 @@ pub fn specs() -> Vec { cmd!(noundo "file.newSample", "Sample Document", ["Help"], None, "{} — a multi-page magazine sample", always, file_sample), cmd!(query "file.presets", "Document Presets", [], None, "{}", always, |_, _| Ok(serde_json::to_value(PRESETS).unwrap_or_default())), cmd!(noundo "file.open", "Open…", ["File"], Some("Cmd+O"), - "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped}", + "{path} — .designcraft or .idml; the fonts in a `Document Fonts` folder beside it load first → {index, documentFonts: faces loaded, warnings: font files skipped (and, for .idml, package parts missing)}", always, file_open), cmd!(noundo "file.openBytes", "Open Bytes", [], None, "{name, base64} — DesignCraft JSON or an IDML package", always, file_open_bytes), cmd!(noundo "file.save", "Save", ["File"], Some("Cmd+S"), "{path?}", has_doc, file_save), diff --git a/crates/engine/src/cmd/interchange.rs b/crates/engine/src/cmd/interchange.rs index 8a65ec91..a0af09f5 100644 --- a/crates/engine/src/cmd/interchange.rs +++ b/crates/engine/src/cmd/interchange.rs @@ -13,7 +13,7 @@ pub fn specs() -> Vec { "{path?, embedImages?: true} — writes an IDML package to `path`, or returns {base64} without a path", has_doc, export_idml), cmd!(noundo "file.openIdml", "Open IDML", [], None, - "{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings}", + "{path | base64, name?} — opens an IDML package as a new document (linked images are read next to the file or from its Links/ folder; the fonts in a `Document Fonts` folder beside it load first) → {index, documentFonts, warnings: package parts missing, font files skipped}", always, open_idml), ] } @@ -34,6 +34,11 @@ fn export_idml(s: &mut Session, p: &Value) -> Result { /// Import IDML bytes; `dir` is the folder the package came from (for relative link lookup). pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { + import_report(bytes, dir).map(|(document, _)| document) +} + +/// [`import`], with the problems that did not stop it (parts missing from the package, …). +pub fn import_report(bytes: &[u8], dir: Option<&std::path::Path>) -> Result<(Document, Vec)> { #[cfg(not(target_arch = "wasm32"))] let resolved = std::cell::RefCell::new(std::collections::HashMap::new()); let read = |link: &str| -> Option> { @@ -49,7 +54,8 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { None } }; - let document = designcraft_idml::import_idml_with(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?; + let designcraft_idml::Imported { document, warnings } = + designcraft_idml::import_idml_report(bytes, &read).map_err(|e| EngineError::Other(e.to_string()))?; #[cfg(not(target_arch = "wasm32"))] let document = { let mut document = document; @@ -62,7 +68,7 @@ pub fn import(bytes: &[u8], dir: Option<&std::path::Path>) -> Result { resolve_packaged_links(&mut document, dir); document }; - Ok(document) + Ok((document, warnings)) } #[cfg(not(target_arch = "wasm32"))] @@ -142,14 +148,15 @@ pub(crate) fn open_idml(s: &mut Session, p: &Value) -> Result { } else { return Err(bad("file.openIdml", "missing `path` or `base64`")); }; - let mut d = import(&bytes, dir.as_deref())?; + let (mut d, mut warnings) = import_report(&bytes, dir.as_deref())?; if let Some(n) = name { d.title = n; } - let (fonts, faces, warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) { + let (fonts, faces, font_warnings) = match str_param(p, "path").filter(|_| str_param(p, "base64").is_none()) { Some(path) => super::file::load_document_fonts(&mut d, path), None => (None, 0, Vec::new()), }; + warnings.extend(font_warnings); // Never save over the .idml with the native format: the document starts unsaved. let mut st = DocState::new(d, None); st.fonts = fonts; diff --git a/crates/idml/src/import.rs b/crates/idml/src/import.rs index 13877526..46c3253b 100644 --- a/crates/idml/src/import.rs +++ b/crates/idml/src/import.rs @@ -48,8 +48,24 @@ fn default_reader(path: &str) -> Option> { /// Import with a custom reader for linked files (`path` → bytes). pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option>) -> Result { + import_idml_report(bytes, read_link).map(|r| r.document) +} + +/// An imported IDML document and what the import could not bring in. +#[derive(Debug)] +pub struct Imported { + pub document: Document, + /// One line per problem that did not stop the import (a part listed in `designmap.xml` but + /// absent from the package, …). + pub warnings: Vec, +} + +/// Import with a custom reader for linked files, reporting the problems that did not stop it. +pub fn import_idml_report(bytes: &[u8], read_link: &dyn Fn(&str) -> Option>) -> Result { let mut zip = zip::ZipArchive::new(std::io::Cursor::new(bytes)).map_err(|e| IdmlError::NotIdml(e.to_string()))?; let mut files: HashMap> = HashMap::new(); + // Normalised part name → entry name, first entry wins (zip order, so the choice is stable). + let mut lenient: HashMap = HashMap::new(); for i in 0..zip.len() { let mut f = zip.by_index(i).map_err(|e| IdmlError::NotIdml(e.to_string()))?; if f.is_dir() { @@ -59,35 +75,81 @@ pub fn import_idml_with(bytes: &[u8], read_link: &dyn Fn(&str) -> Option // The size is what the archive claims: don't reserve more than a sane part up front. let mut buf = Vec::with_capacity((f.size() as usize).min(1 << 24)); f.read_to_end(&mut buf).map_err(|e| IdmlError::Part { part: name.clone(), msg: e.to_string() })?; + lenient.entry(normalize_part_name(&name)).or_insert_with(|| name.clone()); files.insert(name, buf); } - if let Some(m) = files.get("mimetype") + let part = |name: &str| files.get(name).or_else(|| lenient.get(&normalize_part_name(name)).and_then(|n| files.get(n))); + if let Some(m) = part("mimetype") && String::from_utf8_lossy(m).trim() != MIMETYPE { return Err(IdmlError::NotIdml(format!("unexpected mimetype `{}`", String::from_utf8_lossy(m).trim()))); } - let dm = files.get("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?; + let dm = part("designmap.xml").ok_or_else(|| IdmlError::NotIdml("missing designmap.xml".into()))?; let root = parse(dm).map_err(|msg| IdmlError::Part { part: "designmap.xml".into(), msg })?; if root.local() != "Document" { return Err(IdmlError::NotIdml(format!("designmap root is <{}>", root.name))); } // Flatten includes: every `idPkg:*` child is replaced by the children of its part's root. let mut top: Vec = Vec::new(); + let mut warnings = Vec::new(); for c in root.elements() { if c.name.starts_with("idPkg:") { let Some(src) = c.get("src") else { continue }; - let Some(data) = files.get(src) else { continue }; - let part = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?; - top.extend(part.elements().cloned()); + let Some(data) = part(src) else { + warnings.push(format!("{src}: listed in designmap.xml but missing from the package; its contents were not imported")); + continue; + }; + let el = parse(data).map_err(|msg| IdmlError::Part { part: src.into(), msg })?; + top.extend(el.elements().cloned()); } else { top.push(c.clone()); } } let mut im = Importer::new(read_link); im.run(&root, &top)?; - let d = im.finish(&root)?; - d.check().map_err(|e| IdmlError::Invalid(e.to_string()))?; - Ok(d) + let document = im.finish(&root).map_err(|e| match e { + // Without its spread parts there is nothing to open: say which parts were missing. + IdmlError::Invalid(msg) if !warnings.is_empty() => IdmlError::Invalid(format!("{msg} ({})", warnings.join("; "))), + e => e, + })?; + document.check().map_err(|e| IdmlError::Invalid(e.to_string()))?; + Ok(Imported { document, warnings }) +} + +/// A package part name in the form the lookup compares: InDesign and other IDML writers differ +/// in percent-encoding, case, path separators and a leading `./` or `/`. +fn normalize_part_name(name: &str) -> String { + let decoded = percent_decode(&name.replace('\\', "/")); + let mut rest = decoded.as_str(); + loop { + if let Some(r) = rest.strip_prefix("./") { + rest = r; + } else if let Some(r) = rest.strip_prefix('/') { + rest = r; + } else { + break; + } + } + rest.to_lowercase() +} + +/// Decode `%XX` escapes; anything else (including a malformed escape) is kept as written. +pub(crate) fn percent_decode(s: &str) -> String { + let b = s.as_bytes(); + let mut out = Vec::with_capacity(b.len()); + let mut i = 0; + while let Some(&c) = b.get(i) { + if c == b'%' + && let (Some(h), Some(l)) = (b.get(i + 1).and_then(|h| (*h as char).to_digit(16)), b.get(i + 2).and_then(|l| (*l as char).to_digit(16))) + { + out.push((h * 16 + l) as u8); + i += 3; + continue; + } + out.push(c); + i += 1; + } + String::from_utf8_lossy(&out).into_owned() } struct ItemCtx { @@ -2560,21 +2622,9 @@ fn path_of(pg: &El) -> PathData { pub(crate) fn uri_to_path(uri: &str) -> String { let rest = uri.strip_prefix("file://").or_else(|| uri.strip_prefix("file:")).unwrap_or(uri); // `file:///C:/…` → `/C:/…` → `C:/…` - let rest = if rest.len() > 3 && rest.starts_with('/') && rest.as_bytes()[2] == b':' { &rest[1..] } else { rest }; - let mut out = Vec::with_capacity(rest.len()); - let b = rest.as_bytes(); - let mut i = 0; - while i < b.len() { - if b[i] == b'%' - && i + 2 < b.len() - && let Ok(v) = u8::from_str_radix(&rest[i + 1..i + 3], 16) - { - out.push(v); - i += 3; - continue; - } - out.push(b[i]); - i += 1; - } - String::from_utf8_lossy(&out).to_string() + let rest = match rest.as_bytes() { + [b'/', _, b':', _, ..] => rest.get(1..).unwrap_or(rest), + _ => rest, + }; + percent_decode(rest) } diff --git a/crates/idml/src/lib.rs b/crates/idml/src/lib.rs index 71d68825..0526070d 100644 --- a/crates/idml/src/lib.rs +++ b/crates/idml/src/lib.rs @@ -49,7 +49,7 @@ mod xml; mod tests; pub use export::{ExportOptions, export_idml, export_idml_with}; -pub use import::{import_idml, import_idml_with}; +pub use import::{Imported, import_idml, import_idml_report, import_idml_with}; /// The IDML package mimetype (content of the first, stored `mimetype` entry). pub const MIMETYPE: &str = "application/vnd.adobe.indesign-idml-package"; diff --git a/crates/idml/src/tests.rs b/crates/idml/src/tests.rs index 4c781700..23ec99bd 100644 --- a/crates/idml/src/tests.rs +++ b/crates/idml/src/tests.rs @@ -428,6 +428,60 @@ fn imports_scalar_insets_in_object_style_without_replacing_frame_override() { assert_eq!(frame_inset(&d), [3.0; 4]); } +/// A one-page spread part `Self` with one rectangle. +fn one_frame_spread(id: &str) -> String { + format!( + r#" + + + + + + + + + + "# + ) +} + +/// Writers differ in how a designmap `src` spells its part: percent-encoded, another case, +/// backslashes, a leading `./`. Those resolve; a part absent from the package is a warning that +/// names it, not silently dropped objects. +#[test] +fn designmap_parts_match_leniently_and_missing_ones_are_reported() { + let designmap = r#" + + + + "#; + let zip = zip_files(&[ + ("designmap.xml", designmap), + ("Spreads/Spread_u1a b.xml", &one_frame_spread("u1")), + ("Spreads/Spread_u2.xml", &one_frame_spread("u2")), + ]); + let r = import_idml_report(&zip, &|_| None).unwrap(); + assert_eq!(r.document.spreads.len(), 2, "both lenient spellings resolve"); + assert!(r.document.spreads.iter().all(|sp| sp.items.len() == 1), "the spreads keep their frames"); + assert_eq!(r.warnings.len(), 1, "{:?}", r.warnings); + assert!(r.warnings[0].contains("Spreads/Spread_gone.xml"), "{:?}", r.warnings); + + // With every spread part missing the import fails, and the error names the part. + let only_missing = r#" + + "#; + let err = import_idml(&zip_files(&[("designmap.xml", only_missing)])).unwrap_err(); + assert!(err.to_string().contains("Spreads/Spread_gone.xml"), "{err}"); +} + +#[test] +fn percent_decode_keeps_malformed_escapes() { + assert_eq!(import::percent_decode("a%20b%2"), "a b%2"); + assert_eq!(import::percent_decode("%é%zz"), "%é%zz"); + assert_eq!(import::uri_to_path("file:///C:/a%20b"), "C:/a b"); + assert_eq!(import::uri_to_path("file:/a%bé"), "/a%bé"); +} + #[test] fn rejects_non_idml() { assert!(import_idml(b"not a zip").is_err()); diff --git a/crates/ui-egui/Cargo.toml b/crates/ui-egui/Cargo.toml index 92bff320..85090219 100644 --- a/crates/ui-egui/Cargo.toml +++ b/crates/ui-egui/Cargo.toml @@ -30,6 +30,7 @@ log = { workspace = true } [dev-dependencies] designcraft-idml = { workspace = true } +zip = { workspace = true } # Headless (offscreen wgpu) rendering of the whole window for UI screenshots: `examples/ui_shot.rs`. egui_kittest = { workspace = true, features = ["wgpu", "snapshot"] } designcraft-fonts = { workspace = true, features = ["testing"] } diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index c003154a..3d94bdb0 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -812,6 +812,7 @@ impl DesignApp { /// Open (`file.open`, `file.openBytes`) or place (`file.place`) a file the user chose. A file /// that can't be opened or placed gets an alert with the reason, as well as the status line. + /// A successful import with warnings keeps the document and shows the warnings in an alert. pub fn open_file(&mut self, cmd: &str, params: Value) -> Result { let file = match params.get("path").and_then(Value::as_str) { Some(path) => std::path::Path::new(path).file_name().map_or_else(|| path.to_string(), |n| n.to_string_lossy().to_string()), @@ -821,6 +822,20 @@ impl DesignApp { if let Err(e) = &r { let title = if cmd == "file.place" { "Can't Place the File" } else { "Can't Open the File" }; self.ui.dialog = Some(dialogs::Dialog::new("alert", json!({"title": title, "file": file, "message": e}))); + } else if let Ok(result) = &r { + let warnings: Vec<&str> = result + .get("warnings") + .and_then(Value::as_array) + .into_iter() + .flatten() + .filter_map(Value::as_str) + .filter(|warning| !warning.is_empty()) + .collect(); + if !warnings.is_empty() { + self.status(format!("{file}: {}", warnings.join("; "))); + self.ui.dialog = + Some(dialogs::Dialog::new("alert", json!({"title": "Import Warnings", "file": file, "message": warnings.join("\n")}))); + } } r } @@ -1319,6 +1334,99 @@ mod tests { } } +#[cfg(test)] +mod import_warning_tests { + use super::*; + use std::io::Write; + + fn idml(missing_parts: bool) -> Vec { + let missing = if missing_parts { r#""# } else { "" }; + let map = format!( + r#"{missing}"# + ); + let spread = r#""#; + let mut zip = zip::ZipWriter::new(std::io::Cursor::new(Vec::new())); + let options = zip::write::SimpleFileOptions::default().compression_method(zip::CompressionMethod::Stored); + for (name, text) in [("mimetype", designcraft_idml::MIMETYPE), ("designmap.xml", &map), ("Spreads/one.xml", spread)] { + zip.start_file(name, options).unwrap(); + zip.write_all(text.as_bytes()).unwrap(); + } + zip.finish().unwrap().into_inner() + } + + fn app() -> DesignApp { + let mut app = DesignApp::new(Session::new(), Services { inbox: Some(Inbox::default()), ..Default::default() }); + app.run("file.new", json!({"title": "Existing work"})).unwrap(); + app.run("frame.create", json!({"rect": [10, 10, 50, 50]})).unwrap(); + app + } + + fn assert_partial_import_warning(app: &mut DesignApp, file: &str) { + let dialog = app.ui.dialog.as_ref().expect("partial import must show an alert"); + assert_eq!(dialog.id, "alert"); + assert_eq!(dialog.fields["title"], "Import Warnings"); + assert_eq!(dialog.fields["file"], file); + for part in ["Spreads/missing.xml", "Stories/missing.xml"] { + assert!(dialog.fields["message"].as_str().unwrap().contains(part), "every missing part is named"); + assert!(app.ui.status.contains(part), "the status line retains the warning"); + } + assert_eq!(app.session.documents().len(), 2); + assert_eq!(app.session.documents()[0].doc.title, "Existing work"); + assert!(app.session.documents()[0].is_dirty(), "existing unsaved work stays intact"); + assert_eq!(app.session.documents()[0].doc.spreads[0].items.len(), 1); + assert_eq!(app.session.doc().unwrap().doc.spreads.len(), 1); + assert_eq!(app.session.doc().unwrap().doc.spreads[0].items.len(), 1, "available document content is kept"); + dialogs::confirm(app).unwrap(); + assert!(app.ui.dialog.is_none(), "OK dismisses the warning"); + assert_eq!(app.session.documents().len(), 2, "dismissing the warning keeps the imported document"); + } + + #[test] + fn byte_open_reports_partial_import_warnings_without_losing_documents() { + let mut app = app(); + let result = + app.open_file("file.openBytes", json!({"name": "partial.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(true))})).unwrap(); + assert_eq!(result["warnings"].as_array().unwrap().len(), 2); + assert_partial_import_warning(&mut app, "partial.idml"); + } + + #[test] + fn native_open_reports_partial_import_warnings_without_losing_documents() { + let path = std::env::temp_dir().join(format!("designcraft-partial-import-{}", std::process::id())).join("partial.idml"); + std::fs::create_dir_all(path.parent().unwrap()).unwrap(); + std::fs::write(&path, idml(true)).unwrap(); + let picked = path.to_string_lossy().to_string(); + let mut app = app(); + app.services.pick_open = Some(Box::new(move |_| Some(picked.clone()))); + let result = app.run("app.openDialog", json!({})); + std::fs::remove_file(&path).unwrap(); + std::fs::remove_dir(path.parent().unwrap()).unwrap(); + assert_eq!(result.unwrap()["warnings"].as_array().unwrap().len(), 2); + assert_partial_import_warning(&mut app, "partial.idml"); + } + + #[test] + fn inbox_open_reports_partial_import_warnings_without_losing_documents() { + let mut app = app(); + let request = app.import_request("open"); + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request, name: "partial.idml".into(), bytes: idml(true) }); + app.drain_inbox(); + assert_partial_import_warning(&mut app, "partial"); + } + + #[test] + fn complete_import_does_not_show_a_warning() { + let mut app = app(); + let result = app + .open_file("file.openBytes", json!({"name": "complete.idml", "base64": designcraft_engine::cmd::base64_encode(&idml(false))})) + .unwrap(); + assert_eq!(result["warnings"], json!([])); + assert!(app.ui.dialog.is_none()); + assert!(app.ui.status.is_empty()); + assert_eq!(app.session.documents().len(), 2); + } +} + /// The whole window in a headless test harness, at a chosen window size. #[cfg(test)] pub(crate) mod test_window {