Allow hyphen and special characters in Feature File glyph names by iorsh · Pull Request #5358 · fontforge/fontforge

iorsh · 2024-01-29T20:08:20Z

This PR implements support of hyphens in glyph names, per OpenType Feature File Specification 2.f.i. Specifically, hyphen (-) and other special characters _.*+:^|~ must be supported in glyph names, and whenever hyphen is encountered, the implementation shall first attempt to find a glyph with the respective name, and only upon failure the implementation shall regard the hyphen as a range specifier.

Without hyphen support we incorrectly reject feature files for fonts with hyphens in glyph names (Iosevka being a one major example)

The entire issue of glyph names probably warrants some further discussion.

FontForge UI warns to use only alphanumerics, periods and underscores, but accepts everything
FontForge Python interface allows everything
Adobe Font Feature allows all the special characters in "development" mode, but not as the first character.
Microsoft OpenType specification allows only alphanumerics, period and underscore, anywhere in the name.

Type of change

Non-breaking change

iorsh · 2024-02-25T09:09:11Z

This PR mostly affects fea_ParseGlyphClass().

The case if ( tok->type==tk_char && tok->tokbuf[0]=='-' ) is functionally unaffected, only split into two subroutines.

The case if ( tok->type==tk_name ) is first tried as a possible range, then falls back to the existing flow if it's not a range.

skef · 2024-02-25T11:19:18Z

I know I'm supposed to be reviewing this, and I have looked at it. It seems a bit hairy and more risky than some of the other recent PRs and I want to go over it a few more times before I approve.

iorsh · 2024-02-25T11:22:31Z

I know I'm supposed to be reviewing this, and I have looked at it. It seems a bit hairy and more risky than some of the other recent PRs and I want to go over it a few more times before I approve.

I agree with your reserved approach, this stuff is risky indeed. I'll think about some application of bulk runner to get more confidence in it.

iorsh · 2024-02-25T19:36:32Z

Ran tests/bulk_test.sh with the following environment:

FF_PY_SCRIPT_LINE="\
f=fontforge.open(argv[1]);\
f.generateFeatureFile('Out.fea')"$'\n'"\
for l in f.gsub_lookups: f.removeLookup(l)"$'\n'"\
for l in f.gpos_lookups: f.removeLookup(l)"$'\n'"\
f.mergeFeature('Out.fea'); f.save(argv[2])"
FF_OUTPUT_FILE="Output.sfd"

Results (other than simple PASS):

