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

[Benoit Jacob]

2009/9/30 Benoit Jacob <jacob.benoit.1 at gmail.com>:
> 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.

Hi again,

Out of curiosity, I just tested that.

The results are terrible: it is as if FT_LOAD_NO_AUTOHINT has no
effect at all. The results that I get are exactly the same as with
hinted text, and i'm sure that I haven't enabled the bytecode
interpreter so this can't be native hinting (and anyway this is the
default TeX font which I'm pretty sure is Type 1, not TrueType) so the
only possible conclusion is that FT_LOAD_NO_AUTOHINT has no effect and
auto-hinting is still being used.

Again, I don't think that you need to bother at all, my opinion is to
just use my patch from 1st e-mail. But if you really want to implement
the exact intended behavior of the original code, to let it use native
"bytecode" hinting when possible while disabling auto-hinting, the
solution is more complicated than I thought: you have to check if 1)
the configuration has the bytecode interpreter enabled and 2) the font
has bytecode. And again, for all the same reasons already stated, I
still don't expect this to be a good idea.

Benoit


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