Font incorrectly rendered

[GoogleCodeExporter]

The font in the attached sample renders incorrectly with Libass.

Original issue reported on code.google.com by cyber.sp...@gmail.com on 3 Jul 2013 at 12:02

Attachments:

• font_error.mkv
• [28 Days Later.ttf](https://storage.googleapis.com/google-code-attachments/libass/issue-106/comment-0/28 Days Later.ttf)

[GoogleCodeExporter]

Probably a freetype issue? Report to the libfreetype project.

Original comment by nfxjfg@googlemail.com on 3 Jul 2013 at 8:17

[GoogleCodeExporter]

Reported to Freetype here:
https://savannah.nongnu.org/bugs/index.php?39408

Original comment by cyber.sp...@gmail.com on 3 Jul 2013 at 3:28

[GoogleCodeExporter]

So just the size is wrong? I thought it was maybe something about the effect 
the font was trying to produce.

It appears with ftstring (freetype demo that serves as small test utility) 
seems to render the font at the right size.

Anyway, no clue what this could be causing.

Original comment by nfxjfg@googlemail.com on 3 Jul 2013 at 3:57

[GoogleCodeExporter]

Yes, it seems as if the size & scale of the font is being interpreted 
completely wrong. This also causes strange things to happen with the 
positioning. A quick test shows I need to use {\fscx2048\fscy2048} with Libass 
to get the roughly the same size as VSFilter {\fscx100\fscy100} with this font.

Original comment by cyber.sp...@gmail.com on 3 Jul 2013 at 4:13

[GoogleCodeExporter]

Freetype closed the bug as "invalid" with the following comment:

> My tests with ftview show nothing special; everything looks OK.
>
> Note that the font uses an uncommon unitsPerEm value (4096); 
> I suspect that libass doesn't handle this correctly.

Original comment by cyber.sp...@gmail.com on 3 Jul 2013 at 11:15

[GoogleCodeExporter]

I tried changing the Em size from 4096 to 2048 in FontForge, but unfortunately 
that didn't resolve the issue.

Original comment by cyber.sp...@gmail.com on 3 Jul 2013 at 11:39

[GoogleCodeExporter]

After messing around in FontForge some more, the problem actually seems to be 
the negative OS/2 "WinDescent" value, which after a bit of research seems to be 
technically allowed, but very uncommon and strongly not recommended. The glyphs 
in this font actually do reside a ways _above_ the baseline, hence the negative 
value. The font seems to be valid, but definitely a bit strange in how it was 
authored.

Changing "WinDescent" to positive seems to workaround this bug, but that would 
seem to technically make the metrics of this font invalid, even though it seems 
to work anyway.

Original comment by cyber.sp...@gmail.com on 4 Jul 2013 at 12:21

[GoogleCodeExporter]

I've confirmed that this Libass bug occurs with all fonts containing a negative 
OS/2 "WinDescent" value, not only the attached font.

Original comment by cyber.sp...@gmail.com on 4 Jul 2013 at 5:20

[GoogleCodeExporter]

>Freetype closed the bug as "invalid" with the following comment:

OK. Sorry for leading you into the wrong direction and thanks for figuring out 
the cause.

Original comment by nfxjfg@googlemail.com on 4 Jul 2013 at 11:08

[GoogleCodeExporter]

Here's the thing: WinDescent is an *unsigned* short. So it cannot really have 
negative values. According to Microsoft's ttfdump utility, WinDescent is 65195. 
That's unusuably big and can't be right. Possibly GDI implements workarounds 
for buggy fonts that stuff signed, negative values into these fields. We just 
need to interpret the value as signed and flip the sign in case it's negative. 
In this particular case, the signed interpretation is -341, and sign flipped 
it's 341. Which is reasonably and probably correct.

Original comment by g...@chown.ath.cx on 5 Jul 2013 at 5:18

[GoogleCodeExporter]

The attached patch should fix the problem. Please test.

Original comment by g...@chown.ath.cx on 5 Jul 2013 at 5:44

Attachments:

• 0001-Fix-OS-2-usWinDescent-for-quirky-fonts.patch

[GoogleCodeExporter]

I see now that the specification does list WinDescent as USHORT.

> usWinDescent
> Format: 2-byte USHORT
> The descender metric for Windows. For platform 3 encoding 0 fonts, it is the 
same as -yMin.
> Windows will clip the bitmap of any portion of a glyph that appears below 
this value. Some 
> applications use this value to determine default line spacing. This is 
strongly discouraged. The 
> typographic ascender, descender and line gap fields in conjunction with 
unitsPerEm should be used 
> for this purpose. Developers should set this field keeping the above factors 
in mind.
> If any clipping is unacceptable, then the value should be set to yMin.
> However, if a developer desires to provide appropriate default line spacing 
using this field, for 
> those applications that continue to use this field for doing so (against OFF 
recommendations), then 
> the value should be set appropriately. In such a case, it may result in some 
glyph bitmaps
> being clipped.

The problem is that the glyphs of this font really do reside 341 _above_ the 
baseline. So even if it violates the specification, cropping the baseline 
upwards is actually the intended behavior for this font. WinDescent is supposed 
to represent the maximum distance which glyphs of the font reside below the 
baseline. Since positive numbers represent distance below the baseline, 
negative numbers would represent distance above the baseline.

The Windows 7 font viewer for example, crops the font view upwards from the 
baseline with negative WinDescent, while positive WinDescent increases the view 
from the bottom. This seems to suggest that Microsoft ignores the specification 
and treats WinDescent as a signed number to deal with quirky fonts properly.

Freetype by comparison, seems to use the hhea table Descender (defined as 
signed short by OpenType specification) for decent, and only uses WinDescent as 
a last resort:
http://git.savannah.gnu.org/cgit/freetype/freetype2.git/commit/src/sfnt/sfobjs.c
?id=f095744d2db48eddc79bc744c3286e26ebd2c694

        /*      usWinAscent/Descent values -- as a conclusion, the OS/2   */
        /*      table cannot be used to compute the text height reliably! */

        /* The ascender and descender are taken from the `hhea' table. */
        /* If zero, they are taken from the `OS/2' table.              */

The hhea Descender of this font states the same thing as negative WinDescent 
value. The bottom of the lowest glyph is 341 _above_ the baseline. If you are 
not going to allow use of negative WinDescent numbers (positive hhea Descender) 
like Windows does, I think it would be better to treat WinDescent values with 
an incorrect sign as 0, rather than flipping the sign. That way you'd at least 
be half-way correct, instead of completely wrong with such fonts.

Original comment by cyber.sp...@gmail.com on 5 Jul 2013 at 9:34

[GoogleCodeExporter]

> The problem is that the glyphs of this font really do reside 341 _above_ the 
baseline. So even if it violates the specification, cropping the baseline 
upwards is actually the intended behavior for this font.

OK. So be it, then let's just interpret usWinDescent/usWinAscent as signed 
numbers. It's unlikely to have any side effects.

> Freetype by comparison, seems to use the hhea table Descender (defined as 
signed short by OpenType specification) for decent, and only uses WinDescent as 
a last resort

Well, libass uses values from both hhea and os2 to determine font size, and 
prefers os2 for ascender/descender. This seems to be the right thing to do to 
achieve GDI compatibility.

Original comment by g...@chown.ath.cx on 5 Jul 2013 at 10:55

[GoogleCodeExporter]

This very simple patch should actually do the right thing.

Original comment by g...@chown.ath.cx on 5 Jul 2013 at 11:18

Attachments:

• 0001-Fix-OS-2-usWinDescent-usWinAscent-for-quirky-fonts.patch

[GoogleCodeExporter]

> This very simple patch should actually do the right thing.
> 0001-Fix-OS-2-usWinDescent-usWinAscent-for-quirky-fonts.patch 

I've tested and confirmed this patch resolves the issue with my sample.

Original comment by cyber.sp...@gmail.com on 6 Jul 2013 at 12:05

Attachments:

• libass_patched_mpv.png
• XySubFilter_madVR.png

[GoogleCodeExporter]

Pushed.

Original comment by g...@chown.ath.cx on 6 Jul 2013 at 12:12

• Changed state: Fixed

[GoogleCodeExporter]

Issue 108 has been merged into this issue.

Original comment by nfxjfg@googlemail.com on 26 Jul 2013 at 6:04