Merge pull request #3997 from dotCooCoo/fix/native-pretty-save-size

Native format: a document is saved pretty-printed only when its pretty-printed JSON is at most 1 MiB

crates/engine/src/cmd/fileio/native.rs

@@ -146,10 +146,13 @@ pub(super) struct Extras {
 /// are left to [`encode`]).
 pub(super) fn save_options(cmd: &str, f: &Format, doc: &Document, p: &Value) -> Result<(SaveOptions, Extras)> {
     let o: NativeOptions = super::encode::options(f, p)?;
-    let mut so = SaveOptions::for_doc(doc);
-    so.compress = o.compress.unwrap_or(false);
-    so.version = o.version.unwrap_or(vectorcraft_format::VERSION);
-    so.include_linked = o.include_linked;
+    // Pretty or compact is decided by [`encode_to`] on the document it writes.
+    let mut so = SaveOptions {
+        compress: o.compress.unwrap_or(false),
+        version: o.version.unwrap_or(vectorcraft_format::VERSION),
+        include_linked: o.include_linked,
+        ..SaveOptions::default()
+    };
     if o.embed_profiles.unwrap_or(true) {
         so.profiles = embedded_profiles(doc);
     }
@@ -190,6 +193,7 @@ pub(super) fn encode_to(cmd: &str, f: &Format, doc: &Document, p: &Value, w: imp
         }
         other => other,
     };
+    so.pretty = so.pretty_fits(&native);
     vectorcraft_format::save_to(&native, &so, w).map_err(|e| bad(cmd, e.to_string()))
 }
 

crates/engine/src/tests_nativefile.rs

@@ -96,6 +96,31 @@ fn saves_for_older_versions() {
     }
 }
 
+#[test]
+fn an_older_version_is_pretty_only_when_its_own_json_fits() {
+    // One path whose anchors fit the pretty size limit in the current version but not as
+    // version 1 writes them (both handles of every anchor as `{x, y}` maps).
+    use vectorcraft_geom::Point;
+    use vectorcraft_geom::path::{PathData, SubPath};
+    let path_doc = |anchors: usize| {
+        let mut b = vectorcraft_testkit::fixtures::DocBuilder::new(1000.0, 1000.0);
+        let points: Vec<Point> = (0..anchors).map(|j| Point::new(j as f64, (j % 2) as f64)).collect();
+        let node = b.path_node(PathData::single(SubPath::polyline(&points, false)), Default::default());
+        b.add(node);
+        b.doc
+    };
+    let per_anchor = serde_json::to_vec_pretty(&path_doc(1000)).unwrap().len() / 1000;
+    let d = path_doc(vectorcraft_format::PRETTY_MAX_BYTES / 2 / per_anchor);
+    let mut s = Session::new();
+    open(&mut s, "traced.vectorcraft", &vectorcraft_format::save_file(&d));
+    let is_pretty = |bytes: &[u8]| bytes.windows(3).any(|w| w == b"\n  ");
+    assert!(is_pretty(&saved(&mut s, json!({"compress": false}))));
+    let old = saved(&mut s, json!({"version": 1}));
+    assert!(!is_pretty(&old), "{} bytes", old.len());
+    open(&mut s, "old.drawcraft", &old);
+    assert_eq!(s.doc().unwrap().doc.node_count(), 2);
+}
+
 #[test]
 fn preview_is_at_most_256_pixels() {
     let mut s = session();

crates/format/src/lib.rs

@@ -253,17 +253,25 @@ impl SaveOptions {
         Ok(())
     }
 
-    /// The defaults for saving `doc` to disk: pretty when small (see [`PRETTY_MAX_OBJECTS`] and
-    /// [`PRETTY_MAX_BYTES`]).
+    /// The defaults for saving `doc` to disk: pretty when small (see [`Self::pretty_fits`]).
     pub fn for_doc(doc: &Document) -> Self {
+        let o = Self::default();
+        Self { pretty: o.pretty_fits(doc), ..o }
+    }
+
+    /// Is `doc` small enough to save pretty-printed with these options: at most
+    /// [`PRETTY_MAX_OBJECTS`] objects, and at most [`PRETTY_MAX_BYTES`] of pretty-printed JSON
+    /// as [`save_to`] writes the document with them (in their version)?
+    pub fn pretty_fits(&self, doc: &Document) -> bool {
         let objects: usize = doc.layers.iter().map(|l| l.count()).sum();
-        Self { pretty: objects <= PRETTY_MAX_OBJECTS && compact_size_at_most(doc, PRETTY_MAX_BYTES), ..Self::default() }
+        objects <= PRETTY_MAX_OBJECTS && body(doc, self).is_ok_and(|b| pretty_size_at_most(&b, PRETTY_MAX_BYTES))
     }
 }
 
