From 9ef4643ba104acc08cd3a4fce8cfea72253654b1 Mon Sep 17 00:00:00 2001 From: bluebear94 Date: Mon, 26 Jun 2023 07:40:21 -0400 Subject: [PATCH] shape_tofus: respect text direction when inserting tofus (#1519) This ensures that the ranges of the shaped glyphs are monotonically decreasing in right-to-left-text, thus avoiding nonsensical results in find_safe_to_break that later causes a panic (see #1373). Additionally, debug assertions have been added to catch non-monotonic glyph ranges. --- library/src/text/shaping.rs | 57 +++++++++++++++++++++++++++++++++- tests/ref/bugs/bidi-tofus.png | Bin 0 -> 513 bytes tests/typ/bugs/bidi-tofus.typ | 7 +++++ 3 files changed, 63 insertions(+), 1 deletion(-) create mode 100644 tests/ref/bugs/bidi-tofus.png create mode 100644 tests/typ/bugs/bidi-tofus.typ diff --git a/library/src/text/shaping.rs b/library/src/text/shaping.rs index 5a1724aba..469928d76 100644 --- a/library/src/text/shaping.rs +++ b/library/src/text/shaping.rs @@ -1,5 +1,7 @@ use std::borrow::Cow; +use std::cmp::Ordering; use std::ops::Range; +use std::panic; use std::str::FromStr; use az::SaturatingAs; @@ -58,6 +60,12 @@ pub struct ShapedGlyph { /// The byte range of this glyph's cluster in the full paragraph. A cluster /// is a sequence of one or multiple glyphs that cannot be separated and /// must always be treated as a union. + /// + /// The range values of the glyphs in a [`ShapedText`] should not + /// overlap with each other, and they should be monotonically + /// increasing (for left-to-right or top-to-bottom text) or + /// monotonically decreasing (for right-to-left or bottom-to-top + /// text). pub range: Range, /// Whether splitting the shaping result before this glyph would yield the /// same results as shaping the parts to both sides of `text_index` @@ -388,6 +396,11 @@ impl<'a> ShapedText<'a> { ) -> ShapedText<'a> { let text = &self.text[text_range.start - self.base..text_range.end - self.base]; if let Some(glyphs) = self.slice_safe_to_break(text_range.clone()) { + debug_assert!( + all_glyphs_in_range(glyphs, &text_range), + "one or more glyphs in {:?} fell out of range", + text + ); Self { base: text_range.start, text, @@ -559,6 +572,14 @@ pub fn shape<'a>( track_and_space(&mut ctx); calculate_adjustability(&mut ctx, lang, region); + debug_assert!( + all_glyphs_in_range(&ctx.glyphs, &(base..(base + text.len()))), + "one or more glyphs in {:?} fell out of range", + text + ); + #[cfg(debug_assertions)] + assert_glyph_ranges_in_order(&ctx.glyphs, dir); + ShapedText { base, text, @@ -708,7 +729,7 @@ fn shape_segment( /// Shape the text with tofus from the given font. fn shape_tofus(ctx: &mut ShapingContext, base: usize, text: &str, font: Font) { let x_advance = font.advance(0).unwrap_or_default(); - for (cluster, c) in text.char_indices() { + let add_glyph = |(cluster, c): (usize, char)| { let start = base + cluster; let end = start + c.len_utf8(); ctx.glyphs.push(ShapedGlyph { @@ -723,6 +744,11 @@ fn shape_tofus(ctx: &mut ShapingContext, base: usize, text: &str, font: Font) { c, span: ctx.spans.span_at(start), }); + }; + if ctx.dir.is_positive() { + text.char_indices().for_each(add_glyph); + } else { + text.char_indices().rev().for_each(add_glyph); } } @@ -916,3 +942,32 @@ fn language(styles: StyleChain) -> rustybuzz::Language { } rustybuzz::Language::from_str(&bcp).unwrap() } + +/// Returns true if all glyphs in `glyphs` have ranges within the range `range`. +#[cfg(debug_assertions)] +fn all_glyphs_in_range(glyphs: &[ShapedGlyph], range: &Range) -> bool { + glyphs + .iter() + .all(|g| g.range.start >= range.start && g.range.end <= range.end) +} + +/// Asserts that the ranges of `glyphs` is in the proper order according to `dir`. +/// +/// This asserts instead of returning a bool in order to provide a more informative message when the invariant is violated. +fn assert_glyph_ranges_in_order(glyphs: &[ShapedGlyph], dir: Dir) { + // Iterator::is_sorted and friends are unstable as of Rust 1.70.0 + if !glyphs.is_empty() { + for i in 0..(glyphs.len() - 1) { + let a = &glyphs[i]; + let b = &glyphs[i + 1]; + let ord = a.range.start.cmp(&b.range.start); + let ord = if dir.is_positive() { ord } else { ord.reverse() }; + if ord == Ordering::Greater { + panic!( + "glyph ranges should be monotonically {}, but found glyphs out of order:\n\nfirst: {a:#?}\nsecond: {b:#?}", + if dir.is_positive() { "increasing" } else { "decreasing" } + ); + } + } + } +} diff --git a/tests/ref/bugs/bidi-tofus.png b/tests/ref/bugs/bidi-tofus.png new file mode 100644 index 0000000000000000000000000000000000000000..1b7a7d8b4be900f4e874458f731604f2bda17477 GIT binary patch literal 513 zcmeAS@N?(olHy`uVBq!ia0y~yU}OQZ^EiM6!;@;GIH14@PZ!6Kid%1Q*?Oe}inu;J zF4;KoeP#%A#)tLHhT=lTQ=+X925c+s1y`aV>asNa>Xbpz@@v#jWHsR;A3HLVm{dTz@b3FLO=$SYhKkdzjx(ALHWGDw@hAc3jJG<{XXzr zq5h@NKhp0ipT}0+uk0%kb-(bmwWM&RM|f#Ve{*|#Gr#M$Q_3$=CE`Gic2rmYbOPwc z--i!Z_oo0I&-?l3hM<(yHiv%t%$lpcDOC30^2|y7SCnFp3T}H|^!cqvcwx(3`PpU4 zv0DY}%$F^z@CQbifC@I}K^_02|8)llNd4wMpZ{3ne`iM_C~iGn{an^LB{Ts5O3B%; literal 0 HcmV?d00001 diff --git a/tests/typ/bugs/bidi-tofus.typ b/tests/typ/bugs/bidi-tofus.typ new file mode 100644 index 000000000..3b43b2805 --- /dev/null +++ b/tests/typ/bugs/bidi-tofus.typ @@ -0,0 +1,7 @@ +// Test that shaping missing characters in both left-to-right and +// right-to-left directions does not cause a crash. + +--- +#"\u{590}\u{591}\u{592}\u{593}" + +#"\u{30000}\u{30001}\u{30002}\u{30003}"