Bug 85934 - [PATCH] add antialiasing to pdftops 0.28.1
Summary: [PATCH] add antialiasing to pdftops 0.28.1
Status: RESOLVED FIXED
Alias: None
Product: poppler
Classification: Unclassified
Component: utils (show other bugs)
Version: unspecified
Hardware: All All
: medium normal
Assignee: William Bader
QA Contact:
URL:
Whiteboard:
Keywords:
Depends on:
Blocks:
 
Reported: 2014-11-05 20:10 UTC by William Bader
Modified: 2015-02-09 21:31 UTC (History)
1 user (show)

See Also:
i915 platform:
i915 features:


Attachments
patch to add anti-aliasing options to pdftops.cc (1.83 KB, text/plain)
2014-11-05 20:10 UTC, William Bader
Details
screen capture before and after anti-aliasing (748.58 KB, image/png)
2014-11-05 20:11 UTC, William Bader
Details
revised patches to add antialiasing to pdftops (23.72 KB, patch)
2014-11-14 03:12 UTC, William Bader
Details | Splinter Review
revised patches that do not create currentVectorAntialias (16.26 KB, patch)
2014-11-14 21:00 UTC, William Bader
Details | Splinter Review
revised patch the removes extra variables (16.23 KB, patch)
2014-11-14 21:08 UTC, William Bader
Details | Splinter Review
Simplified version of the patch (18.66 KB, patch)
2015-01-02 18:33 UTC, Albert Astals Cid
Details | Splinter Review
Better simplified version of the patch (19.44 KB, text/plain)
2015-01-02 18:38 UTC, Albert Astals Cid
Details
Updated better simplified version of the patch (20.12 KB, patch)
2015-01-14 13:23 UTC, William Bader
Details | Splinter Review
patch to update the man page (1.12 KB, patch)
2015-02-09 03:29 UTC, William Bader
Details | Splinter Review

Description William Bader 2014-11-05 20:10:40 UTC
Created attachment 108984 [details]
patch to add anti-aliasing options to pdftops.cc

