[prev in list] [next in list] [prev in thread] [next in thread]
List: freedesktop-poppler
Subject: Re: [poppler] [PATCH] Fix Splash back-end FT_Load_Glyph flags
From: Albert Astals Cid <aacid () kde ! org>
Date: 2009-10-02 22:23:16
Message-ID: 200910030023.16722.aacid () kde ! org
[Download RAW message or body]
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
>
["okular-zoom.png" (image/png)]
["evince-zoom.png" (image/png)]
_______________________________________________
poppler mailing list
poppler@lists.freedesktop.org
http://lists.freedesktop.org/mailman/listinfo/poppler
[prev in list] [next in list] [prev in thread] [next in thread]
Configure |
About |
News |
Add a list |
Sponsored by KoreLogic