From f3afeaaf540b1bc73e092cc3638592afd538e615 Mon Sep 17 00:00:00 2001 From: dotCooCoo Date: Mon, 26 Oct 2026 23:39:59 -0700 Subject: [PATCH] Refresh the open Print dialog after spooler contention ends Retry printer enumeration after a busy result and restore the available printer controls without reopening the dialog. Cover opening and drawing the dialog while the spooler is held, then drawing it after release. --- crates/engine/src/cmd/print_cmds/tests.rs | 20 +++++---- crates/ui-egui/src/dialogs/file_ui/print.rs | 28 +++++++++--- crates/ui-egui/src/dialogs/file_ui/tests.rs | 44 ++++++++++++++++++- crates/ui-egui/tests/it/custom_print_paper.rs | 2 +- 4 files changed, 78 insertions(+), 16 deletions(-) diff --git a/crates/engine/src/cmd/print_cmds/tests.rs b/crates/engine/src/cmd/print_cmds/tests.rs index 052aba8f2d..6208b53ec5 100644 --- a/crates/engine/src/cmd/print_cmds/tests.rs +++ b/crates/engine/src/cmd/print_cmds/tests.rs @@ -881,15 +881,17 @@ fn uncached_listings_do_not_wait(cache_other: bool) { assert_eq!(asked.load(Ordering::SeqCst), 0); assert_eq!(listed, printers_answer(Vec::new())); assert_eq!(listed_rgb, Err(craft_print::rgb::PrintError::Unsupported)); - let listed = printers_without_waiting(&mut s); - let listed_rgb = rgb_printers_without_waiting(&s); - assert_eq!(listed["printers"], json!([{"name": "Proofing", "default": true}])); - let names = listed_rgb.map(|list| list.into_iter().map(|p| p.name).collect::>()); - if cfg!(windows) { - assert_eq!(names, Ok(vec!["Proofing".to_string()])); - } else { - assert_eq!(names, Err(craft_print::rgb::PrintError::Unsupported)); - } + with_spooler(|| { + let listed = printers_without_waiting(&mut s); + let listed_rgb = rgb_printers_without_waiting(&s); + assert_eq!(listed["printers"], json!([{"name": "Proofing", "default": true}])); + let names = listed_rgb.map(|list| list.into_iter().map(|p| p.name).collect::>()); + if cfg!(windows) { + assert_eq!(names, Ok(vec!["Proofing".to_string()])); + } else { + assert_eq!(names, Err(craft_print::rgb::PrintError::Unsupported)); + } + }); } /// Spooling tests run in a child process: `disable_printing` is process-wide and a test above diff --git a/crates/ui-egui/src/dialogs/file_ui/print.rs b/crates/ui-egui/src/dialogs/file_ui/print.rs index fb5d63796d..9a826611e6 100644 --- a/crates/ui-egui/src/dialogs/file_ui/print.rs +++ b/crates/ui-egui/src/dialogs/file_ui/print.rs @@ -31,8 +31,6 @@ pub fn open_print(app: &mut PhotocraftApp) -> u64 { let mut f = Map::new(); f.insert("__print".into(), json!(true)); f.insert("__label".into(), json!("PhotoCraft Print Settings")); - let printers = photocraft_engine::print_cmds::printers_without_waiting(&mut app.session); - f.insert("__systemPrinters".into(), printers["printers"].clone()); // The settings of the last print, also from an earlier session (#7410). if let Some(Value::Object(m)) = photocraft_engine::print_cmds::last_print(&app.session) { f.extend(m); @@ -62,17 +60,31 @@ pub fn open_print(app: &mut PhotocraftApp) -> u64 { ] { f.entry(k.to_string()).or_insert(v); } - // Windows: the system's printers, and the paper from the chosen printer's driver settings. + refresh_printers(app, &mut f); + app.ui.open_dialog(DialogKind::Command, f) +} + +fn refresh_printers(app: &mut PhotocraftApp, f: &mut Map) { + if photocraft_engine::print_cmds::try_with_spooler(|| take_printer_lists(app, f)).is_some() { + f.remove("__printerListingPending"); + } else { + take_printer_lists(app, f); + f.insert("__printerListingPending".into(), json!(true)); + } +} + +fn take_printer_lists(app: &mut PhotocraftApp, f: &mut Map) { + let printers = photocraft_engine::print_cmds::printers_without_waiting(&mut app.session); + f.insert("__systemPrinters".into(), printers["printers"].clone()); if let Ok(list) = photocraft_engine::print_cmds::rgb_printers_without_waiting(&app.session) { f.insert("__printers".into(), json!(list.iter().map(|p| p.name.clone()).collect::>())); let known = |n: &str| list.iter().any(|p| p.name == n); - if !known(&s(&f, "printer", "")) { + if !known(&s(f, "printer", "")) { let fallback = list.iter().find(|p| p.default).or(list.first()).map(|p| p.name.clone()).unwrap_or_default(); f.insert("printer".into(), json!(fallback)); f.remove("printerSettings"); } } - app.ui.open_dialog(DialogKind::Command, f) } /// Whether the driver's paper, as `driver_page` took it, is wider than tall: the Orientation @@ -288,6 +300,12 @@ pub(super) fn print_preview_image(l: photocraft_engine::print_cmds::PrintLayout, /// Dragging the visible image updates dialog offsets live from a fixed gesture origin; /// the position row stays present while centered so switching to manual cannot move the page. pub(super) fn print_body(app: &mut PhotocraftApp, ui: &mut egui::Ui, f: &mut Map, id: u64) { + if b(f, "__printerListingPending", false) { + refresh_printers(app, f); + if b(f, "__printerListingPending", false) { + ui.ctx().request_repaint_after(std::time::Duration::from_millis(100)); + } + } let t = Tokens::get(ui.ctx()); let Some(doc) = app.session.active().map(|d| d.doc.clone()) else { return }; // PDF exports and previews retain editable paper on Windows too. Only a real native diff --git a/crates/ui-egui/src/dialogs/file_ui/tests.rs b/crates/ui-egui/src/dialogs/file_ui/tests.rs index d68adc24eb..77a0619df2 100644 --- a/crates/ui-egui/src/dialogs/file_ui/tests.rs +++ b/crates/ui-egui/src/dialogs/file_ui/tests.rs @@ -1,5 +1,47 @@ use super::*; +#[test] +fn print_dialog_retries_an_uncached_printer_list_after_the_spooler_is_released() { + use std::sync::atomic::{AtomicBool, AtomicUsize, Ordering}; + let (mut app, ctx) = app(); + let asked = Arc::new(AtomicUsize::new(0)); + let counted = Arc::clone(&asked); + app.session.printer_source = Some(Arc::new(move || { + counted.fetch_add(1, Ordering::SeqCst); + vec![craft_print::Printer { name: "Retry printer".into(), default: true, two_sided: None, description: String::new() }] + })); + let (held_tx, held) = std::sync::mpsc::channel(); + let (release, released_rx) = std::sync::mpsc::channel::<()>(); + let released = Arc::new(AtomicBool::new(false)); + let done = Arc::clone(&released); + let holder = std::thread::spawn(move || { + photocraft_engine::print_cmds::with_spooler(|| { + held_tx.send(()).unwrap(); + let _ = released_rx.recv_timeout(std::time::Duration::from_secs(5)); + done.store(true, Ordering::SeqCst); + }); + }); + held.recv().unwrap(); + let id = open_print(&mut app); + let mut fields = app.ui.dialog_mut(id).unwrap().fields.clone(); + fields.insert("dryRun".into(), json!(true)); + ctx.run_ui(Default::default(), |ui| print_body(&mut app, ui, &mut fields, id)).textures_delta.clear(); + let waited = released.load(Ordering::SeqCst); + let calls_while_busy = asked.load(Ordering::SeqCst); + let _ = release.send(()); + holder.join().unwrap(); + assert!(!waited, "opening or drawing Print waited for the spooler"); + assert_eq!(calls_while_busy, 0); + assert_eq!(fields["__systemPrinters"], json!([])); + photocraft_engine::print_cmds::with_spooler(|| { + ctx.run_ui(Default::default(), |ui| print_body(&mut app, ui, &mut fields, id)).textures_delta.clear(); + assert_eq!(fields["__systemPrinters"], json!([{"name": "Retry printer", "default": true}])); + assert_eq!(fields.get("__printers"), cfg!(windows).then(|| json!(["Retry printer"])).as_ref()); + assert!(!fields.contains_key("__printerListingPending")); + assert!(asked.load(Ordering::SeqCst) > 0); + }); +} + #[test] fn driver_calls_do_not_wait_while_the_spooler_is_busy() { use std::sync::atomic::{AtomicBool, Ordering}; @@ -471,7 +513,7 @@ fn printer_menu_lists_the_spooler_printers_after_the_default() { assert_eq!(printer_options(None, "").len(), 1); // The dialog asks the engine, and the list never reaches file.print's params. let (mut app, ctx) = app(); - let id = crate::menus::invoke(&mut app, &ctx, "file.print", json!({})).unwrap()["dialog"].as_u64().unwrap(); + let id = photocraft_engine::print_cmds::with_spooler(|| crate::menus::invoke(&mut app, &ctx, "file.print", json!({})).unwrap()["dialog"].as_u64().unwrap()); let f = &app.ui.dialog_mut(id).unwrap().fields; assert_eq!(f["__systemPrinters"], json!([{"name": "Test printer", "default": true}])); assert_eq!(f.get("__printers"), cfg!(windows).then(|| json!(["Test printer"])).as_ref()); diff --git a/crates/ui-egui/tests/it/custom_print_paper.rs b/crates/ui-egui/tests/it/custom_print_paper.rs index afba9d0177..d6c66a21e3 100644 --- a/crates/ui-egui/tests/it/custom_print_paper.rs +++ b/crates/ui-egui/tests/it/custom_print_paper.rs @@ -11,7 +11,7 @@ fn app() -> (PhotocraftApp, u64) { vec![craft_print::Printer { name: "Test native printer".into(), default: true, two_sided: None, description: String::new() }] })); app.run("file.new", json!({"width": 120, "height": 80, "name": "Custom paper"})).unwrap(); - let id = open_print(&mut app); + let id = photocraft_engine::print_cmds::with_spooler(|| open_print(&mut app)); assert_no_system_listing(); let fields = &mut app.ui.dialog_mut(id).unwrap().fields; // Every UI path in these fixtures is safe even if an unexpected Enter confirms the dialog.