diff --git a/crates/automation/src/printing.rs b/crates/automation/src/printing.rs index fef5beaaab..9f0eedba95 100644 --- a/crates/automation/src/printing.rs +++ b/crates/automation/src/printing.rs @@ -60,8 +60,10 @@ fn paper_named(name: Option<&str>) -> Result<(usize, (f64, f64))> { /// The spooler job for `doc_print` to `printer`. It feeds the paper the PDF was laid out on /// (`PAPERS[paper]`, as `media` after the driver's `options`) rather than the queue's default -/// paper, which a job naming none got (A4 under Letter sheets on the queue in #3752). -fn spool_job(a: &Args, printer: &str, title: String, duplex: Option, paper: usize) -> Result { +/// paper, which a job naming none got (A4 under Letter sheets on the queue in #3752), and tells +/// CUPS which way the laid-out `sheets` run, as the Print dialog does (#2713): an orientation the +/// caller chose in `options` stands. +fn spool_job(a: &Args, printer: &str, title: String, duplex: Option, paper: usize, sheets: &[print::Sheet]) -> Result { let options = match a.get("options") { None => Vec::new(), Some(Value::Object(m)) => m @@ -81,7 +83,11 @@ fn spool_job(a: &Args, printer: &str, title: String, duplex: Option = options.into_iter().chain(print::media_option(paper)).collect(); + print::add_cups_orientation(&mut options, sheets); + options + }, }) } @@ -213,12 +219,7 @@ impl Automation { out["path"] = json!(path.to_string_lossy()); } (None, Some(printer)) => { - let mut job = spool_job(a, printer, self.doc(a)?.name.clone(), duplex, paper_index)?; - // CUPS is told which way the sheets run, as the Print dialog does (#2713); Windows' - // print ticket already is. An orientation the caller chose in `options` stands. - if !cfg!(windows) && !job.options.iter().any(|(k, _)| k == "orientation-requested") { - job.options.extend(print::cups_orientation(&laid_out)); - } + let job = spool_job(a, printer, self.doc(a)?.name.clone(), duplex, paper_index, &laid_out)?; out["job"] = json!(spool::submit(&bytes, &job).map_err(|e| failed(e.to_string()))?); } _ => return Err(bad("pass either path (save the print-ready PDF) or printer (a name from printers, or \"default\")")), @@ -257,13 +258,31 @@ mod tests { assert!(duplex("sideways", false).is_err()); } + /// #2713, #3739: `doc_print` to a printer tells CUPS when every sheet runs landscape, after + /// the paper, so a landscape sheet is turned onto the portrait paper rather than printed + /// unrotated and cut off. Portrait and mixed jobs say nothing, and an orientation passed in + /// `options` stands and isn't sent twice. + #[test] + fn printing_landscape_sheets_to_a_printer_asks_cups_to_turn_them() { + let sheets = |sizes: &[(f64, f64)]| sizes.iter().map(|&size| print::Sheet { size, ..Default::default() }).collect::>(); + let job = |args: Value, s: &[print::Sheet]| spool_job(&Args(&args), "Office", "a.pdf".into(), None, 0, s).unwrap().options; + let media = ("media".to_string(), "Letter".to_string()); + let landscape = sheets(&[(792.0, 612.0), (792.0, 612.0)]); + let turned = if cfg!(windows) { vec![media.clone()] } else { vec![media.clone(), ("orientation-requested".to_string(), "4".to_string())] }; + assert_eq!(job(json!({}), &landscape), turned); + assert_eq!(job(json!({}), &sheets(&[(612.0, 792.0)])), vec![media.clone()], "portrait sheets"); + assert_eq!(job(json!({}), &sheets(&[(792.0, 612.0), (612.0, 792.0)])), vec![media.clone()], "a job that mixes them"); + let chosen = ("orientation-requested".to_string(), "5".to_string()); + assert_eq!(job(json!({ "options": { "orientation-requested": "5" } }), &landscape), [chosen, media], "the caller's own choice, once"); + } + /// #3752: `doc_print` to a printer sent no paper, so the queue's default paper stood in for /// the one the sheets were laid out on (A4 under Letter sheets on the report's queue). #[test] fn printing_to_a_printer_feeds_the_laid_out_paper() { let job = |args: Value, paper_name: Option<&str>| { let (index, _) = paper_named(paper_name).unwrap(); - spool_job(&Args(&args), "Office", "a.pdf".into(), None, index).unwrap() + spool_job(&Args(&args), "Office", "a.pdf".into(), None, index, &[]).unwrap() }; let media = |m: &str| ("media".to_string(), m.to_string()); // US Letter when no paper is asked for, after the driver's own options. @@ -279,7 +298,7 @@ mod tests { assert_eq!(job(json!({}), Some("letter")).options, [media("Letter")]); assert_eq!(job(json!({}), Some("ARCH D")).options, [media("na_arch-d_24x36in")]); assert!(matches!(paper_named(Some("Quarto")), Err(ToolError::InvalidArgs(m)) if m.contains("US Letter"))); - assert_eq!(spool_job(&Args(&json!({})), "default", String::new(), None, 0).ok().map(|j| j.printer), Some(None)); - assert!(spool_job(&Args(&json!({ "options": { "InputSlot": 3 } })), "Office", String::new(), None, 0).is_err()); + assert_eq!(spool_job(&Args(&json!({})), "default", String::new(), None, 0, &[]).ok().map(|j| j.printer), Some(None)); + assert!(spool_job(&Args(&json!({ "options": { "InputSlot": 3 } })), "Office", String::new(), None, 0, &[]).is_err()); } } diff --git a/crates/print/src/lib.rs b/crates/print/src/lib.rs index b32cb15541..88dfa84960 100644 --- a/crates/print/src/lib.rs +++ b/crates/print/src/lib.rs @@ -373,6 +373,17 @@ pub fn cups_orientation(sheets: &[Sheet]) -> Option<(String, String)> { landscape.then(|| ("orientation-requested".to_string(), "4".to_string())) } +/// Add [`cups_orientation`] to a CUPS job's `options`, for the Print dialog's jobs and +/// `doc_print`'s alike, unless they already name an orientation (a driver choice, or one a +/// caller passed): `lp` must not be given the option twice. Nothing on Windows, whose print +/// ticket sets each sheet's orientation itself. +pub fn add_cups_orientation(options: &mut Vec<(String, String)>, sheets: &[Sheet]) { + if cfg!(windows) || options.iter().any(|(k, _)| k == "orientation-requested") { + return; + } + options.extend(cups_orientation(sheets)); +} + /// Whether most of page `index`'s text runs bottom-to-top as the page displays (a label laid /// sideways on a letter or A4 sheet), counting each text-showing operator once; form XObjects /// are followed a few levels deep. diff --git a/crates/print/src/tests.rs b/crates/print/src/tests.rs index c37abdf8fc..c15fc37254 100644 --- a/crates/print/src/tests.rs +++ b/crates/print/src/tests.rs @@ -489,6 +489,8 @@ fn printer_options_come_from_the_ppd() { assert!(spool::parse_ppd("*OpenUI *Broken\n*Broken A/B\n").is_empty(), "unclosed or malformed blocks are skipped"); } +/// A driver option's current choice is read from `lpoptions`, and a job passes `lp` only the +/// driver options that are well-formed PPD keywords, never the ones PdfCraft sets itself. #[test] fn printer_option_defaults_and_job_arguments() { let current = @@ -509,6 +511,8 @@ fn printer_option_defaults_and_job_arguments() { assert!(spool::printer_options("../../etc/passwd").is_empty(), "not a queue name"); } +/// CUPS is asked to turn the sheets (`orientation-requested` 4) when every one is landscape, a +/// booklet's included; portrait, mixed, square and empty jobs say nothing (#2713). #[test] fn landscape_sheets_ask_cups_to_turn_the_page() { let landscape = |size: (f64, f64)| Sheet { size, ..Sheet::default() }; @@ -531,6 +535,26 @@ fn landscape_sheets_ask_cups_to_turn_the_page() { assert!(lp_args(&job).join(" ").ends_with("-o fit-to-page=false -o orientation-requested=4")); } +/// Both submission paths add the orientation through `add_cups_orientation`: after the job's +/// other options, never over an orientation the job already names (a driver choice), so `lp` is +/// never given it twice, and not on Windows, whose print ticket sets it. +#[test] +fn a_job_that_names_its_orientation_keeps_it() { + let landscape = layout(&[(300.0, 200.0)], &settings(vec![0], Layout::Size(SizeMode::Fit))).unwrap(); + let media = ("media".to_string(), "Letter".to_string()); + let mut options = vec![media.clone()]; + add_cups_orientation(&mut options, &landscape); + let turned = if cfg!(windows) { vec![media] } else { vec![media, ("orientation-requested".to_string(), "4".to_string())] }; + assert_eq!(options, turned); + // Added again, it isn't doubled. + add_cups_orientation(&mut options, &landscape); + assert_eq!(options, turned); + // A choice already made stands. + let mut chosen = vec![("orientation-requested".to_string(), "5".to_string())]; + add_cups_orientation(&mut chosen, &landscape); + assert_eq!(chosen, [("orientation-requested".to_string(), "5".to_string())]); +} + #[cfg(all(unix, not(target_arch = "wasm32")))] mod cups_options { use crate::spool; diff --git a/crates/ui-egui/src/lib.rs b/crates/ui-egui/src/lib.rs index 79f46bee7b..83386eed58 100644 --- a/crates/ui-egui/src/lib.rs +++ b/crates/ui-egui/src/lib.rs @@ -117,7 +117,7 @@ mod signature_drag; pub mod units; mod web_print; mod word_count_ui; -pub use print_ui::{Handling as PrintHandling, OptionsSource, PrintDraft, PrinterSource, Which as PrintWhich, system_printers}; +pub use print_ui::{Handling as PrintHandling, OptionsSource, PrintDraft, PrinterSource, Spooler, Which as PrintWhich, system_printers}; mod redact_ui; pub use redact_ui::{HiddenDraft, PagesDraft as RedactPagesDraft, RedactPrefs, SearchDraft as RedactSearchDraft, SearchMode as RedactSearchMode}; pub mod i18n; @@ -706,6 +706,8 @@ pub struct PdfCraftApp { pub printer_source: Option, /// Where Properties… reads a printer's driver options: the spooler's unless a test stands in. pub options_source: Option, + /// What print jobs go to: the system spooler unless a test stands in ([`Spooler`]). + pub spooler: Option, // A browser has no background print work: the type is empty there. #[cfg_attr(target_arch = "wasm32", allow(dead_code))] pub(crate) print_tasks: print_ui::PrintTasks, @@ -1139,6 +1141,7 @@ impl PdfCraftApp { updates: updates::Updates::default(), printer_source: None, options_source: None, + spooler: None, print_tasks: print_ui::PrintTasks::default(), palette_open: false, palette_query: String::new(), diff --git a/crates/ui-egui/src/print_ui.rs b/crates/ui-egui/src/print_ui.rs index 2a68927f26..382e70df4d 100644 --- a/crates/ui-egui/src/print_ui.rs +++ b/crates/ui-egui/src/print_ui.rs @@ -264,6 +264,10 @@ pub type PrinterSource = std::sync::Arc Vec + Send + /// UI language) unless a test stands in. Called off the GUI thread. pub type OptionsSource = std::sync::Arc Vec + Send + Sync>; +/// What takes a print job in place of the system spooler: a test's stand-in, which gets the +/// print-ready PDF and the job and answers as the spooler would. Called off the GUI thread. +pub type Spooler = std::sync::Arc Result + Send + Sync>; + /// The system spooler's printers, as the desktop app's [`PrinterSource`]. pub fn system_printers() -> PrinterSource { std::sync::Arc::new(spool::printers) @@ -655,10 +659,11 @@ impl PdfCraftApp { } Some(printer) => { let mut job = self.print_draft.job(&name); - // CUPS is told which way the sheets run; Windows' print ticket already is. - if !cfg!(windows) { - let sizes: Vec<(f64, f64)> = doc.info.pages.iter().map(|p| (f64::from(p.width), f64::from(p.height))).collect(); - job.options.extend(print::layout(&sizes, &settings).ok().and_then(|sheets| print::cups_orientation(&sheets))); + // CUPS is told which way the sheets run (#2713), unless a driver choice already + // says; Windows' print ticket already is. + let sizes: Vec<(f64, f64)> = doc.info.pages.iter().map(|p| (f64::from(p.width), f64::from(p.height))).collect(); + if let Ok(sheets) = print::layout(&sizes, &settings) { + print::add_cups_orientation(&mut job.options, &sheets); } self.send_to_printer(printer, bytes, job) } @@ -674,6 +679,7 @@ impl PdfCraftApp { fn send_to_printer(&mut self, printer: String, bytes: Vec, job: spool::Job) -> bool { let (tx, rx) = std::sync::mpsc::channel(); let ctx = self.ctx.clone(); + let spooler = self.spooler.clone(); std::thread::spawn(move || { let repaint = || { if let Some(ctx) = &ctx { @@ -681,11 +687,15 @@ impl PdfCraftApp { } }; // The receiver may be gone (the app quit): nothing to report to then. - let result = spool::submit_with_progress(&bytes, &job, &mut |sent, sheets| { - let _ = tx.send(JobNews::Sent(sent, sheets)); - repaint(); - }); - let _ = tx.send(JobNews::Done(result.map_err(|e| e.to_string()))); + let result = match &spooler { + Some(submit) => submit(&bytes, &job), + None => spool::submit_with_progress(&bytes, &job, &mut |sent, sheets| { + let _ = tx.send(JobNews::Sent(sent, sheets)); + repaint(); + }) + .map_err(|e| e.to_string()), + }; + let _ = tx.send(JobNews::Done(result)); repaint(); }); self.print_tasks.jobs.push(PrintJob { printer, sheets: None, news: rx }); diff --git a/crates/ui-egui/tests/it/print.rs b/crates/ui-egui/tests/it/print.rs index 81025e756f..5d0707ede8 100644 --- a/crates/ui-egui/tests/it/print.rs +++ b/crates/ui-egui/tests/it/print.rs @@ -215,6 +215,8 @@ fn cut_stack_dialog_previews_saves_and_refuses_duplex() { let _ = std::fs::remove_file(out); } +/// Save as PDF to a folder that doesn't exist keeps the Print dialog open, with its settings, +/// and says why. #[test] fn a_failed_save_as_pdf_keeps_the_dialog_open() { let mut h = harness(); @@ -232,6 +234,67 @@ fn a_failed_save_as_pdf_keeps_the_dialog_open() { assert!(toast.contains("Could not save"), "the user is told why: {toast:?}"); } +/// #2713, #3739: Print to a CUPS queue tells it when every sheet runs landscape, so a +/// landscape sheet (an A4 landscape page, or a 6 × 4 label, laid out on Letter) is turned onto +/// the portrait paper instead of printed unrotated at 100% and cut off at the paper's edge. A +/// driver choice for the orientation stands and isn't sent twice. +#[test] +fn printing_landscape_sheets_tells_cups_to_turn_them() { + use pdfcraft_engine::print::spool; + use std::sync::{Arc, Mutex}; + let s = pdfcraft_engine::Session::new(); + let bytes = s.create_blank(842.0, 595.0, 2).unwrap(); + let mut h = Harness::builder().with_size(egui::vec2(1400.0, 900.0)).build_eframe(move |_cc| { + let mut app = PdfCraftApp::new(); + app.set_option("language", "en").unwrap(); + app.open_bytes("landscape.pdf", None, bytes.to_vec()).unwrap(); + app + }); + h.run_steps(4); + let jobs: Arc>> = Arc::default(); + let sent = Arc::clone(&jobs); + h.state_mut().spooler = Some(Arc::new(move |_pdf: &[u8], job: &spool::Job| { + sent.lock().unwrap().push(job.clone()); + Ok(String::new()) + })); + // Print to the queue; `choose` sets the driver's choices once the dialog has its printer. + let print = |h: &mut Harness<'static, PdfCraftApp>, choose: &dyn Fn(&mut pdfcraft_ui_egui::PrintDraft)| -> spool::Job { + let before = jobs.lock().unwrap().len(); + assert!(h.state_mut().execute("print.dialog")); + h.run_steps(3); + h.state_mut().print_draft.printer = Some("Test_Queue".into()); + h.run_steps(2); + choose(&mut h.state_mut().print_draft); + h.run_steps(1); + h.get_by(|n| n.label().as_deref() == Some("Print") && n.role() == Role::Button).click(); + for _ in 0..400 { + h.run_steps(1); + if jobs.lock().unwrap().len() > before { + break; + } + std::thread::sleep(std::time::Duration::from_millis(5)); + } + jobs.lock().unwrap().last().cloned().expect("the job reached the spooler") + }; + let orientation = |job: &spool::Job| job.options.iter().filter(|(k, _)| k == "orientation-requested").map(|(_, v)| v.clone()).collect::>(); + // Fit on Letter with Auto orientation lays both pages out on landscape sheets. + let job = print(&mut h, &|_| {}); + assert_eq!(orientation(&job), if cfg!(windows) { vec![] } else { vec!["4".to_string()] }, "{:?}", job.options); + // A driver choice for the orientation goes instead, once. + let job = print(&mut h, &|d| { + let choice = spool::PrinterOption { + key: "orientation-requested".into(), + label: "Orientation".into(), + group: "General".into(), + choices: vec![("4".into(), "Landscape".into()), ("5".into(), "Reverse landscape".into())], + default: "4".into(), + }; + d.driver_options = Some(("Test_Queue".into(), vec![choice])); + d.driver_choices.insert("orientation-requested".into(), "5".into()); + }); + assert_eq!(orientation(&job), ["5"], "{:?}", job.options); +} + /// Properties… shows the printer driver's own options (CUPS) and sends only those changed. #[test] fn print_dialog_shows_the_printer_driver_options() { diff --git a/parity/acrobat-features.toml b/parity/acrobat-features.toml index a042c2d642..e3319a5fb6 100644 --- a/parity/acrobat-features.toml +++ b/parity/acrobat-features.toml @@ -8144,8 +8144,8 @@ milestone = "M10" status = "shipped" commands = ["print.dialog"] tools = ["doc_print"] -evidence = ["crates/print/src/tests.rs::size_modes", "crates/print/src/tests.rs::architectural_sheets_preserve_full_scale", "crates/automation/tests/it/automation.rs::architectural_and_iso_papers_through_tools", "crates/ui-egui/tests/it/print.rs::architectural_page_can_be_matched_at_100_percent", "crates/print/src/tests.rs::landscape_sheets_ask_cups_to_turn_the_page"] -notes = "auto portrait/landscape. CUPS jobs carry orientation-requested=4 when every sheet is landscape, so a landscape sheet fills its portrait paper instead of being shrunk into it (#2713); a job that mixes orientations is left to the spooler, and Windows sets orientation in its own print ticket. The CUPS option is checked against Apple's filters and one physical printer, not a test that runs a spooler." +evidence = ["crates/print/src/tests.rs::size_modes", "crates/print/src/tests.rs::architectural_sheets_preserve_full_scale", "crates/automation/tests/it/automation.rs::architectural_and_iso_papers_through_tools", "crates/ui-egui/tests/it/print.rs::architectural_page_can_be_matched_at_100_percent", "crates/print/src/tests.rs::landscape_sheets_ask_cups_to_turn_the_page", "crates/print/src/tests.rs::a_job_that_names_its_orientation_keeps_it", "crates/automation/src/printing.rs::printing_landscape_sheets_to_a_printer_asks_cups_to_turn_them", "crates/ui-egui/tests/it/print.rs::printing_landscape_sheets_tells_cups_to_turn_them"] +notes = "auto portrait/landscape. CUPS jobs carry orientation-requested=4 when every sheet is landscape, so a landscape sheet fills its portrait paper instead of being shrunk into it or, with the Mac's filters, printed unrotated and cut off at the paper's edge (#2713, #3739); an orientation the job already names (a driver choice, or doc_print's options) stands, and a job that mixes orientations is left to the spooler, so its landscape sheets still print unturned. Windows sets orientation in its own print ticket. The CUPS option is checked against Apple's filters (cupsfilter on an AirPrint PPD) and one physical printer; the tests check the job both submission paths send, not a spooler's output." [[feature]] id = "print.comments-and-forms"