shifathosn
/
pdfcraft
1
0
Fork 0
Code Issues Pull Requests Packages Projects Releases Wiki Activity

fix(print): custom pages-per-sheet counts no longer overfill sheets (#1336) (#1430) 

Layout::multiple(n) maps counts without a fixed grid to a near-square
grid (3 -> 2x2, 5 -> 3x2), but Layout::Multiple kept only the grid, so
layout() filled every cell: three per sheet put four pages on a sheet.

Layout::Multiple now carries per_sheet, capped by the grid; spare cells
stay blank and cut-stack piles use the same count. The doc_print tool
and the print dialog pass the count through.

Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Filip Pranklin 2026-10-13 06:41:45 +02:00 committed by GitHub
parent fc56888567
commit 4f375bf45c
No known key found for this signature in database
GPG Key ID: B5690EEEBB952194
4 changed files with 46 additions and 13 deletions
Show Stats Download Patch File Download Diff File Expand all files Collapse all files

3
crates/automation/src/printing.rs
View File

@ -65,9 +65,10 @@ impl Automation {
o => return Err(bad(format!("unknown order {o:?}"))),
};
match Layout::multiple(per) {
Layout::Multiple { cols, rows, .. } => Layout::Multiple {
Layout::Multiple { cols, rows, per_sheet, .. } => Layout::Multiple {
cols,
rows,
per_sheet,
order,
border: a.opt_bool("border")?.unwrap_or(false),
auto_rotate: a.opt_bool("auto_rotate")?.unwrap_or(true),

18
crates/print/src/lib.rs
View File

@ -162,6 +162,9 @@ pub enum Layout {
Multiple {
cols: usize,
rows: usize,
/// Source pages on each sheet, at most `cols × rows`: three per sheet uses a 2 × 2 grid
/// with one cell left blank.
per_sheet: usize,
order: PageOrder,
border: bool,
auto_rotate: bool,
@ -198,7 +201,7 @@ impl Layout {
(c, n.div_ceil(c).max(1))
}
};
Layout::Multiple { cols, rows, order: PageOrder::Horizontal, border: false, auto_rotate: true }
Layout::Multiple { cols, rows, per_sheet: per_sheet.max(1), order: PageOrder::Horizontal, border: false, auto_rotate: true }
}
}
@ -343,11 +346,16 @@ pub fn layout(sizes: &[(f64, f64)], settings: &Settings) -> Result<Vec<Sheet>, P
sheets.push(Sheet { size, placed: vec![place_in(p, d, [0.0, 0.0, size.0, size.1], s, false)], ..Sheet::default() });
}
}
Layout::Multiple { cols, rows, order, border, auto_rotate } => {
Layout::Multiple { cols, rows, per_sheet, order, border, auto_rotate } => {
let cells = cols.checked_mul(rows).filter(|&n| n > 0 && n <= 256);
let Some(cells) = cells else {
return Err(PrintError::Invalid("invalid number of pages per sheet".into()));
};
// The grid can have more cells than the pages asked for on each sheet; the rest stay blank.
let per_sheet = per_sheet.min(cells);
if per_sheet == 0 {
return Err(PrintError::Invalid("invalid number of pages per sheet".into()));
}
// A grid with more columns than rows wants a landscape sheet, and the reverse.
let first = pages.first().and_then(|&p| sizes.get(p)).copied().ok_or(PrintError::NoPages)?;
let page_landscape = first.0 > first.1;
@ -366,7 +374,7 @@ pub fn layout(sizes: &[(f64, f64)], settings: &Settings) -> Result<Vec<Sheet>, P
if cw <= 0.0 || ch <= 0.0 {
return Err(PrintError::Invalid("the page grid does not fit on the paper".into()));
}
let sheet_count = pages.len().div_ceil(cells);
let sheet_count = pages.len().div_ceil(per_sheet);
for sheet_index in 0..sheet_count {
let mut sheet = Sheet { size, ..Sheet::default() };
if order == PageOrder::CutStack {
@ -381,13 +389,13 @@ pub fn layout(sizes: &[(f64, f64)], settings: &Settings) -> Result<Vec<Sheet>, P
sheet.lines.extend([[MARGIN, y, MARGIN + 8.0, y], [size.0 - MARGIN - 8.0, y, size.0 - MARGIN, y]]);
}
}
for k in 0..cells {
for k in 0..per_sheet {
// A cell's pile gets consecutive pages. Missing pages leave blank cells,
// rather than shifting later piles left on the final sheets.
let index = if order == PageOrder::CutStack {
k.checked_mul(sheet_count).and_then(|n| n.checked_add(sheet_index))
} else {
sheet_index.checked_mul(cells).and_then(|n| n.checked_add(k))
sheet_index.checked_mul(per_sheet).and_then(|n| n.checked_add(k))
};
let Some(p) = index.and_then(|i| pages.get(i)).copied() else { continue };

34
crates/print/src/tests.rs
View File

@ -155,9 +155,11 @@ fn multiple_pages_per_sheet() {
assert_eq!(two[0].size, (792.0, 612.0));
assert!(two[0].placed[1].matrix.0[4] > two[0].placed[0].matrix.0[4]);
// Vertical order and borders.
let v =
layout(&sizes, &settings(vec![0, 1, 2], Layout::Multiple { cols: 2, rows: 2, order: PageOrder::Vertical, border: true, auto_rotate: false }))
.unwrap();
let v = layout(
&sizes,
&settings(vec![0, 1, 2], Layout::Multiple { cols: 2, rows: 2, per_sheet: 4, order: PageOrder::Vertical, border: true, auto_rotate: false }),
)
.unwrap();
assert!(close(v[0].placed[1].matrix.0[4], v[0].placed[0].matrix.0[4]) && v[0].placed[1].matrix.0[5] < v[0].placed[0].matrix.0[5]);
assert_eq!(v[0].borders.len(), 3);
// Auto-rotate turns a landscape page in a portrait cell.
@ -168,6 +170,28 @@ fn multiple_pages_per_sheet() {
assert_eq!(r[0].placed[1].matrix.0[0], 0.0, "rotated");
}
#[test]
fn custom_pages_per_sheet_never_overfill_the_grid() {
// Three per sheet draws on a 2 × 2 grid, five on 3 × 2: the spare cells stay blank (#1336).
let sizes = vec![(200.0, 300.0); 7];
let pages = |per: usize| sheet_pages(&layout(&sizes, &settings((0..7).collect(), Layout::multiple(per))).unwrap());
assert_eq!(pages(3), [vec![0, 1, 2], vec![3, 4, 5], vec![6]]);
assert_eq!(pages(5), [vec![0, 1, 2, 3, 4], vec![5, 6]]);
assert_eq!(pages(7), [vec![0, 1, 2, 3, 4, 5, 6]]);
assert_eq!(pages(4), [vec![0, 1, 2, 3], vec![4, 5, 6]]);
assert_eq!(pages(2), [vec![0, 1], vec![2, 3], vec![4, 5], vec![6]]);
// Cut-stack piles take the requested count too.
let stacked = Layout::Multiple { cols: 2, rows: 2, per_sheet: 3, order: PageOrder::CutStack, border: false, auto_rotate: false };
assert_eq!(sheet_pages(&layout(&sizes, &settings((0..7).collect(), stacked)).unwrap()), [vec![0, 3, 6], vec![1, 4], vec![2, 5]]);
// A count larger than the grid is capped by the grid; no pages per sheet is refused.
let grid = |per_sheet| Layout::Multiple { cols: 2, rows: 1, per_sheet, order: PageOrder::Horizontal, border: false, auto_rotate: false };
assert_eq!(sheet_pages(&layout(&sizes[..3], &settings(vec![0, 1, 2], grid(9))).unwrap()), [vec![0, 1], vec![2]]);
assert!(matches!(layout(&sizes, &settings(vec![0], grid(0))), Err(PrintError::Invalid(_))));
// The imposed PDF has one page per sheet.
let out = impose(&fixture(7), &settings((0..7).collect(), Layout::multiple(3))).unwrap();
assert_eq!(pdfcraft_model::pages(&Document::open(Arc::new(out)).unwrap()).len(), 3);
}
#[test]
fn booklets_pair_pages_for_folding() {
let sizes = vec![(200.0, 300.0); 6];
@ -309,7 +333,7 @@ fn driverless_printers_join_the_list_without_a_queue() {
}
fn cut_stack(cols: usize, rows: usize) -> Layout {
Layout::Multiple { cols, rows, order: PageOrder::CutStack, border: false, auto_rotate: false }
Layout::Multiple { cols, rows, per_sheet: cols.saturating_mul(rows), order: PageOrder::CutStack, border: false, auto_rotate: false }
}
#[test]
@ -362,7 +386,7 @@ fn cutting_and_stacking_recovers_every_selected_page() {
fn multiple_rejects_hostile_grids_without_panicking() {
let sizes = [(200.0, 300.0)];
for (cols, rows) in [(usize::MAX, 2), (2, usize::MAX), (0, 2), (1, 0), (257, 1)] {
let mode = Layout::Multiple { cols, rows, order: PageOrder::Horizontal, border: false, auto_rotate: false };
let mode = Layout::Multiple { cols, rows, per_sheet: 1, order: PageOrder::Horizontal, border: false, auto_rotate: false };
assert!(matches!(layout(&sizes, &settings(vec![0], mode)), Err(PrintError::Invalid(_))));
}
let tiny = Settings { paper: (72.0, 72.0), ..settings(vec![0], cut_stack(16, 16)) };

4
crates/ui-egui/src/print_ui.rs
View File

@ -148,8 +148,8 @@ impl PrintDraft {
m => m,
}),
Handling::Multiple => match Layout::multiple(self.per_sheet) {
Layout::Multiple { cols, rows, .. } => {
Layout::Multiple { cols, rows, order: self.order, border: self.border, auto_rotate: self.auto_rotate }
Layout::Multiple { cols, rows, per_sheet, .. } => {
Layout::Multiple { cols, rows, per_sheet, order: self.order, border: self.border, auto_rotate: self.auto_rotate }
}
other => other,
},