Repository navigation
pdftopdf: fix back pages rotated 180° in long-edge duplex (#263) - #265
princemittalr wants to merge 1 commit into
Conversation
There was a problem hiding this comment.
Review: duplex back-side orientation fix
Changing sheet_back from rotated to normal addresses the duplicate transformation identified in #263. The documented reproductions cover driverless queues, manufacturer PPDs, PDF and raster output, and a printer requiring a rotated back side. Based on that evidence and the validation below, the fix looks good to me.
Validation performed on c1690952c0536e3b4702b74739d0bdb618c6994a:
- Successfully ran
./autogen.sh,./configure --disable-mutool --disable-static,make -j2, andmake check. - All 10 tests passed. The optional
mutoolintegration was disabled becausemutoolwas unavailable. - Confirmed all 15 CI checks passed, including Build & Test on x86_64, arm64, armv7, and riscv64, along with CodeQL and cppcheck.
Non-blocking follow-up: The existing test suite does not automatically verify back-side orientation. A targeted regression test would be useful once a suitable test harness is available.
The validation above applies to the original commit. The author has since reported rebasing onto current master at 7616418, with all 15 CI checks passing again; I have not repeated the local validation on that revision.
The N²-copy issue tracked in #271 is separate and should not block this fix.
The hardcoded sheet_back="rotated" was causing prepare_documents() to apply a 180-degree rotation transform to every back-side page when printing two-sided-long-edge. This is wrong: back-side orientation is already handled downstream by cfGetBackSideOrientation() in ghostscript.c, pdftoraster.c, pwgtoraster.c, pclmtoraster.c and rastertopwg.c. For PDF output that function always returns "normal". The old QPDF-based pdftopdf had no back-side transform. The bug only became visible after commit 1973c4a (print-scaling=auto default, OpenPrinting#237) stopped ordinary jobs from taking the fast-copy path that bypassed the transform. Fixes: OpenPrinting#263
c169095 to
7616418
Compare
|
Thanks for the thorough review. I’ve rebased onto current master (the changed line is unchanged, head is now 7616418) and all 15 checks pass again. Agreed that back-side orientation needs a regression test; I’d prefer to add it in a follow-up once there’s a suitable harness, to keep this fix minimal. Maintainers: if anyone has a printer that needs the rotated back side, I’d welcome that check. |
|
Tested on hardware with https://aur.archlinux.org/packages/libcupsfilters-duplexfix (libcupsfilters 2.2.1.r32.gd49e9284 with only this change applied):
|
Fixes #263
Problem:
The hardcoded sheet_back = "rotated" in cfFilterPDFToPDF() was causing prepare_documents() to apply a 180° rotation transform to every back-side page when printing two-sided-long-edge. This made page 2, 4, etc. come out upside down.
Root cause:
Back-side orientation is already handled downstream by cfGetBackSideOrientation() in ghostscript.c, pdftoraster.c, pwgtoraster.c, pclmtoraster.c and rastertopwg.c. For PDF output that function always returns "normal". The old QPDF-based pdftopdf had no back-side transform at this stage.
The bug only became visible after commit 1973c4a (print-scaling=auto default, #237) stopped ordinary jobs from taking the fast-copy path that bypassed the transform.
Fix:
Change sheet_back from "rotated" to "normal".
Testing:
Built successfully against PDFio 1.6.4
The one-line change matches the fix suggested in issue #263 by the reporter
Landscape auto-rotation (#237) is unaffected by this change
Note: Not tested on a printer that genuinely requires a rotated back side. Maintainer review on that case is welcome.