./bulk_test.sh: line 56: 341824 Segmentation fault      (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 341829 Segmentation fault      (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
PASS: http://svn.savannah.gnu.org/viewvc/*checkout*/freefont/trunk/freefont/sfd/FreeSansOblique.sfd
./bulk_test.sh: line 56: 342269 Aborted                 (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 342274 Aborted                 (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
FAIL: https://github.com/marutan/riscos-free-fonts/raw/main/Fonts/Sassoon.Primary.Bold.sfd
./bulk_test.sh: line 56: 342526 Segmentation fault      (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 342533 Segmentation fault      (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
FAIL: https://github.com/fontforge/debugfonts/raw/master/jieub-bt-ps.sfd

Failures seem unrelated, as these are crashes on both system and project code, but I'll take a look anyway.

iorsh · 2024-03-15T12:25:25Z

./bulk_test.sh: line 56: 342269 Aborted                 (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 342274 Aborted                 (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
FAIL: https://github.com/marutan/riscos-free-fonts/raw/main/Fonts/Sassoon.Primary.Bold.sfd

This crash is unrelated, but there is a simple fix in 3c9c621 - we are releasing SplineChar object we don't own, and trying to do it multiple times because the font contains multiple duplicate kerning pairs applied to this char.

iorsh · 2024-03-15T19:11:58Z

./bulk_test.sh: line 56: 342526 Segmentation fault      (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 342533 Segmentation fault      (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
FAIL: https://github.com/fontforge/debugfonts/raw/master/jieub-bt-ps.sfd

Unrelated crash, fixed in 30389c7

iorsh · 2024-03-15T19:29:41Z

./bulk_test.sh: line 56: 341824 Segmentation fault      (core dumped) $FF_BIN_SYSTEM -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
./bulk_test.sh: line 56: 341829 Segmentation fault      (core dumped) $FF_BIN_PROJECT -c "$FF_PY_SCRIPT_LINE" $SFD_FILE $FF_OUTPUT_FILE > /dev/null 2>&1
PASS: http://svn.savannah.gnu.org/viewvc/*checkout*/freefont/trunk/freefont/sfd/FreeSansOblique.sfd

Unrelated crash - occurs both in master code and in the pull request.
FF crashes probably because of malformed FEA is applied back to the file, and then file is saved. I prefer not to fix the crash, as it would just occlude the underlying issue.

Stack:

libfontforge.so.4!SFD_DumpSplineFontMetadata(FILE * sfd, SplineFont * sf) (/home/iorsh/devel/fontforge/fontforge/sfd.c:2525)
libfontforge.so.4!SFD_Dump(FILE * sfd, SplineFont * sf, EncMap * map, EncMap * normal, int todir, char * dirname) (/home/iorsh/devel/fontforge/fontforge/sfd.c:2642)
libfontforge.so.4!SFDDump(FILE * sfd, SplineFont * sf, EncMap * map, EncMap * normal, int todir, char * dirname) (/home/iorsh/devel/fontforge/fontforge/sfd.c:2956)
libfontforge.so.4!SFDWrite(char * filename, SplineFont * sf, EncMap * map, EncMap * normal, int todir) (/home/iorsh/devel/fontforge/fontforge/sfd.c:3061)
libfontforge.so.4!SFDWriteBak(char * filename, SplineFont * sf, EncMap * map, EncMap * normal) (/home/iorsh/devel/fontforge/fontforge/sfd.c:3186)
libfontforge.so.4!SFDWriteBakExtended(char * locfilename, SplineFont * sf, EncMap * map, EncMap * normal, int s2d, int localRevisionsToRetain) (/home/iorsh/devel/fontforge/fontforge/sfd.c:3124)
libfontforge.so.4!PyFFFont_Save(PyFF_Font * self, PyObject * args) (/home/iorsh/devel/fontforge/fontforge/python.c:16717)

iorsh · 2024-03-15T19:30:18Z

For me, the bulk tests are clear now.

skef · 2024-10-06T22:32:34Z

@iorsh how thoroughly is the CID path tested? If it's been tested well I'm ready to approve this.

iorsh · 2024-10-15T23:16:36Z

Tested CID path with the following tests/bulk_test.sh environment:

FF_PY_SCRIPT_LINE="\
f=fontforge.open(argv[1]);\
f.cidConvertByCmap('/home/iorsh/devel/cmap-resources/Adobe-Identity-0/CMap/Identity-H')"$'\n'"\
f.generateFeatureFile('Out.fea')"$'\n'"\
for l in f.gsub_lookups: f.removeLookup(l)"$'\n'"\
for l in f.gpos_lookups: f.removeLookup(l)"$'\n'"\
f.mergeFeature('Out.fea'); f.save(argv[2])"
FF_OUTPUT_FILE="Output.sfd"

with some tweaks. All tests passed except for Mothanna.sfd, which crashes on both system and project with unrelated stack.

Tweaks:

Test script crashes FF nearly universally due to 27806fe; I had to apply the fix on the system to obtain meaningful comparison.
Must zeroize "CreationTime" and "ModificationTime" to suppress bogus diffs.

iorsh · 2024-11-10T12:35:57Z

@skef, I tested the CID path with the entire bulk test suite - see above. The results are ok, could you please merge?

iorsh added 7 commits January 26, 2024 18:04

Extract fea_AddGlyphRange()

76bc3ba

Add fea_glyphname_get_silent()

11c18f9

Split fea_AddGlyphRange() to CID / Name cases

2536702

Support hyphen and other characters in glyph names per spec

7a9c3b6

Fix comments

6478208

Add test test_fea_hyphens.py

d30e0aa

Merge branch 'master' into hyphen_names

ab7961b

iorsh force-pushed the hyphen_names branch from 5e7831e to ab7961b Compare February 24, 2024 20:09

Don't release unowned memory

3c9c621

iorsh added 3 commits March 15, 2024 19:27

Merge branch 'master' into hyphen_names

41d695a

Remove shadowed argument in SFScriptsInLookups()

3b77945

Check NULL value in dump_header_languagesystem()

30389c7

iorsh added 2 commits October 16, 2024 00:00

Merge branch 'master' into hyphen_names

1e0b554

Crash fix: fea_cid_validate() must return freshly allocated buffer

27806fe

skef approved these changes Nov 10, 2024

View reviewed changes

skef merged commit 42f3316 into fontforge:master Nov 10, 2024

iorsh deleted the hyphen_names branch March 28, 2025 07:40

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow hyphen and special characters in Feature File glyph names#5358

Allow hyphen and special characters in Feature File glyph names#5358
skef merged 13 commits intofontforge:masterfrom
iorsh:hyphen_names

iorsh commented Jan 29, 2024

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

skef commented Feb 25, 2024

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

skef commented Oct 6, 2024

Uh oh!

iorsh commented Oct 15, 2024

Uh oh!

iorsh commented Nov 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants

Conversation

iorsh commented Jan 29, 2024

Type of change

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

skef commented Feb 25, 2024

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

iorsh commented Feb 25, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

iorsh commented Mar 15, 2024

Uh oh!

skef commented Oct 6, 2024

Uh oh!

iorsh commented Oct 15, 2024

Uh oh!

iorsh commented Nov 10, 2024

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

2 participants