Fix Arabic ligature bidi reordering in textkit#3407
Fix Arabic ligature bidi reordering in textkit#3407e-Naeim wants to merge 1 commit intodiegomura:masterfrom
Conversation
|
There was a problem hiding this comment.
Pull request overview
This PR fixes incorrect bidi reordering behavior in @react-pdf/textkit that could drop Arabic ligatures (when multiple logical clusters share the same glyph id) and could mis-assign glyph clusters to the wrong visual run in mixed bidi text.
Changes:
- Rebuilds reordered runs based on the source run that owns each visual character index, rather than slicing visual indices by original logical run boundaries.
- Dedupes ligature continuations by (owning run, source glyph-array index) instead of by
glyph.id, avoiding false dedupes when glyph ids repeat across clusters. - Adds regression tests for repeated ligature glyph ids and for Arabic text mixed with an LTR span.
Reviewed changes
Copilot reviewed 2 out of 2 changed files in this pull request and generated no comments.
| File | Description |
|---|---|
| packages/textkit/src/layout/bidiReordering.ts | Updates run reconstruction + ligature dedupe logic during bidi reordering to preserve repeated ligatures and keep clusters in the correct visual run. |
| packages/textkit/tests/layout/bidiReordering.test.ts | Adds targeted regression coverage for repeated ligature glyph ids and mixed bidi run ownership. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
Fixes #3406.
What changed
This updates textkit bidi reordering so reordered runs are built from the source run that owns each visual character index.
Ligature continuations are now deduped by source glyph-array index within the owning run, instead of by
glyph.id.Why
Arabic fonts can reuse the same ligature glyph id for different logical clusters. Cairo, for example, can emit the same
شرligature glyph id in separate words such asشراكةandشركات.The previous
addedGlyphs.has(glyph.id)logic treated the second logical cluster as a duplicate and omitted it. With mixed bidi text, slicing reordered visual indices by the original logical run boundaries can also assign glyphs to the wrong visual run.Examples
Reproduction text:
Before:
After:
Validation
bidiReorderingbehavior with both regression inputs.@react-pdf/rendererstack using Cairo to verify the visible Arabic output.I could not run the full upstream workspace locally on Windows because the repository contains an existing test fixture filename under
packages/image/tests/assetsthat cannot be checked out on NTFS. The PR branch itself is created directly againstmasterand only changespackages/textkit.