Bug 13954 - Patroncards: images show inadequate rough pixelation
Summary: Patroncards: images show inadequate rough pixelation
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Label/patron card printing (show other bugs)
Version: master
Hardware: All All
: P4 normal (vote)
Assignee: Chris Nighswonger
QA Contact: Marc Véron
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2015-04-03 20:53 UTC by Heinrich Hartl
Modified: 2016-12-05 21:22 UTC (History)
5 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Trivial patch
Bot Control: ---
When did the bot last check this:
Who signed the patch off:
Text to go in the release notes:


Attachments
pixaleted picture side by side with irfanview window of same picture (78.15 KB, image/jpeg)
2015-04-03 20:53 UTC, Heinrich Hartl
Details
layout of patron card (on template with labels 70*36mm) (52.18 KB, image/jpeg)
2015-04-03 20:56 UTC, Heinrich Hartl
Details
picture used as other image (74.66 KB, image/jpeg)
2015-04-03 20:58 UTC, Heinrich Hartl
Details
Compare PDF with downsampled image (without patch) with PDF internally scaled image (with patch) (346.34 KB, image/png)
2015-04-06 11:54 UTC, Marc Véron
Details
Bug 13954 - Patroncards: images show inadequate rough pixelation (6.10 KB, patch)
2015-04-06 12:19 UTC, Marc Véron
Details | Diff | Splinter Review
[SIGNED-OFF] Bug 13954 - Patroncards: images show inadequate rough pixelation (6.17 KB, patch)
2015-04-06 19:24 UTC, Chris Nighswonger
Details | Diff | Splinter Review
[PASSED QA] Bug 13954 - Patroncards: images show inadequate rough pixelation (6.22 KB, patch)
2015-04-07 20:46 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Heinrich Hartl 2015-04-03 20:53:43 UTC
Created attachment 37493 [details]
pixaleted picture side by side with irfanview window of same picture

I am preparing a patron card layout including a logo. Since the real logo is not already available I decided to use an arbitrary other .jpg instead. The result in the created pdf is showing the picture in right size and position but badly pixelated. Providing the same picture in .png did not help. I suspect that the resizing operation is not done such as to give an acceptable or optimum result.

 Please find the following attachments:

Screenshot of layout used (template has labels 70*36 mm)

Screenshot taken with irfanview showing the pdf created by export viewed 400% and on top of it irfanview Window displaying the same photo - here you can see the pixelated picture side by side with a reasonably resized version of the same picture

The used picture to allow you to reproduce my result or even better to use in a testcase for the resolved problem.
Comment 1 Heinrich Hartl 2015-04-03 20:56:17 UTC
Created attachment 37494 [details]
layout of patron card (on template with labels 70*36mm)
Comment 2 Heinrich Hartl 2015-04-03 20:58:27 UTC
Created attachment 37495 [details]
picture used as other image
Comment 3 Marc Véron 2015-04-06 11:54:35 UTC
Created attachment 37525 [details]
Compare PDF with downsampled image (without patch) with PDF internally scaled image (with patch)

The problem is that the image is downsampled to a very small size before it ist inserted into the PDF file. The downscaling should be done inside PDF.

See screenshots from PDF, image height 67pt, displayed in Adobe Reader at 400%.

