D7949: Allow to print pdf doc directly into a QPrinter

[Oliver Sander]

sander updated this revision to Diff 37521.
sander added a comment.
Restricted Application added a subscriber: okular-devel.


  Applied Henrik's suggestions

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7949?vs=37025=37521

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

AFFECTED FILES
  generators/poppler/generator_pdf.cpp

To: sander, #okular
Cc: okular-devel, asturmlechner, cfeck, ltoscano, rkflx, michaelweghorn, 
ngraham, aacid

D7949: Allow to print pdf doc directly into a QPrinter

[Oliver Sander]

sander updated this revision to Diff 37522.

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D7949?vs=37521=37522

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

AFFECTED FILES
  generators/poppler/generator_pdf.cpp

To: sander, #okular
Cc: okular-devel, asturmlechner, cfeck, ltoscano, rkflx, michaelweghorn, 
ngraham, aacid

D10974: Add option to ignore print margins

[Nathaniel Graham]

ngraham added a comment.


  In D10974#290127 , @rkflx wrote:
  
  > - `Scale to:` `Printable area` | `Paper size`
  
  
  +1, I would be fine with this.

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: rkflx, arthurpeters, ltoscano, okular-devel, aacid, ngraham

D7949: Allow to print pdf doc directly into a QPrinter

[Henrik Fehlauer]

rkflx accepted this revision.
rkflx added a comment.
This revision is now accepted and ready to land.


  Thanks, accepting the Diff from my side (obviously I cannot speak for the 
Okular project).
  
  In D7949#290074 , @sander wrote:
  
  > That is https://phabricator.kde.org/D7962 .  I put it on hold, because I 
didn't want to spend more time on time before knowing whether this patch here 
will get merged or not.
  
  
  Ah, I knew there was a Diff somewhere. Also, there is now D10974: Add option 
to ignore print margins , so this probably 
needs some coordination. Anyway, I don't think it's mandatory to wait for that, 
you could just add a hint to the tooltip that QPrinter does not scale to the 
printable area yet.
  
  BTW, if you write D7962  instead of 
https://phabricator.kde.org/D7962, Phabricator will automatically add mentions 
to the review, which makes discovering things easier. Also, dependencies can be 
expressed with a line in the summary, e.g. `Depends on D7949` 

 (here you could use Edit Related Revisions > Edit Child Revisions).
  
  ---
  
  >> For me, highlighter annotations are rendered opaque. Not a blocker, of 
course, but maybe worth to file a bug for.
  > 
  > That is interesting. Arthur renders them correctly to a screen, but I can 
confirm that they are opaque when printed.
  >  Could this be a QPrinter bug?  It is the same poppler backend code for 
screen and printer output.
  
  You are the expert here, so I guess it's up to you to fix or at least report 
this at the appropriate place ;)

INLINE COMMENTS

> generator_pdf.cpp:105
> +   m_printBackend->setItemData(CUPSBackend, xi18n("Print using the 
> CUPS printing system.This will convert your PDF file to PostScript 
> first,and then send the PostScript file to the printer."), 
> Qt::ToolTipRole);
> +   m_printBackend->setItemData(CUPSBackend, xi18n("Print using the 
> CUPS printing system.This will convert your PDF file to PostScript 
> first,and then send the PostScript file to the printer."), 
> Qt::WhatsThisRole);
> +#endif

As there is no way to access `Qt::WhatsThisRole` from the UI, perhaps the line 
for `Qt::ToolTipRole` is enough?

REPOSITORY
  R223 Okular

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

To: sander, #okular, rkflx
Cc: okular-devel, asturmlechner, cfeck, ltoscano, rkflx, michaelweghorn, 
ngraham, aacid

D10974: Add option to ignore print margins

[Henrik Fehlauer]

rkflx added a comment.


  FWIW, earlier in D7962  there was this 
suggestion:
  
  In D7962#150260 , @rkflx wrote:
  
  > > 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`

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: rkflx, arthurpeters, ltoscano, okular-devel, aacid, ngraham

D7949: Allow to print pdf doc directly into a QPrinter

[Oliver Sander]

sander marked 3 inline comments as done.
sander added a comment.


  >>Unlike for CUPS printing, Qt printing prints on the entire page and does 
not scale to the printable area yet. This is only because it is the easiest 
way. I > >plan to implement a few standard scaling methods in a subsequent 
patch, which will only be a few additional lines of code.
  
  
  
  > Has this been worked on? If not, there should be a hint in the tooltip.
  
  That is https://phabricator.kde.org/D7962 .  I put it on hold, because I 
didn't want to spend more time on time before knowing whether this patch here 
will get merged or not.
  
  >>  being able to use the Arthur backend in my daily work.
  >  I guess you don't print annotations then ;) For me, highlighter 
annotations are rendered opaque. Not a blocker, of course, but maybe worth to 
file a bug for.
  
  That is interesting. Arthur renders them correctly to a screen, but I can 
confirm that they are opaque when printed.
  Could this be a QPrinter bug?  It is the same poppler backend code for screen 
and printer output.
  
  That being said, highlighter annotations were a bit difficult to implement in 
Arthur, because they use a pdf
  feature called 'transparency groups', which maps badly to Qt.
  
  I also implemented all your further suggestions.  I hope I understood them 
correctly: not sure where you wanted the colon to be.

REPOSITORY
  R223 Okular

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

To: sander, #okular
Cc: okular-devel, asturmlechner, cfeck, ltoscano, rkflx, michaelweghorn, 
ngraham, aacid