<html>
<body>
<div style="font-family: Verdana, Arial, Helvetica, Sans-Serif;">
<table bgcolor="#f9f3c9" width="100%" cellpadding="12" style="border: 1px #c9c399 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
This is an automatically generated e-mail. To reply, visit:
<a href="https://git.reviewboard.kde.org/r/130055/">https://git.reviewboard.kde.org/r/130055/</a>
</td>
</tr>
</table>
<br />
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<p style="margin-top: 0;">On July 2nd, 2017, 10:34 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE 4.1 is long gone. What is fileprinter supposed to be replaced with?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Some code in Qt that was never made public</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">What direction should I take to obtain something mergeable?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Get people that actually use Okular to print with different printers to test it. Or otherwise just say "fuck it" and merge it and fix all the things that break afterwards :D</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Have you tested printing to different sizes? What about margins?</p></pre>
</blockquote>
<p>On July 5th, 2017, 7:45 p.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Some code in Qt that was never made public</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does this mean that fileprinter is here to stay now?</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Get people that actually use Okular to print with different printers to test it. Or
otherwise just say "fuck it" and merge it and fix all the things that break afterwards :D</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I tested a few configurations, but I don't think one person can exhaustively test these things anyway. How about the following: we merge it, but we add a new option to the print dialog "Fall back to ghostscript printing" (default: off). That would allow to pacify angry users quickly, and it may help with debugging, too. If things are quiet for a year or two we can then remove the option.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't really have any clue how to do printing with annotations without ghostscript. Any ideas? Will that require poppler support? Like a new option in Poppler::PdfConverter?</p></pre>
</blockquote>
<p>On July 11th, 2017, 11:07 p.m. UTC, <b>Albert Astals Cid</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Does this mean that fileprinter is here to stay now?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Guess so, yeah</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">How about the following: we merge it, but we add a new option to the print dialog "Fall back to ghostscript printing" (default: off). That would allow to pacify angry users quickly, and it may help with debugging, too. If things are quiet for a year or two we can then remove the option.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Postscript, not ghostscript</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't really have any clue how to do printing with annotations without ghostscript. Any ideas? Will that require poppler support? Like a new option in Poppler::PdfConverter?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Yeah guess so.</p></pre>
</blockquote>
<p>On July 12th, 2017, 2:30 p.m. UTC, <b>Oliver Sander</b> wrote:</p>
<blockquote style="margin-left: 1em; border-left: 2px solid #d0d0d0; padding-left: 10px;">
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I don't really have any clue how to do printing with annotations without ghostscript.
Any ideas? Will that require poppler support? Like a new option in
Poppler::PdfConverter?
Yeah guess so.</p>
</blockquote>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I thought about this a bit more. According to the Qt docs, the 'true' way would be to have poppler render the file directly into a QPrinter/QPainter, and then let Qt do the actual printing. That would simplify the Okular printing code considerably, and it would also work on Windows (unlike what we do currently).</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Interestingly, you can kind-of already do that, by using the poppler renderToPainter method. First experiments have been promising, but you need the Arthur backend and I think I am seeing a few of its deficiencies. Is it worth pursuing this approach any further?</p></pre>
</blockquote>
</blockquote>
<pre style="white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">you need the Arthur backend and I think I am seeing a few of its deficiencies.</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">a "few" is being very bening :D</p>
<blockquote style="text-rendering: inherit;padding: 0 0 0 1em;border-left: 1px solid #bbb;white-space: normal;margin: 0 0 0 0.5em;line-height: inherit;">
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Is it worth pursuing this approach any further?</p>
</blockquote>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Unless you want to become the maintainer of a pdf renderer, i would suggest not to.</p></pre>
<br />
<p>- Albert</p>
<br />
<p>On July 12th, 2017, 10:37 a.m. UTC, Oliver Sander wrote:</p>
<table bgcolor="#fefadf" width="100%" cellspacing="0" cellpadding="12" style="border: 1px #888a85 solid; border-radius: 6px; -moz-border-radius: 6px; -webkit-border-radius: 6px;">
<tr>
<td>
<div>Review request for Okular.</div>
<div>By Oliver Sander.</div>
<p style="color: grey;"><i>Updated July 12, 2017, 10:37 a.m.</i></p>
<div style="margin-top: 1.5em;">
<b style="color: #575012; font-size: 10pt;">Repository: </b>
okular
</div>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Description </h1>
<table width="100%" bgcolor="#ffffff" cellspacing="0" cellpadding="10" style="border: 1px solid #b8b5a0">
<tr>
<td>
<pre style="margin: 0; padding: 0; white-space: pre-wrap; white-space: -moz-pre-wrap; white-space: -pre-wrap; white-space: -o-pre-wrap; word-wrap: break-word;"><p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">This patch is an improved version of what is in the pdfprintpdf branch. (There is really only a single patch in that branch.)</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">The patch tries to avoid converting pdf files to ps files for printing as much as possible. In particular:</p>
<ul style="padding: 0;text-rendering: inherit;margin: 0 0 0 1em;line-height: inherit;white-space: normal;">
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">If we are printing a file, and printing of annotations is not requested, then the pdf file is sent straight to the printer.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">If rasterization is requested, then the short previously-windows-only printing code is used, because it does exactly that: printing by rasterization.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">In all other cases, the file is converted to ps as before.</li>
<li style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: normal;">The big #ifdef Q_OS_WIN ... #else ... #endif is removed. All code is compiled on all platforms, but on Windows forceRasterize is always set. Therefore, the behavior on Windows remains unchanged.</li>
</ul>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">There are a few behavioral changes that I know of:
- The resolution of the rasterization most likely changes. Don't know whether this is a problem.
- Previously, when printing without printAnnots to a file, the result file did not have the annotations anymore. With the new code, the annotations are still there, but they are still actual annotations, unlike what you get when using the old code to print annotations. I am not sure whether to call this a regression or a feature.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">I am motivated to improve this patch some more, but I need some guidance. What direction should I take to obtain something mergeable?</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">Also: in the file fileprinter.h it says:</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">// This Class is a temporary addition to Okular for the duration of KDE 4.0.
// In KDE 4.1 this class will either be moved to kdelibs if still required,
// or replaced with a Qt 4.4 based solution.</p>
<p style="padding: 0;text-rendering: inherit;margin: 0;line-height: inherit;white-space: inherit;">KDE 4.1 is long gone. What is fileprinter supposed to be replaced with?</p></pre>
</td>
</tr>
</table>
<h1 style="color: #575012; font-size: 10pt; margin-top: 1.5em;">Diffs</b> </h1>
<ul style="margin-left: 3em; padding-left: 0;">
<li>generators/poppler/generator_pdf.h <span style="color: grey">(a078f50bd)</span></li>
<li>generators/poppler/generator_pdf.cpp <span style="color: grey">(42ccb3a26)</span></li>
<li>part.cpp <span style="color: grey">(df38e85e9)</span></li>
</ul>
<p><a href="https://git.reviewboard.kde.org/r/130055/diff/" style="margin-left: 3em;">View Diff</a></p>
</td>
</tr>
</table>
</div>
</body>
</html>