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

@@ -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>) -> 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);

crates/engine/src/cmd/menucmds.rs

@@ -1306,13 +1306,16 @@ fn convert_to_artboards(s: &mut Session, _: &Value) -> Result<Value> {
         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}"),

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<NodeId> = (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::<Vec<_>>()
+    };
+    let named = |pairs: &[(u32, &str)]| pairs.iter().map(|&(id, n)| (id, n.to_string())).collect::<Vec<_>>();
+
+    // 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<u32> = 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]