[poppler] [PATCH] Fix Splash back-end FT_Load_Glyph flags

[Benoit Jacob]

Hi,

Here on Opensuse 11.2 / KDE 4.3.1, Okular gave me very poor rendering
of PDF files. Example:
http://imagebin.ca/view/Z7jDxF.html

The attached patch fixes it so it now looks like this:
http://imagebin.ca/view/a_rIPp.html

For comparison, here's how it looks in Evince.
http://imagebin.ca/view/hoRjU3.html

As you can see, my fix makes Okular render this document exactly like
Evince does.

*** What it does ***

This patch changes the flags that are passed to FT_Load_Glyph.

First, notice that the Cairo back-end has a very simple logic for
that: it always passes

    FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP

By contrast, the Splash back-end used to have a much more complex
logic, that was buggy and self-inconsistent.

It was buggy because it did this (in the file splash/SplashFTFont.cc):

// if we have the FT2 bytecode interpreter, autohinting won't be used
#ifdef TT_CONFIG_OPTION_BYTECODE_INTERPRETER

the bug here is that this preprocessor define is irrelevant here. It
only tells you that FreeType was built with support for the bytecode
interpreter, but it doesn't tell that the bytecode interpreter is
enabled in the user's configuration. Here on openSUSE 11.2, this
define was defined but the bytecode interpreter was still disabled in
the default user config.

Notice however a very important fact here: it was actually the
intention of whoever wrote this code to disable hinting UNLESS the
bytecode interpreter is enabled. This means that my patch, which
disables hinting, honors the spirit of this code except perhaps in the
case where the bytecode interpreter is enabled; but that case never
was appropriately handled.

Replacing this #ifdef by just "#if 0" was enough to fix the rendering
glitches that I observed.

However, the code also was self-inconsistent, because the
determination of the flags was done at 3 different places in the code,
namely in SplashFTFont::makeGlyph(), in
SplashFTFont::getGlyphAdvance() and in SplashFTFont::getGlyphPath(),
and out of these 3 implementations, only the first two agreed, the 3rd
one was inconsistent with them.

To fix that, my patch introduces a method
SplashFTFont::getFTLoadFlags() that centralizes the computation of
these flags.

Here's its code:

FT_Int32 SplashFTFont::getFTLoadFlags() const
{
  return aa ? FT_LOAD_NO_HINTING | FT_LOAD_NO_BITMAP
            : FT_LOAD_DEFAULT;
}

As you can see, in the anti-aliasing case, it does the exact same
thing as the Cairo back-end.

I preserved the existing logic for the non-anti-aliasing case, as I
don't have an opinion on these matters. The logic of using
FT_LOAD_DEFAULT in that case is what was previously being done in the
Splash back-end and I just kept it without asking questions.

Cheers,
Benoit
-------------- next part --------------
A non-text attachment was scrubbed...
Name: Fix-FT_Load_Glyph-flags.patch
Type: text/x-patch
Size: 5368 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20090930/1a632a45/attachment.bin