-/// Is `doc` at most `limit` bytes as compact JSON? Serializing stops as soon as it is past the
-/// limit, so a large document costs no more than `limit` bytes of work.
-fn compact_size_at_most(doc: &Document, limit: usize) -> bool {
+/// Is the document in `b` at most `limit` bytes as pretty-printed JSON, indented as a pretty file
+/// holds it? Serializing stops as soon as it is past the limit, so a large document costs no more
+/// than `limit` bytes of work.
+fn pretty_size_at_most(b: &Body, limit: usize) -> bool {
     struct Capped {
         left: usize,
     }
@@ -276,7 +284,47 @@ fn compact_size_at_most(doc: &Document, limit: usize) -> bool {
             Ok(())
         }
     }
-    serde_json::to_writer(Capped { left: limit }, doc).is_ok()
+    // The document is a member of the file's top-level object, indented one level more.
+    write_body(Indent { inner: Capped { left: limit }, on: true }, b.v1.as_ref(), &b.d, true).is_ok()
+}
+
+/// The document as [`save_to`] writes it, with the images it lists outside the document.
+struct Body {
+    d: Document,
+    /// The version 1 form of `d`, written instead of it.
+    v1: Option<serde_json::Value>,
+    blobs: BTreeMap<String, ImageBlob>,
+    /// Images only linked images show (see [`Document::linked_only_images`]).
+    linked: std::collections::BTreeSet<String>,
+}
+
+/// `doc` as [`save_to`] writes it with `o`: without editing-mode working copies, with placed
+/// documents as their art for older versions, and its images taken out.
+fn body(doc: &Document, o: &SaveOptions) -> Result<Body, FormatError> {
+    let version = o.version;
+    let mut d = doc.without_edit_modes().into_owned();
+    // Older apps don't know placed documents: they get the art the objects show, with its
+    // resources. Otherwise resources only exporting adds are never saved.
+    if version < VERSION && d.has_placed() {
+        d = d.with_placed_art().into_owned();
+        d.placed_as_groups();
+    } else {
+        d.drop_placed_resources();
+    }
+    let linked = d.linked_only_images();
+    // The blobs go in the file's `images` (only those the document uses).
+    let blobs = std::mem::take(&mut d.images);
+    if version < 3 {
+        d.store_legacy_color_profiles();
+    }
+    let v1 = if version == 1 {
+        let mut v = serde_json::to_value(&d).map_err(encode_err)?;
+        legacy::anchors_to_v1(&mut v);
+        Some(v)
+    } else {
+        None
+    };
+    Ok(Body { d, v1, blobs, linked })
 }
 
 /// Serialize a document (pretty = human-diffable) in the current version. Editing-mode working
