From 8f2451bc6642d4cf7d8995e4cfa0e25e417f3106 Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 14:13:55 -0500 Subject: [PATCH 1/4] Place: a PDF later in a batch joins the place cursor, whatever is selected With a graphic frame selected, `file.place` chose that frame before it looked at the place cursor, so a PDF confirmed with `append` filled the frame and left the images around it queued. An appended file now skips the selected-frame fallback; an explicit `frame` still applies, and a single placed file still fills the selected frame. --- crates/engine/src/cmd/file/place.rs | 9 ++++++--- crates/ui-egui/src/app/files.rs | 21 +++++++++++++++++++++ 2 files changed, 27 insertions(+), 3 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 081e3bd27..5ecd31268 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -100,8 +100,11 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result { (nw, nh) } }; + // A file joining the place cursor's queue goes after the ones already there, whatever is selected. + let load_cursor_param = p.get("loadCursor").and_then(Value::as_bool) == Some(true); + let append = p.get("append").and_then(Value::as_bool) == Some(true); let target_frame = crate::cmd::id_param(p, "frame").or_else(|| { - if ["x", "y", "spread", "width"].iter().any(|key| p.get(*key).is_some_and(|value| !value.is_null())) { + if (load_cursor_param && append) || ["x", "y", "spread", "width"].iter().any(|key| p.get(*key).is_some_and(|value| !value.is_null())) { return None; } let st = s.active()?; @@ -115,8 +118,8 @@ pub(super) fn file_place(s: &mut Session, p: &Value) -> Result { let lid = s.doc()?.active_layer; let px = p.get("x").and_then(Value::as_f64); let py = p.get("y").and_then(Value::as_f64); - if target_frame.is_none() && px.is_none() && py.is_none() && p.get("loadCursor").and_then(Value::as_bool) == Some(true) { - return load_cursor(s, asset, (bw, bh), p.get("append").and_then(Value::as_bool) == Some(true)); + if target_frame.is_none() && px.is_none() && py.is_none() && load_cursor_param { + return load_cursor(s, asset, (bw, bh), append); } s.edit(|d, sel| { let fresh = AssetId(d.alloc()); diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 8b1682d83..48ca90e26 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -1010,6 +1010,27 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.svg", "one.pdf", "b.svg", "c.svg", "a.svg"]); } + /// A PDF later in a batch joins the place cursor with the images around it, even with a + /// graphic frame selected: it doesn't fill that frame and leave the others queued. + #[test] + fn a_pdf_in_a_batch_queues_rather_than_filling_the_selected_frame() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + std::fs::write(path("a.svg"), image()).unwrap(); + std::fs::write(path("one.pdf"), pdf_pages(1)).unwrap(); + std::fs::write(path("b.svg"), image()).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let f = designcraft_doc::ItemId( + app.run("frame.create", json!({"rect": [200, 200, 300, 300], "content": "graphic"})).unwrap()["id"].as_u64().unwrap(), + ); + assert!(app.session.active().unwrap().selection.items.contains(&f)); + app.place_paths(vec![path("a.svg"), path("one.pdf"), path("b.svg")]).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg", "one.pdf", "b.svg"]); + let frame = app.session.active().unwrap().doc.item(f).unwrap(); + assert!(!matches!(frame.content, designcraft_doc::Content::Graphic(_)), "the selected frame stays empty"); + } + /// File › Place with several files chosen in the dialog loads them all into the place /// cursor, in the order chosen; with one, it is placed as before. #[test] From 5868b6036584aff4d155ee181c9124e5ec12f08b Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 14:54:26 -0500 Subject: [PATCH 2/4] Place: the 1000-image cap holds for the whole place cursor when appending Appending only checked each batch against the cap, and a single image or a PDF added with append wasn't checked at all, so repeated appends could grow the cursor without limit. Refused before decoding a batch or adding an asset. --- crates/engine/src/cmd/file/place.rs | 90 ++++++++++++++++++++++++++++- docs/multi-image-placement.md | 2 +- 2 files changed, 90 insertions(+), 2 deletions(-) diff --git a/crates/engine/src/cmd/file/place.rs b/crates/engine/src/cmd/file/place.rs index 5ecd31268..5ac886618 100644 --- a/crates/engine/src/cmd/file/place.rs +++ b/crates/engine/src/cmd/file/place.rs @@ -253,6 +253,7 @@ pub(super) fn place_load(s: &mut Session, p: &Value) -> Result { if sources.is_empty() || sources.len() > MAX_LOADED_IMAGES { return Err(bad("place.load", format!("select between 1 and {MAX_LOADED_IMAGES} images"))); } + check_cursor_room(s, sources.len(), append, "place.load")?; let mut images = Vec::with_capacity(sources.len()); for source in sources { let (bytes, name, link) = read_source(source)?; @@ -280,9 +281,18 @@ pub(super) fn place_load(s: &mut Session, p: &Value) -> Result { Ok(json!({"remaining": loaded_count(s), "loaded": true})) } -/// Most images one `place.load` takes. +/// Most images the place cursor holds, and so most one `place.load` takes. const MAX_LOADED_IMAGES: usize = 1000; +/// Whether `incoming` images fit in the place cursor: appended, they go after the ones it holds. +fn check_cursor_room(s: &Session, incoming: usize, append: bool, command: &str) -> Result<()> { + let held = if append { loaded_count(s) } else { 0 }; + if held.saturating_add(incoming) > MAX_LOADED_IMAGES { + return Err(bad(command, format!("the place cursor holds at most {MAX_LOADED_IMAGES} images, and it has {held}"))); + } + Ok(()) +} + /// How many images the place cursor holds: the current one and those queued after it. fn loaded_count(s: &Session) -> usize { s.loaded_queue.len().saturating_add(usize::from(s.loaded.is_some())) @@ -291,6 +301,7 @@ fn loaded_count(s: &Session) -> usize { /// Add a decoded asset with a fresh id, replacing the previous placement queue (or, with /// `append` and an image already loaded, queued after it). fn load_cursor(s: &mut Session, mut asset: Asset, size: (f64, f64), append: bool) -> Result { + check_cursor_room(s, 1, append, "place.load")?; let name = asset.name.clone(); let (pw, ph) = asset.pixels.unwrap_or_default(); let aid = s.edit(|d, _| { @@ -1067,3 +1078,80 @@ mod nested_frame_tests { assert_eq!(d.spreads[0].items.len(), 1, "no new object"); } } + +#[cfg(test)] +mod cursor_cap_tests { + use serde_json::{Value, json}; + + use super::MAX_LOADED_IMAGES; + use crate::Session; + + fn png() -> String { + let png = designcraft_render::Rendered { width: 4, height: 4, pixels: [200, 90, 30, 255].repeat(16) }.to_png(); + super::super::base64_encode(&png) + } + + /// A session whose place cursor holds `held` images, loaded without decoding each of them. + fn holding(held: usize) -> Session { + let mut s = Session::new(); + s.execute("file.new", &json!({})).unwrap(); + s.execute("place.load", &json!({"base64": png(), "name": "first.png"})).unwrap(); + let first = s.loaded.unwrap(); + s.loaded_queue = std::iter::repeat_n(first, held - 1).collect(); + s + } + + fn count(s: &Session) -> usize { + super::loaded_count(s) + } + + /// Every way of adding to the cursor stops at the cap, and a refused add changes nothing. + #[test] + fn appending_stops_at_the_cap_on_the_whole_cursor() { + let image = png(); + let single = json!({"base64": image, "name": "a.png", "append": true}); + let placed = json!({"base64": image, "name": "a.png", "append": true, "loadCursor": true}); + let two = json!({"files": [{"base64": image, "name": "a.png"}, {"base64": image, "name": "b.png"}], "append": true}); + for (command, params) in [("place.load", &single), ("file.place", &placed), ("place.load", &two)] { + let mut s = holding(MAX_LOADED_IMAGES); + let assets = s.doc().unwrap().doc.assets.len(); + assert!(s.execute(command, params).is_err(), "{command} {params}"); + assert_eq!(count(&s), MAX_LOADED_IMAGES, "{command} {params}"); + assert_eq!(s.doc().unwrap().doc.assets.len(), assets, "{command} {params}"); + } + // One over, as a batch. + let mut s = holding(MAX_LOADED_IMAGES - 1); + assert!(s.execute("place.load", &two).is_err()); + assert_eq!(count(&s), MAX_LOADED_IMAGES - 1); + } + + /// The last places up to the cap load, and an unreadable batch reports the cap before decoding. + #[test] + fn appending_up_to_the_cap_still_loads() { + let image = png(); + for params in + [json!({"base64": image, "name": "a.png", "append": true}), json!({"files": [{"base64": image, "name": "a.png"}], "append": true})] + { + let mut s = holding(MAX_LOADED_IMAGES - 1); + s.execute("place.load", ¶ms).unwrap(); + assert_eq!(count(&s), MAX_LOADED_IMAGES, "{params}"); + } + let mut s = holding(MAX_LOADED_IMAGES); + let error = + s.execute("place.load", &json!({"files": [{"base64": "not an image", "name": "a.png"}], "append": true})).unwrap_err().to_string(); + assert!(error.contains(&MAX_LOADED_IMAGES.to_string()), "{error}"); + } + + /// Without `append` the new images replace the cursor, so what it holds doesn't count. + #[test] + fn replacing_a_full_cursor_is_not_capped() { + let image = png(); + let mut s = holding(MAX_LOADED_IMAGES); + let result: Value = s.execute("place.load", &json!({"base64": image, "name": "a.png"})).unwrap(); + assert_eq!(result["loaded"], true); + assert_eq!(count(&s), 1); + let mut s = holding(MAX_LOADED_IMAGES); + s.execute("place.load", &json!({"files": [{"base64": image, "name": "a.png"}]})).unwrap(); + assert_eq!(count(&s), 1); + } +} diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index cae08a519..bb58b600a 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -9,7 +9,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images. They load in - Press Escape or select another tool to cancel the remaining placement. - Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. -A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. Image batches support up to 1000 images. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. Images here are PNG, JPEG, GIF, WebP, BMP, TIFF, SVG, PSD and EPS files. From 15eeacc8456af6d9e194647b6d11c9b40c40b907 Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 15:00:33 -0500 Subject: [PATCH 3/4] Place: the files after a multi-page PDF in a batch wait for its options They were left behind in place_paths when the PDF's Import Options opened, so the PDF was placed alone and the rest of what was chosen never loaded. They now wait in the dialog and carry on when it is confirmed; Cancel ends the batch. --- crates/ui-egui/src/app/files.rs | 82 +++++++++++++++++++++++++++------ crates/ui-egui/src/dialogs.rs | 16 ++++++- docs/multi-image-placement.md | 2 +- 3 files changed, 84 insertions(+), 16 deletions(-) diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 48ca90e26..6f5368ad8 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -52,18 +52,21 @@ impl DesignApp { self.place_file(if append { json!({"path": path, "append": true}) } else { json!({"path": path}) }) } - /// Place an ordered batch of files, as File › Place with several chosen. The batch stops when - /// a file opens a dialog (a multi-page PDF's Import Options, an alert), as the inbox's does; - /// every file that loads the cursor adds to what the earlier ones loaded, in the order the - /// files were chosen (images in a row load together, before the file after them). A file that - /// can't be placed gets an alert and stops the batch; its error is returned, so automation - /// doesn't carry on as if the batch had loaded. + /// Place an ordered batch of files, as File › Place with several chosen. A multi-page PDF's + /// Import Options pause the batch: the files after it wait in the dialog and carry on when it + /// is confirmed, and Cancel ends the batch. Any other dialog (an alert, one already open) stops + /// it, as the inbox's does. Every file that loads the cursor adds to what the earlier ones + /// loaded, in the order the files were chosen (images in a row load together, before the file + /// after them). A file that can't be placed gets an alert and stops the batch; its error is + /// returned, so automation doesn't carry on as if the batch had loaded. pub(crate) fn place_paths(&mut self, paths: Vec) -> Result<(), String> { let mut images = Vec::new(); // Images already in the place cursor stay: the batch queues after them. let mut loaded = self.session.loaded.is_some(); - // `None` at the end loads the images left. - for path in paths.into_iter().map(Some).chain([None]) { + let mut paths = paths.into_iter(); + loop { + // `None` at the end loads the images left. + let path = paths.next(); if let Some(p) = &path && is_cursor_image(p) { @@ -71,7 +74,7 @@ impl DesignApp { continue; } if path.is_none() && images.is_empty() { - break; + return Ok(()); } if self.ui.dialog.is_some() { self.status(crate::i18n::tr(&self.ui.language, "Place cancelled while a dialog is open")); @@ -81,12 +84,14 @@ impl DesignApp { let r = self.load_place_cursor(std::mem::take(&mut images), loaded)?; loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); } - if let Some(p) = path { - let r = self.place_path(&p, loaded)?; - loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); + let Some(p) = path else { return Ok(()) }; + let r = self.place_path(&p, loaded)?; + loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); + if let Some(d) = self.ui.dialog.as_mut().filter(|d| d.id == "pdfImport") { + d.fields.insert(dialogs::PDF_IMPORT_REST.into(), json!(paths.collect::>())); + return Ok(()); } } - Ok(()) } /// `place.load` of `files` (`{path}` or `{name, base64}` each), queued after the images @@ -943,6 +948,57 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.png", "b.png", "three.pdf"]); } + /// The files after a multi-page PDF in a native batch wait for its Import Options, through a + /// second such PDF too, and keep the order chosen. + #[test] + fn native_batch_continues_after_a_multi_page_pdf_is_confirmed() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + for name in ["a.svg", "b.svg", "c.svg", "d.svg"] { + std::fs::write(path(name), image()).unwrap(); + } + std::fs::write(path("three.pdf"), pdf_pages(3)).unwrap(); + std::fs::write(path("two.pdf"), pdf_pages(2)).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let batch = ["a.svg", "three.pdf", "b.svg", "c.svg", "two.pdf", "d.svg"].map(path).to_vec(); + app.place_paths(batch).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg"], "waiting for the first PDF's options"); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("pdfImport"), "the second PDF asks in its turn"); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg", "c.svg"]); + dialogs::confirm(&mut app).unwrap(); + assert!(app.ui.dialog.is_none()); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg", "c.svg", "two.pdf", "d.svg"]); + } + + /// A refused page number keeps the rest of the batch waiting; Cancel ends the batch, and what + /// loaded before the PDF stays. + #[test] + fn native_batch_waits_through_a_refused_page_and_ends_at_cancel() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + std::fs::write(path("a.svg"), image()).unwrap(); + std::fs::write(path("b.svg"), image()).unwrap(); + std::fs::write(path("three.pdf"), pdf_pages(3)).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let batch = ["a.svg", "three.pdf", "b.svg"].map(path).to_vec(); + app.place_paths(batch.clone()).unwrap(); + app.ui.dialog.as_mut().unwrap().fields.insert("page".into(), json!("4-5")); + assert!(dialogs::confirm(&mut app).is_err()); + app.ui.dialog.as_mut().unwrap().fields.insert("page".into(), json!("2")); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(loaded_names(&app), ["a.svg", "three.pdf", "b.svg"]); + + let mut app = self::app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + app.place_paths(batch).unwrap(); + dialogs::cancel(&mut app); + assert!(app.ui.dialog.is_none()); + assert_eq!(loaded_names(&app), ["a.svg"]); + } + /// A single multi-page PDF still replaces what the cursor holds, as any single Place does. #[test] fn a_single_multi_page_pdf_replaces_the_cursor_when_confirmed() { diff --git a/crates/ui-egui/src/dialogs.rs b/crates/ui-egui/src/dialogs.rs index 4633620f3..b104a0ae1 100644 --- a/crates/ui-egui/src/dialogs.rs +++ b/crates/ui-egui/src/dialogs.rs @@ -1562,6 +1562,9 @@ fn text_frame_params(app: &DesignApp, d: &Dialog) -> Result, Ok(p) } +/// In a native batch's `pdfImport` dialog: the paths chosen after its PDF, placed once it is confirmed. +pub(crate) const PDF_IMPORT_REST: &str = "rest"; + /// File › Document Setup, filled from the document. /// The active document's ruler unit (picas without a document). fn doc_unit(app: &DesignApp) -> Unit { @@ -4711,9 +4714,18 @@ fn confirm_part3(app: &mut DesignApp, d: Dialog) -> Result { if let Some(target) = d.fields.get("target").and_then(Value::as_u64) { app.activate_import_target(&crate::ImportRequest { purpose: "place".into(), target: Some(target) })?; } - app.place_file( + let placed = app.place_file( json!({"path": d.s("path"), "pdfPage": page, "pdfCrop": if crop.is_empty() { "crop".to_string() } else { crop }, "append": d.b("append")}), - ) + )?; + // The files chosen after this PDF, which waited for its options. + let rest: Vec = d + .fields + .get(PDF_IMPORT_REST) + .and_then(Value::as_array) + .map(|paths| paths.iter().filter_map(Value::as_str).map(str::to_string).collect()) + .unwrap_or_default(); + app.place_paths(rest)?; + Ok(placed) } "print" => { let pages = if d.s("range") == "range" { json!(d.s("pages")) } else { Value::Null }; diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index bb58b600a..a99b4e144 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -9,7 +9,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images. They load in - Press Escape or select another tool to cancel the remaining placement. - Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. -A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch stops the batch at its import options; once confirmed, it queues after the images loaded before it. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch pauses the batch at its import options; once confirmed, it queues after the images loaded before it and the rest of the batch follows. Cancelling them ends the batch, and the images already loaded stay. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. Images here are PNG, JPEG, GIF, WebP, BMP, TIFF, SVG, PSD and EPS files. From d5bb8a378c8c3ccb676885c19b2bc1feb49672ca Mon Sep 17 00:00:00 2001 From: texjer Date: Thu, 5 Nov 2026 18:40:41 -0500 Subject: [PATCH 4/4] Place: a browser batch waits for a PDF's options; a PDF first joins the cursor Files delivered after a multi-page PDF were dropped when its Import Options opened, with a status asking to choose them again, and choosing them again replaced the cursor. They now wait with the PDF and carry on when it is confirmed; Cancel ends the batch, as a native one's does. A batch starting with a single-page PDF, or in a browser with one image before a PDF, filled the selected frame instead of entering the place cursor. A browser delivers one Place's files together, so several in one delivery now count as a batch. One file alone still fills the frame. --- crates/ui-egui/src/app.rs | 1 + crates/ui-egui/src/app/files.rs | 145 ++++++++++++++++++++++++++++++-- crates/ui-egui/src/lib.rs | 2 + docs/multi-image-placement.md | 2 +- 4 files changed, 142 insertions(+), 8 deletions(-) diff --git a/crates/ui-egui/src/app.rs b/crates/ui-egui/src/app.rs index b2b433b71..0048a8c4e 100644 --- a/crates/ui-egui/src/app.rs +++ b/crates/ui-egui/src/app.rs @@ -39,6 +39,7 @@ impl DesignApp { let dialog_wake = std::sync::Arc::new(std::sync::OnceLock::new()); DesignApp { pending_pdf: None, + pending_pdf_rest: Vec::new(), #[cfg(not(target_arch = "wasm32"))] file_dialogs: file_dialogs::new_dialogs(dialog_wake.clone()), #[cfg(not(target_arch = "wasm32"))] diff --git a/crates/ui-egui/src/app/files.rs b/crates/ui-egui/src/app/files.rs index 6f5368ad8..712f2b500 100644 --- a/crates/ui-egui/src/app/files.rs +++ b/crates/ui-egui/src/app/files.rs @@ -61,6 +61,8 @@ impl DesignApp { /// returned, so automation doesn't carry on as if the batch had loaded. pub(crate) fn place_paths(&mut self, paths: Vec) -> Result<(), String> { let mut images = Vec::new(); + // Several files go to the place cursor, even a PDF first with a frame selected. + let batch = paths.len() > 1; // Images already in the place cursor stay: the batch queues after them. let mut loaded = self.session.loaded.is_some(); let mut paths = paths.into_iter(); @@ -85,7 +87,7 @@ impl DesignApp { loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); } let Some(p) = path else { return Ok(()) }; - let r = self.place_path(&p, loaded)?; + let r = self.place_path(&p, batch || loaded)?; loaded |= r.get("loaded").and_then(Value::as_bool) == Some(true); if let Some(d) = self.ui.dialog.as_mut().filter(|d| d.id == "pdfImport") { d.fields.insert(dialogs::PDF_IMPORT_REST.into(), json!(paths.collect::>())); @@ -138,16 +140,28 @@ impl DesignApp { } pub(crate) fn cancel_pdf_import(&mut self) { + self.release_pending_pdf(); + } + + fn release_pending_pdf(&mut self) { self.pending_pdf = None; + self.pending_pdf_rest = Vec::new(); } /// Import Options confirmed for the delivered PDF; a placed one is added to the place cursor - /// when `append`, so the images loaded before it stay. + /// when `append`, so the images loaded before it stay. The files of its batch that waited for + /// the options carry on; if it can't be placed, the batch ends, as a native one does. pub(crate) fn confirm_pdf_import(&mut self, page: u64, crop: String, append: bool) -> Result { let file = self.pending_pdf.take().ok_or("PDF import cancelled: no pending file")?; + let rest = std::mem::take(&mut self.pending_pdf_rest); self.activate_import_target(&file.request)?; let params = json!({"name": file.name, "base64": designcraft_engine::cmd::base64_encode(&file.bytes), "pdfPage": page, "pdfCrop": crop, "append": append}); - if file.request.purpose == "place" { self.place_file(params) } else { self.open_file("file.place", params) } + if file.request.purpose != "place" { + return self.open_file("file.place", params); + } + let placed = self.place_file(params)?; + self.import_delivered(rest, true); + Ok(placed) } /// A single unresolved PDF owns its bytes. Later deliveries cancel explicitly rather than @@ -158,7 +172,7 @@ impl DesignApp { let owns_dialog = self.ui.dialog.as_ref().is_some_and(|d| d.id == "pdfImport" && d.fields.get("async").and_then(Value::as_bool) == Some(true)); if !target_exists || !owns_dialog { - self.pending_pdf = None; + self.release_pending_pdf(); if !target_exists { if owns_dialog { self.ui.dialog = None; @@ -169,14 +183,23 @@ impl DesignApp { } let Some(inbox) = self.services.inbox.clone() else { return }; let files = std::mem::take(&mut *inbox.lock().unwrap_or_else(std::sync::PoisonError::into_inner)); + // A browser delivers one Place's files together. Several go to the place cursor, even a + // PDF first with a frame selected, as a native batch's do. + let batch = files.iter().filter(|f| f.request.purpose == "place").count() > 1; + self.import_delivered(files, batch); + } + + /// Imports `files` in the order they came; placed ones are added to the place cursor when + /// `append`, and after the first that loads it. A multi-page PDF's Import Options pause a + /// Place: the files after it wait with it and carry on when it is confirmed, and Cancel ends + /// the batch. + fn import_delivered(&mut self, files: Vec, mut append: bool) { let cursor_image = |file: &ImportedFile| { file.request.purpose == "place" && !file.name.to_ascii_lowercase().ends_with(".pdf") && (is_cursor_image(&file.name) || designcraft_render::image_size(&file.bytes).is_some()) }; - // In the order they came: consecutive images for the same document load the place - // cursor together; after the first placement into it, the later ones are added to it. - let mut append = false; + // Consecutive images for the same document load the place cursor together. let mut files = files.into_iter().peekable(); while let Some(file) = files.next() { if file.request.purpose == "shortcuts" { @@ -220,11 +243,17 @@ impl DesignApp { continue; } let name = file.name.clone(); + let target = file.request.target; match self.import_file(file, append) { Ok(_) if placing => append = true, Ok(_) => {} Err(e) => self.status(format!("{name}: {e}")), } + if placing && self.pending_pdf.is_some() { + while let Some(next) = files.next_if(|n| n.request.purpose == "place" && n.request.target == target) { + self.pending_pdf_rest.push(next); + } + } } } @@ -918,6 +947,108 @@ pub(crate) mod browser_import_tests { assert_eq!(loaded_names(&app), ["a.png", "one.pdf", "b.png"], "in the order dropped, all in the cursor"); } + /// A graphic frame, holding a picture and selected, with an empty place cursor; the frame's id. + fn select_a_graphic_frame(app: &mut DesignApp) -> u64 { + let frame = app.run("frame.create", json!({"rect": [10, 10, 100, 100]})).unwrap()["id"].as_u64().unwrap(); + let b64 = designcraft_engine::cmd::base64_encode(&image()); + app.run("file.place", json!({"base64": b64, "name": "old.svg", "frame": frame})).unwrap(); + app.run("selection.set", json!({"ids": [frame]})).unwrap(); + assert!(app.session.loaded.is_none()); + frame + } + + fn frame_asset(app: &DesignApp, frame: u64) -> String { + let doc = &app.session.active().unwrap().doc; + let designcraft_doc::Content::Graphic(g) = &doc.item(designcraft_doc::ItemId(frame)).unwrap().content else { panic!("a graphic") }; + doc.assets[&g.asset].name.clone() + } + + /// A PDF first in a native batch joins the cursor like the rest; the selected frame keeps its + /// picture. A PDF alone still fills it. + #[test] + fn native_batch_starting_with_a_pdf_leaves_the_selected_frame() { + let dir = tempfile::tempdir().unwrap(); + let path = |n: &str| dir.path().join(n).to_string_lossy().to_string(); + std::fs::write(path("one.pdf"), pdf_pages(1)).unwrap(); + std::fs::write(path("a.svg"), image()).unwrap(); + let mut app = app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let frame = select_a_graphic_frame(&mut app); + app.place_paths(vec![path("one.pdf"), path("a.svg")]).unwrap(); + assert_eq!(loaded_names(&app), ["one.pdf", "a.svg"]); + assert_eq!(frame_asset(&app, frame), "old.svg"); + + let mut app = self::app(); + app.services.read = Some(Box::new(|p| std::fs::read(p).map_err(|e| e.to_string()))); + let frame = select_a_graphic_frame(&mut app); + app.place_paths(vec![path("one.pdf")]).unwrap(); + assert!(app.session.loaded.is_none()); + assert_eq!(frame_asset(&app, frame), "one.pdf"); + } + + /// The same for files a host delivers (a browser's Place), with a PDF or an image first. + #[test] + fn inbox_batch_leaves_the_selected_frame() { + for names in [["one.pdf", "a.png"], ["a.png", "one.pdf"]] { + let mut app = app(); + let frame = select_a_graphic_frame(&mut app); + let request = app.import_request("place"); + for name in names { + let bytes = if name.ends_with(".pdf") { pdf_pages(1) } else { png(200) }; + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request: request.clone(), name: name.into(), bytes }); + } + app.drain_inbox(); + assert_eq!(loaded_names(&app), names); + assert_eq!(frame_asset(&app, frame), "old.svg"); + } + // One file alone still fills it. + let mut app = app(); + let frame = select_a_graphic_frame(&mut app); + let request = app.import_request("place"); + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request, name: "one.pdf".into(), bytes: pdf_pages(1) }); + app.drain_inbox(); + assert_eq!(frame_asset(&app, frame), "one.pdf"); + } + + /// The files a host delivered after a multi-page PDF wait for its Import Options, through a + /// second such PDF too, keep their order and don't replace what the cursor holds. + #[test] + fn inbox_batch_continues_after_a_multi_page_pdf_is_confirmed() { + let mut app = app(); + let request = app.import_request("place"); + let files = + [("a.png", png(200)), ("three.pdf", pdf_pages(3)), ("b.png", png(90)), ("c.png", png(40)), ("two.pdf", pdf_pages(2)), ("d.png", png(20))]; + for (name, bytes) in files { + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request: request.clone(), name: name.into(), bytes }); + } + app.drain_inbox(); + assert_eq!(loaded_names(&app), ["a.png"], "waiting for the first PDF's options"); + assert_eq!(app.pending_pdf_rest.len(), 4); + dialogs::confirm(&mut app).unwrap(); + assert_eq!(app.ui.dialog.as_ref().map(|d| d.id.as_str()), Some("pdfImport"), "the second PDF asks in its turn"); + assert_eq!(loaded_names(&app), ["a.png", "three.pdf", "b.png", "c.png"]); + dialogs::confirm(&mut app).unwrap(); + assert!(app.ui.dialog.is_none()); + assert_eq!(loaded_names(&app), ["a.png", "three.pdf", "b.png", "c.png", "two.pdf", "d.png"]); + assert!(app.pending_pdf.is_none() && app.pending_pdf_rest.is_empty()); + } + + /// Cancel ends a delivered batch, as a native one's does: what it loaded stays, the waiting + /// files are released. + #[test] + fn inbox_batch_ends_when_a_multi_page_pdf_is_cancelled() { + let mut app = app(); + let request = app.import_request("place"); + for (name, bytes) in [("a.png", png(200)), ("three.pdf", pdf_pages(3)), ("b.png", png(90))] { + app.services.inbox.as_ref().unwrap().lock().unwrap().push(ImportedFile { request: request.clone(), name: name.into(), bytes }); + } + app.drain_inbox(); + assert_eq!(app.pending_pdf_rest.len(), 1); + dialogs::cancel(&mut app); + assert!(app.pending_pdf.is_none() && app.pending_pdf_rest.is_empty()); + assert_eq!(loaded_names(&app), ["a.png"]); + } + /// A multi-page PDF after images in a native batch asks which page, then queues after them. #[test] fn native_batch_keeps_its_images_when_a_multi_page_pdf_is_confirmed() { diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 6775c7755..ef7b7b0a4 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -131,6 +131,8 @@ struct ShotRequest { pub struct DesignApp { pending_pdf: Option, + /// The files of the batch after `pending_pdf`, which wait for its Import Options. + pending_pdf_rest: Vec, /// File and message dialogs: asked for during a frame, shown by the host at its end, answered /// on a later frame (see `app/file_dialogs.rs`). #[cfg(not(target_arch = "wasm32"))] diff --git a/docs/multi-image-placement.md b/docs/multi-image-placement.md index a99b4e144..c6b719584 100644 --- a/docs/multi-image-placement.md +++ b/docs/multi-image-placement.md @@ -9,7 +9,7 @@ Choose **File > Place** (Cmd+D on macOS) and select several images. They load in - Press Escape or select another tool to cancel the remaining placement. - Switching to, opening or closing a document also cancels it, and goes back to the Selection tool. -A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. A multi-page PDF in a batch pauses the batch at its import options; once confirmed, it queues after the images loaded before it and the rest of the batch follows. Cancelling them ends the batch, and the images already loaded stay. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. +A single selected file retains the normal Place behavior, including filling a selected frame and PDF import options. Several files go to the place cursor whatever is selected, even with a PDF first. A multi-page PDF in a batch pauses the batch at its import options; once confirmed, it queues after the images loaded before it and the rest of the batch follows. Cancelling them ends the batch, and the images already loaded stay. The place cursor holds up to 1000 images, counting those already in it when a batch adds to them. Rows and columns each range from 1 to 100. A batch containing an unreadable image fails without replacing the existing queue or changing the document. An invalid grid area also leaves the document and queue unchanged. Images here are PNG, JPEG, GIF, WebP, BMP, TIFF, SVG, PSD and EPS files.