You signed in with another tab or window. Reload to refresh your session.You signed out in another tab or window. Reload to refresh your session.You switched accounts on another tab or window. Reload to refresh your session.Dismiss alert
The "Don't generate FFTM tables" option was not only not working, its partial implementation was causing a segfault when user aksed not to include FFTM and generated a TTC file.
See #5508 and #4417
The issue was:
WriteTTC (tottf.c) called ttc_prep (tottf.c), which called ttf_fftm_dump (ttfspecial.c), and there the check "if ( at->gi.flags & ttf_flag_noFFTMtable )" succeeded, function returned false and no FFTM table was generated.
The line "tab->offset = ftell(ttc);" after "tab = findtabindir(&all[cnt].tabdir,CHR('F','F','T','M'));" in ttc_dump (tottf.c) caused a segfault, due to the pointer pointing at a place outside of the file.
There was, surprisingly, no FFTM check at all at all places where FFTM is mentionned in tottf.c, is to say in:
* ttc_perfonttables, called by ttc_dump (which is called by WriteTTC)
* buildtablestructures, called by:
** initTables (which is called by _WriteTTFFont)
** ttc_dump (two places) (which is called by WriteTTC)
* ttc_dump, called by WriteTTC
* ttc_prep, called by WriteTTC
* initTables, called by _WriteTTFFont
I fixed this issue by first propagating the flags already present in _WriteTTFFont and WriteTTC, down to initTables, ttc_dump, then to buildtablestructures, and performing the necessary check in ttc_perfonttables, buildtablestructures, ttc_dump, ttc_prep and initTables. This includes before calling ttf_fftm_dump, thus I moved the FFTM check outside of ttf_fftm_dump (we shouldn't get in there if no FFTM is to be generated) and reformed the function.
Since all of these FFTM-related stuff were entangled in _WriteTTFFont and WriteTTC, this patch should fix the issue I reported and also those related to FFTM being generated against users' will.
I tested generating TrueType, TrueType TTC, OpenType CID, all of them with FFTM checked and unchecked, Mac checked and unchecked, and everything seems to work.
Conversely, I could have accessed fftm option through at->gi.flags everywhere (exporting options are saved in glyphinfo?!?), but this seems less reliable than what's passed from the UI down to _WriteTTFFont and WriteTTC. That is, in both these functions there is an alltabs variable, but it is generated there, not passed down. And it is again re-generated in ttc_prep, I suppose from the output of all open fonts. At each of these regenerations of alltabs we'd have to make sure that the flags are copied properly.
P.S.: I did some additional testing on the repository's original code. When generating a TTC file, the value of at->gi.flags in ttf_fftm_dump does not change no matter if you check FFTM or not. It does change when you generate a single font, though. So maybe FontForge gets confused as to which opened font's glyphinfo flags has to be used; I think it picks up the last one, which does not guarantee that is where the user has initiated the generate TTC command (and changed the settings). So in both cases (TTC or single font), I think it's better to use the value that is passed down from _WriteTTFFont and WriteTTC.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
The "Don't generate FFTM tables" option was not only not working, its partial implementation was causing a segfault when user aksed not to include FFTM and generated a TTC file.
See #5508 and #4417
The issue was:
There was, surprisingly, no FFTM check at all at all places where FFTM is mentionned in tottf.c, is to say in:
* ttc_perfonttables, called by ttc_dump (which is called by WriteTTC)
* buildtablestructures, called by:
** initTables (which is called by _WriteTTFFont)
** ttc_dump (two places) (which is called by WriteTTC)
* ttc_dump, called by WriteTTC
* ttc_prep, called by WriteTTC
* initTables, called by _WriteTTFFont
I fixed this issue by first propagating the flags already present in _WriteTTFFont and WriteTTC, down to initTables, ttc_dump, then to buildtablestructures, and performing the necessary check in ttc_perfonttables, buildtablestructures, ttc_dump, ttc_prep and initTables. This includes before calling ttf_fftm_dump, thus I moved the FFTM check outside of ttf_fftm_dump (we shouldn't get in there if no FFTM is to be generated) and reformed the function.
Since all of these FFTM-related stuff were entangled in _WriteTTFFont and WriteTTC, this patch should fix the issue I reported and also those related to FFTM being generated against users' will.
I tested generating TrueType, TrueType TTC, OpenType CID, all of them with FFTM checked and unchecked, Mac checked and unchecked, and everything seems to work.