Repository navigation
Stop morx state machines that never advance (#175) - #428
Closed
iliev-blnk wants to merge 1 commit into
Closed
iliev-blnk wants to merge 1 commit into
iliev-blnk wants to merge 1 commit into
Conversation
Transitions with DontAdvance now use up a budget of max(4096 * glyphs, 65536) steps per glyph run, as in HarfBuzz. Once it runs out, the machine advances anyway, so a font that cycles through substitutions without advancing no longer hangs layout. The test font is TestMORXTwentyfour.ttf (MORX-24) from Unicode text-rendering-tests, under the Apache License 2.0 in test/data/fonttest/LICENSE.txt.
This was referenced Oct 28, 2026
Member
|
Sorry. I merged other PR first |
Contributor
Author
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.
Fixes #175
Cause
AATStateMachine.processadvances only when an entry has no DontAdvance flag. A font whose transitions all carry DontAdvance keeps the machine on the same glyph forever, sofont.layout()never returns. Test case MORX-24 does this with a contextual subtable that substitutes A → B → C → D → E → A.Fix
Like HarfBuzz, each glyph run gets a budget of
max(4096 * glyphs, 65536)steps, the same values asHB_BUFFER_MAX_OPS_FACTORandHB_BUFFER_MAX_OPS_MIN. A transition that doesn't advance uses up one step. Once the budget runs out, the machine advances anyway. The budget is shared by all subtables in the run. Fonts that use DontAdvance as intended spend only a few steps of it.The cost is still proportional to the budget: 5 glyphs of MORX-24 now finish in about 20 ms, and 10,000 glyphs take a few seconds instead of hanging.
Tests
test/issues.js: MORX-24 lays outABCDEand returns 5 glyphs. On master, the test hangs.TestMORXTwentyfour.ttffrom Unicode text-rendering-tests, added totest/data/fonttest/next to the other fonts from there, under the Apache License 2.0 intest/data/fonttest/LICENSE.txt.npm test: 704 passing.npm run format:checkpasses.