pdftops -level1 reverts to rasterizing PDFs that it can not convert to Level 1 postscript (which includes PDFs that use transparencies or pattern image masks).
This patch adds -aa and -aaVector options identical to the options in pdftoppm.
Splash already implements anti-aliasing.  The patch just adds code in pdftops.cc to parse the command line options and set the corresponding global parameters.
I will attach the patch and a screen capture of the comparison using the PDF from https://bugs.freedesktop.org/show_bug.cgi?id=31926 made with
./pdftops -level1sep -eps S33948.pdf x1.ps
./pdftops -level1sep -eps -aa yes -aaVector yes S33948.pdf x2.ps
Comment 1 William Bader 2014-11-05 20:11:34 UTC
Created attachment 108985 [details]
screen capture before and after anti-aliasing
Comment 2 Adrian Johnson 2014-11-05 20:44:12 UTC
(In reply to William Bader from comment #1)
> Created attachment 108985 [details]
> screen capture before and after anti-aliasing

What does it look like when printed?
Comment 3 William Bader 2014-11-05 21:39:26 UTC
Yes, there is a difference.  PDFs like this have been a problem for several years.  If they print the PDF directly it looks OK, but if they print it through my application, it goes through a workflow that pushes it through pdftops -level1sep so that it is software separable.  I got complaints about the quality, and I have been working around it by adding -r 400, but it almost doubles the size of the files.
Comment 4 William Bader 2014-11-12 19:58:56 UTC
Does this patch to pdftops have any chance of being accepted?

The anti-aliasing code paths are well tested, so enabling anti-aliasing should be safe: GlobalParams.cc initializes antialias to gTrue, so pdftoppm has anti-aliasing enabled by default, and pdftocairo enables anti-aliasing for bitmapped output.  

The generated postscript should remain the same unless pdftops has to fall back to rasterizing the image, so normal ps output will be unaffected: PSOutputDev.cc uses the anti-alias flag only in checkPageSlice() when it needs to create a SplashOutputDev.

Why does GlobalParams.cc initialize antialiasPrinting to gFalse?  Is it for performance or memory issues or does enabling anti-aliasing have other side effects that produce bad postscript output?

In my case, I am using pdftops to make an eps that is one of many images embedded on a postscript page, and then the page is both printed and converted to PDF for viewing over the internet.  When pdftops rasterizes an eps, anti-aliasing makes text in that eps look better in the printed version of the complete document and much better when viewing the PDF version of the complete document.
Comment 5 Albert Astals Cid 2014-11-12 20:24:45 UTC
Can i convince you of trying to remove those properties from GlobalParams and move it to a proper setter in the needed classes? GlobalParams is a thing of the past of when xpdf had a single global configuration file that has to die.
Comment 6 William Bader 2014-11-12 21:11:19 UTC
I could do that, but what class should hold the flags?

OutputDevices looks like a good place, for example, poppler/Gfx.cc uses getVectorAntialias() in OutputDevice.

But poppler/SplashOutputDev.cc initializes SplashOutputDev() using globalParams->getVectorAntialias(), so the three anti-aliasing flags would need to be parameters to the constructor.
Comment 7 Albert Astals Cid 2014-11-12 21:18:41 UTC
no need for new paramaters, just settings and getters.
Comment 8 Adrian Johnson 2014-11-12 21:30:02 UTC
(In reply to William Bader from comment #4)
> Why does GlobalParams.cc initialize antialiasPrinting to gFalse?

Because it is the right thing to do. For antialising to work correctly the image resolution needs to exactly match the resolution that color images are rendered on the output device. When printing you generally don't know this so is is better to send a higher resolution image and let the printer generate the optimum output.

I don't have any objection to adding an -aa option as long as it does not change the default.
Comment 9 William Bader 2014-11-12 22:11:01 UTC
Is it OK if I add setters and getters for the antialiasing flags to OutputDev?
Should I remove them from GlobalParams or keep them to avoid changing the API?
Comment 10 Albert Astals Cid 2014-11-12 22:29:50 UTC
kill globalparams. If outputdev works, do it :)
Comment 11 William Bader 2014-11-14 03:12:01 UTC
Created attachment 109447 [details] [review]
revised patches to add antialiasing to pdftops

I moved the antialias flags from GlobalParams to OutputDev.
SplashOutputDev used to have a private copy of the vector antialias flag.
I made it more general by moving it to OutputDev and tracking both the original and current value of the flag.
Comment 12 William Bader 2014-11-14 03:20:20 UTC
I looked at it again, and I think I missed that changing vector antialias in SplashOutputDev also needs to set splash->setVectorAntialias(vaa).
Is the rest OK otherwise?
Comment 13 Albert Astals Cid 2014-11-14 10:26:31 UTC
Do we really need the currentVectorAntialias one?
Comment 14 William Bader 2014-11-14 20:09:44 UTC
I am redoing it not to add any new variables like currentVectorAntialias.

poppler current has two copies of that flag, one copy in GlobalParams and a second copy in SplashOutputDev.

SplashOutputDev sets its copy to false if colorMode != splashModeMono1.

I had made the GlobalParams copy into OutputDev vectorAntialias and the SplashOutputDev copy into currentVectorAntialias.

If I remove currentVectorAntialias and use vectorAntialias throughout, then if you call SplashOutputDev::startPage() with a different colorMode, you might have to reset vectorAntialias before calling startPage().  Is that a problem?
Comment 15 William Bader 2014-11-14 21:00:16 UTC
Created attachment 109482 [details] [review]
revised patches that do not create currentVectorAntialias
Comment 16 William Bader 2014-11-14 21:08:46 UTC
Created attachment 109484 [details] [review]
revised patch the removes extra variables
Comment 17 William Bader 2014-12-01 19:46:11 UTC
Have you had a chance to look at the patch that I submitted on Nov 14?
Comment 18 Albert Astals Cid 2014-12-01 19:48:16 UTC
Not really sorry, it's on my long list, i promise i'll get to it on the xmas break.
Comment 19 Albert Astals Cid 2015-01-02 18:32:41 UTC
I've come up with a simplified version that still achieves the same.

What do you think?
Comment 20 Albert Astals Cid 2015-01-02 18:33:34 UTC
Created attachment 111668 [details] [review]
Simplified version of the patch
Comment 21 Albert Astals Cid 2015-01-02 18:38:03 UTC
Created attachment 111669 [details]
Better simplified version of the patch
Comment 22 William Bader 2015-01-14 13:23:14 UTC
Created attachment 112215 [details] [review]
Updated better simplified version of the patch

I tested the "better simplified" patch with a 14 Jan 2015 git clone.
I manually applied a rejected hunk to pdftops.cc, and I changed the error message in pdftops.cc to say "Bad '-aaRaster' value" instead of "Bad '-aaVector' value".
Can you consider applying the updated patch?
Comment 23 Albert Astals Cid 2015-01-14 22:29:08 UTC
Pushed.
Comment 24 William Bader 2015-01-15 00:10:13 UTC
Thanks!
Comment 25 William Bader 2015-02-09 03:29:21 UTC
Created attachment 113264 [details] [review]
patch to update the man page

This patch adds a description of the -aaRaster option to the pdftops man page.
It also adds a description of the -overprint option.
Comment 26 Albert Astals Cid 2015-02-09 21:31:15 UTC
Pushed


Use of freedesktop.org services, including Bugzilla, is subject to our Code of Conduct. How we collect and use information is described in our Privacy Policy.