| ngraham | |
| sander |
| Okular |
This adds another 'FilePrinter::printFile' method that
accepts an additional parameter to specify whether or not to
do scaling and passes the 'fit-to-page' to CUPS dependent
on what is specified.
If FilePrinter is used, The PDF generator now passes this
option depending on the scaling mode that was selected in the
custom print options widget, which is therefore now enabled
for non-rasterized printing as well.
| No Linters Available |
| No Unit Test Coverage |
| Buildable 7216 | |
| Build 7234: arc lint + arc unit |
Maybe an enum is better than a bool so if in the future more scaling options are implemented we don't need to change the signature again?
| core/fileprinter.h | ||
|---|---|---|
| 82 ↗ | (On Diff #49715) | this is not binary compatible, we try not to break the binary compatibility of the okular public classes. Add another method (without the default values) and make this one call the new one. Same for all the functions you're adding new parameters to. Or you can try to convince me that maintainting binary compatibilty is not needed :D |
Sounds reasonable. I'll change that in the next version.
| core/fileprinter.h | ||
|---|---|---|
| 82 ↗ | (On Diff #49715) | I don't really feel up to the task of convincing you. :D Otherwise, I don't mind adding the suggested extra methods. Will those extra methods then stay there forever or can the methods then be "merged" again once another ABI-incompatible change is done? Just out of curiosity: Do you know of specific applications/third-party generators,... that use libokularcore? |
| core/fileprinter.h | ||
|---|---|---|
| 82 ↗ | (On Diff #49715) | I'm almost sure that doesn't break binary compatibility since it's a template and thus the code would end up on the app side and not the library side, but i didn't want to lose sleep making sure. Once we break ABI we could merge them back, if people realize :D calligra has a few generators for odt, odp using calligra libs. |
Implement improvements suggested by Albert:
What's a bit ugly here is that 'scaleMode' cannot be the last
parameter in the new 'FilePrinter::printFile' method, as it
has to come before those params that have a default value.
We would need a @since marker for the enum and for the new methods and a
// TODO merge with function above when a BIC change happens somehwere else
would also make sense
| core/fileprinter.h | ||
|---|---|---|
| 58 | if this is going to be flags, it should be like this enum ScaleMode { so the next flags can be 2, 4, 8, etc, meaning you can use binary or and binary and on them and it'll all work | |
Changes in this revision:
I added these. I did not add a TODO comment to merge for the new printFiles() method, though. In the current constellation, merging them would break the API, since the position of the 'scaleMode' parameter is another one (can't be the last param for the new version, since it has no default value) and I guess this is undesirable, even when an ABI change appears?.
If you have any other idea on how to avoid that issue with the printFiles() methods, I'd be thankful to hear about this.
Changes in this revision:
Thanks, that looks good. I have done it this way now.
(Note to self: Make sure not to have strange settings in $HOME/.cups/lpoptions while testing ...)
This patch no longer applies cleanly to master:
Switched to branch 'master'
Your branch is up to date with 'origin/master'.
Already up to date.
Deleted branch arcpatch-D18179 (was 8bf1a9117).
INFO Base commit is not in local repository; trying to fetch.
Created and checked out branch arcpatch-D18179.
Created and checked out branch arcpatch-D10974.
Checking patch part.cpp...
Checking patch interfaces/printinterface.h...
Checking patch generators/poppler/generator_pdf.h...
Checking patch generators/poppler/generator_pdf.cpp...
Checking patch core/printoptionswidget.h...
Checking patch core/printoptionswidget.cpp...
Checking patch core/fileprinter.cpp...
Checking patch core/document.h...
Checking patch CMakeLists.txt...
Applied patch part.cpp cleanly.
Applied patch interfaces/printinterface.h cleanly.
Applied patch generators/poppler/generator_pdf.h cleanly.
Applied patch generators/poppler/generator_pdf.cpp cleanly.
Applied patch core/printoptionswidget.h cleanly.
Applied patch core/printoptionswidget.cpp cleanly.
Applied patch core/fileprinter.cpp cleanly.
Applied patch core/document.h cleanly.
Applied patch CMakeLists.txt cleanly.
Cherry Pick Failed!
Exception
Command failed with error #1!
COMMAND
git cherry-pick 'arcpatch-D10974'
STDOUT
On branch arcpatch-D18179
You are currently cherry-picking commit 4cc9a85f0.
nothing to commit, working tree clean
STDERR
The previous cherry-pick is now empty, possibly due to conflict resolution.
If you wish to commit it anyway, use:
git commit --allow-empty
Otherwise, please use 'git reset'
(Run with `--trace` for a full exception trace.)Could you rebase on master?
Yet, it also took me a while to figure this out... Looking forward to the GitLab migration :)
All options work perfectly in my testing, both with and without rasterized printing enabled. If @aacid is good with the code, I think this could land.
Changes in this revision:
i have not tested it, if you think it works, i guess it's ok from the pure code point of view.
OK, let's go for it!
@michaelweghorn I hope you know this means you're on the hook to fix any bugs that crop up, right? :)