fix(rio-grid): prevent RTL glyphs from collapsing into one cell - #1929
Open
SagiMedina wants to merge 1 commit into
Open
SagiMedina wants to merge 1 commit into
SagiMedina wants to merge 1 commit into
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
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.
RTL glyphs can collapse onto one terminal cell because the grid emitter only advances its cluster-to-cell cursor. For example, source clusters
[3, 2, 1, 0]currently produce cell indices[3, 3, 3, 3]. This change maps them to[3, 2, 1, 0], retaining each glyph's logical terminal cell and its foreground color.The mapping keeps the existing forward walk for ascending clusters and uses a binary search when a glyph belongs to an earlier cell. It supports direction changes within a run without classifying the whole run by its first and last clusters. Both platform offset conventions (UTF-16 on macOS, UTF-8 elsewhere) and the existing wide-cell column mapping are preserved.
This fixes cell attribution for RTL scripts, including Hebrew, Arabic, and Persian. It preserves Rio's logical cell order; it does not implement Unicode BiDi visual reordering, proportional RTL layout, or new combining-mark positioning. Those need coordinated cursor, selection, and mouse mappings.
Related: #1877, which identified the same forward-only mapping bug. This is a narrower alternative that retains terminal cell positions. The broader BiDi feature request #1257 remains open.
Validation on macOS arm64 with Rust 1.96.1:
rio-gridtests pass. Coverage includes RTL and mixed-direction clusters, UTF-8/UTF-16 offsets, marks and ligatures, actual Hebrew/Arabic/Persian shaping, and row emission into a CPU atlas with per-cell colors and cursor/selection run breaks.cargo test --features wgpu --lockedpasses across the full workspace (1374 tests passed, 11 ignored, including doc tests).cargo fmt --all -- --checkandcargo clippy --all-targets --all-features --lockedpass.Interactive GPU rendering and Android/tmux handoff have not been manually tested with this patch.