Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 10, 2017, 3:53 p.m., Christoph Feck wrote:
> > generators/poppler/generator_pdf.cpp, line 1086
> > 
> >
> > Coding style issues:
> > - there is a space after '(' but no space before ')'
> > - the code after '{' is not indented

> there is a space after '(' but no space before ')'

I promise to fix that in a later commit

> the code after '{' is not indented

That was intentional.  Indenting it properly would have led to a patch with 
lots of whitespace changes.  For easier reviewing I left the indentation as it 
was.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103593
---


On Aug. 10, 2017, 3:32 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 10, 2017, 3:32 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Christoph Feck]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103593
---




generators/poppler/generator_pdf.cpp (line 1086)


Coding style issues:
- there is a space after '(' but no space before ')'
- the code after '{' is not indented


- Christoph Feck


On Aug. 10, 2017, 5:32 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 10, 2017, 5:32 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/
---

(Updated Aug. 10, 2017, 3:32 p.m.)


Status
--

This change has been marked as submitted.


Review request for Okular.


Changes
---

Submitted with commit 0c4c2ddbbc5e509c69b72c99bf6e83a356669eba by Oliver Sander 
to branch master.


Repository: okular


Description
---

When the users chooses to print with rasterization and annotations, it is easy 
to print directly to a QPrinter, rather than converting to PostScript and then 
using CUPS tools.  The code for it was already there, but it was hidden behind 
an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  If nothing else, 
it will make an interesting debugging tool, because it allows to bypass the 
postscript & cups toolchain from the GUI. This may allow to track down some of 
the numerous my-printer-settings-are-getting-ignored bugs.

Incidentally, this patch does fix at least one bug for me: Without it, my 
printer will happily ignore the 'print in grayscale' button.  With the patch, 
that button is suddenly honoured.

This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post it 
here separately because I am not sure anymore whether the other stuff in that 
larger patch is a good idea.


Diffs
-

  generators/poppler/generator_pdf.cpp 42ccb3a26 

Diff: https://git.reviewboard.kde.org/r/130218/diff/


Testing
---

Printed a few test sheets, to a printer and a file.


Thanks,

Oliver Sander

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 9, 2017, 9:19 p.m., Albert Astals Cid wrote:
> > Ok, you're the printing maintainer now ;)
> > 
> > Good luck!
> > 
> > P.S: I can still try to review your patches if you want :)

Thanks.  I still need lots of help.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103584
---


On Aug. 9, 2017, 8:58 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 9, 2017, 8:58 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103584
---


Ship it!




Ok, you're the printing maintainer now ;)

Good luck!

P.S: I can still try to review your patches if you want :)

- Albert Astals Cid


On Aug. 9, 2017, 8:58 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 9, 2017, 8:58 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?
> 
> Oliver Sander wrote:
> Yes I tried that, and it looks very good.  It looks so good I wasn't even 
> able to tell that it was rasterized before printing.  The resolution chosen 
> by the code adapts to the printer
> 
>  QImage img = pp->renderToImage(  printer.physicalDpiX(), 
> printer.physicalDpiY() );
> 
> and I guess it must be pretty large when printing to an actual printer.
> 
> Albert Astals Cid wrote:
> So we will kill cheap printers/computers by using lots of memory? Can you 
> try to see how much memory is used printing an A4 page? Also make sure that 
> printing N pages doesn't need N times the memory (i guess using heaptrack or 
> similar)
> 
> Oliver Sander wrote:
> Quick first results: printer.physicalDpiX() returns a value of 1200, both 
> for printing to my office printer and for printing to a file.  An A4 printout 
> results in a 9921 x 14032 bitmap; the corresponding file size is 804 KB 
> (greyscale).  Testing the actual memory consumption will take me a bit more 
> time.
> 
> I don't actually care about the exact resolution.  If you say "bah, just 
> hard-code 72dpi there" that's fine with me.
> 
> Albert Astals Cid wrote:
> >  9921 x 14032 bitmap
> 
> Does that mean that you need 9921x14032x32 bits in memory? That's around 
> 530 MB, right? I guess it's not horrible in todays big scheme of things if it 
> doesn't get multiplied by the number of pages.
> 
> Oliver Sander wrote:
> I now tried to measure the heap consumptions using valgrind/massif.  I 
> printed to files exclusively, to save some trees.
> 
> If I read the massif output correctly, the new code needs about 1GB of 
> heap space to print A4 pages, and that is independent of the number of pages.
> 
> On the other hand, printing an A0-size poster fails with the friendly 
> message "Bogus memory allocation size".  Not really surprising: that would 
> need an enormous amount of memory.
> 
> Albert Astals Cid wrote:
> > On the other hand, printing an A0-size poster fails with the friendly 
> message "Bogus memory allocation size".  Not really surprising: that would 
> need an enormous amount of memory.
> 
> I'm not sure i'm ok with that.
> 
> Why are we discussing this instead of printing pdf directly?

> I'm not sure i'm ok with that.

