From 0a447ee1ab772a64dd48c2de92952a574418c829 Mon Sep 17 00:00:00 2001 From: Fabio Lussi Date: Mon, 12 Oct 2026 03:22:12 +0200 Subject: [PATCH] Comments panel: show Persian and Arabic comments in reading order (#1079) (#1138) * Comments panel: show Persian and Arabic comments in reading order Comment bodies and replies with right-to-left script were laid out with their words in logical order, left to right. They now wrap through bidi::wrap: each paragraph takes its direction from its first strong character, lines break in reading order, each line is reordered (UAX #9) and right-to-left lines sit against the right edge. Author names get the same treatment on one line. Persian and Arabic-Indic digits, which egui's shaper drew reversed, now read left to right, file names included. Fixes #1079 Co-Authored-By: Claude Opus 5.5 * RTL comments: cache wrapped lines between frames wrapped_text measured every candidate line of every right-to-left comment on every frame. The wrapped lines are now kept in egui's temp data, keyed by a hash of (text, font, width, pixels per point, collapsed); the cache starts over at 256 entries, so it stays bounded. A collapsed comment wraps only its first 1024 characters (it shows one line), still with the ellipsis when the text goes on. Test: second frame reuses the wrap, the cache stays bounded, a very long collapsed comment is one line. Co-Authored-By: Claude Opus 5.5 --------- Co-authored-by: Fabio Lussi <198260595+itrycodinganditsbad@users.noreply.github.com> Co-authored-by: Claude Opus 5.5 Co-authored-by: Brandon Thomas --- crates/ui-egui/src/bidi.rs | 323 ++++++++++++++++++++++++--- crates/ui-egui/src/comments.rs | 2 +- crates/ui-egui/src/comments_panel.rs | 36 +-- crates/ui-egui/src/widgets.rs | 167 ++++++++++++++ 4 files changed, 476 insertions(+), 52 deletions(-) diff --git a/crates/ui-egui/src/bidi.rs b/crates/ui-egui/src/bidi.rs index 709e34b9..50cae547 100644 --- a/crates/ui-egui/src/bidi.rs +++ b/crates/ui-egui/src/bidi.rs @@ -10,8 +10,9 @@ //! catalog), whose base direction is right to left. use std::borrow::Cow; +use std::ops::Range; -use unicode_bidi::{BidiInfo, Level}; +use unicode_bidi::{BidiInfo, Level, ParagraphInfo}; /// `text` rearranged for an egui call that lays out one line. Text without right-to-left /// characters comes back borrowed and unchanged. @@ -32,18 +33,170 @@ pub fn visual(text: &str) -> Cow<'_, str> { fn push_visual(out: &mut String, text: &str, base: Level) { let info = BidiInfo::new(text, Some(base)); for para in &info.paragraphs { - let (levels, runs) = info.visual_runs(para, para.range.clone()); - for run in runs { - let rtl = levels.get(run.start).is_some_and(Level::is_rtl); - // Runs lie on character boundaries; a missing slice is skipped, never a panic. - let Some(piece) = text.get(run) else { continue }; - if rtl { - push_rtl_run(out, piece); - } else { - out.extend(piece.chars().filter(|c| !is_bidi_control(*c))); - } + push_line(out, text, &info, para, para.range.clone()); + } +} + +/// The `line` of paragraph `para` (a non-empty byte range of `text`) in display order, as egui +/// needs it, appended to `out`. +fn push_line(out: &mut String, text: &str, info: &BidiInfo<'_>, para: &ParagraphInfo, line: Range) { + let (levels, runs) = info.visual_runs(para, line); + // First the true display order, every character left to right as it appears on screen. + let mut shown = String::with_capacity(text.len()); + for run in runs { + let rtl = levels.get(run.start).is_some_and(Level::is_rtl); + // Runs lie on character boundaries; a missing slice is skipped, never a panic. + let Some(piece) = text.get(run) else { continue }; + let chars = piece.chars().filter(|c| !is_bidi_control(*c)); + if rtl { + shown.extend(chars.rev().map(mirrored)); + } else { + shown.extend(chars); } } + push_for_shaper(out, &shown); +} + +/// `shown` (in screen order) as egui must get it: egui hands each stretch of right-to-left +/// script to the shaper as one run, which lays it out right to left (and joins its letters), so +/// each such stretch goes back to logical order. What lies between them (spaces, punctuation, +/// Latin) is drawn by the Latin face left to right and stays as it is. The stretches include +/// Persian and Arabic-Indic digits: Arabic script to the shaper, though they read left to right. +fn push_for_shaper(out: &mut String, shown: &str) { + let mut stretch: Vec = Vec::new(); + for c in shown.chars() { + if is_rtl_script(c) || is_joiner(c) { + stretch.push(c); + } else { + out.extend(stretch.drain(..).rev()); + out.push(c); + } + } + out.extend(stretch.drain(..).rev()); +} + +/// One line of wrapped text (see [`wrap`]). +#[derive(Clone, Debug, PartialEq, Eq)] +pub struct Line { + /// The line in display order, for one egui call that lays out one line. + pub text: String, + /// The paragraph reads right to left: the line belongs against the right edge. + pub rtl: bool, +} + +/// Whether `text` holds any right-to-left script, so [`wrap`] or [`visual_auto`] is needed. +pub fn has_rtl(text: &str) -> bool { + text.chars().any(is_rtl_script) +} + +/// Multi-line text with right-to-left script (a comment in Persian, Arabic, Hebrew), broken +/// into lines that `fits` accepts (it measures a line in logical order) and each put in display +/// order by the Unicode bidirectional algorithm (UAX #9). +/// +/// Every paragraph takes its direction from its first strong character, as `dir="auto"` does, +/// so a Persian comment reads right to left with its Latin words and numbers left to right +/// inside it. Lines break between words, in reading order, and a word wider than a line +/// between characters. Use it for painting; copying and accessibility keep the logical text. +pub fn wrap(text: &str, mut fits: impl FnMut(&str) -> bool) -> Vec { + let info = BidiInfo::new(text, None); + let mut lines = Vec::new(); + for para in &info.paragraphs { + let whole = text.get(para.range.clone()).unwrap_or_default(); + // unicode-bidi ends a paragraph at the `\r` of `\r\n`; the `\n` is no new paragraph. + if whole == "\n" && text.get(..para.range.start).is_some_and(|before| before.ends_with('\r')) { + continue; + } + let content = para.range.start..para.range.start.saturating_add(whole.trim_end_matches(is_paragraph_separator).len()); + let rtl = para.level.is_rtl(); + for line in break_lines(text, content, &mut fits) { + let mut out = String::new(); + // unicode-bidi indexes the first level of a line, so an empty one would panic. + if !line.is_empty() { + push_line(&mut out, text, &info, para, line); + } + lines.push(Line { text: out, rtl }); + } + } + lines +} + +/// One line of `text` in display order, its direction from its first strong character (an +/// author's name). Text without right-to-left characters comes back borrowed and unchanged. +pub fn visual_auto(text: &str) -> Cow<'_, str> { + if !has_rtl(text) { + return Cow::Borrowed(text); + } + let lines: Vec = wrap(text, |_| true).into_iter().map(|l| l.text).collect(); + Cow::Owned(lines.join("\n")) +} + +/// The byte ranges of the lines `range` of `text` breaks into: as many words as `fits` accepts +/// per line, a word too wide for any line split between characters. Always at least one line. +fn break_lines(text: &str, range: Range, fits: &mut impl FnMut(&str) -> bool) -> Vec> { + let mut lines = Vec::new(); + let mut current: Option> = None; + for word in words(text, range.clone()) { + if let Some(line) = current.as_mut() { + if fits(text.get(line.start..word.end).unwrap_or_default()) { + line.end = word.end; + continue; + } + lines.push(line.clone()); + } + // The word starts a line. + let mut start = word.start; + loop { + let end = fitting_prefix(text, start..word.end, fits); + if end >= word.end { + current = Some(start..word.end); + break; + } + lines.push(start..end); + start = end; + } + } + lines.push(current.unwrap_or(range.start..range.start)); + lines +} + +/// The end of the longest start of `range` that `fits`, at least one character. +fn fitting_prefix(text: &str, range: Range, fits: &mut impl FnMut(&str) -> bool) -> usize { + let piece = text.get(range.clone()).unwrap_or_default(); + let mut end = range.end; + for (i, (at, c)) in piece.char_indices().enumerate() { + let next = range.start.saturating_add(at).saturating_add(c.len_utf8()); + if i > 0 && !fits(text.get(range.start..next).unwrap_or_default()) { + end = range.start.saturating_add(at); + break; + } + } + end +} + +/// The byte ranges of the whitespace-separated words of `range` in `text`. +fn words(text: &str, range: Range) -> Vec> { + let mut words = Vec::new(); + let mut start = None; + for (at, c) in text.get(range.clone()).unwrap_or_default().char_indices() { + let at = range.start.saturating_add(at); + match (c.is_whitespace(), start) { + (true, Some(s)) => { + words.push(s..at); + start = None; + } + (false, None) => start = Some(at), + _ => {} + } + } + if let Some(s) = start { + words.push(s..range.end); + } + words +} + +/// Characters that end a paragraph (bidi class B). +fn is_paragraph_separator(c: char) -> bool { + matches!(c, '\n' | '\r' | '\u{1C}'..='\u{1E}' | '\u{85}' | '\u{2029}') } /// A label of a right-to-left interface language, in logical order, rearranged for egui: each @@ -159,34 +312,18 @@ fn is_modifier_key(c: char) -> bool { matches!(c, '\u{2318}' | '\u{21E7}' | '\u{2325}' | '\u{2303}') } -/// One right-to-left run, in display order. Stretches of right-to-left script stay in logical -/// order (egui gives each to the shaper as one face run, which reverses and joins it); what -/// lies between them (spaces, punctuation: drawn by the Latin face, left to right) is reversed -/// here, with brackets mirrored. -fn push_rtl_run(out: &mut String, run: &str) { - let mut pieces: Vec<(bool, String)> = Vec::new(); - for c in run.chars().filter(|c| !is_bidi_control(*c)) { - let script = is_rtl_script(c); - match pieces.last_mut() { - Some((kind, piece)) if *kind == script => piece.push(c), - _ => pieces.push((script, c.to_string())), - } - } - for (script, piece) in pieces.iter().rev() { - if *script { - out.push_str(piece); - } else { - out.extend(piece.chars().rev().map(mirrored)); - } - } -} - /// Characters of the right-to-left scripts' blocks: letters, marks and the scripts' own /// punctuation, all drawn by the script's face. fn is_rtl_script(c: char) -> bool { matches!(u32::from(c), 0x0590..=0x08FF | 0xFB1D..=0xFDFF | 0xFE70..=0xFEFC | 0x1_0800..=0x1_0FFF | 0x1_E800..=0x1_EFFF) } +/// The zero-width (non-)joiners inside a word (Persian `می‌روم`): egui shapes them with the +/// letter before them. +fn is_joiner(c: char) -> bool { + matches!(c, '\u{200C}' | '\u{200D}') +} + /// Direction marks, embeddings, overrides and isolates: they steer the reordering and are not drawn. fn is_bidi_control(c: char) -> bool { matches!(c, '\u{061C}' | '\u{200E}' | '\u{200F}' | '\u{202A}'..='\u{202E}' | '\u{2066}'..='\u{2069}') @@ -324,6 +461,126 @@ mod tests { } } + /// `wrap` with a width of `n` characters. + fn wrap_at(text: &str, n: usize) -> Vec { + wrap(text, |s| s.chars().count() <= n) + } + + fn rtl(text: &str) -> Line { + Line { text: text.to_owned(), rtl: true } + } + + fn ltr(text: &str) -> Line { + Line { text: text.to_owned(), rtl: false } + } + + #[test] + fn persian_paragraph_reads_right_to_left() { + // "I go to school": the words come out in reverse for egui's left-to-right placement. + assert_eq!(wrap_at("من به مدرسه می‌روم", 80), [rtl("می‌روم مدرسه به من")]); + // The zero-width non-joiner keeps a word whole, so the shaper gets it as one run. + assert_eq!(wrap_at("کتاب‌ها", 80), [rtl("کتاب‌ها")]); + } + + #[test] + fn arabic_lines_break_in_reading_order() { + // The first words go on the first line, each line reordered on its own. + assert_eq!(wrap_at("واحد اثنين ثلاثة أربعة", 11), [rtl("اثنين واحد"), rtl("أربعة ثلاثة")]); + assert_eq!(wrap_at("واحد اثنين ثلاثة أربعة", 80), [rtl("أربعة ثلاثة اثنين واحد")]); + } + + #[test] + fn base_direction_comes_from_the_first_strong_character() { + // Latin first: a left-to-right paragraph with an Arabic phrase inside it. + assert_eq!(wrap_at("See واحد اثنين here", 80), [ltr("See اثنين واحد here")]); + // Persian first: right to left, the Latin words and the number left to right inside it. + assert_eq!(wrap_at("فایل PDF نسخه 2 است", 80), [rtl("است 2 نسخه PDF فایل")]); + assert_eq!(wrap_at("سلام Hello World", 80), [rtl("Hello World سلام")]); + // Leading neutrals and numbers don't decide it. + assert_eq!(wrap_at("12. واحد اثنين", 80), [rtl("اثنين واحد .12")]); + // Each paragraph finds its own direction. + assert_eq!(wrap_at("واحد اثنين\nOK then\n\nשלום עולם", 80), [rtl("اثنين واحد"), ltr("OK then"), ltr(""), rtl("עולם שלום")]); + } + + #[test] + fn rtl_punctuation_and_brackets() { + // The sentence's full stop ends up on the left, brackets flip. + assert_eq!(wrap_at("سلام دنیا.", 80), [rtl(".دنیا سلام")]); + assert_eq!(wrap_at("واحد (اثنين) ثلاثة", 80), [rtl("ثلاثة (اثنين) واحد")]); + assert_eq!(wrap_at("آیا آماده است؟", 80), [rtl("است؟ آماده آیا")]); + } + + #[test] + fn persian_digits_go_to_the_shaper_reversed() { + // Persian and Arabic-Indic digits are Arabic script, which egui's shaper lays out right to + // left; they read left to right, so they are handed over reversed. + assert_eq!(wrap_at("سال ۱۴۰۲ شمسی", 80), [rtl("شمسی ۲۰۴۱ سال")]); + assert_eq!(wrap_at("صفحه ١٢", 80), [rtl("٢١ صفحه")]); + // Glued to a word, number and word swap places within one shaped run. + assert_eq!(wrap_at("ب۱۲", 80), [rtl("ب۲۱")]); + } + + #[test] + fn wrapping_handles_odd_widths_and_input() { + assert_eq!(wrap_at("", 10), Vec::::new()); + // A word wider than the line is broken between characters. + assert_eq!(wrap_at("abcdefghij واحد", 4), [ltr("abcd"), ltr("efgh"), ltr("ij"), ltr("واحد")]); + // Extra spaces between words don't make empty lines; \r\n is one break. + assert_eq!(wrap_at("واحد اثنين\r\nب", 5), [rtl("واحد"), rtl("اثنين"), rtl("ب")]); + // Nothing fits: one character per line, never an endless loop. + assert_eq!(wrap("واحد", |_| false).len(), 4); + let long = "ا ب ۱ (ج) ".repeat(5_000); + for s in ["\u{202E}ا\u{202C}b", "\u{064B}", "\n\n", "\u{2029}ا", long.as_str()] { + let lines = wrap_at(s, 7); + let shown: usize = lines.iter().map(|l| l.text.chars().count()).sum(); + assert!(shown <= s.chars().count(), "{:?}", s.chars().take(12).collect::()); + } + } + + /// The characters egui puts on screen for each line of `text`, left to right, zero-width + /// ones left out, with the interface fonts. + fn on_screen(text: &str) -> Vec { + let ctx = egui::Context::default(); + ctx.set_fonts(crate::theme::font_definitions()); + let mut shown = Vec::new(); + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + for line in wrap(text, |_| true) { + let galley = ui.painter().layout_no_wrap(line.text, egui::FontId::proportional(13.0), egui::Color32::WHITE); + let glyphs = galley.rows.iter().flat_map(|row| row.glyphs.iter()); + shown.push(glyphs.filter(|g| g.advance_width > 0.0).map(|g| g.chr).collect()); + } + }); + output.drop_without_applying_deltas(); + shown + } + + fn rev(s: &str) -> String { + s.chars().rev().collect() + } + + #[test] + fn egui_draws_lines_in_reading_order() { + // Only with an Arabic-script face (the craft-fonts build input, as in CI); without one + // egui has no shaper run to check. + if pdfcraft_fonts::ui_arabic_fonts().is_empty() { + return; + } + assert_eq!(on_screen("من به مدرسه می\u{200C}روم"), [rev("من به مدرسه میروم")]); + assert_eq!(on_screen("فایل PDF نسخه 2 است"), [format!("{} 2 {} PDF {}", rev("است"), rev("نسخه"), rev("فایل"))]); + assert_eq!(on_screen("سال ۱۴۰۲ شمسی"), [format!("{} ۱۴۰۲ {}", rev("شمسی"), rev("سال"))]); + assert_eq!(on_screen("ب۱۲"), ["۱۲ب"]); + assert_eq!(on_screen("See واحد اثنين here"), [format!("See {} {} here", rev("اثنين"), rev("واحد"))]); + assert_eq!(on_screen("واحد (اثنين)."), [format!(".({}) {}", rev("اثنين"), rev("واحد"))]); + } + + #[test] + fn single_line_with_its_own_direction() { + assert!(matches!(visual_auto("Ada Lovelace"), Cow::Borrowed("Ada Lovelace"))); + assert_eq!(visual_auto("علی رضایی"), "رضایی علی"); + assert_eq!(visual_auto("علی 2"), "2 علی"); + assert_eq!(visual_auto("Team علی رضایی"), "Team رضایی علی"); + } + #[test] fn odd_input_never_panics() { let long = "ا ب ".repeat(20_000); diff --git a/crates/ui-egui/src/comments.rs b/crates/ui-egui/src/comments.rs index bce3d18d..b0321210 100644 --- a/crates/ui-egui/src/comments.rs +++ b/crates/ui-egui/src/comments.rs @@ -1140,7 +1140,7 @@ pub(crate) fn composer(ctx: &egui::Context, view: &mut DocView, info: &DocInfo, egui::Frame::popup(ui.style()).inner_margin(egui::Margin::same(12)).corner_radius(CornerRadius::same(8)).show(ui, |ui| { ui.set_width(260.0); ui.horizontal(|ui| { - ui.label(egui::RichText::new(&prefs.author).font(theme::semibold(13.0))); + ui.label(egui::RichText::new(crate::bidi::visual_auto(&prefs.author)).font(theme::semibold(13.0))); ui.label(egui::RichText::new(tl!(title)).font(theme::regular(11.5)).color(t.text_faint)); }); ui.add_space(6.0); diff --git a/crates/ui-egui/src/comments_panel.rs b/crates/ui-egui/src/comments_panel.rs index e89c7de6..901493ad 100644 --- a/crates/ui-egui/src/comments_panel.rs +++ b/crates/ui-egui/src/comments_panel.rs @@ -10,10 +10,9 @@ use pdfcraft_render::{Annotation, DocInfo}; use crate::canvas::DocView; use crate::comments::{CommentPrefs, CommentTool, SortBy, color32, status_badge}; -use crate::icons; use crate::panels::Nav; use crate::theme::{self, Tokens}; -use crate::widgets; +use crate::{icons, widgets}; /// Accent bar of the selected card (11-visual-spec §3: `#0165DD`). const CARD_ACCENT: Color32 = Color32::from_rgb(0x01, 0x65, 0xDD); @@ -208,7 +207,8 @@ pub(crate) fn show( if g.is_some() && g != group { ui.add_space(6.0); let title = g.clone().unwrap_or_default(); - let shown = if sort == SortBy::Author && a.author.is_some() { title.as_str() } else { tl!(&title) }; + // Catalog labels are already in display order; an author's name is put in it here. + let shown = if sort == SortBy::Author && a.author.is_some() { crate::bidi::visual_auto(&title) } else { tl!(&title).into() }; ui.label(egui::RichText::new(shown).font(theme::semibold(12.5)).color(t.text_muted)); group = g; } @@ -266,7 +266,8 @@ fn card( let mut buttons_left = f32::INFINITY; let head = ui.horizontal(|ui| { avatar(ui, t, Some(subtype_icon(&a.subtype)), None, color); - ui.label(egui::RichText::new(a.author.as_deref().unwrap_or_else(|| tl!("Unknown author"))).font(theme::semibold(12.5)).color(t.text)); + let author = a.author.as_deref().map_or_else(|| tl!("Unknown author").into(), crate::bidi::visual_auto); + ui.label(egui::RichText::new(author).font(theme::semibold(12.5)).color(t.text)); if let Some(m) = &a.modified { ui.add(egui::Label::new(egui::RichText::new(m).font(theme::regular(11.0)).color(t.text_faint)).truncate()); } @@ -312,16 +313,16 @@ fn card( }); } else { let body = a.contents.as_deref().unwrap_or(""); - let text = if body.is_empty() { - egui::RichText::new(tl!(kind_label(a))).italics().color(t.text_faint) + if body.is_empty() { + let text = egui::RichText::new(tl!(kind_label(a))).italics().color(t.text_faint).font(theme::regular(13.0)); + let mut label = egui::Label::new(text).wrap(); + if !selected { + label = label.truncate(); + } + ui.add(label); } else { - egui::RichText::new(body).color(t.text) - }; - let mut label = egui::Label::new(text.font(theme::regular(13.0))).wrap(); - if !selected { - label = label.truncate(); + widgets::wrapped_text(ui, body, theme::regular(13.0), t.text, !selected); } - ui.add(label); } if let Some((icon, label, c)) = status.and_then(status_badge) { ui.add_space(4.0); @@ -339,15 +340,13 @@ fn card( ui.vertical(|ui| { ui.spacing_mut().item_spacing.y = 2.0; ui.horizontal(|ui| { - ui.label(egui::RichText::new(r.author.as_deref().unwrap_or_else(|| tl!("Reply"))).font(theme::semibold(12.0))); + let author = r.author.as_deref().map_or_else(|| tl!("Reply").into(), crate::bidi::visual_auto); + ui.label(egui::RichText::new(author).font(theme::semibold(12.0))); if let Some(m) = &r.modified { ui.add(egui::Label::new(egui::RichText::new(m).font(theme::regular(11.0)).color(t.text_faint)).truncate()); } }); - ui.add( - egui::Label::new(egui::RichText::new(r.contents.as_deref().unwrap_or("")).font(theme::regular(12.5)).color(t.text)) - .wrap(), - ); + widgets::wrapped_text(ui, r.contents.as_deref().unwrap_or(""), theme::regular(12.5), t.text, false); }); }); let x = resp.response.rect.left() + 12.0; @@ -549,7 +548,8 @@ pub(crate) fn header_controls(ui: &mut egui::Ui, info: &DocInfo, view: &mut DocV ui.separator(); ui.label(egui::RichText::new(tl!("Author")).small()); for a in &authors { - toggle(ui, &mut cv.hidden_authors, a, if a.is_empty() { tl!("Unknown author") } else { a }); + let name = crate::bidi::visual_auto(a); + toggle(ui, &mut cv.hidden_authors, a, if a.is_empty() { tl!("Unknown author") } else { &name }); } ui.separator(); ui.label(egui::RichText::new(tl!("Status")).small()); diff --git a/crates/ui-egui/src/widgets.rs b/crates/ui-egui/src/widgets.rs index 2cca166d..1610691a 100644 --- a/crates/ui-egui/src/widgets.rs +++ b/crates/ui-egui/src/widgets.rs @@ -286,6 +286,95 @@ pub fn row_text(ui: &egui::Ui, pos: egui::Pos2, text: impl Into, font: e ui.painter().galley(rect.min, galley, color); } +/// Text wrapped to the available width, for text people write (comments, replies). A paragraph +/// in a right-to-left script (Persian, Arabic, Hebrew) reads right to left and sits against the +/// right edge ([`crate::bidi::wrap`]); text without one is an ordinary wrapping label. +/// `truncate` keeps the first line only, with an ellipsis where the text goes on. +pub fn wrapped_text(ui: &mut egui::Ui, text: &str, font: egui::FontId, color: Color32, truncate: bool) -> Response { + if !crate::bidi::has_rtl(text) { + let mut label = egui::Label::new(egui::RichText::new(text).font(font).color(color)).wrap(); + if truncate { + label = label.truncate(); + } + return ui.add(label); + } + let width = ui.available_width().max(1.0); + let lines = wrapped_lines(ui, text, &font, width, truncate); + let galleys: Vec<_> = lines.iter().map(|line| (line.rtl, ui.painter().layout_no_wrap(line.text.clone(), font.clone(), color))).collect(); + let height = galleys.iter().map(|(_, g)| g.size().y).sum(); + let (rect, resp) = ui.allocate_exact_size(vec2(width, height), Sense::hover()); + // Painted text only: the accessibility name keeps the logical order. + resp.widget_info(|| egui::WidgetInfo::labeled(egui::WidgetType::Label, true, text)); + let mut y = rect.top(); + for (rtl, galley) in galleys { + let x = if rtl { rect.right() - galley.size().x } else { rect.left() }; + let h = galley.size().y; + ui.painter().galley(egui::pos2(x, y), galley, color); + y += h; + } + resp +} + +const ELLIPSIS: &str = "\u{2026}"; + +/// Wrapped right-to-left text kept between frames. Wrapping measures many candidate lines, so +/// re-wrapping every comment on every frame would make a panel of long comments slow; the +/// result depends only on the text, font, width, scale and collapsed state, so it is looked up. +#[derive(Clone, Default)] +struct WrapCache { + lines: std::collections::HashMap>, +} + +/// How many wrapped texts the cache holds before it starts over (each costs its text's size). +const WRAP_CACHE_MAX: usize = 256; + +/// A collapsed comment shows one line, so no more than this many characters are wrapped for it: +/// a long comment doesn't cost a full wrap just to show its start. +const COLLAPSED_WRAP_CHARS: usize = 1024; + +/// The lines `wrapped_text` paints for a right-to-left `text`, from the cache when the same text +/// was wrapped the same way before. +fn wrapped_lines(ui: &egui::Ui, text: &str, font: &egui::FontId, width: f32, truncate: bool) -> std::sync::Arc<[crate::bidi::Line]> { + use std::hash::{Hash, Hasher}; + let mut hasher = std::collections::hash_map::DefaultHasher::new(); + (text, font, width.to_bits(), ui.ctx().pixels_per_point().to_bits(), truncate).hash(&mut hasher); + let key = hasher.finish(); + let cache_id = egui::Id::new("pdfcraft-rtl-wrap-cache"); + if let Some(hit) = ui.ctx().data_mut(|d| d.get_temp_mut_or_default::(cache_id).lines.get(&key).cloned()) { + return hit; + } + // Collapsed: only the start is wrapped (cut at a char boundary, so the slice can't fail). + let (measured, cut) = match text.char_indices().nth(COLLAPSED_WRAP_CHARS) { + Some((at, _)) if truncate => (text.get(..at).unwrap_or(text), true), + _ => (text, false), + }; + let measure = |f: &mut egui::epaint::text::FontsView<'_>, s: &str| f.layout_no_wrap(s.to_owned(), font.clone(), Color32::WHITE).size().x; + let mut lines = ui.fonts_mut(|f| { + let room = if truncate { width - measure(f, ELLIPSIS) } else { width }; + crate::bidi::wrap(measured, |s| measure(f, s) <= room) + }); + if truncate && (lines.len() > 1 || cut) { + lines.truncate(1); + if let Some(line) = lines.first_mut() { + // The ellipsis goes where the line's reading ends: the left of a right-to-left line. + if line.rtl { + line.text.insert_str(0, ELLIPSIS); + } else { + line.text.push_str(ELLIPSIS); + } + } + } + let lines: std::sync::Arc<[crate::bidi::Line]> = lines.into(); + ui.ctx().data_mut(|d| { + let cache = &mut d.get_temp_mut_or_default::(cache_id).lines; + if cache.len() >= WRAP_CACHE_MAX { + cache.clear(); + } + cache.insert(key, lines.clone()); + }); + lines +} + /// Transient message at the bottom centre. pub fn toast(app: &mut PdfCraftApp, ctx: &egui::Context) { let Some((msg, start)) = app.toast.clone() else { return }; @@ -630,6 +719,84 @@ mod tests { assert!((again.width() - short.width()).abs() < 1.0, "long notices must not impose a permanent minimum width"); } + /// The text shapes `wrapped_text` paints in a 120 pt wide column at x = 0. + fn wrapped_shapes(text: &str, truncate: bool) -> Vec<(Pos2, Rect, String)> { + let ctx = egui::Context::default(); + ctx.set_fonts(theme::font_definitions()); + let output = ctx.run_ui(egui::RawInput::default(), |ui| { + let column = Rect::from_min_size(Pos2::ZERO, vec2(120.0, 400.0)); + ui.scope_builder(egui::UiBuilder::new().max_rect(column), |ui| { + wrapped_text(ui, text, theme::regular(13.0), Color32::WHITE, truncate); + }); + }); + let shapes = output + .shapes + .iter() + .filter_map(|shape| if let egui::Shape::Text(t) = &shape.shape { Some((t.pos, t.galley.rect, t.galley.job.text.clone())) } else { None }) + .collect(); + output.drop_without_applying_deltas(); + shapes + } + + /// Right-to-left text is wrapped once and looked up on later frames; the cache stays bounded + /// however many comments go by, and a collapsed comment of any length still shows one line. + #[test] + fn rtl_wrapping_is_cached_and_bounded() { + let ctx = egui::Context::default(); + ctx.set_fonts(theme::font_definitions()); + let cache_len = + |ctx: &egui::Context| ctx.data_mut(|d| d.get_temp_mut_or_default::(egui::Id::new("pdfcraft-rtl-wrap-cache")).lines.len()); + let persian = "این یک نظر طولانی به زبان فارسی است که باید در چند خط شکسته شود"; + let long = persian.repeat(2_000); + let mut first = None; + for _ in 0..3 { + ctx.run_ui(egui::RawInput::default(), |ui| { + let lines = wrapped_lines(ui, persian, &theme::regular(13.0), 120.0, false); + assert!(lines.len() > 1); + if let Some(prev) = &first { + assert!(std::sync::Arc::ptr_eq(prev, &lines), "the second frame reuses the first frame's wrap"); + } + first = Some(lines); + let collapsed = wrapped_lines(ui, &long, &theme::regular(13.0), 120.0, true); + assert_eq!(collapsed.len(), 1); + assert!(collapsed[0].text.starts_with(ELLIPSIS), "{:?}", collapsed[0].text); + }) + .drop_without_applying_deltas(); + } + assert_eq!(cache_len(&ctx), 2, "one entry per text and layout"); + ctx.run_ui(egui::RawInput::default(), |ui| { + for i in 0..WRAP_CACHE_MAX + 50 { + wrapped_lines(ui, &format!("{persian} {i}"), &theme::regular(13.0), 120.0, false); + } + }) + .drop_without_applying_deltas(); + assert!(cache_len(&ctx) <= WRAP_CACHE_MAX, "bounded: {}", cache_len(&ctx)); + } + + #[test] + fn rtl_comment_wraps_against_the_right_edge() { + let persian = "این یک نظر طولانی به زبان فارسی است که باید در چند خط شکسته شود"; + let lines = wrapped_shapes(persian, false); + assert!(lines.len() > 1, "{lines:?}"); + let right = lines.iter().map(|(pos, rect, _)| pos.x + rect.width()).fold(0.0, f32::max); + for (pos, rect, text) in &lines { + assert!(pos.x >= -0.5 && (pos.x + rect.width() - right).abs() < 0.5, "{text:?} at {pos:?} must be right-aligned"); + } + assert!(right <= 120.5, "lines stay inside the column: {right}"); + // The first words read on the first line, at the top. + let top = lines.iter().min_by(|a, b| a.0.y.total_cmp(&b.0.y)).unwrap(); + assert!(top.2.contains("این"), "{top:?}"); + // Collapsed: one line, its ellipsis on the left where right-to-left reading ends. + let collapsed = wrapped_shapes(persian, true); + assert_eq!(collapsed.len(), 1); + assert!(collapsed[0].2.starts_with('\u{2026}'), "{collapsed:?}"); + // Text without right-to-left script is a plain label, as before. + let latin = wrapped_shapes("A short note", false); + assert_eq!(latin.len(), 1); + assert_eq!(latin[0].2, "A short note"); + assert!(latin[0].0.x.abs() < 0.5); + } + #[test] fn line_field_is_at_least_thirty_points_tall() { let ctx = egui::Context::default();