@@ -289,9 +337,9 @@ pub fn save(doc: &Document, pretty: bool) -> Vec<u8> {
 /// compact, where indentation would multiply the file size and slow opening down.
 pub const PRETTY_MAX_OBJECTS: usize = 2000;
 
-/// Documents whose compact JSON is larger than this are saved compact even with few objects:
-/// pretty-printing spreads every path anchor over several lines, so traced or generated art (in
-/// layers, symbols or patterns alike) would grow about tenfold.
+/// Documents whose pretty-printed JSON is larger than this are saved compact even with few
+/// objects: pretty-printing spreads every path anchor over several lines, so traced or generated
+/// art (in layers, symbols or patterns alike) grows about fivefold.
 pub const PRETTY_MAX_BYTES: usize = 1 << 20;
 
 /// Serialize for saving to disk: pretty when small, compact when large.
@@ -317,28 +365,7 @@ const JSON_CHUNK: usize = 64 << 10;
 pub fn save_to(doc: &Document, o: &SaveOptions, w: impl Write) -> Result<(), FormatError> {
     o.check()?;
     let pretty = o.pretty && !o.compress;
-    let mut d = doc.without_edit_modes().into_owned();
-    // Older apps don't know placed documents: they get the art the objects show, with its
-    // resources. Otherwise resources only exporting adds are never saved.
-    if o.version < VERSION && d.has_placed() {
-        d = d.with_placed_art().into_owned();
-        d.placed_as_groups();
-    } else {
-        d.drop_placed_resources();
-    }
-    let linked = d.linked_only_images();
-    // The blobs go in the file's `images` (only those the document uses).
-    let blobs = std::mem::take(&mut d.images);
-    if o.version < 3 {
-        d.store_legacy_color_profiles();
-    }
-    let v1 = if o.version == 1 {
-        let mut v = serde_json::to_value(&d).map_err(encode_err)?;
-        legacy::anchors_to_v1(&mut v);
-        Some(v)
-    } else {
-        None
-    };
+    let Body { d, v1, blobs, linked } = body(doc, o)?;
     let entries = Entries { blobs: &blobs, linked: &linked, include_linked: o.include_linked };
     let mut out_blobs = Blobs { tail: vec![], at: 0, outside: o.version >= 4 };
     // Images are only referenced by image objects' `key` (wherever the objects are: layers,

crates/format/tests/it/save_options.rs

@@ -127,10 +127,70 @@ fn few_objects_with_many_anchors_are_saved_compact() {
     assert_eq!(load(&bytes).unwrap().layers[0].count(), d.layers[0].count());
 }
 
+/// Anchors per path that make a document of `paths` paths about `bytes` of pretty-printed JSON.
+fn pretty_anchors_for(paths: usize, bytes: usize) -> usize {
+    let per_anchor = serde_json::to_vec_pretty(&paths_doc(1, 1000)).unwrap().len() / 1000;
+    bytes / per_anchor / paths
+}
+
 #[test]
 fn documents_under_the_size_limit_stay_pretty() {
-    let d = paths_doc(2, anchors_for(2, PRETTY_MAX_BYTES / 2));
+    let d = paths_doc(2, pretty_anchors_for(2, PRETTY_MAX_BYTES / 2));
     assert!(SaveOptions::for_doc(&d).pretty);
+    assert!(is_pretty(&save_file(&d)));
+}
+
+#[test]
+fn the_limit_is_on_the_pretty_printed_size() {
+    // One traced-sized path: well under the limit as compact JSON, over it pretty-printed.
+    let d = paths_doc(1, 20_000);
+    let compact = serde_json::to_vec(&d).unwrap().len();
+    let pretty = serde_json::to_vec_pretty(&d).unwrap().len();
+    assert!(compact < PRETTY_MAX_BYTES && pretty > PRETTY_MAX_BYTES, "compact {compact} bytes, pretty {pretty} bytes");
+    assert!(!SaveOptions::for_doc(&d).pretty);
+    let bytes = save_file(&d);
+    assert!(!is_pretty(&bytes));
+    assert_eq!(load(&bytes).unwrap().layers[0].count(), d.layers[0].count());
+}
+
+#[test]
+fn the_files_indentation_counts_toward_the_limit() {
+    // The pretty JSON's size and line breaks; the file indents every line after the first by two spaces more.
+    let measure = |anchors: usize| {
+        let json = serde_json::to_vec_pretty(&paths_doc(1, anchors)).unwrap();
+        (json.len(), json.iter().filter(|&&b| b == b'\n').count())
+    };
+    // The most anchors for which `fits` holds (it holds for 1 and not past twice the estimate).
+    let most = |fits: &dyn Fn((usize, usize)) -> bool| {
+        let (mut lo, mut hi) = (1, 2 * pretty_anchors_for(1, PRETTY_MAX_BYTES));
+        while lo < hi {
+            let mid = (lo + hi).div_ceil(2);
+            if fits(measure(mid)) { lo = mid } else { hi = mid - 1 }
+        }
+        lo
+    };
+    // Without the file's indentation this many anchors fit the limit, with it they do not.
+    let bare = most(&|(len, _)| len <= PRETTY_MAX_BYTES);
+    let (len, newlines) = measure(bare);
+    assert!(len + 2 * newlines > PRETTY_MAX_BYTES, "{len} bytes, {newlines} line breaks");
+    assert!(!SaveOptions::for_doc(&paths_doc(1, bare)).pretty);
+    // Indented, this many fit.
+    let indented = most(&|(len, newlines)| len + 2 * newlines <= PRETTY_MAX_BYTES);
+    assert!(indented < bare);
+    assert!(SaveOptions::for_doc(&paths_doc(1, indented)).pretty);
+    assert!(!SaveOptions::for_doc(&paths_doc(1, indented + 1)).pretty);
+}
+
+#[test]
+fn the_limit_is_on_the_version_written() {
+    // Version 1 writes every anchor with both handles as `{x, y}` maps: this document fits in the
+    // current version and not in version 1.
+    let d = paths_doc(1, pretty_anchors_for(1, PRETTY_MAX_BYTES / 2));
+    let v1 = SaveOptions { version: 1, ..SaveOptions::default() };
+    assert!(SaveOptions::default().pretty_fits(&d));
+    assert!(!v1.pretty_fits(&d));
+    let pretty = save_with(&d, &SaveOptions { pretty: true, ..v1.clone() }).unwrap();
+    assert!(pretty.len() > PRETTY_MAX_BYTES, "{} bytes", pretty.len());
 }
 
 #[test]

docs/mcp.md

@@ -580,6 +580,8 @@ their content). `version: 2` or `1` writes a file older VectorCraft versions ope
 compressed), and `preview: true` embeds a PNG of the first artboard, at most 256 px on its longer side. A save writes
 a temporary file and renames it over the old one, so a failed save never damages the file; only the images the
 document uses are written, and document keys written by a newer version are kept.
+An uncompressed file is indented JSON when the document has at most 2,000 layers and objects and the indented
+document, as the chosen version writes it, is at most 1 MiB; otherwise the file is compact JSON.
 
 ## Resources