Okay.  I changed the patch to always use 300dpi when the platform is not 
Windows.  That is the resolution that was used previously (see 
poppler/PSOutputDev.cc:1260). Therefore, this patch does not bring functional 
changes anymore: with and without the patch, the rasterization resolution is 
printer.physicalDpiX() for Windows, and 300 for all others.

> Why are we discussing this instead of printing pdf directly?

- Printing the pdf does not rasterize it, but this patch is about rasterized 
printing
- The new code does not involve the detour via postscript
- It does not rely on CUPS tools like 'lpr'
- It is platform-independent (exception: the rasterization resolution)
- It is short and elegant
- It fixes at least one bug: greyscale printing

Follow-up patches are in the pipeline.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 9, 2017, 8:58 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 9, 2017, 8:58 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> 

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/
---

(Updated Aug. 9, 2017, 8:58 p.m.)


Review request for Okular.


Changes
---

UNIX: Hardwire rasterization resolution to 300dpi


Repository: okular


Description
---

When the users chooses to print with rasterization and annotations, it is easy 
to print directly to a QPrinter, rather than converting to PostScript and then 
using CUPS tools.  The code for it was already there, but it was hidden behind 
an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  If nothing else, 
it will make an interesting debugging tool, because it allows to bypass the 
postscript & cups toolchain from the GUI. This may allow to track down some of 
the numerous my-printer-settings-are-getting-ignored bugs.

Incidentally, this patch does fix at least one bug for me: Without it, my 
printer will happily ignore the 'print in grayscale' button.  With the patch, 
that button is suddenly honoured.

This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post it 
here separately because I am not sure anymore whether the other stuff in that 
larger patch is a good idea.


Diffs (updated)
-

  generators/poppler/generator_pdf.cpp 42ccb3a26 

Diff: https://git.reviewboard.kde.org/r/130218/diff/


Testing
---

Printed a few test sheets, to a printer and a file.


Thanks,

Oliver Sander

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Albert Astals Cid]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?
> 
> Oliver Sander wrote:
> Yes I tried that, and it looks very good.  It looks so good I wasn't even 
> able to tell that it was rasterized before printing.  The resolution chosen 
> by the code adapts to the printer
> 
>  QImage img = pp->renderToImage(  printer.physicalDpiX(), 
> printer.physicalDpiY() );
> 
> and I guess it must be pretty large when printing to an actual printer.
> 
> Albert Astals Cid wrote:
> So we will kill cheap printers/computers by using lots of memory? Can you 
> try to see how much memory is used printing an A4 page? Also make sure that 
> printing N pages doesn't need N times the memory (i guess using heaptrack or 
> similar)
> 
> Oliver Sander wrote:
> Quick first results: printer.physicalDpiX() returns a value of 1200, both 
> for printing to my office printer and for printing to a file.  An A4 printout 
> results in a 9921 x 14032 bitmap; the corresponding file size is 804 KB 
> (greyscale).  Testing the actual memory consumption will take me a bit more 
> time.
> 
> I don't actually care about the exact resolution.  If you say "bah, just 
> hard-code 72dpi there" that's fine with me.
> 
> Albert Astals Cid wrote:
> >  9921 x 14032 bitmap
> 
> Does that mean that you need 9921x14032x32 bits in memory? That's around 
> 530 MB, right? I guess it's not horrible in todays big scheme of things if it 
> doesn't get multiplied by the number of pages.
> 
> Oliver Sander wrote:
> I now tried to measure the heap consumptions using valgrind/massif.  I 
> printed to files exclusively, to save some trees.
> 
> If I read the massif output correctly, the new code needs about 1GB of 
> heap space to print A4 pages, and that is independent of the number of pages.
> 
> On the other hand, printing an A0-size poster fails with the friendly 
> message "Bogus memory allocation size".  Not really surprising: that would 
> need an enormous amount of memory.

> On the other hand, printing an A0-size poster fails with the friendly message 
> "Bogus memory allocation size".  Not really surprising: that would need an 
> enormous amount of memory.

I'm not sure i'm ok with that.

Why are we discussing this instead of printing pdf directly?


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?
> 
> Oliver Sander wrote:
> Yes I tried that, and it looks very good.  It looks so good I wasn't even 
> able to tell that it was rasterized before printing.  The resolution chosen 
> by the code adapts to the printer
> 
>  QImage img = pp->renderToImage(  printer.physicalDpiX(), 
> printer.physicalDpiY() );
> 
> and I guess it must be pretty large when printing to an actual printer.
> 
> Albert Astals Cid wrote:
> So we will kill cheap printers/computers by using lots of memory? Can you 
> try to see how much memory is used printing an A4 page? Also make sure that 
> printing N pages doesn't need N times the memory (i guess using heaptrack or 
> similar)
> 
> Oliver Sander wrote:
> Quick first results: printer.physicalDpiX() returns a value of 1200, both 
> for printing to my office printer and for printing to a file.  An A4 printout 
> results in a 9921 x 14032 bitmap; the corresponding file size is 804 KB 
> (greyscale).  Testing the actual memory consumption will take me a bit more 
> time.
> 
> I don't actually care about the exact resolution.  If you say "bah, just 
> hard-code 72dpi there" that's fine with me.
> 
> Albert Astals Cid wrote:
> >  9921 x 14032 bitmap
> 
> Does that mean that you need 9921x14032x32 bits in memory? That's around 
> 530 MB, right? I guess it's not horrible in todays big scheme of things if it 
> doesn't get multiplied by the number of pages.

