Skip to content

fix(rtl): lay a line out in the page's direction, not its first letter #13

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
alientechsw merged 1 commit into from
Oct 29, 2026
Merged

fix(rtl): lay a line out in the page's direction, not its first letter #13

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion crates/ui-egui/src/app.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -339,7 +339,7 @@ fn page_title(ui: &mut Ui, ed: &mut Editor, tokens: &Tokens, title_id: egui::Id,
out.response
} else {
let pieces = crate::bidi::plain(&ed.title_buf, 26.0 * zoom, tokens);
let (drawn, size) = crate::bidi::runs(ui, &ed.title_buf, &pieces, 26.0 * zoom, width);
let (drawn, size) = crate::bidi::runs(ui, &ed.title_buf, &pieces, 26.0 * zoom, width, ed.page_dir);
let (rect, resp) = ui.allocate_exact_size(vec2(width, size.y.max(34.0)), egui::Sense::click());
crate::bidi::paint(ui, rect.min, &drawn);
if resp.clicked() {
Expand Down
137 changes: 93 additions & 44 deletions crates/ui-egui/src/bidi.rs
View file
Open in desktop

Large diffs are not rendered by default.

29 changes: 15 additions & 14 deletions crates/ui-egui/src/pageedit.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -1165,7 +1165,7 @@ pub fn show_blocks(
if let Some(done) = lead(ui, &block, t) {
ticked = Some(done);
}
let w = write_rtl(ui, c, size, t, id);
let w = write_rtl(ui, c, size, t, id, page);
let resp = w.response.clone();
wrote = Some(w);
resp
Expand Down Expand Up @@ -1766,14 +1766,15 @@ pub struct Wrote {
/// (`crates/ui-egui/tests/arabic_layout.rs`). So the line is drawn by the bidi painter,
/// which puts the runs where the Unicode algorithm says, and the caret is NoteCraft's own:
/// a click asks parley which character is under the pointer, and the arrow keys step the
/// way the eye moves, not the way the bytes run.
pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id) -> Wrote {
/// way the eye moves, not the way the bytes run. `page` is the page's direction, which the
/// line reads in when it has no mark of its own, the same as when it is not being written in.
pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id, page: Option<notecraft_model::Dir>) -> Wrote {
let width = ui.available_width().max(32.0);
let pieces = vec![(
0..c.text.len(),
egui::text::TextFormat { font_id: egui::FontId::proportional(size), color: t.text, ..egui::text::TextFormat::default() },
)];
let (drawn, used) = crate::bidi::runs(ui, &c.text, &pieces, size, width);
let (drawn, used) = crate::bidi::runs(ui, &c.text, &pieces, size, width, page);
// The block's own id, the same one the egui text box uses for a line without
// right-to-left text in it, so the focus carries over when a line becomes one or stops
// being one under the caret.
Expand Down Expand Up @@ -1822,7 +1823,7 @@ pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id
if let Some(p) = resp.interact_pointer_pos().or(carried) {
let point = p - rect.min;
if resp.double_clicked() {
let word = crate::bidi::word_at(&c.text, point, size, width);
let word = crate::bidi::word_at(&c.text, point, size, width, page);
anchor = word.start;
at = word.end;
resp.request_focus();
Expand All @@ -1833,15 +1834,15 @@ pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id
// recognised and nothing is ever selected inside a block the caret already sat
// in. That is what the owner saw.
let began = ui.input(|i| i.pointer.press_origin()).unwrap_or(p);
anchor = crate::bidi::hit(&c.text, began - rect.min, size, width);
at = crate::bidi::hit(&c.text, point, size, width);
anchor = crate::bidi::hit(&c.text, began - rect.min, size, width, page);
at = crate::bidi::hit(&c.text, point, size, width, page);
resp.request_focus();
} else if resp.clicked() && !resp.dragged() {
at = crate::bidi::hit(&c.text, point, size, width);
at = crate::bidi::hit(&c.text, point, size, width, page);
anchor = at;
resp.request_focus();
} else if resp.dragged() || carried.is_some() {
at = crate::bidi::hit(&c.text, point, size, width);
at = crate::bidi::hit(&c.text, point, size, width, page);
resp.request_focus();
}
}
Expand All @@ -1862,10 +1863,10 @@ pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id
let delete = pressed(Key::Delete).is_some();
let moved = left || right || home || end;
if left {
at = crate::bidi::step(&c.text, at, false, size, width);
at = crate::bidi::step(&c.text, at, false, size, width, page);
}
if right {
at = crate::bidi::step(&c.text, at, true, size, width);
at = crate::bidi::step(&c.text, at, true, size, width, page);
}
if home {
at = 0;
Expand Down Expand Up @@ -1926,7 +1927,7 @@ pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id

// What is selected, painted behind the words, one rectangle per run it covers.
if resp.has_focus() && anchor != at {
let shapes: Vec<egui::Shape> = crate::bidi::selection(ui, &c.text, &pieces, anchor, at, size, width)
let shapes: Vec<egui::Shape> = crate::bidi::selection(ui, &c.text, &pieces, anchor, at, size, width, page)
.into_iter()
.map(|r| {
let r = egui::Rect::from_min_max(rect.min + r.min.to_vec2(), rect.min + r.max.to_vec2());
Expand All @@ -1937,7 +1938,7 @@ pub fn write_rtl(ui: &mut Ui, c: &mut Caret, size: f32, t: &Tokens, id: egui::Id
}
// The caret itself, drawn where parley says it belongs on the line.
if resp.has_focus()
&& let Some(bar) = crate::bidi::caret(ui, &c.text, &pieces, at, size, width)
&& let Some(bar) = crate::bidi::caret(ui, &c.text, &pieces, at, size, width, page)
{
let bar = egui::Rect::from_min_max(rect.min + bar.min.to_vec2(), rect.min + bar.max.to_vec2());
let bar = egui::Rect::from_min_size(bar.min, vec2(1.5, bar.height().max(size)));
Expand Down Expand Up @@ -2193,7 +2194,7 @@ fn char_under_drawn(ui: &Ui, block: &Block, resp: &egui::Response, pos: egui::Po
let column = (resp.rect.width() - lead).max(32.0);
let (size, strong) = size_of(block);
let at = if crate::bidi::paints(&drawn, page) {
let byte = crate::bidi::hit(&drawn, vec2(x, rel.y), size, column);
let byte = crate::bidi::hit(&drawn, vec2(x, rel.y), size, column, page);
drawn.get(..byte).unwrap_or(drawn.as_str()).chars().count()
} else {
char_at(ui, &drawn, size, strong, t, column, vec2(x, rel.y), page)
Expand Down
2 changes: 1 addition & 1 deletion crates/ui-egui/src/preview.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -317,7 +317,7 @@ pub fn link_at_point(block: &Block, at: egui::Vec2, width: f32, page: Option<Dir
let rtl = dir_of(&spans, page).is_rtl();
let x = if rtl { at.x } else { at.x - lead };
let column = (width - lead).max(32.0);
let byte = crate::bidi::hit(&text, egui::vec2(x, at.y), size_of_block(block), column);
let byte = crate::bidi::hit(&text, egui::vec2(x, at.y), size_of_block(block), column, page);
let mut start = 0usize;
for s in &spans {
if s.image.is_some() {
Expand Down
2 changes: 1 addition & 1 deletion crates/ui-egui/tests/arabic_layout.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ fn a_number_inside_an_arabic_line_is_drawn_in_order() {

// The digits are one piece, written the way they were typed, and they sit between the
// two Arabic runs rather than at one end of the line.
let (drawn, size) = run_in_ui(|ui| bidi::runs(ui, line, &bidi::plain(line, 16.0, &t), 16.0, 400.0));
let (drawn, size) = run_in_ui(|ui| bidi::runs(ui, line, &bidi::plain(line, 16.0, &t), 16.0, 400.0, None));
assert!(size.x > 0.0);
let digits = drawn.iter().position(|d| d.galley.text().contains("17"));
let at = digits.unwrap_or(usize::MAX);
Expand Down
98 changes: 80 additions & 18 deletions crates/ui-egui/tests/rtl_quality.rs
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -58,8 +58,8 @@ fn home_and_end_go_to_the_visual_ends_of_a_right_to_left_line() {
let t = Theme::DEFAULT.tokens();
let (home, end) = in_frame(|ui| {
let pieces = bidi::plain(LINE, 16.0, &t);
let home = bidi::caret(ui, LINE, &pieces, 0, 16.0, 400.0).map(|r| r.min.x).unwrap_or(0.0);
let end = bidi::caret(ui, LINE, &pieces, LINE.len(), 16.0, 400.0).map(|r| r.min.x).unwrap_or(0.0);
let home = bidi::caret(ui, LINE, &pieces, 0, 16.0, 400.0, None).map(|r| r.min.x).unwrap_or(0.0);
let end = bidi::caret(ui, LINE, &pieces, LINE.len(), 16.0, 400.0, None).map(|r| r.min.x).unwrap_or(0.0);
(home, end)
});
assert!(home > end + 50.0, "Home is at the right edge and End at the left: home {home}, end {end}");
Expand All @@ -79,11 +79,11 @@ fn a_step_never_lands_between_a_letter_and_its_diacritic() {
// Every stop the caret makes walking the word. In a right-to-left word the left
// arrow is the one that goes forward through the letters, which is itself worth
// saying: at the start of the word the right arrow has nowhere to go.
assert_eq!(bidi::step(VOWELLED, 0, true, 16.0, 400.0), 0, "the right arrow is already at the start of a right-to-left word");
assert_eq!(bidi::step(VOWELLED, 0, true, 16.0, 400.0, None), 0, "the right arrow is already at the start of a right-to-left word");
let mut stops = vec![0usize];
let mut at = 0usize;
for _ in 0..12 {
let next = bidi::step(VOWELLED, at, false, 16.0, 400.0);
let next = bidi::step(VOWELLED, at, false, 16.0, 400.0, None);
if next == at {
break;
}
Expand All @@ -107,7 +107,7 @@ fn a_click_inside_a_vowelled_letter_lands_beside_it() {
// Points across the word, from its left edge to its right.
for step in 0..40 {
let x = step as f32 * 3.0;
let at = bidi::hit(VOWELLED, egui::vec2(x, 8.0), 16.0, 400.0);
let at = bidi::hit(VOWELLED, egui::vec2(x, 8.0), 16.0, 400.0, None);
assert!(!marks.contains(&at), "a click at x={x} landed on the mark at {at}");
}
}
Expand Down Expand Up @@ -141,7 +141,7 @@ fn a_bracket_in_a_right_to_left_line_reaches_egui_as_it_was_typed() {
let t = Theme::DEFAULT.tokens();
let drawn: Vec<char> = in_frame(|ui| {
let pieces = bidi::plain(BRACKETED, 16.0, &t);
let (runs, _) = bidi::runs(ui, BRACKETED, &pieces, 16.0, 400.0);
let (runs, _) = bidi::runs(ui, BRACKETED, &pieces, 16.0, 400.0, None);
runs.iter().flat_map(|d| d.galley.text().chars().collect::<Vec<_>>()).collect()
});
assert!(drawn.contains(&'('), "the opening bracket is handed over as itself: {drawn:?}");
Expand Down Expand Up @@ -175,17 +175,17 @@ fn a_line_is_justified_by_stretching_the_letters_not_the_spaces() {
// wraps the line is no width at all, which is how the search knows to stop.
let line_height = {
let pieces = bidi::plain(SENTENCE, 18.0, &t);
bidi::runs(ui, SENTENCE, &pieces, 18.0, COLUMN).1.y
bidi::runs(ui, SENTENCE, &pieces, 18.0, COLUMN, None).1.y
};
let mut width = |s: &str| {
let pieces = bidi::plain(s, 18.0, &t);
// Laid out in the column, the painter reports the column's own width, so the
// height is what says whether the line still fits; the ink width comes from
// laying the same line out with nothing to wrap against.
if bidi::runs(ui, s, &pieces, 18.0, COLUMN).1.y > line_height + 1.0 {
if bidi::runs(ui, s, &pieces, 18.0, COLUMN, None).1.y > line_height + 1.0 {
return f32::INFINITY;
}
bidi::runs(ui, s, &pieces, 18.0, f32::INFINITY).1.x
bidi::runs(ui, s, &pieces, 18.0, f32::INFINITY, None).1.x
};
let natural = width(SENTENCE);
// The strongest of Word's three: this test is about whether kashida can fill a
Expand Down Expand Up @@ -230,7 +230,7 @@ fn a_whole_paragraph_is_justified_except_its_last_line() {
let pieces = bidi::plain(PARAGRAPH, SIZE, &t);
let (out, moved) = bidi::stretched(ui, PARAGRAPH, &pieces, SIZE, COLUMN, notecraft_model::justify::Strength::High);
// Every line as it is drawn: its left edge and its right edge in the column.
let (drawn, _) = bidi::runs(ui, &out, &moved, SIZE, COLUMN);
let (drawn, _) = bidi::runs(ui, &out, &moved, SIZE, COLUMN, None);
let mut rows: Vec<(f32, f32, f32)> = Vec::new();
for d in &drawn {
let (left, right) = (d.at.x, d.at.x + d.galley.rect.width());
Expand Down Expand Up @@ -281,7 +281,7 @@ fn paragraph_picture(plain: &str, justified: &str, column: f32, size: f32) {
ui.add_space(16.0);
for text in [plain.as_str(), justified.as_str()] {
let pieces = bidi::plain(text, size, &t);
let (drawn, used) = bidi::runs(ui, text, &pieces, size, column);
let (drawn, used) = bidi::runs(ui, text, &pieces, size, column, None);
let (rect, _) = ui.allocate_exact_size(egui::vec2(column, used.y.max(24.0)), egui::Sense::hover());
bidi::paint(ui, rect.min, &drawn);
ui.painter().rect_stroke(
Expand Down Expand Up @@ -313,7 +313,7 @@ fn picture(plain: &str, justified: &str, column: f32) {
ui.add_space(20.0);
for line in [plain.as_str(), justified.as_str()] {
let pieces = bidi::plain(line, 18.0, &t);
let (drawn, size) = bidi::runs(ui, line, &pieces, 18.0, column);
let (drawn, size) = bidi::runs(ui, line, &pieces, 18.0, column, None);
let (rect, _) = ui.allocate_exact_size(egui::vec2(column, size.y.max(24.0)), egui::Sense::hover());
bidi::paint(ui, rect.min, &drawn);
// The column the line is being justified into, so the eye can see which
Expand Down Expand Up @@ -529,7 +529,7 @@ fn the_three_justify_strengths_differ_on_the_page() {
fn drawn_lines(text: &str, size: f32, column: f32, t: &notecraft_ui_egui::theme::Tokens) -> Vec<(f32, f32, f32)> {
in_frame(|ui| {
let pieces = bidi::plain(text, size, t);
let (drawn, _) = bidi::runs(ui, text, &pieces, size, column);
let (drawn, _) = bidi::runs(ui, text, &pieces, size, column, None);
let mut rows: Vec<(f32, f32, f32)> = Vec::new();
for d in &drawn {
let (left, right) = (d.at.x, d.at.x + d.galley.rect.width());
Expand Down Expand Up @@ -560,7 +560,7 @@ fn strengths_picture(plain: &str, stretched: &[String; 3], column: f32, size: f3
ui.add_space(14.0);
for (label, text) in labels.iter().zip(&all) {
let pieces = bidi::plain(text, size, &t);
let (drawn, used) = bidi::runs(ui, text, &pieces, size, column);
let (drawn, used) = bidi::runs(ui, text, &pieces, size, column, None);
let (rect, _) = ui.allocate_exact_size(egui::vec2(column + 70.0, used.y.max(24.0)), egui::Sense::hover());
bidi::paint(ui, rect.min, &drawn);
ui.painter().text(
Expand Down Expand Up @@ -626,7 +626,7 @@ fn an_arrow_key_walks_a_mixed_line_the_way_it_points() {
let x_at = |at: usize| -> f32 {
in_frame(|ui| {
let pieces = bidi::plain(line, 16.0, &t);
bidi::caret(ui, line, &pieces, at, 16.0, W).map(|r| r.min.x).unwrap_or(f32::NAN)
bidi::caret(ui, line, &pieces, at, 16.0, W, None).map(|r| r.min.x).unwrap_or(f32::NAN)
})
};
for right in [false, true] {
Expand All @@ -638,7 +638,7 @@ fn an_arrow_key_walks_a_mixed_line_the_way_it_points() {
let mut was = x_at(at);
let mut stops = 1;
for _ in 0..64 {
let next = bidi::step(line, at, right, 16.0, W);
let next = bidi::step(line, at, right, 16.0, W, None);
if next == at {
break;
}
Expand All @@ -662,10 +662,72 @@ fn an_arrow_key_walks_a_mixed_line_the_way_it_points() {
for line in ["السعر 123 ريال", "مرحبا hello بك"] {
let mut at = line.len();
for _ in 0..6 {
let forward = bidi::step(line, at, true, 16.0, W);
let back = bidi::step(line, forward, false, 16.0, W);
let forward = bidi::step(line, at, true, 16.0, W, None);
let back = bidi::step(line, forward, false, 16.0, W, None);
assert_eq!(back, at, "{line:?}: right then left came back to {at}, not {back}");
at = forward;
}
}
}

/// 1.4 of the bar. A line whose direction comes from the page reads that way inside, and
/// not only from that edge.
///
/// Measured: **the bar**, from 2026-10-29, and a plain fault before that. parley takes a
/// paragraph's base direction from its first strong letter and cannot be told otherwise, so
/// a list item `TODO: راجع هذه الصفحة` on a page set right to left was hung from the right
/// edge and laid out left to right inside it: `TODO:` was drawn at the far left end of the
/// line, after the Arabic it comes before, which is what `docs/gaps.md` called the line
/// that "flips". A browser with `dir="rtl"` on the page puts `TODO:` at the right edge,
/// where the line starts. The line is laid out with the page's direction mark in front now.
#[test]
fn a_line_that_starts_in_latin_reads_the_way_the_page_does() {
const TODO: &str = "TODO: راجع هذه الصفحة";
let t = Theme::DEFAULT.tokens();
// Where `TODO:` and the first Arabic word are drawn, by the page painter.
let places = |page: Option<notecraft_model::Dir>| -> (f32, f32) {
let h = harness();
let blocks = notecraft_ui_egui::preview::blocks_for(notecraft_model::PageFormat::Cmgfm, TODO);
let mut frame = h.ctx.run_ui(egui::RawInput::default(), |ui| {
fonts::install_once(ui.ctx());
ui.set_width(560.0);
notecraft_ui_egui::preview::show(ui, &blocks, std::path::Path::new("."), &t, page, &Default::default());
});
let at = |word: &str| {
frame
.shapes
.iter()
.find_map(|c| match &c.shape {
// The colon after `TODO` is neutral, so on a right-to-left page it goes
// with the Arabic and `TODO` is a run of its own; on a left-to-right one
// the two are drawn together.
egui::Shape::Text(text) if text.galley.text().trim_end_matches(':') == word => Some(text.pos.x),
_ => None,
})
.unwrap_or(f32::NAN)
};
let out = (at("TODO"), at("راجع"));
frame.textures_delta.clear();
out
};

let (latin, arabic) = places(Some(notecraft_model::Dir::Rtl));
assert!(latin.is_finite() && arabic.is_finite(), "both words were drawn: {latin} and {arabic}");
assert!(latin > arabic, "on a right-to-left page the line starts at the right, with TODO there: {latin} and {arabic}");
// A page that says nothing leaves the line to its first letter, which is Latin.
let (latin, arabic) = places(None);
assert!(latin < arabic, "with no page direction the line reads left to right: {latin} and {arabic}");

// A click and the caret go through the same layout as the drawing, or a click on
// `TODO:` would land in the Arabic beside it.
let rtl = Some(notecraft_model::Dir::Rtl);
let near_right = bidi::hit(TODO, egui::vec2(395.0, 8.0), 16.0, 400.0, rtl);
assert!(near_right <= "TODO:".len(), "a click at the right edge lands in TODO: (byte {near_right})");
let near_left = bidi::hit(TODO, egui::vec2(5.0, 8.0), 16.0, 400.0, rtl);
assert!(near_left > "TODO:".len(), "a click at the left edge lands in the Arabic (byte {near_left})");
let start = in_frame(|ui| {
let pieces = bidi::plain(TODO, 16.0, &t);
bidi::caret(ui, TODO, &pieces, 0, 16.0, 400.0, rtl).map(|r| r.min.x).unwrap_or(f32::NAN)
});
assert!(start > 300.0, "the start of the line is at the right edge, and so is a caret there: {start}");
}
Loading