Fix Arabic / RTL text rendering in terminal output Closes #10355#10809
Fix Arabic / RTL text rendering in terminal output Closes #10355#10809BSN4 wants to merge 1 commit into
Conversation
|
Thank you for your pull request and welcome to our community. We require contributors to sign our Contributor License Agreement, and we don't seem to have the users @BSN4 on file. In order for us to review and merge your code, each contributor must visit https://cla.warp.dev to read and agree to our CLA. Once you have done so, please comment |
|
This PR is not linked to an issue that is marked with Issue-state enforcement details:
Readiness check:
To continue, link this PR to a same-repo issue such as Powered by Oz |
![oz-for-oss[bot]](https://avatars.githubusercontent.com/in/3450139?s=60&v=4){kind=small}
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-implement.
Issue-state enforcement details:
-
Associated same-repo issues checked: none
-
Required readiness label:
ready-to-implement
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-implement.
Powered by Oz
github-actions
Bot
added
the
external-contributor
BSN4
changed the title
|
@cla-bot check |
|
The cla-bot has been summoned, and re-checked this pull request! |
|
/oz-review |
There was a problem hiding this comment.
This PR is not linked to an issue that is marked with ready-to-implement.
Issue-state enforcement details:
-
Associated same-repo issues checked: #10355
-
Required readiness label:
ready-to-implement
Readiness check:
- #10355: missing
ready-to-implement; readiness labels present: none
To continue, link this PR to a same-repo issue such as Closes #123 in the PR description, and make sure that issue has ready-to-implement.
Powered by Oz
|
Hi @BSN4 — a reviewer requested changes on this PR and it hasn't had activity from you in 23 days. When you get a chance, please push updates or reply to the review so a reviewer can take another look. Without activity, this PR will be automatically closed after 30 days of inactivity. |
Terminal output rendered Arabic (and other complex / RTL scripts) left to right with isolated letter forms, while the input editor rendered the same text correctly. The two paths diverged at the painter, not at the shaper. The input editor renders through `TextLayoutSystem::layout_text`, which runs `BidiParagraphs` + `ShapeLine` with `Shaping::Advanced`, so bidi reorder and rustybuzz Arabic shaping both apply. The terminal grid had two paths, and both broke that: * Without ligatures (default), `render_grid_without_ligatures` walks cells one by one and calls `glyph_for_char` per codepoint — no shaping, no bidi, single isolated glyph per cell. * With ligatures, `render_grid_with_ligatures` does call `layout_line`, but `paint_line` then discarded the shaper's `glyph.x` and forced every glyph back to `original_logical_column * cell_width`. That undid the bidi reorder and disconnected joined glyphs. This change keeps the existing fast cell-by-cell path for plain LTR rows and only diverts rows that need bidi + shaping: * Add `is_complex_script_char` covering Arabic (all blocks), Hebrew, Syriac, Thaana, NKo, and Arabic Presentation Forms A/B. * Add `row_needs_complex_layout` (row predicate) and `frame_contains_complex_script` (frame pre-scan, short-circuits). * When the frame contains any complex-script char, the dispatcher forces the ligature path so RTL rows reach `layout_line`. * Inside `render_grid_with_ligatures`, complex-script rows are routed to a new `paint_complex_line` that positions glyphs at `glyph.position_along_baseline` — the shaper-produced visual x — preserving bidi reorder and contextual joining. Non-RTL rows continue through `paint_line` unchanged. Regression tests cover the three predicates against representative RTL and non-RTL inputs, including the short-circuit on row range. Deferred (separate change): cursor placement, selection highlight rect, mouse hit-testing, and per-cell backgrounds on RTL rows still use logical cell coordinates and will be visually offset from the shaped glyphs. This is most visible in alt-screen apps (vim, less, htop) where the cursor lives inside terminal output. Fixing it correctly needs a visual<->logical cell map shared between the renderer and those consumers; doing it in this PR would balloon the diff and risks regressing non-RTL behaviour. Refs: warpdotdev#10355, warpdotdev#3589, warpdotdev#279, warpdotdev#125
BSN4
force-pushed
the
bader/fix-arabic-rtl-terminal-output
branch
from
67d0d2e to
e80814d
Compare
1 participant
Description
Terminal output rendered Arabic and other RTL scripts left-to-right with isolated (unjoined) forms, while the input editor rendered the same text correctly. The two paths diverged at the painter, not at the shaper:
TextLayoutSystem::layout_text, which runsBidiParagraphs+ShapeLinewithShaping::Advanced— bidi reorder and rustybuzz Arabic shaping both apply.render_grid_without_ligatureswalks cells one by one and callsglyph_for_charper codepoint — no shaping, no bidi.render_grid_with_ligaturesdoes calllayout_line, butpaint_linethen discarded the shaper'sglyph.xand forced every glyph back tooriginal_logical_column * cell_width, undoing the bidi reorder and disconnecting joined glyphs.This change keeps the existing fast cell-by-cell path for plain LTR rows and only diverts rows that need bidi + shaping:
is_complex_script_charcovers Arabic (all blocks), Hebrew, Syriac, Thaana, NKo, Arabic Presentation Forms A/B.row_needs_complex_layout(row predicate) andframe_contains_complex_script(frame pre-scan, short-circuits).layout_line.paint_complex_linethat positions glyphs atglyph.position_along_baseline— the shaper-produced visual x — preserving bidi reorder and contextual joining.paint_lineunchanged.Deferred (separate change): cursor placement, selection highlight rect, mouse hit-testing, and per-cell backgrounds on RTL rows still use logical cell coordinates and will be visually offset from the shaped glyphs. Most visible in alt-screen apps (vim/less/htop). Fixing it correctly needs a visual↔logical cell map shared between the renderer and those consumers; bundling it here would balloon the diff and risks regressing non-RTL behaviour.
Linked Issue
Fixes #10355 — RTL (Right-to-Left) text direction support for Hebrew, Arabic, and other languages (rendering subset; cursor / selection / hit-test on RTL rows tracked as a follow-up; see Deferred note above). Also addresses #3589, #279, #125, #7889 (the recurring "RTL works in command line but not in result blocks" symptom).
Testing
Screenshots / Videos
Before
After
Agent Mode
CHANGELOG-BUG-FIX: Fix Arabic / RTL text in terminal output being rendered left-to-right with unjoined letter forms.