From 605a74f19f656c4d8331e0ff0f6ba4e2e2bb0777 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Wed, 11 Nov 2026 20:56:36 -0800 Subject: [PATCH] Artboards: Convert to Artboards gives each new artboard its id without reading every artboard Convert to Artboards found the highest artboard id twice for each object it converted: once for the new artboard's name and once in Document::next_artboard_id. Converting N objects read about N^2 artboards. The command now keeps the highest id as it adds artboards, and Document::next_artboard_id_above gives the next id from it. When an artboard has id u32::MAX, it falls back to next_artboard_id, which takes the smallest free id. The ids and names it gives are unchanged. --- crates/doc/src/document_guides.rs | 13 ++++++++++++ crates/engine/src/cmd/menucmds.rs | 7 +++++-- crates/engine/src/tests/menucmds.rs | 32 +++++++++++++++++++++++++++++ 3 files changed, 50 insertions(+), 2 deletions(-) diff --git a/crates/doc/src/document_guides.rs b/crates/doc/src/document_guides.rs index ca6b5ed88..4ec7aab1f 100644 --- a/crates/doc/src/document_guides.rs +++ b/crates/doc/src/document_guides.rs @@ -503,6 +503,19 @@ impl Document { id } + /// [`Document::next_artboard_id`] when `top` is the highest id the artboards have (`None`: there + /// are none). It reads no artboard unless that id is `u32::MAX`, so a caller that adds many + /// artboards and keeps `top` up to date gets each id in constant time. + pub fn next_artboard_id_above(&self, top: Option) -> u32 { + match top.unwrap_or(0).max(self.retired_artboard_id.get()).checked_add(1) { + Some(id) => { + self.retired_artboard_id.raise(id); + id + } + None => self.next_artboard_id(), + } + } + /// Record that the artboard with `id` was deleted: no later artboard takes its id. pub fn retire_artboard_id(&self, id: u32) { self.retired_artboard_id.raise(id); diff --git a/crates/engine/src/cmd/menucmds.rs b/crates/engine/src/cmd/menucmds.rs index 8ea016db6..90e02bac7 100644 --- a/crates/engine/src/cmd/menucmds.rs +++ b/crates/engine/src/cmd/menucmds.rs @@ -1306,13 +1306,16 @@ fn convert_to_artboards(s: &mut Session, _: &Value) -> Result { let symbols = d.symbol_boxes(); let boxes: Vec<(NodeId, Rect)> = roots.iter().filter_map(|id| Some((*id, d.node(*id)?.geometric_bounds_with(&symbols)?))).collect(); let mut converted = vec![]; + // The highest artboard id the document has, kept up to date as artboards are added. + let mut top = d.artboards.iter().map(|a| a.id).max(); for (id, b) in boxes { if b.width() <= 0.0 || b.height() <= 0.0 { continue; } // Named one past the highest artboard id the document has, whatever id it is given. - let number = d.artboards.iter().map(|a| a.id).max().map_or(1, |m| m.saturating_add(1)); - let aid = d.next_artboard_id(); + let number = top.map_or(1, |m| m.saturating_add(1)); + let aid = d.next_artboard_id_above(top); + top = top.max(Some(aid)); d.artboards.push(vectorcraft_doc::Artboard { id: aid, name: format!("Artboard {number}"), diff --git a/crates/engine/src/tests/menucmds.rs b/crates/engine/src/tests/menucmds.rs index fccbe4a2d..28cb0a159 100644 --- a/crates/engine/src/tests/menucmds.rs +++ b/crates/engine/src/tests/menucmds.rs @@ -1855,6 +1855,38 @@ fn convert_to_artboards_names_by_the_artboards_the_document_has() { assert_eq!(name_after(&|s| _ = s.execute("edit.undo", &json!({})).unwrap()), "Artboard 3"); } +/// Convert to Artboards on several objects gives each new artboard an id above the ids given out +/// before and the deleted artboard's, and names it one past the highest id the artboards have, +/// counting the artboards it has just added. With an artboard whose id is `u32::MAX` (a damaged +/// file), each new artboard takes the smallest free id and the name stays at `u32::MAX`. +#[test] +fn convert_to_artboards_gives_each_object_the_next_id_and_name() { + let convert = |s: &mut Session| { + let ids: Vec = (0..3).map(|i| rect(s, 3000.0 + 300.0 * f64::from(i), 0.0, 200.0, 100.0)).collect(); + sel(s, &ids); + assert_eq!(s.execute("artboard.convertToArtboards", &json!({})).unwrap()["artboards"], 3); + let d = &s.doc().unwrap().doc; + d.artboards.iter().map(|a| (a.id, a.name.clone())).collect::>() + }; + let named = |pairs: &[(u32, &str)]| pairs.iter().map(|&(id, n)| (id, n.to_string())).collect::>(); + + // Artboards 1 and 2 are left after artboard 3 is deleted, so its id stays given out. + let mut s = session(); + s.execute("artboard.new", &json!({})).unwrap(); + s.execute("artboard.new", &json!({})).unwrap(); + s.execute("artboard.delete", &json!({"index": 2})).unwrap(); + let ids: Vec = s.doc().unwrap().doc.artboards.iter().map(|a| a.id).collect(); + assert_eq!(ids, [1, 2]); + let got = convert(&mut s); + assert_eq!(got, named(&[(1, "Artboard 1"), (2, "Artboard 2"), (4, "Artboard 3"), (5, "Artboard 5"), (6, "Artboard 6")])); + + let mut s = session(); + std::sync::Arc::make_mut(&mut s.doc_mut().unwrap().doc).artboards[0].id = u32::MAX; + let got = convert(&mut s); + let top = format!("Artboard {}", u32::MAX); + assert_eq!(got, named(&[(u32::MAX, "Artboard 1"), (1, &top), (2, &top), (3, &top)])); +} + /// Rearrange All Artboards refuses `columns` that are not a whole number (a fraction, a negative number or a string), /// and the artboards stay where they were; 1.0, as the Rearrange All Artboards dialog sends a count, is one column. #[test]