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

[Albert Astals Cid]

A Dimecres, 30 de setembre de 2009, Benoit Jacob va escriure:
> Further remark.
> 
> If /really/ you want to keep the idea that you want to use the font's
> native hinting when possible and only disable auto-hinting, I just
> found that there is FT_LOAD_NO_AUTOHINT that does just that. So if you
> want to do that, just edit my function getFTLoadFlags like this:
> 
> FT_Int32 SplashFTFont::getFTLoadFlags() const
> {
>  return aa ? FT_LOAD_NO_AUTOHINT | FT_LOAD_NO_BITMAP
> 
>            : FT_LOAD_DEFAULT;
> 
> }
> 
> I'm not saying that that's a good idea. I just mention it FYI.
> 
> However, before you do that, please check with the authors of the
> Cairo back-end as there may be a good reason why they /don't/ do that.
> 
> I don't have a PDF file with fonts that do native hinting, so I can't test.
> 
> My subjective opinion is : don't bother at all, as hinting seems
> always a bad idea to me in a PDF viewer; however I thought that I'd
> mention that so you know how to implement that if that's what you
> really want.

The code has been there for ages, xpdf has it, so it's none of use that has 
implemented, actually the question should be the reverse, why didn't the Cairo 
backend devels ask us about the flags we used?

Albert

> 
> Benoit
> 
> 2009/9/30 Benoit Jacob <jacob.benoit.1 at gmail.com>:
> > 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
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>