Patch follows.
Comment 4 Marc Véron 2015-04-06 12:19:50 UTC Comment hidden (obsolete)
Comment 5 Chris Nighswonger 2015-04-06 13:01:49 UTC
(In reply to Marc Véron from comment #4)

> This patch provides a much better quality of images on patron cards.
> It inserts images at a higher resolution into the PDF file and then scales
> it down internally in PDF.
> 

I think $oversize_factor should not be hard-coded. Based on past experience (at least in this area of the code), hard-coded vars end up making it very difficult to figure out what is going on both for the end user and for devs working on bugs, etc.

Furthermore, it may be desirable to have this as a per-image parameter to allow the end user to decide what amount of over-sizing is needed to achieve the desired end results for each image.
Comment 6 Katrin Fischer 2015-04-06 13:15:00 UTC
Not sure if this value would be easy to understand/use for users from the GUI. It seems to fix a problem that some people have reported in the past - could this be a first step with more refinement later?
Comment 7 Marc Véron 2015-04-06 13:43:49 UTC
The way how the images are first downsampled and then added to the PDF file is simply wrong. It leads to a very poor quality of the printed cards. The images should be added to the PDF with a higher quality and downscaled (transformed) with PDF internal operators. Rule of thumb for this kind of printing is to have it 5 times bigger (I know such things; I do PDF stuff since 1995). This is fixed in my patch, and the wole card creation works the same way as before.

I do not think that it makes much sense to expose this var to the end user, because it is merely a technical thing.

For the encanced quality, see: 
http://bugs.koha-community.org/bugzilla3/attachment.cgi?id=37525
Comment 8 Chris Nighswonger 2015-04-06 14:03:01 UTC
Marc, thanks for you work on this. Nothing here is meant to discredit that or convey anything other than great appreciation for all of the help I can get on bugs in this code.

I've only done PDF stuff since 2000, so you've go the goods on me there, however, I would really like to have time to convince myself that this is where/what the fix should be. If you feel that the problem really lies elsewhere in the code, I would prefer that the fix be made there rather than in this location.

ATM, I am attempting to duplicate your issue on the current master. After that, if the fix as it is makes sense to me, I'll have no problem signing off on it.

Also, citing some sort of documentation on scale-up/scale-down inside the pdf as the preferred technique would be appreciated as well. I have tried to do this as comments in the code where I felt it might be helpful to my own poor memory as well as to the next sad soul who has to work on the "code soup" I have written.

AAMOF, if doing the scaling inside the pdf both works in every case and is the preferred method, then perhaps we can eliminate a large chunk of code moving the scaling entirely to the pdf renderer. I'm all for purging old/bad/poorly-written/unnecessary code.
Comment 9 Marc Véron 2015-04-06 18:48:49 UTC
Chris, thank you very much for your comment.

There is an example for scaling images in PDF Reference:

http://wwwimages.adobe.com/www.adobe.com/content/dam/Adobe/en/devnet/pdf/pdfs/pdf_reference_1-7.pdf


Page 339, Example 4.72
% = comment

---snip--- 
  q                                        % Save graphics state
  1 0 0 1 100 200 cm                       % Translate
  0.7071 0.7071 −0.7071 0.7071 0 0 cm      % Rotate [not yet used in our case, MV]
  150 0 0 80 0 0 cm                        % Scale
  /Image1 Do                               % Paint image
  Q                                        % Restore graphics state
---snip---

- Translate puts the image at a given position
- Rotate is not used at in Patroncard.pm (but could be used at a later stage)
- Scale (new line in Patroncard.pm) does what it says (it does not shrink the image data, however, it scales it).
- The Do operator paints the image using the transformation rules above.

Page 206 (Chapter 4.2 Coordinate Systems -> 4.2.2. Common Transformatons) says:
---snip---
If several transformations are combined, the order in which they are applied is significant. For example, first scaling and then translating the x axis is not the same as first translating and then scaling it. In general, to obtain the expected results, transformations should be done in the following order:
1.Translate
2.Rotate
3.Scale or skew
---snip---

On the same page is a figure that shows the big differences in the result if you apply the transformations in different sequences.

In our case the issue was that the image was downsampled / shrinked to a very small size (using ImageMagick) before it was applied to the PDF. The resulting image had (in my test case) a height of about 70 units (pixels). The quality was like (or even less) the quality of a thumbnail picture. The PDF viewer then had to scale it up for displaying or printing - that's why it looked so 'pixelated'. If the picture has a height of 350 units (pixels) and is transformed down to 70 while preparing it for the display it, the result is much better, simply because we have more image data available.

In short, the steps are:
- Prepare a normalized jpg image with a good quality / size using ImageMagick
- Position this image on the PDF page (Translate)
- (Rotate /chew it if necessary, we don't do it here)
- Scale it down to the final display size

PS: I can imagine some sophisticatd enhancements as well. 
For example, it would be nice to have the possibility to put an image as background or to rotate an image etc., with appropriate parameters in the UI. But that would not be in the scope of this Bug.
Comment 10 Chris Nighswonger 2015-04-06 19:24:43 UTC Comment hidden (obsolete)
Comment 11 Chris Nighswonger 2015-04-06 19:25:17 UTC
I can't tell you how many times I've looked at that particular part of the spec and completely missed the scale options... thanks for pointing it out.

I ran some tests on your patch using some extremely large, compressed jpegs and it works fine on everything I've thrown at it so far.

I still have an issue (not too much of one) with the approach to the fix. I'd love to see us rip out the ImageMagick stuff, keep some restrictions on the size of what can be uploaded, and let the scaling be handled during rendering.

Its been a while since I worked on all of the affected sections of code, so this might be a big project.

With that in mind, I'll drop my objection to this patch (and even sign-off on it) with the condition that a new enhancement request be opened to suggest a more through cleanup of this.

Thanks again, Marc. I have never really liked ImageMagick as it always seemed a bit klungy.
Comment 12 Katrin Fischer 2015-04-07 20:45:18 UTC
Thx for the screenshots - it made testing this a lot easier. The image quality is greatly improved by this patch.
Comment 13 Katrin Fischer 2015-04-07 20:46:33 UTC
Created attachment 37557 [details] [review]
[PASSED QA] Bug 13954 - Patroncards: images show inadequate rough pixelation

This patch provides a much better quality of images on patron cards.
It inserts images at a higher resolution into the PDF file and then scales it down internally in PDF.

Additionaly, the patch removes the follwing warning:
"my" variable $template masks earlier declaration in same scope at /usr/share/kohaclone/patroncards/create-pdf.pl line 66

To test:

- Create patron cards, save them as PDF and display them with a PDF viewer, e.g. at 400%
- Without patch, the images are very pixelated.
- Apply patch
- Verify that in the PDF the images now display with a much better quality.

Signed-off-by: Chris Nighswonger <cnighswonger@foundations.edu>
Signed-off-by: Katrin Fischer <Katrin.Fischer.83@web.de>
Comment 14 Tomás Cohen Arazi 2015-04-08 14:00:04 UTC
Patch pushed to master.

Thanks Marc!
Comment 15 Marc Véron 2015-04-08 15:31:38 UTC
Hi Tomás,

Thank you for pushing this one.
Is it possible to push Bug 8796 as well? - Would be great.
Comment 17 Katrin Fischer 2015-12-17 12:57:06 UTC
I've hidden the last comment as spam.