Closed Bug 1669905 Opened 8 months ago Closed 5 months ago

Support "2" and "6" as values for pages-per-sheet

Categories

(Core :: Printing: Output, enhancement)

enhancement

Tracking

()

VERIFIED FIXED
Tracking Status
firefox86 --- verified
firefox87 --- verified

People

(Reporter: dholbert, Assigned: dholbert)

References

Details

Attachments

(6 files, 1 obsolete file)

3.05 KB, image/png
Details
7.91 KB, image/png
Details
47 bytes, text/x-phabricator-request
Details | Review
47 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review
48 bytes, text/x-phabricator-request
Details | Review

In bug 1631452 right now, I'm only adding support for perfect-square numbers of pages per sheet (1, 4, 9, 16) since they're more straightforward -- the scale factor is always 1/(num-columns) and we never need to consider whether to rotate the sheet to get the best fit.

I'm filing this followup bug on adding support for "2" and "6" as values for pages-per-sheet. These are a bit more complex because usually they'll require rotation.

(To determine if rotation is required, I think we want to compare the numCols/numRows aspect ratio to the page's aspect ratio, and depending on which side of that comparison we come out on, that tells us whether it makes sense to rotate. Or something like that.)

we never need to consider whether to rotate the sheet to get the best fit

Hmm, auto-rotating the sheet seems confusing to me. Personally, I'd rather do that manually.

Here's the four plausible page layouts for 2-pages-per-sheet in Portrait mode.
The two on the left doesn't seem particularly useful, but I think that's only because the sheet size in this example is "tall". It seems like all four could be useful if the sheet size is square (or close to).

Similarly, there are four plausible page layouts for Landscape mode. Note that Landscape mode isn't just the example in the 4th quadrant rotated 90CW because they have the page header/footer on different sides.

So I think there a three orthogonal parameters involved:

  1. page placement main axis: horizontal or vertical
  2. page rotation (only multiples of 90deg?)
  3. sheet mode: Portrait or Landscape

(In reply to Mats Palmgren (:mats) from comment #2)

Here's the four plausible page layouts for 2-pages-per-sheet in Portrait mode.
The two on the left doesn't seem particularly useful, but I think that's only because the sheet size in this example is "tall". It seems like all four could be useful if the sheet size is square (or close to).

So there's one invariant that I think you might be missing, based on your diagram: the "virtual pages" are supposed to represent miniature shrunken-down versions of the same paper size that's being printed onto. So, the "pages" and the "sheet" should all have the same aspect ratio. (At least, that's how pages-per-sheet works everywhere else that I've tested it -- which is GTK Print Dialog, Adobe PDF Viewer, and Chrome.)

The version shown in your dialog looks a bit more similar to a version of CSS Multicol where the virtual columns are simply space-filling. That is an interesting feature that a printing application might consider supporting -- but it's not how "N pages-per-sheet" works in any application that I've tested, and it's not a behavior that falls easily out of the pages-per-sheet implementation that we've got right now (which assumes that the pages and sheet are the same physical size, modulo a scaling factor).

This design / behavior has the convenient simplifying factor that it's trivially decideable whether or not it's worth rotating the pages on the physical sheet or not -- there's no need to leave it up to the user. We should simply choose the option that requires the least shrinking (and therefore makes the best use of the available area on the sheet).

For typical rectangular paper sizes, it almost always makes sense to rotate. But for sufficiently tall-and-skinny pages (e.g. 1in by 100in), it's actually better not to rotate. (In that scenario, for 2 pages-per-sheet, rotating would result in a 100x scale-down factor, whereas not-rotating would result in only a 0.5 scale-down factor.) We could work out the scaling factor for both options (given the pages-per-sheet choice and the paper size) and pick the higher one -- but as noted in comment 0, I think we can take a shortcut by just comparing the paper's aspect ratio to the pages-per-sheet's numRows/numCols and making a judgement based on that. (I worked it out a few weeks ago, but I forget the exact logic to follow for that decision.)

So really, given that constraint, the two options for plausible layouts are actually like this screenshot (taken from https://jsfiddle.net/dholbert/f92xyk5a/ which requires Nightly to render since it uses the aspect-ratio property)

As you can see, the option on the right makes better use of the available page area, which means it will produce a less-severe scale factor; so it's what applications use in practice when doing 2-pages-per-sheet (for this paper size, at least -- and really, all commonly-used page sizes. The theoretical 1x100in page size is the exception where the preferred option would be reversed.).

Attachment #9183780 - Attachment description: Plausible layouts for 2 pages per sheet setting, in Portrait mode. → Plausible layouts for 2 pages per sheet setting, in Portrait mode, if there were no requirement for pages to match the sheet's aspect-ratio

the "virtual pages" are supposed to represent miniature shrunken-down versions of the same paper size that's being printed onto

Ah, interesting. I haven't seen your new pages-per-sheet feature in action yet, so I didn't know that's how it worked. I agree that choosing an optimal scaling/placement automatically makes sense for this scaled-down page model.

The version shown in your dialog looks a bit more similar to a version of CSS Multicol where the virtual columns are simply space-filling.

Yes, these are different models indeed. The layout-preserving scaled-down pages approach makes sense for media that is inherently page-oriented, like PDF/PS etc. The divide-the-page-into-columns model is more interesting for fluid layouts that can adapt to different viewport sizes, like plain text, or indeed, web content. I remember from my distant university days that we printed source code using this model to maximize the number of lines you could print on a sheet (double-sided 2-column landscape mode was the best for legible text IIRC). There are likely many other use cases for this as well, for example if you have a long list of names that would normally just use the left side of the sheet, which would be more optimal using 3 or more columns in landscape mode. Just scaling things down is in many cases sub-optimal since it makes the text illegible pretty quickly.

But you're quite right that what I was thinking about is more like a page column layout - having more than one row per sheet doesn't really make sense in this model.

BTW, the example on the right in your screenshot has wrong orientation? I would expect it to be rotated 180deg so that the start of the page is on the left and text flowing down to the right (at least for writing-mode:horizontal-tb). Anyway, I think it makes more sense to choose the layout on the left and leave the other one for Landscape mode. I think if we place pages in a (physical) top-left to bottom-right sequence in both Portrait/Landscape then it should cover all useful layouts.

EDIT: deleted the comment about Chrome not preserving the sheet ratio, I think I was just confused how it looked in Okular which had automatically zoomed it to fit the window...

Funny, it seems Chrome does the opposite of what I suggested for 2 pages per sheet. It automatically rotates the sheet in the Print Preview (so it appears like Landscape) and place the pages horizontally side-by-side in Portrait mode, and rotates the sheet (so it appears like Portrait) and place the pages vertically in Landscape mode. Wow, that's really confusing.

(In reply to Mats Palmgren (:mats) from comment #6)

BTW, the example on the right in your screenshot has wrong orientation?

Yeah, rotating the sheet 180deg there would be fine; I haven't tested to see which makes sense. I think this is just a question of "which edge of the paper comes out of the printer first", which I haven't thought about for the rotated-page case but which I assume is an answerable question.

Chrome's behavior here (at least in Print Preview and print-to-PDF) makes sense to me and matches my mental model, FWIW. We're giving the user 2 portrait-sized pages, with those pages themselves oriented in a "portrait" view for easy reading, stacked on the sheet in the way that makes the best use of the available area on the sheet & requires the least-severe scaling factor. (In this case, for US Letter and I assume also A4, that means printing onto landscape-oriented sheets of paper, with the page-content rendered into side-by-side portrait-oriented "cards").

Assignee: nobody → dholbert
Status: NEW → ASSIGNED

This patch doesn't change behavior. It's just a simplification of the data
that we track for our different pages-per-sheet mode (with some minor
refactoring of the logic involved).

This is a necessary step towards implementing support for 2 and 6
pages-per-sheet (which are referenced in comments included in this patch).

Attachment #9192112 - Attachment description: Bug 1669905 part 1: Simplify pages-per-sheet data to only track a single row or column count (since the other one is implicit). r?TYLin → Bug 1669905 part 1: Simplify pages-per-sheet data & logic to only store a single track count (either the row or column count), since the other one is implicit. r?TYLin

I'm hoping to have the main patch up soon - I was hoping to post it today to land before the beta freeze tomorrow, but I'm not sure that'll happen. I've been needing to do a bit more digging than I was expecting, in order to get the orthogonal-page-vs-sheet-orientation stuff printing properly.

There's a bit of complexity from the fact that this is creating a mode where the print settings say "portrait mode" and yet we really want to print landscape-oriented sheets of paper, which is necessary in order to e.g. have generated PDFs display the sheets on a virtual landscape-oriented piece of paper. I've got things working in print preview, and I had been naively imagining I could just adjust the sheet size in PrintedSheetFrame::Reflow, but in fact we need to do some special handling when generating the actual print target, and when responding to the various GetPageSize / GetOrientation APIs, since different callers may want different answers depending on whether they're asking about the physical sheets vs. the layout on pages. I'm still hoping this will end up relatively targeted & will be safe to land during the soft freeze and/or to uplift in early beta -- we'll see...

Depends on: 1681623

Comment on attachment 9192112 [details]
Bug 1669905 part 1: Simplify pages-per-sheet data & logic to only store a single track count (either the row or column count), since the other one is implicit. r?TYLin

Revision D99180 was moved to bug 1681623. Setting attachment 9192112 [details] to obsolete.

Attachment #9192112 - Attachment is obsolete: true

(So far, this works on Linux, and on Windows with "save-to-PDF", but it doesn't
rotate the page correctly with my real printer on Windows -- it prints
landscape-mode pages running off the side of a landscape-mode sheet.)

Depends on: 1682403
No longer depends on: 1682403
Depends on: 1681344
Attachment #9192750 - Attachment description: Bug 1669905 part 1: (WIP) Support 2 and 6 as options for pages-per-sheet, when printing. → Bug 1669905 part 2: Add support for 2 and 6 as options for pages-per-sheet, in the platform code. r?
Attachment #9192751 - Attachment description: Bug 1669905 part 2: Show 2 and 6 as pages-per-sheet options in modal print UI, now that they're supported on the platform side. → Bug 1669905 part 3: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side. r?

Without this patch, the rest of this bug's patch-stack can end up improperly
clipping content at the bottom of pages, when printing in 2- and
6-pages-per-sheet configurations.

This clipping problem happens because we build the nsPageFrame's display items
without direct knowledge of the fact that it's scaled down, and that ends up
confusing the display list code about whether content is visible. Here's what
happens, when we improperly clip (in a build with all of this bug's patches
except for this one):
(1) We size & scale the print-preview scrollport to display a
landscape-oriented sheet (onto which we plan to lay out two
portrait-oriented pages, if we're e.g. rendering with 2 pages-per-sheet).
(2) The scrollport area gets passed down as the visible area in our
BuildDisplayList codepath.
(3) When we build the display list for a nsPageFrame (before we've applied its
pages-per-sheet scale-down transform), the display list code correctly
judges that some of the page's display items (e.g. its footers) are beyond
the bottom of the scrollport and can safely be clipped. (At first, they
seem like they're beyond the bottom of the scrollport, since they're part
of a tall-and-skinny portrait-oriented nsPageFrame that we're laying out
in a scrollport that's been sized for a short-and-wide landscape-oriented
sheet).
(4) Then we scale down the rendered nsPageFrame to place it on our sheet, and
we find that part of the page is missing -- the part that was out of the
scrollport from the display list code's perspective -- even though it's
now transformed to be fully visible.

The simplest fix for this is to temporarily override the visible rect to be the
nsPageFrame's full mRect. This fix prevents us from thinking that any of the
nsPageFrame should be clipped (except for parts that legitimately overflow the
nsPageFrame, which of course are still appropriate to clip).

Note that nsPageFrame itself does this same visible-rect-replacement trick,
when it renders its own nsPageContentFrame child. This patch is partially
cribbed from nsPageFrame's similar code.

Attachment #9192750 - Attachment description: Bug 1669905 part 2: Add support for 2 and 6 as options for pages-per-sheet, in the platform code. r? → Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the platform code (and add tests for these values). r?
Attachment #9192751 - Attachment description: Bug 1669905 part 3: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side. r? → Bug 1669905 part 4: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side.
Attachment #9192750 - Attachment description: Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the platform code (and add tests for these values). r? → Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the platform code (and add tests for these values).

This is necessary to support 2 and 6 pages-per-sheet, for which a
user-specified "portrait mode" actually means "portrait-mode pages on a
landscape-mode sheet".

At this point in the patch stack, we don't yet support 2 or 6 pages-per-sheet,
so it's not yet possible for pages and sheets to be orthogonal, in practice.
This patch is just adding & using some new APIs so that we'll handle things
properly when we do add support for these values (later in the patch stack).

Attachment #9192750 - Attachment description: Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the platform code (and add tests for these values). → Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the print-related frame classes (and add tests for these values).
Attachment #9194629 - Attachment description: Bug 1669905 part 2: Add nsIPrintSettings APIs to handle the possibility that pages and sheets may have orthogonal orientation, in pages-per-sheet printouts. → Bug 1669905 part 2: Add nsIPrintSettings APIs to handle the possibility that pages and sheets may have orthogonal orientation, in pages-per-sheet printouts. r?jfkthame
Attachment #9192750 - Attachment description: Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the print-related frame classes (and add tests for these values). → Bug 1669905 part 3: Add support for 2 and 6 as options for pages-per-sheet, in the print-related frame classes (and add tests for these values). r?TYLin
Attachment #9192751 - Attachment description: Bug 1669905 part 4: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side. → Bug 1669905 part 4: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side. r?mstriemer

Triggered autoland for parts 1-3.

Keywords: leave-open
Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/7c46b561db00
part 1: Adjust PrintedSheetFrame::BuildDisplayList to avoid clipping nsPageFrame children. r=TYLin
https://hg.mozilla.org/integration/autoland/rev/45b225f453d6
part 2: Add nsIPrintSettings APIs to handle the possibility that pages and sheets may have orthogonal orientation, in pages-per-sheet printouts. r=jfkthame
https://hg.mozilla.org/integration/autoland/rev/00d01574ddef
part 3: Add support for 2 and 6 as options for pages-per-sheet, in the print-related frame classes (and add tests for these values). r=TYLin

I'm just remembering that we still kinda need to wait on bug 1681344 before landing part 4 here.

Without that, printing 2 or 6 pages per sheet (on Mac) will end up flipping the dialog's remembered default-orientation, as noted in bug 1681344 comment 4.

Pushed by dholbert@mozilla.com:
https://hg.mozilla.org/integration/autoland/rev/e70da7a43c3c
part 4: Offer 2 and 6 in the "pages per sheet" dropdown-menu in modal print UI, now that they're supported on the platform side. r=mstriemer

All the patches in this bug landed. Can this be closed?

Flags: needinfo?(dholbert)

Yes, this can be closed; I forgot to remove leave-open before landing the final patch.

Thanks!

Status: ASSIGNED → RESOLVED
Closed: 5 months ago
Flags: needinfo?(dholbert)
Keywords: leave-open
Resolution: --- → FIXED
Regressions: 1691286

Hello,

Confirming this issue as verified fixed using 86.0b8(20210209185733) and 87.0a1(20210209214921) on a Windows 10x64, Ubuntu 20 and macOS 10.15.7

Status: RESOLVED → VERIFIED
Flags: qe-verify+
You need to log in before you can comment on or make changes to this bug.