prawnpdf / ttfunk

Font Metrics Parser for Prawn
http://prawnpdf.org
Other
125 stars 75 forks source link

Fix maxp table #78

Closed pointlessone closed 11 months ago

pointlessone commented 4 years ago

Original approach was right but there was an error in field sizes. This lead to shift of the following field. One of the fields is maxStackElements which defines stack size for font program. FreeFont (also used in Ghostscript) is very strict about stack size. Incorrect maxStackElements value lead to stack overflow in FreeFont. Consequently, Ghostscript couldn't parse the font and completely discarded it.

pointlessone commented 4 years ago

@camertron This is the same approach I took in 1.6.1 However, there I fixed only half of the issue: table encoding. The other half remained: parsing.

I kept you as the author for this commit as most of the code is yours. I only change 3 lines.

Also with revised table size we might not need check for "sometimes missing" max_component_depth. It's the last field of the table and naturally it would fall off since all fields would shift by 2 bytes. I assume, it would've been sometimes present because the following table was aligned to 4 bytes leaving 2 byte padding ad the end of maxp table.

Please take a look when you have time.

pointlessone commented 4 years ago

@mojavelinux Will you be able to test this branch with your ghostscript use case?

mojavelinux commented 4 years ago

I tested it and I can confirm there are no ghostscript warnings. Btw, the ghostscript use case is nothing more than running ghostscript on the generated PDF (something that could be done in CI, I suppose).

I'll note that I'm seeing a slight difference in output when using ttfunk 1.6 instead of 1.5. In the i18n character test, some of the characters are being swapped. Here's the sample document: https://github.com/asciidoctor/asciidoctor-pdf/blob/master/spec/fixtures/i18n-font-test.adoc The affected characters are wrong in both versions, just wrong in different ways. In some cases, a random glyph is being selected in place of the .notdef glyph. This happens based on what other characters are in the line. For example, \u01ea gets replaced with what looks like a "c".

mojavelinux commented 4 years ago

The affected characters are wrong in both versions

I meant to say this only affects characters with missing glyphs. Because the font is missing the glyph, the expected character doesn't appear. But it seems that in some cases it uses a (seemingly) random character instead of the .notdef glyph (usually a box).

pointlessone commented 4 years ago

@mojavelinux Thank you for testing.

I'm seeing a slight difference in output when using ttfunk 1.6 instead of 1.5. In the i18n character test, some of the characters are being swapped.

At first glance it seems to be a different issue but thank you for reporting.

i18n-font-test.adoc

Could you please submit a minimal reproduction script that doesn't use asciidoctor?

mojavelinux commented 4 years ago

At first glance it seems to be a different issue but thank you for reporting.

Yes, sorry for taking the conversation on track. It's likely something that I'm just now noticing. When I get a chance, I'll submit something that can be reproduced (once I narrow down what the trigger is).