Bug 1681623: 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

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,
and which will be implemented separately in bug 1669905)

Differential Revision: https://phabricator.services.mozilla.com/D99180

layout/generic/PrintedSheetFrame.cpp

@@ -8,6 +8,8 @@
 
 #include "mozilla/PrintedSheetFrame.h"
 
+#include <tuple>
+
 #include "mozilla/StaticPrefs_print.h"
 #include "nsCSSFrameConstructor.h"
 #include "nsPageFrame.h"
@@ -29,6 +31,16 @@ NS_QUERYFRAME_TAIL_INHERITING(nsContainerFrame)
 
 NS_IMPL_FRAMEARENA_HELPERS(PrintedSheetFrame)
 
+std::tuple<uint32_t, uint32_t> GetRowAndColFromIdx(uint32_t aIdxOnSheet,
+                                                   uint32_t aNumCols) {
+  // Compute the row index by *dividing* the item's ordinal position by how
+  // many items fit in each row (i.e. the number of columns), and flooring.
+  // Compute the column index by getting the remainder of that division:
+  // Notably, mNumRows is irrelevant to this computation; that's because
+  // we're adding new items column-by-column rather than row-by-row.
+  return {aIdxOnSheet / aNumCols, aIdxOnSheet % aNumCols};
+}
+
 // Helper for BuildDisplayList:
 gfx::Matrix4x4 ComputePagesPerSheetTransform(nsIFrame* aFrame,
                                              float aAppUnitsPerPixel) {
@@ -48,8 +60,8 @@ gfx::Matrix4x4 ComputePagesPerSheetTransform(nsIFrame* aFrame,
     if (ppsInfo->mNumPages > 1) {
       scale = pd->mPagesPerSheetScale;
       gridOrigin = pd->mPagesPerSheetGridOrigin;
-      std::tie(rowIdx, colIdx) =
-          ppsInfo->GetRowAndColFromIdx(pageFrame->IndexOnSheet());
+      std::tie(rowIdx, colIdx) = GetRowAndColFromIdx(pageFrame->IndexOnSheet(),
+                                                     pd->mPagesPerSheetNumCols);
     }
   }
 
@@ -293,8 +305,16 @@ void PrintedSheetFrame::ComputePagesPerSheetOriginAndScale() {
   // Compute the full size of the "page grid" that'll we'll be scaling down &
   // placing onto a given sheet:
   const auto* ppsInfo = mPD->PagesPerSheetInfo();
-  nsSize pageGridFullSize(ppsInfo->mNumCols * pageSize.width,
-                          ppsInfo->mNumRows * pageSize.height);
+
+  // XXXdholbert Bug 1669905 will add a bit more reasoning around here, to
+  // ensure that we use the larger track-count in the sheet's longer axis, to
+  // make the best use of space. (At this point, we don't have to worry about
+  // that, because we only support pages-per-sheet values that have the same
+  // number of rows and columns.)
+  uint32_t numCols = ppsInfo->mLargerNumTracks;
+  uint32_t numRows = ppsInfo->mNumPages / numCols;
+
+  nsSize pageGridFullSize(numCols * pageSize.width, numRows * pageSize.height);
 
   if (MOZ_UNLIKELY(availSpaceOnSheet.IsEmpty() || pageGridFullSize.IsEmpty())) {
     // Either we have a 0-sized available area, or we have a 0-sized page-grid
@@ -306,6 +326,7 @@ void PrintedSheetFrame::ComputePagesPerSheetOriginAndScale() {
     // vScale computations below.
     NS_WARNING("Zero area for pages-per-sheet grid, or zero-sized grid");
     mPD->mPagesPerSheetGridOrigin = pageGridOrigin;
+    mPD->mPagesPerSheetNumCols = 1;
     mPD->mPagesPerSheetScale = 0.0f;
     return;
   }
@@ -342,6 +363,7 @@ void PrintedSheetFrame::ComputePagesPerSheetOriginAndScale() {
 
   // Update the nsSharedPageData member data:
   mPD->mPagesPerSheetGridOrigin = pageGridOrigin;
+  mPD->mPagesPerSheetNumCols = numCols;
   mPD->mPagesPerSheetScale = scale;
 }
 

layout/generic/nsPageSequenceFrame.cpp

@@ -49,12 +49,12 @@ nsPageSequenceFrame* NS_NewPageSequenceFrame(PresShell* aPresShell,
 NS_IMPL_FRAMEARENA_HELPERS(nsPageSequenceFrame)
 
 static const nsPagesPerSheetInfo kSupportedPagesPerSheet[] = {
-    {1, 1, 1},  // Note: we default to this if no match is found.
+    {1, 1},  // Note: we default to this if no match is found.
     // {2, ... }, // XXXdholbert Coming in bug 1669905.
-    {4, 2, 2},
+    {4, 2},
     // {6, ... }, // XXXdholbert Coming in bug 1669905.
-    {9, 3, 3},
-    {16, 4, 4},
+    {9, 3},
+    {16, 4},
 };
 
 inline void SanityCheckPagesPerSheetInfo() {
@@ -68,16 +68,12 @@ inline void SanityCheckPagesPerSheetInfo() {
   uint16_t prevInfoPPS = 0;
   for (const auto& info : kSupportedPagesPerSheet) {
     MOZ_ASSERT(info.mNumPages > prevInfoPPS,
-               "page count field should be nonzero & monotonically increase");
-    // The uint32_t cast here is to ensure this assertion is robust even if the
-    // mNumRows * mNumCols multiplication would overflow past the maximum
-    // representable uint16_t value. That overflow would be bad because it
-    // could result in us incorrectly passing this assertion.  We prevent this
-    // problem by doing the operation in 32-bit number space; that way, the
-    // multiplication (of two known-to-be-16-bit values) can't overflow.
-    MOZ_ASSERT(
-        (uint32_t)info.mNumRows * (uint32_t)info.mNumCols == info.mNumPages,
-        "page count should match rows*cols");
+               "page count field should be positive & monotonically increase");
+    MOZ_ASSERT(info.mLargerNumTracks > 0,
+               "page grid has to have a positive number of tracks");
+    MOZ_ASSERT(info.mNumPages % info.mLargerNumTracks == 0,
+               "page count field should be evenly divisible by "
+               "the given track-count");
     prevInfoPPS = info.mNumPages;
   }
 #endif

layout/generic/nsPageSequenceFrame.h

@@ -6,8 +6,6 @@
 #ifndef nsPageSequenceFrame_h___
 #define nsPageSequenceFrame_h___
 
-#include <tuple>
-
 #include "mozilla/Attributes.h"
 #include "mozilla/UniquePtr.h"
 #include "nsContainerFrame.h"
@@ -31,18 +29,11 @@ struct nsPagesPerSheetInfo {
   static const nsPagesPerSheetInfo& LookupInfo(int32_t aPPS);
 
   uint16_t mNumPages;
-  uint16_t mNumRows;
-  uint16_t mNumCols;
 
-  std::tuple<uint16_t, uint16_t> GetRowAndColFromIdx(
-      uint16_t aIdxOnSheet) const {
-    // Compute the row index by *dividing* the item's ordinal position by how
-    // many items fit in each row (i.e. the number of columns), and flooring.
-    // Compute the column index by getting the remainder of that division:
-    // Notably, mNumRows is irrelevant to this computation; that's because
-    // we're adding new items column-by-column rather than row-by-row.
-    return {aIdxOnSheet / mNumCols, aIdxOnSheet % mNumCols};
-  }
+  // This is the larger of the row-count vs. column-count for this layout
+  // (if they aren't the same). We'll aim to stack this number of pages
+  // in the sheet's longer axis.
+  uint16_t mLargerNumTracks;
 };
 
 /**
@@ -80,13 +71,21 @@ class nsSharedPageData {
   // frames that overflowed.  It's 1.0 if none overflowed horizontally.
   float mShrinkToFitRatio = 1.0f;
 
-  // These are only used if PagesPerSheetInfo()->mNumPages > 1.  They're
-  // initialized with reasonable defaults here (which correspond to what we do
-  // for the regular 1-page-per-sheet scenario, though we don't actually use
-  // these members in that case).  If we're in >1 pages-per-sheet scenario,
-  // then these members will be assigned "real" values during the reflow of the
-  // first PrintedSheetFrame.
+  // The mPagesPerSheet{...} members are only used if
+  // PagesPerSheetInfo()->mNumPages > 1.  They're initialized with reasonable
+  // defaults here (which correspond to what we do for the regular
+  // 1-page-per-sheet scenario, though we don't actually use these members in
+  // that case).  If we're in >1 pages-per-sheet scenario, then these members
+  // will be assigned "real" values during the reflow of the first
+  // PrintedSheetFrame.
   float mPagesPerSheetScale = 1.0f;
+  // Number of "columns" in our pages-per-sheet layout. For example: if we're
+  // printing with 6 pages-per-sheet (once we've fixed Bug 1669905), then this
+  // could be either 3 or 2, depending on whether we're printing
+  // portrait-oriented pages onto a landscape-oriented sheet (3 cols) vs. if
+  // we're printing landscape-oriented pages onto a portrait-oriented sheet (2
+  // cols).
+  uint32_t mPagesPerSheetNumCols = 1;
   nsPoint mPagesPerSheetGridOrigin;
 
   // Lazy getter, to look up our pages-per-sheet info based on mPrintSettings