From 7f8340f2a055c830cefdad99bfdb81ee1f49bfa1 Mon Sep 17 00:00:00 2001 From: Eslam Ahmad Date: Sun, 2 Aug 2026 19:18:50 +0800 Subject: [PATCH] fix(renderer): shape complex scripts instead of painting them in storage order (#194) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Native `draw_text` painted every run through `Canvas::draw_str`, which maps to Skia's `drawSimpleText` — a plain cmap lookup with no bidi reordering and no contextual glyph selection. Arabic therefore rendered in storage order with isolated letterforms. CanvasKit's `drawText` has the same limitation on web, and `drawScriptRun` there segmented a run by script before drawing the segments left to right, so bidi was resolved inside each segment and then undone across them. The paragraph shaper was bypassed natively on purpose: jian's `draw_text_paragraph` builds a fresh `FontCollection` on every call, which is what produced the ~605ms chrome frames noted in crates/CLAUDE.md. That cost is avoidable rather than inherent — `ParagraphBaseline` already keeps a generation-guarded collection — so this caches the collection and routes only the runs that genuinely need a shaper. Latin and CJK stay on the cmap fast path, leaving the frame budget and their rendered output untouched. Measurement moves with paint. `measure_text_*` summed isolated-glyph advances, so wrap decisions and caret geometry were computed against widths the painter never used and text overflowed its boxes. The routing predicate lives in op-editor-core so the two hosts cannot disagree about which runs are shaped, and so paint and measure within a host cannot drift apart. Hebrew is deliberately out of scope and stays on the fast path. Co-authored-by: Claude Opus 5 (1M context) --- crates/op-editor-core/src/lib.rs | 2 + crates/op-editor-core/src/text_script.rs | 113 ++++++++++++ .../op-editor-core/src/text_script_tests.rs | 120 ++++++++++++ crates/op-host-native/src/backend/skia.rs | 12 ++ .../src/backend/skia/complex_script_tests.rs | 173 ++++++++++++++++++ .../src/backend/skia/shaped_text.rs | 168 +++++++++++++++++ .../op-host-native/src/backend/skia/text.rs | 45 ++++- crates/op-host-web/src/canvaskit/backend.rs | 40 +++- crates/op-host-web/src/canvaskit/bindings.rs | 32 ++++ crates/op-host-web/src/op_ck_bridge.js | 60 +++++- crates/op-host-web/tests/paint_order.rs | 62 ++++++- 11 files changed, 806 insertions(+), 21 deletions(-) create mode 100644 crates/op-editor-core/src/text_script.rs create mode 100644 crates/op-editor-core/src/text_script_tests.rs create mode 100644 crates/op-host-native/src/backend/skia/complex_script_tests.rs create mode 100644 crates/op-host-native/src/backend/skia/shaped_text.rs diff --git a/crates/op-editor-core/src/lib.rs b/crates/op-editor-core/src/lib.rs index 7230afea4..102c50d91 100644 --- a/crates/op-editor-core/src/lib.rs +++ b/crates/op-editor-core/src/lib.rs @@ -144,6 +144,8 @@ pub fn svg_path_data_bounds(d: &str) -> Option<(f32, f32, f32, f32)> { } pub mod text_edit; pub mod text_input_focus; +pub mod text_script; +mod text_script_tests; pub mod theme_presets; pub mod tool; pub mod toolbar_state; diff --git a/crates/op-editor-core/src/text_script.rs b/crates/op-editor-core/src/text_script.rs new file mode 100644 index 000000000..c6493e189 --- /dev/null +++ b/crates/op-editor-core/src/text_script.rs @@ -0,0 +1,113 @@ +//! Complex-script detection + base-direction resolution. +//! +//! Both paint backends keep a fast path that maps codepoints straight to +//! glyphs (`Canvas::draw_str` on native, `canvas.drawText` on CanvasKit). +//! That path does no bidi reordering and no contextual glyph selection, so +//! it renders Arabic in storage order with isolated letterforms. It is +//! still the right path for Latin and CJK, which are 1:1 cmap lookups — +//! routing those through a shaper costs frame time for no visual gain. +//! +//! These predicates are the fork between the two paths. They live here, in +//! the wasm32-clean core, so the native and web hosts make the identical +//! routing decision and — critically — so paint and measurement can never +//! disagree about which path a run takes. + +/// Resolved paragraph direction, per UAX#9 rules P2/P3. +#[derive(Debug, Clone, Copy, PartialEq, Eq)] +pub enum BaseDirection { + Ltr, + Rtl, +} + +/// Lowest codepoint in any table below. Latin, Greek, Cyrillic and Armenian +/// all sit beneath it, so the overwhelmingly common case rejects +/// on one compare per character instead of walking the range tables. +const FIRST_COMPLEX_CODEPOINT: u32 = 0x0600; + +/// Right-to-left scripts. These need bidi reordering; the Arabic-family +/// ones additionally need contextual joining. +const RTL_BLOCKS: &[(u32, u32)] = &[ + (0x0600, 0x06FF), // Arabic + (0x0700, 0x074F), // Syriac + (0x0750, 0x077F), // Arabic Supplement + (0x0780, 0x07BF), // Thaana + (0x07C0, 0x07FF), // NKo + (0x0800, 0x083F), // Samaritan + (0x0840, 0x085F), // Mandaic + (0x0860, 0x086F), // Syriac Supplement + (0x0870, 0x089F), // Arabic Extended-B + (0x08A0, 0x08FF), // Arabic Extended-A + (0xFB50, 0xFDFF), // Arabic Presentation Forms-A + (0xFE70, 0xFEFF), // Arabic Presentation Forms-B +]; + +/// Left-to-right scripts that still need a shaper: conjunct formation, +/// reordered vowel signs, and stacked marks all mean the painted glyph +/// sequence differs from the stored codepoint sequence. +const COMPLEX_LTR_BLOCKS: &[(u32, u32)] = &[ + (0x0900, 0x097F), // Devanagari + (0x0980, 0x09FF), // Bengali + (0x0A00, 0x0A7F), // Gurmukhi + (0x0A80, 0x0AFF), // Gujarati + (0x0B00, 0x0B7F), // Oriya + (0x0B80, 0x0BFF), // Tamil + (0x0C00, 0x0C7F), // Telugu + (0x0C80, 0x0CFF), // Kannada + (0x0D00, 0x0D7F), // Malayalam + (0x0D80, 0x0DFF), // Sinhala + (0x0E00, 0x0E7F), // Thai + (0x0E80, 0x0EFF), // Lao + (0x0F00, 0x0FFF), // Tibetan + (0x1000, 0x109F), // Myanmar + (0x1780, 0x17FF), // Khmer +]; + +/// Explicit bidi formatting characters. Text that is otherwise entirely +/// Latin still reorders when one of these is present. +const BIDI_CONTROLS: &[(u32, u32)] = &[ + (0x200E, 0x200F), // LRM / RLM + (0x202A, 0x202E), // LRE / RLE / PDF / LRO / RLO + (0x2066, 0x2069), // LRI / RLI / FSI / PDI +]; + +fn in_blocks(cp: u32, blocks: &[(u32, u32)]) -> bool { + blocks.iter().any(|&(lo, hi)| cp >= lo && cp <= hi) +} + +fn is_strong_rtl(c: char) -> bool { + let cp = c as u32; + cp >= FIRST_COMPLEX_CODEPOINT && in_blocks(cp, RTL_BLOCKS) +} + +/// Whether `text` needs a real shaping engine (bidi reordering and/or +/// contextual glyph selection) rather than the 1:1 cmap fast path. +pub fn needs_complex_shaping(text: &str) -> bool { + text.chars().any(|c| { + let cp = c as u32; + cp >= FIRST_COMPLEX_CODEPOINT + && (in_blocks(cp, RTL_BLOCKS) + || in_blocks(cp, COMPLEX_LTR_BLOCKS) + || in_blocks(cp, BIDI_CONTROLS)) + }) +} + +/// Resolve the base paragraph direction from the first strong directional +/// character, defaulting to [`BaseDirection::Ltr`] when there is none. +/// +/// This is UAX#9 P2/P3 narrowed to what paint needs: digits, punctuation +/// and whitespace are weak or neutral and must not decide direction, so a +/// string like `"50 ك.م"` resolves right-to-left on its Arabic tail rather +/// than left-to-right on its leading digits. +pub fn base_direction(text: &str) -> BaseDirection { + for c in text.chars() { + if is_strong_rtl(c) { + return BaseDirection::Rtl; + } + // Everything else with a letter category is strong left-to-right — + // Latin, Greek, Cyrillic, CJK, and the complex-but-LTR scripts. + if c.is_alphabetic() { + return BaseDirection::Ltr; + } + } + BaseDirection::Ltr +} diff --git a/crates/op-editor-core/src/text_script_tests.rs b/crates/op-editor-core/src/text_script_tests.rs new file mode 100644 index 000000000..b4610adf0 --- /dev/null +++ b/crates/op-editor-core/src/text_script_tests.rs @@ -0,0 +1,120 @@ +//! Sibling test file for `text_script.rs` (800-line cap convention) — +//! complex-script detection and UAX#9 base-direction resolution, the fork +//! that decides whether a run is painted by the cmap fast path or a shaper. + +#![cfg(test)] + +use crate::text_script::*; + +#[test] +fn ascii_text_stays_on_the_simple_paint_path() { + assert!(!needs_complex_shaping("Design System")); +} + +#[test] +fn cjk_text_stays_on_the_simple_paint_path() { + // CJK is a 1:1 cmap lookup — no joining, no reordering. Sending it to + // the shaper would spend frame time for an identical result. + assert!(!needs_complex_shaping("设计系统")); +} + +#[test] +fn empty_text_stays_on_the_simple_paint_path() { + assert!(!needs_complex_shaping("")); +} + +#[test] +fn arabic_text_needs_shaping() { + assert!(needs_complex_shaping("العربية")); +} + +#[test] +fn devanagari_text_needs_shaping() { + assert!(needs_complex_shaping("नमस्ते")); +} + +#[test] +fn thai_text_needs_shaping() { + assert!(needs_complex_shaping("สวัสดี")); +} + +#[test] +fn arabic_presentation_forms_need_shaping() { + // U+FB50..U+FDFF and U+FE70..U+FEFF carry pre-joined Arabic glyphs. + // Joining is already baked in, but they still need bidi reordering. + assert!(needs_complex_shaping("\u{FEF2}")); +} + +#[test] +fn latin_mixed_with_arabic_needs_shaping() { + // Digits next to an Arabic unit abbreviation — "50 km". + assert!(needs_complex_shaping("50 ك.م")); +} + +#[test] +fn every_supported_complex_block_is_detected() { + // One representative letter per block the range table claims to cover, + // so the table can't silently lose an entry. + for (script, sample) in [ + ("Syriac", "\u{0710}"), + ("Thaana", "\u{0780}"), + ("NKo", "\u{07CA}"), + ("Bengali", "\u{0985}"), + ("Gurmukhi", "\u{0A05}"), + ("Gujarati", "\u{0A85}"), + ("Oriya", "\u{0B05}"), + ("Tamil", "\u{0B85}"), + ("Telugu", "\u{0C05}"), + ("Kannada", "\u{0C85}"), + ("Malayalam", "\u{0D05}"), + ("Sinhala", "\u{0D85}"), + ("Lao", "\u{0E81}"), + ("Tibetan", "\u{0F40}"), + ("Myanmar", "\u{1000}"), + ("Khmer", "\u{1780}"), + ] { + assert!( + needs_complex_shaping(sample), + "{script} must route to the shaper" + ); + } +} + +#[test] +fn explicit_bidi_controls_need_shaping() { + // A right-to-left override reorders even otherwise-Latin text, so the + // fast path would paint it in the wrong order. + assert!(needs_complex_shaping("\u{202E}abc")); +} + +#[test] +fn latin_paragraph_resolves_left_to_right() { + assert_eq!(base_direction("Hello"), BaseDirection::Ltr); +} + +#[test] +fn arabic_paragraph_resolves_right_to_left() { + assert_eq!(base_direction("مرحبا"), BaseDirection::Rtl); +} + +#[test] +fn leading_digits_do_not_decide_direction() { + // UAX#9 P2: digits are weak, so the first *strong* character decides. + // A leading number does not make this a left-to-right paragraph. + assert_eq!(base_direction("50 ك.م"), BaseDirection::Rtl); +} + +#[test] +fn leading_punctuation_does_not_decide_direction() { + assert_eq!(base_direction("«مرحبا»"), BaseDirection::Rtl); +} + +#[test] +fn first_strong_character_wins_in_mixed_text() { + assert_eq!(base_direction("Hello مرحبا"), BaseDirection::Ltr); +} + +#[test] +fn text_without_strong_characters_defaults_left_to_right() { + assert_eq!(base_direction("123 456"), BaseDirection::Ltr); +} diff --git a/crates/op-host-native/src/backend/skia.rs b/crates/op-host-native/src/backend/skia.rs index 5567ec914..23cac391f 100644 --- a/crates/op-host-native/src/backend/skia.rs +++ b/crates/op-host-native/src/backend/skia.rs @@ -73,6 +73,7 @@ mod image; mod image_diagnostics; mod layer; mod path; +mod shaped_text; mod text; #[cfg(test)] use image::{cover_rect, figma_image_local_matrix, image_adjustment_matrix}; @@ -96,6 +97,11 @@ pub struct NativeBackend { font_resolver: jian_skia::FontResolver, /// Cached Paragraph shaper used only for authored line-box baselines. paragraph_baseline: jian_skia::ParagraphBaseline, + /// Cached Paragraph shaper for complex scripts (Arabic and friends), + /// which the `draw_str` fast path cannot reorder or join. Shares the + /// same generation-guarded `FontCollection` discipline so the shaper + /// stays affordable on the paint path. + shaped_text: shaped_text::ShapedText, /// Pre-rasterized image cache keyed by stable source id. Paint only /// reads this cache; encoded bytes are decoded on desktop workers. image_cache: std::collections::HashMap, @@ -236,11 +242,13 @@ impl NativeBackend { let font_resolver = jian_skia::FontResolver::with_default_typeface(font_mgr, default_typeface); let paragraph_baseline = jian_skia::ParagraphBaseline::new(&font_resolver); + let shaped_text = shaped_text::ShapedText::new(&font_resolver); let mut this = Self { skia, dpi, font_resolver, paragraph_baseline, + shaped_text, image_cache: std::collections::HashMap::new(), image_cache_bytes: 0, image_cache_tick: 0, @@ -722,6 +730,10 @@ mod image_thumb_tests; #[path = "skia/font_fallback_tests.rs"] mod font_fallback_tests; +#[cfg(test)] +#[path = "skia/complex_script_tests.rs"] +mod complex_script_tests; + // Gated off Windows: exercises `jian_skia::register_imported_font` (skia // `FontMgr::new_from_data`, DirectWrite on Windows) from parallel test-worker // threads, which segfaults in Windows CI. Production resolves fonts on the diff --git a/crates/op-host-native/src/backend/skia/complex_script_tests.rs b/crates/op-host-native/src/backend/skia/complex_script_tests.rs new file mode 100644 index 000000000..a39a558b8 --- /dev/null +++ b/crates/op-host-native/src/backend/skia/complex_script_tests.rs @@ -0,0 +1,173 @@ +//! Complex-script paint + measure routing. +//! +//! Arabic must reach the Skia paragraph shaper (bidi reordering + contextual +//! joining) instead of the 1:1 cmap fast path, while Latin must stay on the +//! fast path so the documented chrome frame budget is unaffected. +//! +//! macOS-gated for the same reason the sibling font-fallback tests are: they +//! assert against system typefaces, and a bare Linux CI box has no Arabic +//! face to resolve. + +use super::*; +use op_editor_ui::TextLayout; + +const SIZE: f32 = 40.0; + +/// Raw unshaped width — skia's default per-codepoint advances with no +/// joining applied. This is exactly what the cmap fast path produces, so it +/// is the oracle a genuinely shaped measurement has to diverge from. +#[cfg(target_os = "macos")] +fn isolated_width(be: &mut NativeBackend, text: &str) -> f32 { + let tf = be + .typeface_for_char(text.chars().next().expect("non-empty sample"), 400) + .expect("the system covers the sample script"); + let font = skia_safe::Font::new(tf, SIZE); + font.measure_str(text, None).0 +} + +/// Ink mass either side of the painted bounding box's own midpoint. +/// Normalising against the ink box rather than the surface keeps the +/// assertion independent of where the run happens to be anchored. +#[cfg(target_os = "macos")] +fn ink_halves(pixels: &skia_safe::Pixmap, w: i32, h: i32) -> (usize, usize) { + let inked = |x: i32, y: i32| { + let c = pixels.get_color((x, y)); + c.r() < 245 || c.g() < 245 || c.b() < 245 + }; + let mut min_x = w; + let mut max_x = -1; + for y in 0..h { + for x in 0..w { + if inked(x, y) { + min_x = min_x.min(x); + max_x = max_x.max(x); + } + } + } + assert!(max_x > min_x, "the sample must paint visible ink"); + let mid = (min_x + max_x) / 2; + let (mut left, mut right) = (0usize, 0usize); + for y in 0..h { + for x in min_x..=max_x { + if inked(x, y) { + if x < mid { + left += 1; + } else { + right += 1; + } + } + } + } + (left, right) +} + +#[cfg(target_os = "macos")] +#[test] +fn arabic_measures_with_joining_applied() { + let mut be = NativeBackend::with_dpi(1.0); + + // Arabic letters join into a connected form. Isolated letterforms carry + // entry/exit flourishes that the joined run drops, so a shaped width is + // strictly narrower than the sum of default advances. + let shaped = be.measure_text_family_styled("مرحبا", SIZE, "", 400, false); + let isolated = isolated_width(&mut be, "مرحبا"); + + assert!(shaped > 0.0, "Arabic must measure to a real width"); + assert!( + shaped < isolated, + "joined Arabic ({shaped}) should be narrower than isolated ({isolated})" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn latin_measurement_stays_on_the_unshaped_fast_path() { + let mut be = NativeBackend::with_dpi(1.0); + + // Guard on the perf fork, not a driver: routing *all* text through the + // shaper (the alternative design) would change Latin advances via + // kerning and break this. Latin must keep matching raw cmap advances. + let measured = be.measure_text_family_styled("Design System", SIZE, "", 400, false); + let raw = isolated_width(&mut be, "Design System"); + + assert!( + (measured - raw).abs() < 0.01, + "Latin must stay unshaped: measured {measured}, raw {raw}" + ); +} + +#[cfg(target_os = "macos")] +#[test] +fn arabic_paints_right_to_left() { + let mut be = NativeBackend::with_dpi(1.0); + + // Six meems then a lone alef. Meems are dense and round, alef is a thin + // vertical stroke, so the ink is heavily lopsided toward the meems. + // Arabic reads right-to-left, so the meems — first in storage order — + // must paint on the RIGHT. Painting them left is precisely the + // logical-order bug this routing exists to fix. + const TEXT: &str = "مممممم ا"; + let (w, h) = (480, 120); + + let mut surface = skia_safe::surfaces::raster_n32_premul((w, h)).unwrap(); + surface.canvas().clear(skia_safe::Color::WHITE); + let layout = TextLayout::single_run(TEXT, "", SIZE, Color::BLACK.to_jian(), Point2D::ZERO); + be.draw_text(surface.canvas(), &layout, Point2D::new(20.0, 80.0)); + + let image = surface.image_snapshot(); + let pixels = image.peek_pixels().expect("peek raster pixels"); + let (left, right) = ink_halves(&pixels, w, h); + + assert!( + right > left, + "meems must paint right of the alef (left ink {left}, right ink {right})" + ); +} + +/// Visual proof sheet — not an assertion. Renders real strings from a live +/// document through the fixed `draw_text` alongside the raw `draw_str` call +/// the old code made, so the two can be compared by eye. Ignored by default. +/// cargo test -p op-host-native --lib arabic_proof_sheet -- --ignored +#[cfg(target_os = "macos")] +#[test] +#[ignore] +fn arabic_proof_sheet() { + let mut be = NativeBackend::with_dpi(1.0); + let (w, h) = (1040, 460); + let mut surface = skia_safe::surfaces::raster_n32_premul((w, h)).unwrap(); + surface.canvas().clear(skia_safe::Color::WHITE); + + let label = |be: &mut NativeBackend, canvas: &skia_safe::Canvas, s: &str, x: f32, y: f32| { + let l = TextLayout::single_run(s, "", 18.0, Color::BLACK.to_jian(), Point2D::ZERO); + be.draw_text(canvas, &l, Point2D::new(x, y)); + }; + label(&mut be, surface.canvas(), "NEW - shaped + bidi", 30.0, 34.0); + label(&mut be, surface.canvas(), "OLD - raw draw_str", 560.0, 34.0); + + // Generic samples, each covering a distinct rendering feature: + // plain joining, the definite article, a shadda diacritic, and digits + // beside an Arabic unit abbreviation ("50 km"). + let samples = ["مرحبا بالعالم", "اللغة العربية", "معلّم اللغة", "50 ك.م"]; + let size = 34.0; + for (i, s) in samples.iter().enumerate() { + let y = 110.0 + (i as f32) * 88.0; + let layout = TextLayout::single_run(s, "", size, Color::BLACK.to_jian(), Point2D::ZERO); + be.draw_text(surface.canvas(), &layout, Point2D::new(30.0, y)); + + let tf = be + .typeface_for_char(s.chars().next().unwrap(), 400) + .expect("Arabic face"); + let font = skia_safe::Font::new(tf, size); + let mut p = skia_safe::Paint::new(skia_safe::Color4f::new(0.75, 0.1, 0.1, 1.0), None); + p.set_anti_alias(true); + surface.canvas().draw_str(s, (560.0, y), &font, &p); + } + + let image = surface.image_snapshot(); + let data = image + .encode(None, skia_safe::EncodedImageFormat::PNG, 100) + .expect("encode png"); + let out = std::env::temp_dir().join("openpencil-arabic-proof.png"); + std::fs::write(&out, data.as_bytes()).expect("write png"); + println!("proof sheet written to {}", out.display()); +} diff --git a/crates/op-host-native/src/backend/skia/shaped_text.rs b/crates/op-host-native/src/backend/skia/shaped_text.rs new file mode 100644 index 000000000..b28bc44bf --- /dev/null +++ b/crates/op-host-native/src/backend/skia/shaped_text.rs @@ -0,0 +1,168 @@ +//! Shaped paint + measure for complex scripts. +//! +//! The surrounding fast path (`text.rs`) maps codepoints straight to glyphs +//! via `Canvas::draw_str`, which does no bidi reordering and no contextual +//! glyph selection — correct and cheap for Latin and CJK, wrong for Arabic. +//! Runs that `op_editor_core::text_script::needs_complex_shaping` flags come +//! here instead and go through Skia's Paragraph shaper (ICU bidi + +//! HarfBuzz). +//! +//! The reason jian's `draw_text_paragraph` was bypassed natively is that it +//! builds a fresh `FontCollection` on every call (~605 ms chrome frames). +//! This type keeps the collection cached with the same generation-guard +//! `jian_skia::ParagraphBaseline` uses, so the shaper is affordable on the +//! paint path. + +use std::rc::Rc; + +use jian_skia::FontResolver; +use op_editor_core::text_script::{base_direction, BaseDirection}; +use skia_safe::{ + font_style::{Slant, Weight, Width}, + textlayout::{FontCollection, Paragraph, ParagraphBuilder, ParagraphStyle, TextStyle}, + Canvas, FontStyle, +}; + +/// The default family jian seeds its own collections with, so a shaped run +/// falls back to the same face an unshaped one would. +const DEFAULT_FAMILY: &str = "Roboto"; + +/// Effectively unbounded wrap budget. Every run reaching paint is already a +/// single line (wrapping happens upstream in `wrap_text`), so the paragraph +/// must not break; `max_intrinsic_width` then reports the natural width. +const NATURAL_LAYOUT_BUDGET: f32 = 1.0e6; + +/// One styled run to shape. Grouped into a struct so paint and measure take +/// the identical inputs — if they ever drifted, wrapped text would be laid +/// out against one width and painted at another. +pub(crate) struct ShapedRun<'a> { + pub text: &'a str, + pub family: &'a str, + pub font_size: f32, + pub weight: u16, + pub italic: bool, + pub line_height: f32, + pub color: skia_safe::Color, +} + +pub(crate) struct ShapedText { + collection: Rc, + built_generation: u64, +} + +impl ShapedText { + pub(crate) fn new(font_resolver: &FontResolver) -> Self { + jian_skia::with_font_lock(|| Self { + // Read the generation before building: a concurrent font + // registration can at worst leave this stale-low, costing one + // harmless rebuild. + built_generation: jian_skia::font_generation(), + collection: Rc::new(build_collection(font_resolver)), + }) + } + + fn collection(&mut self, font_resolver: &FontResolver) -> Rc { + let generation = jian_skia::font_generation(); + if generation != self.built_generation { + self.collection = Rc::new(build_collection(font_resolver)); + self.built_generation = generation; + } + Rc::clone(&self.collection) + } + + /// Natural, unwrapped advance width of the shaped run. + pub(crate) fn measure(&mut self, font_resolver: &FontResolver, run: &ShapedRun<'_>) -> f32 { + jian_skia::with_font_lock(|| { + let mut paragraph = self.build(font_resolver, run); + paragraph.layout(NATURAL_LAYOUT_BUDGET); + paragraph.max_intrinsic_width() + }) + } + + /// Paint the run with `baseline_y` on its alphabetic baseline, matching + /// the `draw_str` convention the unshaped path uses, so shaped and + /// unshaped text sit on one line. + pub(crate) fn paint( + &mut self, + canvas: &Canvas, + font_resolver: &FontResolver, + run: &ShapedRun<'_>, + x: f32, + baseline_y: f32, + ) { + jian_skia::with_font_lock(|| { + let mut paragraph = self.build(font_resolver, run); + // Two passes. A right-to-left paragraph anchors against the RIGHT + // edge of its layout box, so laying out once against the probe + // budget would fling Arabic a million pixels off-screen. Measure + // the natural width first, then re-lay out into a box of exactly + // that width so the run occupies the same span at `x` either way. + // + // `ceil` rather than the raw width: a box even a float-epsilon + // narrower than the text makes the shaper wrap, turning one line + // into two. Rounding up trades a sub-pixel anchor shift for + // never breaking a line that should not break. + paragraph.layout(NATURAL_LAYOUT_BUDGET); + let natural = paragraph.max_intrinsic_width(); + paragraph.layout(natural.ceil()); + // `Paragraph::paint` places the line-box top; callers hand us a + // baseline. + let top = baseline_y - paragraph.alphabetic_baseline(); + paragraph.paint(canvas, (x, top)); + }); + } + + fn build(&mut self, font_resolver: &FontResolver, run: &ShapedRun<'_>) -> Paragraph { + let collection = self.collection(font_resolver); + let mut paragraph_style = ParagraphStyle::new(); + // Without this the shaper bidi-reorders correctly but anchors the + // paragraph left-to-right, so a trailing neutral (the "." in an + // abbreviation like "ك.م") lands on the wrong end. + paragraph_style.set_text_direction(match base_direction(run.text) { + BaseDirection::Rtl => skia_safe::textlayout::TextDirection::RTL, + BaseDirection::Ltr => skia_safe::textlayout::TextDirection::LTR, + }); + + let mut text_style = TextStyle::new(); + text_style.set_font_size(run.font_size); + text_style.set_color(run.color); + let families = font_resolver.font_families_for_shaping(if run.family.is_empty() { + None + } else { + Some(run.family) + }); + let family_refs = families.iter().map(String::as_str).collect::>(); + if !family_refs.is_empty() { + text_style.set_font_families(&family_refs); + } + text_style.set_font_style(FontStyle::new( + Weight::from(i32::from(run.weight)), + Width::NORMAL, + if run.italic { + Slant::Italic + } else { + Slant::Upright + }, + )); + if run.line_height > 0.0 { + text_style.set_height(run.line_height); + text_style.set_height_override(true); + text_style.set_half_leading(true); + } + + let mut builder = ParagraphBuilder::new(¶graph_style, (*collection).clone()); + builder.push_style(&text_style); + builder.add_text(run.text); + builder.pop(); + builder.build() + } +} + +fn build_collection(font_resolver: &FontResolver) -> FontCollection { + let mut collection = FontCollection::new(); + collection.set_default_font_manager(font_resolver.ordered_font_manager(), Some(DEFAULT_FAMILY)); + if let Some(provider) = jian_skia::bundled_fonts::asset_provider() { + collection.set_asset_font_manager(Some(provider.into())); + } + collection +} diff --git a/crates/op-host-native/src/backend/skia/text.rs b/crates/op-host-native/src/backend/skia/text.rs index 89b478aa5..0fa423bd8 100644 --- a/crates/op-host-native/src/backend/skia/text.rs +++ b/crates/op-host-native/src/backend/skia/text.rs @@ -4,8 +4,10 @@ //! support landed; the typeface caches stay fields on the spine //! struct, this sibling only houses the `impl` block. +use op_editor_core::text_script::needs_complex_shaping; use op_editor_ui::{Point2D, TextBaselineRequest, TextLayout}; +use super::shaped_text::ShapedRun; use super::NativeBackend; pub(super) fn draw_text_runs(layout: &TextLayout) -> &[jian_core::render::TextRun] { @@ -90,6 +92,24 @@ impl NativeBackend { weight: u16, italic: bool, ) -> f32 { + if needs_complex_shaping(text) { + // Paint sends this run to the shaper, so measurement has to go + // the same way. Measuring joined Arabic as a sum of isolated + // advances would wrap lines and place carets against widths the + // painter never uses. + return self.shaped_text.measure( + &self.font_resolver, + &ShapedRun { + text, + family, + font_size, + weight, + italic, + line_height: 0.0, + color: skia_safe::Color::BLACK, + }, + ); + } self.font_resolver .measure_text(text, font_size, Some(family), weight, italic) } @@ -172,6 +192,30 @@ impl NativeBackend { jian_skia::with_font_lock(|| { let italic = layout.italic(); for run in draw_text_runs(layout) { + let jc = run.color; + if needs_complex_shaping(run.content.as_str()) { + // Arabic and friends need bidi reordering plus contextual + // joining, neither of which `draw_str` performs. Hand the + // whole run to the cached Paragraph shaper; segmentation + // would defeat shaping anyway, since joining is decided + // across the run, not per typeface span. + self.shaped_text.paint( + canvas, + &self.font_resolver, + &ShapedRun { + text: run.content.as_str(), + family: run.font_family.as_str(), + font_size: run.font_size, + weight: run.font_weight, + italic, + line_height: run.line_height, + color: skia_safe::Color::from_argb(jc.a(), jc.r(), jc.g(), jc.b()), + }, + origin.x + run.origin.x, + origin.y + run.origin.y, + ); + continue; + } let segments = self.font_resolver.segment_text( run.content.as_str(), Some(&run.font_family), @@ -181,7 +225,6 @@ impl NativeBackend { if segments.is_empty() { continue; } - let jc = run.color; let mut paint = skia_safe::Paint::new( skia_safe::Color4f::new( f32::from(jc.r()) / 255.0, diff --git a/crates/op-host-web/src/canvaskit/backend.rs b/crates/op-host-web/src/canvaskit/backend.rs index fc4953892..9f2e1e026 100644 --- a/crates/op-host-web/src/canvaskit/backend.rs +++ b/crates/op-host-web/src/canvaskit/backend.rs @@ -380,6 +380,30 @@ impl RenderBackend for CanvasKitBackend { // `TextRun.color` is `jian_core::scene::Color` (0-255 u8 channels), // unlike the f32-channel `Color` the fill ops take. let c = run.color; + let (cr, cg, cb, ca) = ( + f32::from(c.r()) / 255.0, + f32::from(c.g()) / 255.0, + f32::from(c.b()) / 255.0, + f32::from(c.a()) / 255.0, + ); + // Same predicate the native host uses, so the two never disagree + // about which runs need bidi + contextual joining. + if op_editor_core::text_script::needs_complex_shaping(run.content.as_str()) { + self.ck.draw_shaped_text( + run.content.as_str(), + run.font_family.as_str(), + x, + y, + run.font_size, + run.font_weight as i32, + italic, + cr, + cg, + cb, + ca, + ); + continue; + } self.ck.draw_text( run.content.as_str(), run.font_family.as_str(), @@ -388,10 +412,10 @@ impl RenderBackend for CanvasKitBackend { run.font_size, run.font_weight as i32, italic, - f32::from(c.r()) / 255.0, - f32::from(c.g()) / 255.0, - f32::from(c.b()) / 255.0, - f32::from(c.a()) / 255.0, + cr, + cg, + cb, + ca, ); } } @@ -615,6 +639,14 @@ impl RenderBackend for CanvasKitBackend { weight: u16, italic: bool, ) -> f32 { + if op_editor_core::text_script::needs_complex_shaping(text) { + // Paint routes this run to the browser shaper, so measurement has + // to as well — otherwise wrapping and carets are computed against + // advances the painter never uses. + return self + .ck + .measure_shaped_text(text, family, font_size, i32::from(weight), italic); + } self.ck .measure_text_family_styled(text, family, font_size, i32::from(weight), italic) } diff --git a/crates/op-host-web/src/canvaskit/bindings.rs b/crates/op-host-web/src/canvaskit/bindings.rs index 7222f4123..686baac52 100644 --- a/crates/op-host-web/src/canvaskit/bindings.rs +++ b/crates/op-host-web/src/canvaskit/bindings.rs @@ -429,6 +429,38 @@ extern "C" { weight: i32, italic: bool, ) -> f32; + /// Paint a complex-script run (Arabic and friends) through the browser's + /// own text engine, which resolves bidi and contextual joining that + /// CanvasKit's `drawText` cmap lookup cannot. The run is passed whole — + /// segmenting it first would reorder within segments but lay the segments + /// out in storage order. + #[wasm_bindgen(method, js_name = drawShapedText)] + #[allow(clippy::too_many_arguments)] + pub(super) fn draw_shaped_text( + this: &OpCk, + t: &str, + family: &str, + x: f32, + y: f32, + sz: f32, + weight: i32, + italic: bool, + r: f32, + g: f32, + b: f32, + a: f32, + ); + /// Measure a complex-script run on the same engine `draw_shaped_text` + /// paints with, so wrap decisions and caret geometry match the glyphs. + #[wasm_bindgen(method, js_name = measureShapedText)] + pub(super) fn measure_shaped_text( + this: &OpCk, + t: &str, + family: &str, + sz: f32, + weight: i32, + italic: bool, + ) -> f32; #[wasm_bindgen(method, js_name = registerSystemFont)] pub(super) fn register_system_font(this: &OpCk, family: &str, bytes: &[u8]) -> bool; /// Register a user-imported font face; the family becomes selectable by diff --git a/crates/op-host-web/src/op_ck_bridge.js b/crates/op-host-web/src/op_ck_bridge.js index 542db5090..d3b1b1806 100644 --- a/crates/op-host-web/src/op_ck_bridge.js +++ b/crates/op-host-web/src/op_ck_bridge.js @@ -336,12 +336,32 @@ export async function opCkInit(canvasId) { // rather than a second cache since text draws are far less frequent than // the primitive-shape hot path this task targets. const allocFillPaint = (r, g, b, a) => { const p = new CK.Paint(); p.setColor(col(r, g, b, a)); p.setAntiAlias(true); setPaintStyle(p, CK.PaintStyle.Fill); return p; }; - const browserTextFont = (sz, weight, italic) => `${italic ? 'italic ' : ''}${Math.max(100, Math.min(900, Math.round(weight || 400)))} ${Math.max(1, sz)}px ${browserTextFontStack}`; + // Quote the authored family so names with spaces stay one CSS token, and + // strip quote/backslash so a family name can't break out of the shorthand. + // Only families the BROWSER knows resolve here; faces registered into + // CanvasKit as raw bytes are invisible to CSS and fall through the stack. + const cssFamilyToken = (family) => { + const name = String(family || '').trim().replace(/["\\]/g, ''); + return name ? `"${name}", ` : ''; + }; + const browserTextFont = (sz, weight, italic, family) => `${italic ? 'italic ' : ''}${Math.max(100, Math.min(900, Math.round(weight || 400)))} ${Math.max(1, sz)}px ${cssFamilyToken(family)}${browserTextFontStack}`; const shouldUseBrowserTextFallback = (_t, _emojiRun) => Boolean(browserTextCtx); + // Complex scripts (Arabic and friends — see `op_editor_core::text_script`, + // which owns the predicate for BOTH hosts). CanvasKit's `canvas.drawText` + // is a 1:1 cmap lookup: no bidi reordering, no contextual joining, so it + // paints Arabic in storage order with isolated letterforms. The browser's + // own 2D text engine does both correctly and `drawBrowserText` already + // wraps it, so complex runs are sent there instead. + // + // They must go WHOLE. `drawScriptRun` splits a run into script segments and + // advances left to right, so it would resolve bidi within each segment and + // then lay the segments out in storage order — a mixed digit/Arabic run + // like "50 ك.م" would still come out wrong. + const shapedTextUnavailable = () => !browserTextCtx; const allSegmentsUseBrowserTextFallback = (segs) => segs.length > 0 && segs.every((seg) => shouldUseBrowserTextFallback(seg.text, seg.emoji)); - const browserTextMeasure = (t, sz, weight = 400, italic = false) => { + const browserTextMeasure = (t, sz, weight = 400, italic = false, family = '') => { if (!browserTextCtx) return 0; - browserTextCtx.font = browserTextFont(sz, weight, italic); + browserTextCtx.font = browserTextFont(sz, weight, italic, family); return browserTextCtx.measureText(t).width; }; const effectiveTextScale = () => { @@ -354,19 +374,21 @@ export async function opCkInit(canvasId) { // a SrcIn tint at draw time so differently coloured runs share a bitmap. // Emoji retain their legacy RGBA-keyed, untinted raster path because colour // glyphs can ignore fillStyle. Cache-hit reinsertion keeps eviction LRU. - const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a) => { + const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a, family = '') => { if (!browserTextCtx) return null; const ss = effectiveTextScale(); + // `family` participates in the key: the same string shaped in two + // different families is two different bitmaps. const key = emoji - ? ['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss].join('\n') - : [t, sz, weight, italic ? 1 : 0, ss].join('\n'); + ? ['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss, family].join('\n') + : [t, sz, weight, italic ? 1 : 0, ss, family].join('\n'); const hit = browserTextCache.get(key); if (hit) { browserTextCache.delete(key); browserTextCache.set(key, hit); return hit; } - const font = browserTextFont(sz, weight, italic); + const font = browserTextFont(sz, weight, italic, family); browserTextCtx.font = font; const metrics = browserTextCtx.measureText(t); // Logical (CSS-px) box the glyphs occupy; positioning stays in CSS units. @@ -443,8 +465,8 @@ export async function opCkInit(canvasId) { // allocFillPaint exists). antiAlias stays at the CK.Paint default (false) so // image sampling matches the prior no-paint drawImage exactly. const cachedTintPaint = new CK.Paint(); - const drawBrowserText = (t, x, y, sz, weight, italic, r, g, b, a, emoji) => { - const entry = browserTextImage(t, sz, weight, italic, emoji, r, g, b, a); + const drawBrowserText = (t, x, y, sz, weight, italic, r, g, b, a, emoji, family = '') => { + const entry = browserTextImage(t, sz, weight, italic, emoji, r, g, b, a, family); if (!entry) return 0; // Emoji rasters bake their own colour and draw UNTINTED (legacy path); // text rasters are white masks tinted here. For text: tint to (r,g,b) via a @@ -1219,6 +1241,26 @@ export async function opCkInit(canvasId) { } } }, + drawShapedText(t, family, x, y, sz, weight, italic, r, g, b, a) { + if (!t) return; + if (shapedTextUnavailable()) { + // No 2D context (headless embed / hostile sandbox). The segmented + // path at least paints glyphs rather than nothing, though it cannot + // reorder or join them. + drawScriptRun(t, x, y, sz, weight, italic, r, g, b, a); + return; + } + drawBrowserText(t, x, y, sz, weight, italic, r, g, b, a, false, family); + }, + measureShapedText(t, family, sz, weight, italic) { + if (!t) return 0; + // Must mirror `drawShapedText` exactly — measuring on one engine and + // painting on another is what desyncs wrapping and caret geometry. + if (shapedTextUnavailable()) { + return this.measureTextFamilyStyled(t, family, sz, weight, italic); + } + return browserTextMeasure(t, sz, weight, italic, family); + }, measureText(t, sz) { return this.measureTextStyled(t, sz, 400, false); }, diff --git a/crates/op-host-web/tests/paint_order.rs b/crates/op-host-web/tests/paint_order.rs index 832e35f24..c59271eec 100644 --- a/crates/op-host-web/tests/paint_order.rs +++ b/crates/op-host-web/tests/paint_order.rs @@ -453,7 +453,7 @@ fn canvaskit_browser_text_raster_key_drops_color_and_alpha() { // TEXT segments are keyed on (text, size, weight, italic, supersample) // only — colour is applied at draw time, so N colours reuse ONE raster. assert!( - source.contains(": [t, sz, weight, italic ? 1 : 0, ss].join('\\n');"), + source.contains(": [t, sz, weight, italic ? 1 : 0, ss, family].join('\\n');"), "text raster cache key must contain no colour/alpha component" ); assert!( @@ -469,11 +469,13 @@ fn canvaskit_browser_text_raster_key_drops_color_and_alpha() { // Emoji / colour-glyph segments keep the exact legacy baked-colour path // (they ignore fillStyle, so tinting a white mask would flatten them). assert!( - source.contains("const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a) =>"), + source.contains( + "const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a, family = '') =>" + ), "browserTextImage must take an `emoji` selector + colour for the legacy emoji path" ); assert!( - source.contains("['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss].join('\\n')"), + source.contains("['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss, family].join('\\n')"), "emoji segments must keep a colour-keyed raster (legacy baked-colour path)" ); assert!( @@ -494,7 +496,7 @@ fn canvaskit_text_rasters_refresh_at_effective_scale() { .find("const effectiveTextScale = () =>") .expect("effective text scale helper exists"); let image_fn = source - .find("const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a) =>") + .find("const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a, family = '') =>") .expect("browserTextImage exists"); let image_end = source[image_fn..] .find("const TINT_FILTER_CACHE_CAP = 64;") @@ -525,8 +527,8 @@ fn canvaskit_text_rasters_refresh_at_effective_scale() { } for marker in [ - "['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss].join('\\n')", - ": [t, sz, weight, italic ? 1 : 0, ss].join('\\n');", + "['e', t, sz, weight, italic ? 1 : 0, r, g, b, a, ss, family].join('\\n')", + ": [t, sz, weight, italic ? 1 : 0, ss, family].join('\\n');", ] { assert!( image_body.contains(marker), @@ -594,7 +596,7 @@ fn canvaskit_browser_text_cache_is_lru() { // A cache hit re-inserts the entry (delete + set) so eviction drops the // least-recently-used run, not the oldest-inserted (FIFO). let image_fn = source - .find("const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a) =>") + .find("const browserTextImage = (t, sz, weight, italic, emoji, r, g, b, a, family = '') =>") .expect("browserTextImage marker exists"); let hit = source[image_fn..] .find("const hit = browserTextCache.get(key);") @@ -749,3 +751,49 @@ fn canvaskit_source() -> String { } parts.join("\n") } + +#[test] +fn canvaskit_bridge_routes_complex_scripts_through_the_browser_shaper() { + let source = std::fs::read_to_string(format!( + "{}/src/op_ck_bridge.js", + env!("CARGO_MANIFEST_DIR") + )) + .expect("CanvasKit bridge source is readable"); + + // CanvasKit's `canvas.drawText` maps codepoints straight to glyphs — no + // bidi, no joining — so Arabic would paint in storage order. The browser's + // own 2D text engine does both, and the bridge already wraps it. The whole + // run must reach it in ONE call: the script-segmented path draws segment + // by segment left to right, which breaks bidi across segment boundaries. + for marker in [ + "drawShapedText(t, family, x, y, sz, weight, italic, r, g, b, a)", + "measureShapedText(t, family, sz, weight, italic)", + "shapedTextUnavailable", + ] { + assert!( + source.contains(marker), + "CanvasKit bridge must preserve `{marker}` so complex scripts reach the browser shaper unsegmented" + ); + } +} + +#[test] +fn canvaskit_backend_asks_the_shared_predicate_which_text_path_to_use() { + // Spans the whole `canvaskit/` module set: the externs live in + // `bindings.rs` and the routing in `backend.rs`. + let backend = canvaskit_source(); + + // The routing decision lives in `op_editor_core::text_script` so native and + // web can never disagree about which runs get shaped, and so paint and + // measure on THIS host can never disagree either. + for marker in [ + "needs_complex_shaping", + "draw_shaped_text", + "measure_shaped_text", + ] { + assert!( + backend.contains(marker), + "CanvasKit backend must reference `{marker}` to route complex scripts to the shaper" + ); + } +}