Repository navigation
Fix repha, pre-base vowel and dotted circle placement in USE - #400
Merged
Merged
Conversation
reorder() in the Universal Shaping Engine differed from HarfBuzz in three places: - The dotted circle of a broken cluster was inserted after the first non-repha glyph instead of before it, and shared that glyph's shaperInfo, so it was never treated as a base. - Moving a repha read glyphs[i] after splicing the array, which duplicated a glyph or inserted undefined, so a repha followed by a base (e.g. RA + virama + KA in Modi or Tirhuta) crashed or gave extra glyphs. Move it before the first post-base glyph, as HarfBuzz does. - Every part of a pre-base vowel split by a multiple substitution was moved before the base. Only move the first, like HarfBuzz and the Indic shaper.
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.
reorder()in the Universal Shaping Engine differs from HarfBuzz in three ways that depend on each other: once the dotted circle of a broken cluster is a base, broken clusters with a repha or a split vowel go through the other two.Dotted circle. It's inserted after the first non-repha glyph instead of before it, so a lone vowel sign comes out as the sign followed by the circle:
The circle also shared the first glyph's
shaperInfo, so it never counted as a base. It now gets its own (GB, the category of U+25CC), like in the Indic shaper, and goes before the first non-repha glyph, like in HarfBuzz'shb_syllabic_insert_dotted_circles.Repha.
glyphs.splice(start, 0, ...glyphs.splice(start + 1, i - start), glyphs[i])readsglyphs[i]after the inner splice has shortened the array, so it inserts a duplicate glyph orundefined. Any repha followed by a base hits it: RA + virama + KA in Modi or Tirhuta throwsCannot read properties of undefined (reading 'shaperInfo'), and with a vowel sign after the base that sign is duplicated. This moves the repha before the first post-base glyph, or to the end, as HarfBuzz'sreorder_syllable_usedoes. Fixing only the splice and keeping the old "after the first base" rule still puts the repha in the wrong place when a nukta or a conjunct follows the base.Split pre-base vowels. When a font splits a pre-base vowel sign with a multiple substitution, every part is moved before the base, so in Noto Sans Siddham KA + vowel sign AI starts with the above-base part. HarfBuzz only moves the first component, and the Indic shaper already checks
isMultipliedfor this.Tests, with HarfBuzz's output as the expected results: dotted circles with the Balinese font in
test/data, repha cases with Noto Sans Tirhuta and the split vowel with Noto Sans Siddham (both OFL, from google/fonts, with their licenses next to them). Five of the six fail onmaster. The sixth, a lone pre-base vowel sign, already passes and is there to keep it working.I also compared about 16,000 strings with 196 fonts against HarfBuzz with and without this change, including every combining mark of each font on its own:
One interaction: in Noto Sans Siddham the dotted circle has NULL anchors for some mark classes. Now that lone vowel signs attach to the circle, 21 of those strings reach the NULL anchor crash from #367. With #374 applied as well nothing crashes, so it's best to merge #374 first.
npm testpasses and the changed files are Prettier-clean.