Fix TTF validation on load for fixed pitch fonts #5562
Conversation
| LogError(_("A point in %s is outside the font bounding box data.\n"), sc->name ); | ||
| info->bad_cff = true; | ||
| } | ||
| if ( info->isFixedPitch && i>2 && sc->width!=info->advanceWidthMax ) |
There was a problem hiding this comment.
What is the reason for the i>2 condition in the existing code? Are the first two glyphs special/ignorable for some reason?
There was a problem hiding this comment.
This is a very old code, so the first glyph is probably .notdef, and the second also used to have some special meaning back then, maybe \n. I tend to kill unexplained cryptic stuff, especially when it only affects a warning message.
| if ( info->isFixedPitch && i>2 && sc->width!=info->advanceWidthMax ) | ||
| LogError(_("The advance width of %s (%d) does not match the font's advanceWidthMax (%d) and this is a fixed pitch font\n"), | ||
| if ( info->isFixedPitch ) { | ||
| bool valid = (sc->width==0 || sc->width==info->advanceWidthMax || sc->width==info->advanceWidthMax / 2); |
There was a problem hiding this comment.
Is the sc->width==0 test here for the same reasons (hidden characters) -- and/or others? -- outlined in:
fontforge/fontforge/splinesaveafm.c
Lines 3064 to 3065 in 770356c
There was a problem hiding this comment.
(and either way, maybe worth adding a comment about the reason here?)
There was a problem hiding this comment.
Modern Unicode fonts tend to have lots of zero-width characters like control stuff, e.g. LRM or diacritics. These characters don't have an advance width and shouldn't affect pitch computation.
Good idea about comment.
jayaddison
left a comment
There was a problem hiding this comment.
@iorsh I approved this without testing it, but then thought today: I really should have applied the patch and checked the results first.
So, I applied this branch as a patch against Debian's packaged version of the 20230101 (1:20230101~dfsg-4, specifically), and rebuilt that. Unfortunately the warning messages continue to appear:
The advance width of space (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of exclam (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of quotedbl (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of numbersign (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of dollar (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of percent (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of ampersand (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of quotesingle (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of parenleft (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of parenright (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of asterisk (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of plus (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of comma (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
The advance width of hyphen (500) does not match the font's advanceWidthMax (1000) and this is a fixed pitch font
...
I'm not certain why they continue to appear despite the changes; I'll try to figure that out.
I think this may be a testing error by me; I think that although I ran the |
My apologies; yes, after an isolated rebuild of the package with this branch (at e9fa9a3) applied as a patch, the warnings have disappeared. The resulting output of the $ fontlint ~/Iosevka-Thin.ttf 2>&1 | more
Copyright (c) 2000-2024. See AUTHORS for Contributors.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
with many parts BSD <http://fontforge.org/license.html>. Please read LICENSE.
Version: 20230101
Based on sources from 2024-09-06 01:07 UTC-ML-D-GDK3.
PythonUI_Init()
copyUIMethodsToBaseTable()
Program root: /usr
Attempt to unget two characters
CHECKING /root/Iosevka-Thin.ttf
The following table(s) in the font have been ignored by FontForge
Ignoring 'DSIG' digital signature table
ERROR 2 Self-intersecting glyph
ERROR 5 Missing points at extrema
ERROR 9 Bad glyph name
FAIL /root/Iosevka-Thin.ttfThank you for the fix! |
When checking glyph width validity, consider zero, full width and double width for fixed-pitch fonts.
Type of change