I now tried to measure the heap consumptions using valgrind/massif.  I printed 
to files exclusively, to save some trees.

If I read the massif output correctly, the new code needs about 1GB of heap 
space to print A4 pages, and that is independent of the number of pages.

On the other hand, printing an A0-size poster fails with the friendly message 
"Bogus memory allocation size".  Not really surprising: that would need an 
enormous amount of memory.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?
> 
> Oliver Sander wrote:
> Yes I tried that, and it looks very good.  It looks so good I wasn't even 
> able to tell that it was rasterized before printing.  The resolution chosen 
> by the code adapts to the printer
> 
>  QImage img = pp->renderToImage(  printer.physicalDpiX(), 
> printer.physicalDpiY() );
> 
> and I guess it must be pretty large when printing to an actual printer.
> 
> Albert Astals Cid wrote:
> So we will kill cheap printers/computers by using lots of memory? Can you 
> try to see how much memory is used printing an A4 page? Also make sure that 
> printing N pages doesn't need N times the memory (i guess using heaptrack or 
> similar)

Quick first results: printer.physicalDpiX() returns a value of 1200, both for 
printing to my office printer and for printing to a file.  An A4 printout 
results in a 9921 x 14032 bitmap; the corresponding file size is 804 KB 
(greyscale).  Testing the actual memory consumption will take me a bit more 
time.

I don't actually care about the exact resolution.  If you say "bah, just 
hard-code 72dpi there" that's fine with me.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Albert Astals Cid]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?
> 
> Oliver Sander wrote:
> Yes I tried that, and it looks very good.  It looks so good I wasn't even 
> able to tell that it was rasterized before printing.  The resolution chosen 
> by the code adapts to the printer
> 
>  QImage img = pp->renderToImage(  printer.physicalDpiX(), 
> printer.physicalDpiY() );
> 
> and I guess it must be pretty large when printing to an actual printer.

So we will kill cheap printers/computers by using lots of memory? Can you try 
to see how much memory is used printing an A4 page? Also make sure that 
printing N pages doesn't need N times the memory (i guess using heaptrack or 
similar)


- Albert


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Oliver Sander]

> On Aug. 6, 2017, 3:17 p.m., Albert Astals Cid wrote:
> > Rasterizing usually makes stuff look quite bad (because the wrong 
> > resolution is chosen) , have you tried printing text or things with sharp 
> > lines?

Yes I tried that, and it looks very good.  It looks so good I wasn't even able 
to tell that it was rasterized before printing.  The resolution chosen by the 
code adapts to the printer

 QImage img = pp->renderToImage(  printer.physicalDpiX(), 
printer.physicalDpiY() );

and I guess it must be pretty large when printing to an actual printer.


- Oliver


---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---


On Aug. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated Aug. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>

Re: Review Request 130218: Print via QPrinter when rasterizing and printing annotations

[Albert Astals Cid]

---
This is an automatically generated e-mail. To reply, visit:
https://git.reviewboard.kde.org/r/130218/#review103569
---



Rasterizing usually makes stuff look quite bad (because the wrong resolution is 
chosen) , have you tried printing text or things with sharp lines?

- Albert Astals Cid


On ago. 4, 2017, 8:20 p.m., Oliver Sander wrote:
> 
> ---
> This is an automatically generated e-mail. To reply, visit:
> https://git.reviewboard.kde.org/r/130218/
> ---
> 
> (Updated ago. 4, 2017, 8:20 p.m.)
> 
> 
> Review request for Okular.
> 
> 
> Repository: okular
> 
> 
> Description
> ---
> 
> When the users chooses to print with rasterization and annotations, it is 
> easy to print directly to a QPrinter, rather than converting to PostScript 
> and then using CUPS tools.  The code for it was already there, but it was 
> hidden behind an #ifdef Q_OS_WIN. This patch enables it for all plattforms.  
> If nothing else, it will make an interesting debugging tool, because it 
> allows to bypass the postscript & cups toolchain from the GUI. This may allow 
> to track down some of the numerous my-printer-settings-are-getting-ignored 
> bugs.
> 
> Incidentally, this patch does fix at least one bug for me: Without it, my 
> printer will happily ignore the 'print in grayscale' button.  With the patch, 
> that button is suddenly honoured.
> 
> This patch is a part of https://git.reviewboard.kde.org/r/130055/ .  I post 
> it here separately because I am not sure anymore whether the other stuff in 
> that larger patch is a good idea.
> 
> 
> Diffs
> -
> 
>   generators/poppler/generator_pdf.cpp 42ccb3a26 
> 
> Diff: https://git.reviewboard.kde.org/r/130218/diff/
> 
> 
> Testing
> ---
> 
> Printed a few test sheets, to a printer and a file.
> 
> 
> Thanks,
> 
> Oliver Sander
> 
>