From 2633fe8b5dded65f7faeaf666de93b7668fcaa14 Mon Sep 17 00:00:00 2001 From: Andee Collard Date: Sat, 24 Oct 2026 13:15:46 +0100 Subject: [PATCH] Format: choose the native version in one place; share v4 with #1671 skylartaylor's review: #1647 and #1671 each introduced native format 4 for a different capability, so a reader that understood only one could accept a file carrying the other and silently drop its settings. No release has shipped v4 (main and v0.12.0 still write at most 3), so v4 is defined as one capability covering both retained placed-graphic layer options and explicit all-on PDF layer choices; #1671 is stacked on this branch. Move the version choice into required_version so every new capability is decided in one place, document the shared v4 contract on VERSION, and test that plain, EPS-only and layer-option documents keep 2, 3 and 4 respectively. Co-Authored-By: Claude Opus 5.5 --- crates/format/src/lib.rs | 49 ++++++++++++++++++++++++++--------- docs/graphic-layer-options.md | 5 ++++ 2 files changed, 42 insertions(+), 12 deletions(-) diff --git a/crates/format/src/lib.rs b/crates/format/src/lib.rs index 4a3144f08..d119c3538 100644 --- a/crates/format/src/lib.rs +++ b/crates/format/src/lib.rs @@ -22,7 +22,10 @@ use serde_json::{Value, json}; pub const MIME: &str = "application/vnd.designcraft+zip"; /// Latest reader capability. Version 2 pinned the Keep Lines Together and drop cap defaults; -/// version 3 adds rendering of retained EPS source assets; version 4 retains placed graphic layer options. +/// version 3 adds rendering of retained EPS source assets; version 4 retains placed graphic layer +/// options. Version 4 is one capability shared with explicit all-on placed-PDF layer choices +/// (storytold/designcraft#1671): it must not be released until a reader understands both, and any +/// later capability needs version 5. [`required_version`] is the only place that picks a version. pub const VERSION: u32 = 4; #[derive(Debug, thiserror::Error)] @@ -67,6 +70,19 @@ pub fn save_with_preview(doc: &Document, preview: Option<&[u8]>) -> Result u32 { + // Older readers ignore unknown Graphic fields and would silently drop layer choices. + if doc.has_graphic_layer_options() { + return 4; + } + // Older readers would silently omit EPS artwork. Preview-only legacy assets and documents + // without EPS stay at 2 so their existing readers keep working; writing 1 would make a + // reader re-run the version 2 paragraph upgrade over current defaults. + if doc.assets.values().any(|a| designcraft_images::is_eps(&a.data)) { 3 } else { 2 } +} + fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_entry: usize) -> Result, FormatError> { // JSON writes infinity and NaN as null, which no load accepts: the file would not open. if let Some(id) = doc.non_finite_item() { @@ -84,17 +100,7 @@ fn save_with_large_entries_from(doc: &Document, preview: Option<&[u8]>, large_en z.start_file("mimetype", stored).map_err(io)?; z.write_all(MIME.as_bytes()).map_err(|e| FormatError::Io(e.to_string()))?; z.start_file("meta.json", deflate).map_err(io)?; - // Older readers would silently omit EPS artwork. Preview-only legacy assets and - // documents without EPS stay at 2 so their existing readers keep working; writing 1 - // would make a reader re-run the version 2 paragraph upgrade over current defaults. - // Older readers ignore unknown Graphic fields and would silently drop layer choices. - let version = if doc.has_graphic_layer_options() { - 4 - } else if doc.assets.values().any(|a| designcraft_images::is_eps(&a.data)) { - 3 - } else { - 2 - }; + let version = required_version(doc); let meta = json!({"format": "designcraft", "version": version, "generator": concat!("DesignCraft ", env!("CARGO_PKG_VERSION"))}); z.write_all(meta.to_string().as_bytes()).map_err(|e| FormatError::Io(e.to_string()))?; if let Some(png) = preview { @@ -481,6 +487,25 @@ mod tests { assert_eq!(version(&d), 2, "legacy EPS previews stay compatible"); } + #[test] + fn one_place_picks_the_oldest_reader_that_keeps_the_document() { + use designcraft_doc::{Content, GraphicLayerOptions}; + let mut d = Document::new(&NewDocument::default()); + assert_eq!(required_version(&d), 2, "plain documents keep the version 2 reader"); + let aid = AssetId(d.alloc()); + let eps = b"%!PS-Adobe-3.0 EPSF-3.0\n%%BoundingBox: 0 0 20 10\n%%EOF\n".to_vec(); + d.assets.insert( + aid, + Arc::new(Asset { id: aid, name: "logo.eps".into(), mime: "application/postscript".into(), data: Arc::new(eps), ..Default::default() }), + ); + let id = place(&mut d, aid, 0.0); + assert_eq!(required_version(&d), 3, "EPS sources alone keep the version 3 reader"); + let Some(Content::Graphic(g)) = d.item_mut(id).map(|it| &mut it.content) else { panic!("placed graphic") }; + g.layer_options = Some(GraphicLayerOptions { layers: vec![[("Name".to_string(), "Art".to_string())].into()], ..Default::default() }); + assert_eq!(required_version(&d), 4, "retained graphic layer options need version 4 even with EPS"); + assert_eq!(required_version(&load(&save(&d).unwrap()).unwrap()), 4); + } + #[test] fn roundtrip_with_assets() { let mut d = Document::new(&NewDocument { pages: 3, ..Default::default() }); diff --git a/docs/graphic-layer-options.md b/docs/graphic-layer-options.md index 53a527991..f38d82d34 100644 --- a/docs/graphic-layer-options.md +++ b/docs/graphic-layer-options.md @@ -27,6 +27,11 @@ field. Documents without layer settings retain their existing version 2/3 requirements. Older graphics without `layerOptions` still deserialize with `None`, and their JSON representation is unchanged. +Version 4 is a single reader capability shared with explicit all-on placed-PDF +layer choices (#1671): a version 4 reader must understand both, so the two +changes ship in the same release. One function in the format crate, +`required_version`, chooses the version for every save. + Synthetic regressions cover independent overrides on two placements sharing one asset, metadata round trips, native reader requirements, nested cells, nested layer hierarchy, preflight/PDF warnings after rasterization and flattening,