D7962: Implement several new print scaling options

[Henrik Fehlauer]

rkflx added a comment.


  > Summary
  
  I'm not able to test real printing at the moment. Could you add a "Test Plan" to show what simulated/real testing you did on your side, at least?
  
  > Scale mode: fit-to-page vs. shrink-to-page vs. none
  >  Scale to: printable area vs. full page
  
  Having "page" in the names for the scale modes while "Scale to" has "page" in the name half of the time seems like it could confuse some users. "Mode" is a quite technical (and often meaningless) term, too. In addition, it's very hard without looking at the code and with no extra explanation available to know what the actual difference between "fit" and "shrink" is. Ideas (could be improved, though):
  
  - `Scaling:` `Shrink oversized pages only` | `Expand or shrink to fit to page` | `None` (← Note how the first/default mode is different – I don't think users would expect their printouts getting scaled up by default like in your Diff.)
  - `Scale to:` `Printable area` | `Paper size`
  
  Optional improvement, not a blocker: The dialog would look nicer if all combo boxes had the same width. Maybe look at other code snippets to see how it's done there?
  
  > The patch requires https://phabricator.kde.org/D7949.
  
  You could use "Depends on https://phabricator.kde.org/D7949" in the summary, this way `arc patch` would automatically download both Diffs for the reviewer and Phab would add the info to the "Stack" tab.

INLINE COMMENTS

> generator_pdf.cpp:115
> +           m_scaleMode = new QComboBox;
> +           m_scaleMode->insertItem(0, "Fit to page");
> +           m_scaleMode->insertItem(1, "Shrink to page");

Use the `enum` like in the other Diff.

> generator_pdf.cpp:170
> +       {
> +           if (m_scaleMode->currentIndex()==0)
> +           {

Use the `return` like in the other Diff.

> generator_pdf.cpp:1217
> +                    // We use the smaller of the two for both directions, to keep the aspect ratio
> +                    scaling = std::min(horizontalScaling,verticalScaling);
> +                }

Missing space: `g,v`.

REPOSITORY
  R223 Okular

REVISION DETAIL
  https://phabricator.kde.org/D7962

To: sander, #okular
Cc: rkflx, michaelweghorn, ngraham, aacid
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://mail.kde.org/pipermail/okular-devel/attachments/20170928/db63bff9/attachment-0001.html>