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

[Albert Astals Cid]

A Dissabte, 3 d'octubre de 2009, Albert Astals Cid va escriure:
> 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?

Forgot to mention we already have a way to disable autohinting provided by the 
noah flag, but it's probably wrong too since if no aa is set it passes 
FT_LOAD_DEFAULT that does autohinting.

I'll have a look at all that code and try to find a way so everything is 
configurable so everyone can pick his poison.

Albert

> 
> 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
> 
> _______________________________________________
> poppler mailing list
> poppler at lists.freedesktop.org
> http://lists.freedesktop.org/mailman/listinfo/poppler
>