M10: one guarded orientation for both print paths, with a test at each (#2713, #3739)

Review follow-ups to the landscape fix rebased from #3068:

- Both submission paths add the orientation through
  pdfcraft_print::add_cups_orientation, which never adds it over an
  orientation the job already names (a driver choice in the Print
  dialog, or a caller's doc_print options), so lp never gets it twice.
- doc_print adds it inside spool_job, next to the paper (#3752), so its
  test checks the job it sends.
- The Print dialog takes a stand-in for the spooler in tests
  (PdfCraftApp::spooler), like printer_source and options_source, and a
  test follows a real Print click to the job.
- Doc comments for the tests beside the change.

Checked on macOS with cupsfilter (cgpdftoraster, AirPrint PPD): a landscape
sheet sent as before lands unrotated at 100% and cut off at the paper's
right edge (#3739); with the option it is turned inside the margins.

Co-authored-by: Zach <90340773+zdblazer@users.noreply.github.com>

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<spool::Duplex>, paper: usize) -> Result<spool::Job> {
+/// 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<spool::Duplex>, paper: usize, sheets: &[print::Sheet]) -> Result<spool::Job> {
     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<spool::Duple
         scaling: spool::Scaling::Exact,
         grayscale: a.opt_bool("grayscale")?.unwrap_or(false),
         title,
-        options: options.into_iter().chain(print::media_option(paper)).collect(),
+        options: {
+            let mut options: Vec<(String, String)> = 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::<Vec<_>>();
+        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());
     }
 }

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.

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;

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<PrinterSource>,
     /// Where Properties… reads a printer's driver options: the spooler's unless a test stands in.
     pub options_source: Option<OptionsSource>,
+    /// What print jobs go to: the system spooler unless a test stands in ([`Spooler`]).
+    pub spooler: Option<Spooler>,
     // 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(),

crates/ui-egui/src/print_ui.rs

@@ -264,6 +264,10 @@ pub type PrinterSource = std::sync::Arc<dyn Fn() -> Vec<spool::Printer> + Send +
 /// UI language) unless a test stands in. Called off the GUI thread.
 pub type OptionsSource = std::sync::Arc<dyn Fn(&str) -> Vec<spool::PrinterOption> + 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<dyn Fn(&[u8], &spool::Job) -> Result<String, String> + 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<u8>, 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 });

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<Mutex<Vec<spool::Job>>> = 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::<Vec<_>>();
+    // 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() {

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"