fix: reject fonts with reserved bit 7 in glyf simple glyph flags#13144
fix: reject fonts with reserved bit 7 in glyf simple glyph flags#13144FurryR wants to merge 1 commit into
Conversation
DroidSansFallback.ttf (and Full variant) has reserved bit 7 set on 19959/19961 simple glyph coordinate flags. While ttf_parser tolerates this, skrifa/swash enforces the spec and rejects all outlines, causing CJK glyphs to render as .notdef squares. Reuse fontdb's mmap'd data to check for this corruption when loading file-based fonts. Corrupted fonts are removed from the database immediately so they cannot be matched by future font fallback queries. Non-corrupted fonts incur zero extra I/O. Signed-off-by: FurryR <awathefox@gmail.com>
|
I'm starting a first review of this pull request. You can view the conversation on Warp. I completed the review and no human review was requested for this pull request. Comment Powered by Oz |
There was a problem hiding this comment.
Overview
This PR adds a font-load-time check that removes file-backed fonts whose simple glyf flags use reserved bit 7 so CJK fallback can skip corrupted Linux fonts.
Concerns
- The corruption check only inspects the first face returned by fontdb, so font collections can still load a corrupted requested face when it is not first.
- The detector samples a small fixed set of glyphs and checks only the first flag byte in each sampled glyph, so it can miss fonts that still contain invalid simple-glyph flags.
- This is a user-visible rendering fix, but the PR description has no screenshot or recording demonstrating CJK rendering before/after as required for user-visible changes.
Verdict
Found: 0 critical, 3 important, 0 suggestions
Request changes
Comment /oz-review on this pull request to retrigger a review (up to 3 times on the same pull request).
Powered by Oz
| // Corrupted fonts are removed from the database immediately so they | ||
| // cannot be matched by future font fallback queries. | ||
| if let Some(ref path) = pre_loaded_path { | ||
| if let Some(&first_id) = fontdb_ids.first() { |
There was a problem hiding this comment.
load_font_source can return multiple faces for a collection, but this checks only first_id; if the requested face is later in the collection and corrupted, it remains loaded and can still be selected. Check each loaded face (or at least the requested index) and remove only the corrupted faces.
There was a problem hiding this comment.
Nice finding. I will fix it later! The final logic would be iterating through font faces until finding a valid one.
| if flag_offset >= offe || flag_offset >= glyf_data.len() { | ||
| continue; | ||
| } | ||
| if glyf_data[flag_offset] & 0x80 != 0 { |
There was a problem hiding this comment.
There was a problem hiding this comment.
If we parse full flag stream, the performance would be possibly degraded. Since the check already worked for the font, is it better to keep it?
Description
Reject fonts with reserved bit 7 set on simple glyph coordinate flags in the
glyftable. Some Linux fonts (notably DroidSansFallback.ttf fromttf-droid) have this corruption —skrifa/swashcorrectly enforces the spec and rejects all outlines, causing CJK glyphs to render as.notdefsquares.Detection runs at font-load time using
fontdb::with_face_data(reuses fontdb's existing mmap; zero extra I/O for healthy fonts). Corrupted faces are removed from the database so they can never be matched by future fallback queries.Linked Issue
ready-to-specorready-to-implement.Closes #9372.
Testing
./script/runManual verification:
has_reserved_bit7_in_glyph_flagsreturnstrueforDroidSansFallback.ttfandDroidSansFallbackFull.ttf,falseforDroidSansFallbackLegacy.ttf.DroidSansFallback.ttfrender as empty.We can actually use DroidSansFallback.ttf but it requires some hacky patching. If that is allowed then I will refactor this fix.
Agent Mode
Changelog Entries for Stable
CHANGELOG-BUG-FIX: Reject fonts with corrupted glyf table (reserved bit 7 on simple glyph flags) that caused CJK glyphs to render as .notdef squares on Linux