Bug 85934

Created attachment 108985 [details]
screen capture before and after anti-aliasing

(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?

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.

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.

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.

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.

no need for new paramaters, just settings and getters.

(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.

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?

kill globalparams. If outputdev works, do it :)

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.

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?

Do we really need the currentVectorAntialias one?

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?

Created attachment 109482 [details] [review]
revised patches that do not create currentVectorAntialias

Created attachment 109484 [details] [review]
revised patch the removes extra variables

Have you had a chance to look at the patch that I submitted on Nov 14?

Not really sorry, it's on my long list, i promise i'll get to it on the xmas break.

I've come up with a simplified version that still achieves the same.

What do you think?

Created attachment 111668 [details] [review]
Simplified version of the patch

Created attachment 111669 [details]
Better simplified version of the patch

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?

Pushed.

Thanks!

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.

Pushed