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

[Albert Astals Cid]

A Dimecres, 30 de setembre de 2009, Benoit Jacob va escriure:
> 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.

Actually it does not, see the attached images, but i agree they look much 
closer now ;-)

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

Interesting findings, have you tried that with the autohinter enabled?

Albert

> 
> Cheers,
> Benoit
> 

-------------- next part --------------
A non-text attachment was scrubbed...
Name: okular-zoom.png
Type: image/png
Size: 2487 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20091003/0f2817e8/attachment.png 
-------------- next part --------------
A non-text attachment was scrubbed...
Name: evince-zoom.png
Type: image/png
Size: 2544 bytes
Desc: not available
Url : http://lists.freedesktop.org/archives/poppler/attachments/20091003/0f2817e8/attachment-0001.png