From 2f0886038318f6f87b27a51f52b2f56927c9d762 Mon Sep 17 00:00:00 2001 From: Wolf Date: Mon, 19 Oct 2026 21:46:41 +0200 Subject: [PATCH 1/2] Print: Portrait and Landscape in the Print dialog on Windows MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit On Windows the Print dialog takes the paper and its orientation from the printer driver, and had no way to change the orientation short of the driver's own Print Settings… dialog. Photoshop has Layout buttons for it. A print laid out landscape on portrait driver settings (from a script, the CLI or another app) stopped with "the driver paper or orientation differs from the print layout". - The dialog has an Orientation row (Portrait, Landscape) under Print Settings…. It turns the driver settings through file.print.page and takes the paper from the driver again, so the paper and preview turn. - file.print.page takes "orientation": "portrait|landscape" and returns the turned settings as printerSettings, with the page they give. - print_windows turns the settings (the given ones or the printer's defaults) to the layout's orientation when the driver has the layout's paper the other way round; another paper is still refused. Turning sets dmOrientation and its dmFields bit in the DEVMODE and keeps every other byte, the driver's private settings included; the driver validates the result as it does settings from Print Settings…. No new UI strings: "Orientation", "Portrait" and "Landscape" are in every catalog. Tests: the turned settings (orientation set, all else kept, with and without private driver bytes), when a paper counts as turned, an unknown orientation refused before any printer is asked, the dialog's reading of the driver paper and its error path, and, run by hand on Windows, a real "Microsoft Print to PDF" driver turning for a landscape layout. Co-Authored-By: Claude Opus 5.5 --- crates/engine/src/print_cmds.rs | 88 ++++++++++++++++++++++++--- crates/engine/src/print_cmds/tests.rs | 73 ++++++++++++++++++++++ crates/ui-egui/src/file_ui.rs | 65 ++++++++++++++++++++ docs/printing.md | 11 ++++ 4 files changed, 230 insertions(+), 7 deletions(-) diff --git a/crates/engine/src/print_cmds.rs b/crates/engine/src/print_cmds.rs index 0a1701a166..483267d333 100644 --- a/crates/engine/src/print_cmds.rs +++ b/crates/engine/src/print_cmds.rs @@ -708,6 +708,61 @@ fn printer_settings(p: &Value) -> Result> { } } +/// `dmFields` and `dmOrientation` in DEVMODEW (wingdi.h: after the 32-wchar device name and four +/// header words), and the `dmFields` bit that says `dmOrientation` is set. +const DM_FIELDS_AT: usize = 72; +const DM_ORIENTATION_AT: usize = 76; +const DM_ORIENTATION: u32 = 0x1; +/// `DMORIENT_PORTRAIT` and `DMORIENT_LANDSCAPE`. +const DMORIENT_PORTRAIT: i16 = 1; +const DMORIENT_LANDSCAPE: i16 = 2; + +/// Driver settings turned to portrait or landscape, as Photoshop's Layout buttons turn them: +/// `dmOrientation` set and flagged in `dmFields`, every other byte (the driver's private settings +/// included) kept. The driver validates the result when it lays out the page, as it does for +/// settings from Print Settings…. +pub fn turned_settings(settings: &craft_print::rgb::DevMode, landscape: bool) -> Result { + let mut b = settings.as_bytes().to_vec(); + let short = || other("printer settings are too short to hold an orientation"); + let fields: [u8; 4] = b.get(DM_FIELDS_AT..DM_FIELDS_AT + 4).and_then(|s| s.try_into().ok()).ok_or_else(short)?; + let fields = u32::from_le_bytes(fields) | DM_ORIENTATION; + b.get_mut(DM_FIELDS_AT..DM_FIELDS_AT + 4).ok_or_else(short)?.copy_from_slice(&fields.to_le_bytes()); + let orientation = if landscape { DMORIENT_LANDSCAPE } else { DMORIENT_PORTRAIT }; + b.get_mut(DM_ORIENTATION_AT..DM_ORIENTATION_AT + 2).ok_or_else(short)?.copy_from_slice(&orientation.to_le_bytes()); + craft_print::rgb::DevMode::from_bytes(b).map_err(other) +} + +/// Whether a driver page of `driver` points is the layout's `paper` turned the other way round +/// (within the 2 pt the driver and layout may differ by): then turning the driver settings, not +/// choosing another paper, makes them agree. A square paper is never turned. +fn turned_paper(driver: (f64, f64), paper: (f64, f64)) -> bool { + let near = |a: f64, b: f64| (a - b).abs() <= 2.0; + let same = near(driver.0, paper.0) && near(driver.1, paper.1); + !same && near(driver.0, paper.1) && near(driver.1, paper.0) +} + +/// The driver settings and page for printing a layout of `paper` points on `printer`: the given +/// settings (or the printer's own) as they are, or turned to the layout's orientation when the +/// driver has that paper the other way round. A landscape layout so prints landscape with portrait +/// Print Settings, as a script or another app asking for landscape expects. +fn settings_for_layout( + printer: &str, + settings: Option, + paper: (f64, f64), +) -> Result<(Option, craft_print::rgb::Page)> { + let page = craft_print::rgb::page(printer, settings.as_ref()).map_err(other)?; + if !turned_paper(page.paper, paper) { + return Ok((settings, page)); + } + let base = match settings { + Some(s) => s, + None => craft_print::rgb::default_settings(printer).map_err(other)?, + }; + let turned = turned_settings(&base, paper.0 > paper.1)?; + let page = craft_print::rgb::page(printer, Some(&turned)).map_err(other)?; + Ok((Some(turned), page)) +} + /// The page's image as 8-bit RGB rows (gray expanded), mirrored for Emulsion Down. pub fn page_rgb(page: &PrintPage) -> Result> { let img = &page.image; @@ -742,8 +797,7 @@ pub fn page_rect_top_left(page: &PrintPage) -> (f64, f64, f64, f64) { /// PDF-only (no font/text layout in the GDI path yet). fn print_windows(page: &PrintPage, p: &Value, title: &str, copies: u32) -> Result { let printer = printer_name(p)?; - let settings = printer_settings(p)?; - let device_page = craft_print::rgb::page(&printer, settings.as_ref()).map_err(other)?; + let (settings, device_page) = settings_for_layout(&printer, printer_settings(p)?, page.paper)?; if (device_page.paper.0 - page.paper.0).abs() > 2.0 || (device_page.paper.1 - page.paper.1).abs() > 2.0 { return Err(other("the driver paper or orientation differs from the print layout; choose the matching paper in Print Settings before printing")); } @@ -780,11 +834,31 @@ fn printers(_: &mut Session, _: &Value) -> Result { } fn printer_page(_: &mut Session, p: &Value) -> Result { + // Checked before the printer, so a bad value is the error whether or not a printer answers. + let landscape = match p.get("orientation").filter(|v| !v.is_null()) { + None => None, + Some(v) => match v.as_str() { + Some("portrait") => Some(false), + Some("landscape") => Some(true), + _ => return Err(bad("file.print.page", "orientation is \"portrait\" or \"landscape\"")), + }, + }; let printer = printer_name(p)?; - let pg = craft_print::rgb::page(&printer, printer_settings(p)?.as_ref()).map_err(other)?; - Ok( - json!({"printer": printer, "paper": [pg.paper.0, pg.paper.1], "printable": [pg.printable.0, pg.printable.1, pg.printable.2, pg.printable.3], "dpi": [pg.dpi.0, pg.dpi.1]}), - ) + let mut settings = printer_settings(p)?; + if let Some(landscape) = landscape { + let base = match settings { + Some(s) => s, + None => craft_print::rgb::default_settings(&printer).map_err(other)?, + }; + settings = Some(turned_settings(&base, landscape)?); + } + let pg = craft_print::rgb::page(&printer, settings.as_ref()).map_err(other)?; + let mut out = json!({"printer": printer, "paper": [pg.paper.0, pg.paper.1], "printable": [pg.printable.0, pg.printable.1, pg.printable.2, pg.printable.3], "dpi": [pg.dpi.0, pg.dpi.1]}); + // The turned settings, to pass on as `printerSettings` (the Print dialog's Orientation does). + if let (Some(_), Some(s), Some(o)) = (landscape, settings, out.as_object_mut()) { + o.insert("printerSettings".into(), json!(s.to_hex())); + } + Ok(out) } /// Set once by test harnesses (the panic hunt) so no command in the process can reach a printer. @@ -1014,7 +1088,7 @@ pub fn specs() -> Vec { label: "Printer Page", menu: &[], shortcut: None, - params: r##"{"printer":name? (default printer),"printerSettings":hex?} → {printer, paper: [w,h] pt, printable: [x,y,w,h] pt from the top-left, dpi: [x,y]} (Windows)"##, + params: r##"{"printer":name? (default printer),"printerSettings":hex?,"orientation":"portrait|landscape"? (turn the settings, or the printer's own, that way)} → {printer, paper: [w,h] pt, printable: [x,y,w,h] pt from the top-left, dpi: [x,y], printerSettings: hex (only with orientation: the turned settings)} (Windows)"##, enabled: |_| Ok(()), journal: false, run: printer_page, diff --git a/crates/engine/src/print_cmds/tests.rs b/crates/engine/src/print_cmds/tests.rs index c54dddf6f8..685c0ec4d3 100644 --- a/crates/engine/src/print_cmds/tests.rs +++ b/crates/engine/src/print_cmds/tests.rs @@ -207,6 +207,79 @@ fn printer_commands_answer_without_a_document_and_reject_bad_settings() { assert!(s.execute("file.print", json!({"printer": "Anything", "printerSettings": "0102", "send": true})).is_err()); } +/// Synthetic driver settings: a DEVMODEW of `size` bytes plus `extra` private driver bytes, every +/// byte a pattern so a stray write shows, with `dmFields` saying only the paper size is set. +fn devmode(size: u16, extra: u16) -> Vec { + let mut b: Vec = (0..usize::from(size) + usize::from(extra)).map(|i| (i % 251) as u8).collect(); + b[68..70].copy_from_slice(&size.to_le_bytes()); + b[70..72].copy_from_slice(&extra.to_le_bytes()); + b[72..76].copy_from_slice(&0x2u32.to_le_bytes()); // DM_PAPERSIZE + b +} + +/// Photoshop's Layout buttons: driver settings turned to landscape or portrait set `dmOrientation` +/// and its `dmFields` bit, and keep every other byte, the driver's private settings included. +#[test] +fn turned_settings_set_the_orientation_and_keep_the_rest() { + for (size, extra) in [(220u16, 0u16), (220, 48)] { + let before = devmode(size, extra); + let dm = craft_print::rgb::DevMode::from_bytes(before.clone()).unwrap(); + for (landscape, orient) in [(true, 2i16), (false, 1i16)] { + let after = turned_settings(&dm, landscape).unwrap().as_bytes().to_vec(); + assert_eq!(after.len(), before.len()); + assert_eq!(i16::from_le_bytes([after[76], after[77]]), orient, "{size}+{extra} landscape={landscape}"); + assert_eq!(u32::from_le_bytes([after[72], after[73], after[74], after[75]]), 0x3, "DM_ORIENTATION added, DM_PAPERSIZE kept"); + for (i, (a, b)) in after.iter().zip(&before).enumerate() { + if !(72..76).contains(&i) && !(76..78).contains(&i) { + assert_eq!(a, b, "byte {i} changed"); + } + } + } + } +} + +/// The driver's paper is turned only when it is the layout's paper the other way round (within +/// the 2 pt a driver's paper may differ by): not for the same paper, another paper or a square one. +#[test] +fn only_the_same_paper_the_other_way_round_is_turned() { + let (a3, a3_wide) = ((842.29, 1191.14), (1190.55, 841.89)); + assert!(turned_paper(a3, a3_wide), "portrait driver, landscape layout"); + assert!(turned_paper((1191.14, 842.29), (841.89, 1190.55)), "landscape driver, portrait layout"); + assert!(!turned_paper(a3, (841.89, 1190.55)), "same orientation"); + assert!(!turned_paper((595.32, 841.92), a3_wide), "an A4 driver for an A3 layout"); + assert!(!turned_paper((612.0, 612.0), (612.0, 612.0)), "square"); +} + +/// `file.print.page` takes an orientation and refuses any other value before asking a printer, so +/// the error is the same with or without one. +#[test] +fn printer_page_refuses_an_unknown_orientation() { + let mut s = Session::new(); + for bad in [json!("sideways"), json!(2), json!(true)] { + let e = s.execute("file.print.page", json!({"printer": "Anything", "orientation": bad})).unwrap_err().to_string(); + assert!(e.contains("orientation"), "{bad}: {e}"); + } +} + +/// A real driver turns: Windows' "Microsoft Print to PDF" on its default (portrait) settings, for a +/// landscape A4 layout, gets settings turned to landscape and a wide page; a portrait layout keeps +/// the driver's own settings. Needs that printer, so it runs by hand: +/// `cargo test -p photocraft-engine --lib a_portrait_driver -- --ignored`. +#[cfg(windows)] +#[test] +#[ignore] +fn a_portrait_driver_turns_for_a_landscape_layout() { + let printer = "Microsoft Print to PDF"; + let defaults = craft_print::rgb::default_settings(printer).unwrap(); + let a4 = craft_print::rgb::page(printer, Some(&defaults)).unwrap().paper; + let (settings, page) = settings_for_layout(printer, None, (a4.1, a4.0)).unwrap(); + assert!(settings.is_some(), "turned settings come back"); + assert!((page.paper.0 - a4.1).abs() <= 2.0 && (page.paper.1 - a4.0).abs() <= 2.0, "wide: {:?}", page.paper); + let (settings, page) = settings_for_layout(printer, None, a4).unwrap(); + assert!(settings.is_none(), "nothing to turn"); + assert!((page.paper.0 - a4.0).abs() <= 2.0, "{:?}", page.paper); +} + #[test] fn printer_profile_can_be_an_installed_icc_file() { let dir = tmp("icc"); diff --git a/crates/ui-egui/src/file_ui.rs b/crates/ui-egui/src/file_ui.rs index 14a512c288..7573e1757b 100644 --- a/crates/ui-egui/src/file_ui.rs +++ b/crates/ui-egui/src/file_ui.rs @@ -772,6 +772,34 @@ pub fn open_print(app: &mut PhotocraftApp) -> u64 { app.ui.open_dialog(DialogKind::Command, f) } +/// Whether the driver's paper, as `driver_page` took it, is wider than tall: the Orientation +/// buttons show landscape then. +fn driver_paper_is_wide(f: &Map) -> bool { + let side = |i: usize| f.get("paper").and_then(Value::as_array).and_then(|a| a.get(i)).and_then(Value::as_f64); + matches!((side(0), side(1)), (Some(w), Some(h)) if w > h) +} + +/// The Orientation buttons: the driver settings (or the printer's own) turned portrait or +/// landscape through `file.print.page`, then the paper taken from the driver again. An error stays +/// on the dialog, with the settings left as they were. +fn turn_driver(app: &mut PhotocraftApp, f: &mut Map, landscape: bool) { + let mut q = json!({"printer": s(f, "printer", ""), "orientation": if landscape { "landscape" } else { "portrait" }}); + if let (Some(h), Some(o)) = (f.get("printerSettings").cloned(), q.as_object_mut()) { + o.insert("printerSettings".into(), h); + } + match app.session.execute("file.print.page", q) { + Ok(r) => { + if let Some(h) = r.get("printerSettings").cloned() { + f.insert("printerSettings".into(), h); + driver_page(f); + } + } + Err(e) => { + f.insert("__pageError".into(), json!(e.to_string())); + } + } +} + /// Takes the paper from the printer's driver settings (Windows), as Photoshop does. fn driver_page(f: &mut Map) { let printer = s(f, "printer", ""); @@ -1006,6 +1034,17 @@ fn print_body(app: &mut PhotocraftApp, ui: &mut egui::Ui, f: &mut Map format!("{:.0} × {:.0} mm · {}", w / 72.0 * 25.4, h / 72.0 * 25.4, tl!("set in Print Settings…")), @@ -1530,6 +1569,32 @@ mod tests { std::fs::remove_file(out).unwrap(); } } + + /// The Orientation buttons show the driver paper's shape: landscape when it is wider than tall. + #[test] + fn orientation_follows_the_driver_paper() { + let with = |paper: Value| { + let mut f = Map::new(); + f.insert("paper".into(), paper); + f + }; + assert!(driver_paper_is_wide(&with(json!([1191.14, 842.29])))); + assert!(!driver_paper_is_wide(&with(json!([842.29, 1191.14])))); + assert!(!driver_paper_is_wide(&with(json!("a3"))), "a named paper is not a driver page"); + assert!(!driver_paper_is_wide(&Map::new())); + } + + /// Turning a printer whose driver can't be asked keeps its settings and says why on the dialog. + #[test] + fn turning_without_a_driver_keeps_the_settings_and_shows_the_error() { + let (mut app, _) = app(); + let mut f = Map::new(); + f.insert("printer".into(), json!("photocraft-no-such-printer")); + f.insert("printerSettings".into(), json!("00ff")); + turn_driver(&mut app, &mut f, true); + assert_eq!(f.get("printerSettings"), Some(&json!("00ff"))); + assert!(f.get("__pageError").is_some()); + } } #[cfg(test)] diff --git a/docs/printing.md b/docs/printing.md index 68015d278e..399944d57d 100644 --- a/docs/printing.md +++ b/docs/printing.md @@ -38,6 +38,17 @@ with a `note` when no server answers. `"printServer"` on `file.print` sends one server; `"dryRun": true` renders the page and reports the equivalent `lp` command without spooling. +## Orientation (Windows) + +On Windows the paper and its orientation are the driver's: the Print dialog takes them from the +printer's settings. Its Orientation buttons (Portrait, Landscape; Photoshop's Layout) turn those +settings, as Print Settings… would, and the paper and preview turn with them. +`file.print.page {"orientation": "portrait|landscape"}` does the same for scripts: it returns the +turned settings as `printerSettings`, with the page they give. A print sent to a Windows printer +whose layout is the driver's paper the other way round (a landscape layout, portrait settings) +turns the settings to the layout before printing, rather than stopping with "the driver paper or +orientation differs"; another paper is still refused. + ## Results `file.print` returns `sent`, `backend` (`lp`, `windows`, `ipp`, `portal`), `spooler` (where the From f8117a535c2f20de9a15280e3c2f70412622da27 Mon Sep 17 00:00:00 2001 From: Wolf Date: Mon, 19 Oct 2026 22:54:46 +0200 Subject: [PATCH 2/2] Print: a turn takes settings and paper from one answer (#7847 review) The Orientation buttons stored the turned settings from file.print.page and then asked the driver again (driver_page) for the paper. If that second query failed, the dialog kept the new settings with the old paper. take_turned_page now takes both, and the orientation, from the one answer; an answer without both leaves the dialog as it was and shows why. The no-driver test used "00ff", which fails when the settings are read, before any printer is asked, so it never tested the failed-driver path. It now uses valid settings (a minimal DEVMODEW) and asserts the error comes from the printer, not from reading the settings; with "00ff" it fails. a_turn_takes_settings_and_paper_together covers the success path and partial answers. Reported by CodeRabbit in review. Co-Authored-By: Claude Opus 5.5 --- crates/ui-egui/src/file_ui.rs | 65 +++++++++++++++++++++++++++++------ 1 file changed, 54 insertions(+), 11 deletions(-) diff --git a/crates/ui-egui/src/file_ui.rs b/crates/ui-egui/src/file_ui.rs index 7573e1757b..32f97bc4e8 100644 --- a/crates/ui-egui/src/file_ui.rs +++ b/crates/ui-egui/src/file_ui.rs @@ -780,26 +780,36 @@ fn driver_paper_is_wide(f: &Map) -> bool { } /// The Orientation buttons: the driver settings (or the printer's own) turned portrait or -/// landscape through `file.print.page`, then the paper taken from the driver again. An error stays -/// on the dialog, with the settings left as they were. +/// landscape through `file.print.page`. An error stays on the dialog, with the settings left as +/// they were. fn turn_driver(app: &mut PhotocraftApp, f: &mut Map, landscape: bool) { let mut q = json!({"printer": s(f, "printer", ""), "orientation": if landscape { "landscape" } else { "portrait" }}); if let (Some(h), Some(o)) = (f.get("printerSettings").cloned(), q.as_object_mut()) { o.insert("printerSettings".into(), h); } match app.session.execute("file.print.page", q) { - Ok(r) => { - if let Some(h) = r.get("printerSettings").cloned() { - f.insert("printerSettings".into(), h); - driver_page(f); - } - } + Ok(r) => take_turned_page(f, &r), Err(e) => { f.insert("__pageError".into(), json!(e.to_string())); } } } +/// The turned settings and the paper they give, both from the one `file.print.page` answer, as +/// `driver_page` takes a page: asking the driver again could fail and leave new settings with the +/// old paper. An answer without both leaves the dialog as it was, with the reason shown. +fn take_turned_page(f: &mut Map, r: &Value) { + let paper = r.get("paper").and_then(Value::as_array).and_then(|a| Some((a.first()?.as_f64()?, a.get(1)?.as_f64()?))); + let (Some(settings), Some((w, h))) = (r.get("printerSettings").and_then(Value::as_str), paper) else { + f.insert("__pageError".into(), json!("the printer did not report the turned page")); + return; + }; + f.insert("printerSettings".into(), json!(settings)); + f.insert("paper".into(), json!([w, h])); + f.insert("orientation".into(), json!("portrait")); + f.remove("__pageError"); +} + /// Takes the paper from the printer's driver settings (Windows), as Photoshop does. fn driver_page(f: &mut Map) { let printer = s(f, "printer", ""); @@ -1587,13 +1597,46 @@ mod tests { /// Turning a printer whose driver can't be asked keeps its settings and says why on the dialog. #[test] fn turning_without_a_driver_keeps_the_settings_and_shows_the_error() { + // Valid driver settings (a minimal DEVMODEW: 220 bytes, no private part), so the command + // gets past reading them and fails where it asks the (missing) printer for its page. + let mut dm = vec![0u8; 220]; + dm[68..70].copy_from_slice(&220u16.to_le_bytes()); + let hex: String = dm.iter().map(|b| format!("{b:02x}")).collect(); + assert!(craft_print::rgb::DevMode::from_hex(&hex).is_ok()); let (mut app, _) = app(); let mut f = Map::new(); f.insert("printer".into(), json!("photocraft-no-such-printer")); - f.insert("printerSettings".into(), json!("00ff")); + f.insert("printerSettings".into(), json!(hex)); + f.insert("paper".into(), json!([595.28, 841.89])); turn_driver(&mut app, &mut f, true); - assert_eq!(f.get("printerSettings"), Some(&json!("00ff"))); - assert!(f.get("__pageError").is_some()); + assert_eq!(f.get("printerSettings"), Some(&json!(hex))); + assert_eq!(f.get("paper"), Some(&json!([595.28, 841.89]))); + let error = f.get("__pageError").and_then(Value::as_str).unwrap_or_default(); + // Reading bad settings fails with "printer settings are too short / damaged"; this must not. + assert!(!error.is_empty() && !error.contains("printer settings are"), "failed at the printer, not at reading the settings: {error}"); + } + + /// A turn takes the settings and the paper from the same answer, never one without the other. + #[test] + fn a_turn_takes_settings_and_paper_together() { + let fields = || { + let mut f = Map::new(); + f.insert("printerSettings".into(), json!("old")); + f.insert("paper".into(), json!([595.28, 841.89])); + f.insert("__pageError".into(), json!("earlier")); + f + }; + let mut f = fields(); + take_turned_page(&mut f, &json!({"printerSettings": "new", "paper": [841.89, 595.28]})); + assert_eq!((f.get("printerSettings"), f.get("paper")), (Some(&json!("new")), Some(&json!([841.89, 595.28])))); + assert_eq!(f.get("orientation"), Some(&json!("portrait")), "the driver paper is already turned"); + assert!(f.get("__pageError").is_none()); + for partial in [json!({"printerSettings": "new"}), json!({"paper": [841.89, 595.28]}), json!({"printerSettings": "new", "paper": [841.89]})] { + let mut f = fields(); + take_turned_page(&mut f, &partial); + assert_eq!((f.get("printerSettings"), f.get("paper")), (Some(&json!("old")), Some(&json!([595.28, 841.89]))), "{partial}"); + assert!(f.get("__pageError").is_some(), "{partial}"); + } } }