Support fonts that use MarkGlyphSets - #1
Merged
Conversation
A GSUB lookup can set the UseMarkFilteringSet flag (0x0010) to ignore every mark except those in a GDEF mark glyph set. mPDF parses those sets already, and _checkGSUBignore()/_checkGCOMignore() consult them, but the two functions that build the ignore pattern threw instead — in both cases directly above a working implementation left unreachable and commented "Not tested yet". Drop the two throws so the existing code runs, keeping a guard for a filtering set GDEF does not define. Fonts built by fontmake use the flag routinely: every current Noto Sans Sinhala release is rejected today. tests/data/ttf/NotoSansSinhala-Subset.ttf is Noto Sans Sinhala (OFL 1.1, no reserved font name) subsetted to the sample text, 7.5 KB, keeping 7 GSUB lookups that carry the flag. Both new tests fail without the change, and each covers one of the two throws.
jakejackson1
force-pushed
the
fix/mark-glyph-sets
branch
from
September 7, 2026 00:00
9958730 to
e863edc
Compare
jakejackson1
force-pushed
the
gravitypdf
branch
from
September 7, 2026 00:00
2c544d1 to
389e19e
Compare
6 tasks
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
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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.
Summary
mPDF rejects any font whose GSUB lookups use a mark filtering set. That takes out every current release of Noto Sans Sinhala, among others, at any non-zero
useOTL— the exception isFont "..." contains MarkGlyphSets which is not supported.The support is already written. A GSUB lookup can set the
UseMarkFilteringSetflag (0x0010) to ignore every mark except those in a GDEF mark glyph set; mPDF parses those sets when it reads GDEF, and bothTTFontFile::_checkGSUBignore()andOtl::_checkGCOMignore()consult them. Only the two functions that build the ignore pattern refuse — and in both, the throw sits directly above a working implementation the author left unreachable, commented "Not tested yet" and "Change also in ttfontsuni.php".This drops those two throws so the existing code runs, keeping a guard for the one case that is genuinely broken: a filtering set GDEF does not define. Four lines of
src/.This was originally committed to the Font Packages PR, where it does not belong — it is independent of font packages. It is split out here.
Try it
On
gravitypdf-7.0that throws. On this branch it renders, and theශ්රීconjunct matches what HarfBuzz produces for the same font and string.Test plan
composer test— 1000 tests, 2291 assertions, green.composer cs— clean.src/change, and they fail on different throws: reverting onlyOtl.phpleavesTTFontFileTest::testGetMetricsWithMarkGlyphSetspassing andMarkGlyphSetsTestfailing. Neither change is redundant.More info
The two call sites
TTFontFile::_getGSUBignoreString()(src/TTFontFile.php)throw new FontException('... contains MarkGlyphSets which is not supported'), with$str = $this->MarkGlyphSets[$MarkFilteringSet];unreachable beneath it$str, throwing only if GDEF defines no such setOtl::_getGCOMignoreString()(src/Otl.php)throw new MpdfException('... contains MarkGlyphSets - Not tested yet'), with a$MarkFilteringSet == ''guard and the assignment unreachable beneath it$str$ignoreflagis dead in both functions — it survives only in the commented-out[IGNORE$ignoreflag]scheme the author never finished — so the assignment to$stris the whole behaviour.Why this shows up now
fontmakesets the flag routinely, so it is not a corner case: it is most current Google/Noto releases with mark positioning. The existingtests/data/ttf/NotoSans-Regular.ttffixture does define four mark glyph sets, but only its GPOS lookups reference them, and neither throw is on a GPOS path — which is why the suite has been green while the bug was live.The fixture
tests/data/ttf/NotoSansSinhala-Subset.ttfis Noto Sans Sinhala (OFL 1.1, no reserved font name) subsetted withfontToolsto the sample text with--layout-features='*'. 7,656 bytes, 32 glyphs, retaining 2 mark glyph sets and 7 GSUB lookups that carry the flag — an order of magnitude smaller than the other fixtures in that directory.