D10974: Add option to ignore print margins

2018-05-24 Thread Nathaniel Graham
ngraham added a comment.


  I took the time to read the attached bug report and I take back what I said 
in the prior comment. I see now why one would want to manually trigger this 
behavior.
  
  This is basically a scaling setting, but we also want it to be really obvious 
what each state is for, because it's a somewhat technical feature.
  
  If we use a checkbox, the string I would recommend is "Scale down to fit 
within printable area".
  
  Another option is to use a radio button:
  
Scaling: (o) None
 ( ) Auto-fit within printable area
  
  A nice side-effect of the radio button approach is that when we add fixed 
percentage scaling, it naturally fits in the same group:
  
Scaling: (o) None
 ( ) Auto-fit within printable area
 ( ) Scale to fixed percentage: [100%]
  
  Or even:
  
Scaling: (o) [100%]
 ( ) Auto-fit within printable area
  
  A further enhancement would be to automatically select an appropriate option 
by default according to the logic I outlined above: if the document actually 
does fit within the printable area due to its own margins, or because the 
printer supports borderless printing, don't scale it down by default unless the 
user deliberately chooses that option.
  
  Does that make sense?

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Nathaniel Graham
ngraham added a comment.


  Stepping back a bit, I can identify the following two use cases:
  
  - The document has its own margins that will make it fit within the printer's 
printable area without any scaling (e.g. a scientific paper or an eBook)
  - The document will not fit within the printer's printable area without some 
scaling, either because it has no margins, or because its own margins are not 
big enough (e.g a flyer, magazine article, or advertisement)
  
  In the first case, you never need additional margins and always want the 
document printed with no scaling.
  
  In the second case, you always need to scale the document down to fit within 
the printable area, or else the edges will be clipped and you'll lose some of 
the content.
  
  If this analysis is complete, then we don't actually need a user-facing 
setting here at all; we should instead infer the correct setting for ourselves 
by seeing whether the document to be printed has its own margins that make it 
fit within the printer's printable area: if it does, print as-is; if it 
doesn't, then scale it until it fits.
  
  Am I on the right track, or is this an incomplete assessment?

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Michael Weghorn
michaelweghorn added a comment.


  In D10974#267891 , @aacid wrote:
  
  > In D10974#267814 , 
@michaelweghorn wrote:
  >
  > > @aacid: What's the right way to properly indicate that an important part 
of this
  > >  was actually done by you (add you in the copyright, mention in the 
commit message,...)?
  >
  >
  > Just add a line to the commit message and it's fine for me
  
  
  OK, will do with the next revision.

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Michael Weghorn
michaelweghorn added a comment.


  In D10974#267887 , @ngraham wrote:
  
  > From a user perspective, I'm not sure I would know what "Ignore printer 
margins" means. What's a printer margin? If I turn this on or off, what will 
happen? It's not very clear at the moment, IMHO.
  
  
  Thanks for your opinion on this. Do you have a suggestion on how to improve 
this?

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Albert Astals Cid
aacid added a comment.


  In D10974#267814 , @michaelweghorn 
wrote:
  
  > @aacid: What's the right way to properly indicate that an important part of 
this
  >  was actually done by you (add you in the copyright, mention in the commit 
message,...)?
  
  
  Just add a line to the commit message and it's fine for me

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Nathaniel Graham
ngraham added a comment.


  From a user perspective, I'm not sure I would know what "Ignore printer 
margins" means. What's a printer margin? If I turn this on or off, what will 
happen? It's not very clear at the moment, IMHO.

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Michael Weghorn
michaelweghorn added a comment.


  In D10974#267814 , @michaelweghorn 
wrote:
  
  > [] 
  >  (Side note: I missed this at first, since values for the margins in the Qt 
print
  >  dialog currently don't seem to be initialized with the PPD default values.)
  
  
  Or, to be more precise after a glance: A minimum of 10 pt is used for the 
initialization and printer defaults for the print margins are only used when 
they are greater, which does make some sense. (Margins smaller than 10pt can 
still be set manually if needed). But this is rather off-topic for this change, 
anyway...

REPOSITORY
  R223 Okular

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

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10974: Add option to ignore print margins

2018-05-24 Thread Michael Weghorn
michaelweghorn updated this revision to Diff 34821.
michaelweghorn retitled this revision from "PDF: Allow to ignore print margins" 
to "Add option to ignore print margins".
michaelweghorn edited the summary of this revision.
michaelweghorn edited the test plan for this revision.
michaelweghorn added a comment.


  Changes to previous version:
  
  1. take over Albert's suggestion to make option available to all generators 
"as is" (except for fixing a minor typo in a comment ("cad" -> "can") (A big 
thanks to Albert!)
  2. rename dialog option from "Fit to printable area" to "Ignore printer 
margins" and adapt variable names etc. accordingly
  3. rebase onto current git master
  4. Fix issue that hardware printer margins were still taken into account when 
option was set not to do so.
  
  Detail on 4): Further testing with more real printers showed that just passing
  the option "fit-to-page" to CUPS without explicitly setting any margins causes
  the defaults from the PPD to be used, which is not what is excpected.
  This is fixed now by explicitly passing '0'.
  
  (Side note: I missed this at first, since values for the margins in the Qt 
print
  dialog currently don't seem to be initialized with the PPD default values.)
  
  @aacid: What's the right way to properly indicate that an important part of 
this
  was actually done by you (add you in the copyright, mention in the commit 
message,...)?

REPOSITORY
  R223 Okular

CHANGES SINCE LAST UPDATE
  https://phabricator.kde.org/D10974?vs=29095&id=34821

BRANCH
  michaelweghorn/WIP_update_D10974

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

AFFECTED FILES
  CMakeLists.txt
  core/document.cpp
  core/document.h
  core/fileprinter.cpp
  core/printoptionswidget.cpp
  core/printoptionswidget.h
  generators/poppler/generator_pdf.cpp
  generators/poppler/generator_pdf.h
  interfaces/printinterface.h
  part.cpp

To: michaelweghorn, #okular
Cc: okular-devel, aacid, ngraham


D10932: [Okular] Option to reset forms

2018-05-24 Thread Albert Astals Cid
aacid requested changes to this revision.
This revision now requires changes to proceed.
Restricted Application added a subscriber: okular-devel.

REPOSITORY
  R223 Okular

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

To: ahmadosama, #okular, aacid
Cc: okular-devel, aheinecke, rkflx, cfeck, ngraham, aacid, #okular


D12250: Install okularpart with the rest of kparts

2018-05-24 Thread Albert Astals Cid
aacid requested changes to this revision.
This revision now requires changes to proceed.
Restricted Application added a subscriber: okular-devel.

REPOSITORY
  R223 Okular

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

To: apol, #okular, aacid
Cc: okular-devel, ngraham, aacid