Created attachment 26510 [details]
pdf with formular, on page2 bottom

Quoting the e-Mail I posted yesterday on poppler@lists.freedesktop.org:

I have several PDFs which consist of low-resolution scanned images.

These PDFs are rendered better and /much/ more readable in Adobe Reader
(e.g. Version 8.1.3) than in Okular or in other applications using
libpoppler version 0.10.5.

The reason appears to me to be a much better antialiasing technique in
Adobe Reader.


Once again, this report does NOT concern the rendering of fonts, but the
rendering of images!

I've uploaded an example file and a screenshot with explanations for you:

http://datenparkplatz.de/DiesUndDas/poppler.adobe.comparison.jpg
http://datenparkplatz.de/DiesUndDas/af2.pdf


I'm using Ubuntu Jaunty 9.04 AMD64.

This bug is genuinely a problem.  Tossing in my two cents.

Note that this is not anything special in Adobe - cairo just isn't doing any basic antialiasing at all.

Compare the image from Poppler-Evince to the same image (extracted from Evince's PDF) in the GNOME Image Viewer.

Created attachment 31093 [details]
description of poppler (cairo)'s pitiful image rendering

I apologize for using slides from class, but you can clearly see the image difference, even in JPG format - cairo's rendering is absolutely pitiful.

The lack of antialiasing means that many images aren't recognizable at all, even in fullscreen.

Congratulatios Jud, you totally hijacked a but not related to your problem, do you feel a better person now?

If you are not absolutely sure the problem is the same always open a different bug, and yours is obviously a different problem since Bernd is complaining about the Splash backend output and you are complaining about the Cairo backend output.

I did not realize.  I truly apologize.

Please forgive me.  The bug description looked nearly exact; I assumed Okular and Evince ran on the same engine; I missed the "component" line above.  A newbie mistake.

*** Bug 48513 has been marked as a duplicate of this bug. ***

https://bugs.freedesktop.org/show_bug.cgi?id=48513 has the testcase showing how text-as-image image quality in evince is worse compared to okular, even though both of them use poppler. Adobe actually produces even worse quality compared to okular/evince on this testcase.

Created attachment 59827 [details]
Hotfix for Okular low-resolution image rendering problems

Mr Lukas stop being useless.

How about switching from Bresenham to Lanczos for upscaling in splash/Splash.cc ?
scaleImageYdXu(), scaleImageYuXd() and scaleImageYuXu() could be replaced by a new scaleImageLanczos() function.

Stefan: are you volunteering to do so?

Created attachment 65962 [details] [review]
imagemagick patch

Created attachment 65963 [details]
the imagemagick rendering of Buschinski and Lukas' example pdfs

My approach was to use imagemagick (actually Magick++, the C++ wrapper) to scale all the bitmaps.

the good:
- optional dependency
- popular library, usually installed by default
- good resizing for soft masked images (using the "sample" method that doesn't introduce new colors in order to avoid a black halo)
- excellent resizing for the rest of the images (using the "resize" method that defaults to the Lanczos filter for upsizing). Now it's finally possible to read PDFs with scanned book pages and comics in okular.

the bad:
- we're adding a new dependency
- the rendering is slower. 1.42 times slower in pdftoppm for Buschinski's Uebung_DBV_Bildverbesserung_090320.pdf on my machine.

the ugly:
- I'm using a PNG intermediary when converting from SplashBitmap to Magick::Image instead of a more straightforward conversion.

Hmmm, i am not sure we want to add a dependency on ImageMagick in Splash

What do others think?

I personally don't like a dependency to imagemagick. But propably I don't have an open mind for imagemagick, I had some bad experiences with it a couple of years ago (nothing to do with raster image operations). But if it stays configurable and the default is "no", why not?
On the other hand I would prefer a splash internal solution. I.e. using nearest neighbour resampling for downscaling (I could provide that) and use a Lanczos filter for upsizing (I know the theory but have no clue how difficult it is to implement a performant algorithm). But because this hasn't a high priority for me and so I don't know when and if I have the time to do it, the imagemagick solution could be the way for those who look for a better quality right now.

Can you check if http://kdepepo.wordpress.com/2012/08/15/reverse-antialiasing-for-image-scaling/ is an option for upscaling? There is a test program linked from one of the comments. It can currently only double pixel size, but for any scaling factors != 2.0 it could be combined with bilinear interpolation.

Created attachment 66192 [details] [review]
imagemagick patch without gamma errors

Changes:
- ImageMagick support now disabled by default
- the gamma error associated with scaling has been corrected by doing the resize in a linear colorspace. Details:
http://www.4p8.com/eric.brasseur/gamma.html
http://www.imagemagick.org/Usage/resize/#resize_colorspace
- the intermediary PNG conversion was removed. SplashBitmap objects are converted to Magick::Image using the ImgWriter mechanism
- speed and memory usage improvements

Created attachment 66193 [details]
side by side comparison with gamma error example

This is a side by side comparison of the previous soft-masked formula and low-res page scan with an added test image with high contrast to show the effect of gamma errors during resize: http://eoimages.gsfc.nasa.gov/ve//1438/earth_lights_4800.tif

Created attachment 66245 [details] [review]
imagemagick patch without gamma errors

added the ImageMagick include dir to poppler.pc so cups-filters (which includes Splash.h) can be built against poppler with IM enabled

I'm a bit confused by your implementation of MagickWriter::init, why aren't you using the FILE * anywhere?

Even though it inherits from ImgWriter, MagickWriter does not write to a file. Its only purpose is to facilitate the conversion to ImageMagick's data type for use in Splash::bitmapToMagickImage() :

  writer = new MagickWriter();
  src->writeImgFile(writer, NULL, 0, 0);
  *dst = ((MagickWriter*)writer)->image;

This is how I was able to use writeImgFile() instead of duplicating the bitmap type dependent logic for reading. The result is retrieved from the custom "image" member of the MagickWriter object.

It is not acceptable to provide a class that doesn't do what it is expected to do. Please provide an alternate way of doing that.

It is not acceptable that you maintain code you don't understand. I give up trying to submit this patch.

Really? Why? Because i told you your code needs to be improved? That's a really low threshold of collaboration and commenting you're taking, can you explain why so low?

Sure, Albert, I'll explain it. You need 3 months to start reading my patch. When you start reading it, you start asking questions that show you can't be bothered to finish reading or that you don't understand what you read.

That's OK, questions are not a problem, but you go on asking for modifications that further show your incompetence. At this point you destroy the fine balance between feeling good for contributing to the community and feeling frustrated because my effort is met with hostility and ignorance.

So no, I don't have any more time and patience to take shit from you. Keep your line algorithm for bitmap scaling, I already run patched versions of poppler on my computers.

For the future, please refrain from asking other potential contributors for patches when you obviously don't want them. You'd save everybody precious time with an honest "go away".

This is simply (In reply to comment #27)
> Sure, Albert, I'll explain it. You need 3 months to start reading my patch.

Yes, and I am sorry that no one else takes responsability in the poppler community to review other people patches, given I do this on my free time I do stuff on an as fast as I can basis, that obviously is not enough, but unless we find the magial 30 hours a day it's the best I can do

> When you start reading it, you start asking questions that show you can't be
> bothered to finish reading or that you don't understand what you read.

I do understand what i read, i understand that you have implemented an ImgWriter that does not work as an ImgWriter and that's simply wrong.

And I won't bother answering the rest of personal insults.

FWIW this was planned to be included in next poppler release as you can see in http://freedesktop.org/wiki/Software/poppler?rev=52 (by mistake i wrote the name of the guy that created the bug instead of yours) but since you are not interested in helping anymore I've removed it from the list freedesktop.org/wiki/Software/poppler

Created attachment 70258 [details] [review]
Implement bilinear scaling

Here's a proof of concept of implementing bilinear scaling in Splash::scaleImageYuXu. Testing with the two pdfs test cases in this bug shows a big improvement. The quality is about the same as the cairo output (which also uses bilinear scaling). Performance is slightly slower than before but the patch has plenty of scope for optimizing (such as moving the SplashMode switch to work on the source row).

If you are interested in this I can continue to work on it.

Thanks for the patch Adrian, i'll try to have a look to squeeze it in before the 0.22 beta 2 this thursday.

Thomas, as Splash guru, can you give it a quick look too?

Created attachment 70301 [details] [review]
Implement bilinear scaling

Updated to fix a bug.

Created attachment 70302 [details] [review]
Implement bilinear scaling using fixed point

This version has been changed to used fixed point integers to see if I can make it faster. It barely made any difference. So take your pick on which version you prefer.

(In reply to comment #31)
> Thanks for the patch Adrian, i'll try to have a look to squeeze it in before
> the 0.22 beta 2 this thursday.
> 
> Thomas, as Splash guru, can you give it a quick look too?

I hadn't a look the algorithm itself and I didn't test the patch. You should see in Your regtest if it has the desired result. But what I can say is that it replaces the correct function: it is called only when there is an upscale in both dimensions (x and y), and hopefully here we get a better quality. So one hint for Your tests: You should render some PDF with images at least with 300 dpi. With 150 dpi this function in splash is seldom called, because most images inside Your PDF Suite have at least 150 dpi (as far as I remember).
And just to be complete: for downscaling an image it isn't necessary, there the Bresenham algorithm is well enough, so the patch should do what is expected.
So in my eyes the patch is okay!

Had a look at a lot of files on the regtest and while most look better there are at least 3 that look much worse when rendered with -r200 on pdftoppm than without the patch, listing them here

altona_technical_1v2_x3.pdf columns 4 and 5 get all blurry, this does not happen in acroread neither in the cairo outputdev

http://bugsfiles.kde.org/attachment.cgi?id=12083 page 12, there is a blocky graph that also gets all blurry, this does not happen in acroread neither in the cairo outputdev

File_that_i_will_attach page 7 bottom left image gets "destroyed", this happpens a bit on both acroread and cairooutputdev, but not as massive

I'm concerned about them all, but http://bugsfiles.kde.org/attachment.cgi?id=12083 seems like the most important one since the clear data shown by the graph is lost.

Any idea how to fix this?

Created attachment 70393 [details]
Page 7 of the file i wanted to attach since the whole file was too big

In cairo the function CairoOutputDev::getFilterForSurface selects between bilinear or nearest neighbour scaling depending on the interpolation flag and the scaling factor.

Fixing this should just be a case of putting the original spash code back (for nearest neighbour scaling) and adding a similar function to splash to switch select between my bilinear code or the splash nearest neighbour code.

Do you prefer using my floating point or fixed point patch?

Floating point.

Fabio was having some problem with the fixed point (he told me gave him some glitches in a file)

Created attachment 70423 [details] [review]
Implement bilinear image scaling in splash

Here's the updated patch that switches between bilinear and nearest depending on the interpolate flag and scale factor.

This fixes the regression in altona_technical_1v2_x3.pdf and  http://bugsfiles.kde.org/attachment.cgi?id=12083.

I have not been able to figure out how to fix https://bugs.freedesktop.org/attachment.cgi?id=70393. It seems to be caused by an smask with a transfer function. The transfer function messes up the smooth interpolation introduced by the bilinear scaling. The output is very similar to acroread so I'm not really sure what else I can do.

re floating vs. fixed point, there might be a bigger difference on a tablet or netbook than on a laptop.

Pushed!