D7949: Allow to print pdf doc directly into a QPrinter

[Michael Weghorn]

michaelweghorn added a comment.


  In general, I really like the idea of providing this new option.
  
  Two notes after having a quick look (besides the two comments directly on the code):
  
  - Depending on what the use case for the option is (Will users who want to use this experimental feature have it enabled "all of the time" or is it actually an option that is meant to be changed per print job?), it might possibly be an alternative to move it to the config options of the PDF backend (which are available at "Options" -> "Configure backends" -> "PDF"), which would even further "hide" it from users who don't know about it. (Just to mention it, I don't have a strong opinion on this at the moment.)
  - As far as I can see, this also changes how the "Force rasterization" behaves for the CUPS case, since it now actually no longer uses QPrinter for this case now. This is possibly intended. (Just to mention that this will change the current behaviour. here also when the new experimental option is not selected.)
  
  (I did not find the time to actually test or look at the change more intensely yet.)

INLINE COMMENTS

> generator_pdf.cpp:99
> +
> +           m_printBackend = new QComboBox;
> +           // Windows can only print with the QPrinter backend, because the CUPS backend

Should "this" be passed in the constructor to have this auto-destroyed and avoid a memory leak?

> generator_pdf.cpp:1349
>          {
> +                if ( forceRasterize )
> +                {

The indentation here is a little odd in my eyes, since the "if" is indented further than the following lines inside of the "if" block. I think moving the "if" (and "else" below) one indentation level to the "left" and the block one indentation level to the "right" would be the usual way to do it.

REPOSITORY
  R223 Okular

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

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