Choose correct shaper and avoid infinite loop #478
Merged
Conversation
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
There was a problem hiding this comment.
Pull request overview
This pull request fixes issue #475 by improving script detection and fallback handling in OpenType GPOS/GSUB tables to prevent infinite loops and exceptions when rendering text in scripts not supported by the font. The main changes introduce a ScriptClass.Default fallback mechanism that gracefully handles unsupported scripts.
Key Changes:
- Introduced
ScriptClass.Defaultas a fallback when fonts don't support a script, preventing exceptions and infinite loops - Added
GetScriptClassmethod in GSubTable and GPosTable to determine script support and return Default when not present - Fixed a bug in
GlyphPositioningCollection.TryUpdatewhere fallback glyph removal logic was using the wrong variable
Reviewed changes
Copilot reviewed 10 out of 14 changed files in this pull request and generated 1 comment.
Show a summary per file
| File | Description |
|---|---|
| src/SixLabors.Fonts/Unicode/ScriptClass.cs | Added ScriptClass.Default enum value (999) to represent fallback script |
| src/SixLabors.Fonts/Tables/AdvancedTypographic/UnicodeScriptTagMap.cs | Added mappings for Default script to DFLT/dflt/latn tags; simplified Lazy initialization |
| src/SixLabors.Fonts/Tables/AdvancedTypographic/GSubTable.cs | Added GetScriptClass method for script fallback; updated to use modern collection initializers |
| src/SixLabors.Fonts/Tables/AdvancedTypographic/GPosTable.cs | Added GetScriptClass method for script fallback; updated to use modern collection initializers |
| src/SixLabors.Fonts/Tables/AdvancedTypographic/GSub/LookupType2SubTable.cs | Made class sealed; updated variable declarations to use explicit types |
| src/SixLabors.Fonts/GlyphPositioningCollection.cs | Fixed bug: replaced j == 0 with replacementCount == 0 for correct fallback glyph removal |
| tests/SixLabors.Fonts.Tests/TestFonts.cs | Renamed NotoSansSCThinFile to NotoSansSCThinBad and added new NotoSansSCThin property for clarity |
| tests/SixLabors.Fonts.Tests/Issues/Issues_475.cs | Added regression test verifying no exceptions with unsupported scripts and correct glyph count |
| tests/SixLabors.Fonts.Tests/Issues/Issues_469.cs | Updated to use correct NotoSansSCThin font reference |
| tests/SixLabors.Fonts.Tests/FontMetricsTests.cs | Updated to use renamed NotoSansSCThinBad font |
| tests/SixLabors.Fonts.Tests/Fonts/NotoSansSC-Thin-Bad.ttf | Font file renamed from NotoSansSC-Thin.ttf for clarity |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
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.
Prerequisites
Description
Fixes #475
This pull request introduces several improvements and bug fixes to the advanced typographic shaping logic and font test infrastructure. The main focus is on more robust script detection and fallback handling in the OpenType GPOS/GSUB tables, as well as enhancements to test coverage and test font management.
Advanced Typographic Shaping Improvements:
ScriptClass.Defaultvalue to represent a fallback/default script, and updated theUnicodeScriptTagMapto include appropriate tags for this default. This allows the shaping engine to gracefully fall back to a default shaper if a script is not present in the font. [1] [2]GetScriptClassmethod in bothGSubTableandGPosTableto determine whether the current script is supported by the font, and to returnDefaultif not. Updated relevant shaping logic to use this method, improving script fallback handling and preventing incorrect shaping when text contains unsupported scripts. [1] [2] [3] [4] Fb617cd5L319R319, [5][]). [1] [2] [3] [4] [5] [6]Bug Fixes and Logic Corrections:
GlyphPositioningCollection.TryUpdatewhere the check for removing fallback glyphs now correctly usesreplacementCount == 0instead ofj == 0.LookupType2Format1SubTablea sealed class and improved variable declarations for clarity and consistency. [1] [2] [3]Test Infrastructure and Coverage:
Issues_475) to verify correct glyph counting and fallback handling for scripts not supported by the provided fonts, ensuring no exceptions are thrown and the correct glyph count is returned.NotoSansSCThinBadtest font where appropriate, and fixed a reference in an issue test to use the correct font. [1] [2] [3]These changes collectively improve the robustness of script detection and fallback in font shaping, clarify test font usage, and expand test coverage for edge cases.