Read the Script property for every character, and match HarfBuzz's default cluster level

docs/STABILITY.md

@@ -86,9 +86,9 @@ signatures only change in a major version.
     `Buffer::{set_insert_dotted_circle, insert_dotted_circle}` pair that 0.22.0
     development builds had.
   - `ClusterLevel` and `Buffer::{set_cluster_level, cluster_level}`: HarfBuzz's four
-    cluster levels. A Rust `Buffer` defaults to `MonotoneCharacters`; the C API
-    defaults to HarfBuzz's `MONOTONE_GRAPHEMES`. Both settings survive
-    `Buffer::clear`.
+    cluster levels. A Rust `Buffer` and the C API both default to HarfBuzz's
+    `MonotoneGraphemes` (`MONOTONE_GRAPHEMES`); before 0.24.0 a Rust `Buffer`
+    defaulted to `MonotoneCharacters`. Both settings survive `Buffer::clear`.
 
 `sigilbuzz-render`:
 

src/buffer.rs

@@ -198,7 +198,7 @@ pub struct Buffer {
     /// `true` once a caller picked the direction with
     /// [`Buffer::set_direction`]. While `false`, `direction` is only
     /// the LTR default and `shape()` may choose vertical layout for
-    /// Mongolian-dominant text. [`Buffer::clear`] resets it.
+    /// Mongolian text. [`Buffer::clear`] resets it.
     pub(crate) direction_explicit: bool,
     /// Script the whole buffer shapes as, set by
     /// [`Buffer::set_script`]. `None` segments the text into script
@@ -277,8 +277,8 @@ impl Buffer {
     /// [`Self::direction`] goes back to the [`Direction::Ltr`] default
     /// and [`Self::has_explicit_direction`] to `false`, so
     /// [`crate::shape`] once again picks the layout itself (vertical for
-    /// Mongolian-dominant text). The text, script, language, and
-    /// context are kept.
+    /// text that starts with a Mongolian character). The text, script,
+    /// language, and context are kept.
     ///
     /// ```
     /// use sigilbuzz::{Buffer, Direction};
@@ -300,9 +300,11 @@ impl Buffer {
     /// [`Self::set_direction`], false while
     /// [`Self::direction`] only reports the LTR default.
     ///
-    /// [`crate::shape`] lays out Mongolian-dominant text vertically
-    /// (top to bottom) only while no direction is explicit; an
-    /// explicit [`Direction::Ltr`] keeps it horizontal.
+    /// [`crate::shape`] lays out text that starts with a character of
+    /// the Mongolian script from the Mongolian block (U+1800..U+18AF)
+    /// vertically (top to bottom) only while no direction is explicit;
+    /// an explicit [`Direction::Ltr`] keeps it horizontal. HarfBuzz has
+    /// no such default.
     ///
     /// ```
     /// use sigilbuzz::{Buffer, Direction};
@@ -339,18 +341,21 @@ impl Buffer {
     }
 
     /// Splits the buffer's text into maximal script runs and yields
-    /// one `ScriptRun` per run. Consecutive codepoints sharing the
-    /// same resolved script collapse into a single run; `COMMON`
-    /// (digits, punctuation, ASCII space, ZWJ/ZWNJ/bidi marks, the
-    /// tatweel, the dandas) and `INHERITED` (combining marks)
+    /// one `ScriptRun` per run. Each character takes the bucket of its
+    /// Unicode Script property ([`crate::script_of`]), and consecutive
+    /// codepoints sharing the same resolved script collapse into a
+    /// single run; `COMMON` (digits, punctuation, ASCII space,
+    /// ZWJ/ZWNJ/bidi marks, the tatweel, the dandas), `INHERITED`
+    /// (combining marks), and `UNKNOWN` (unassigned and private-use)
     /// codepoints, as the Unicode Script property gives them, extend
     /// whichever real script ran before them, matching HarfBuzz's
     /// `select_shaper_for_script` segmentation. A Hangul tone mark
     /// (U+302E, U+302F) extends the run before it too.
     ///
-    /// A leading `COMMON`/`INHERITED` span before the first real
-    /// script codepoint joins that script's run, the way HarfBuzz
-    /// gives a buffer the script of its first non-`COMMON` character.
+    /// A leading `COMMON`/`INHERITED`/`UNKNOWN` span before the first
+    /// real script codepoint joins that script's run, the way HarfBuzz
+    /// gives a buffer the script of its first character of a real
+    /// script (`hb_buffer_guess_segment_properties`).
     /// Text with no real script at all is one `Script::Other` run
     /// with the default `DFLT` priority, same treatment HarfBuzz gives
     /// a pure-digits or pure-punctuation run.
@@ -372,17 +377,11 @@ impl Buffer {
         let mut current: Option<(Script, usize)> = None;
         for (byte, ch) in self.text.char_indices() {
             let raw = script_of(ch);
-            // `COMMON` / `INHERITED` extend the previous real-script
-            // run if one exists. In sigilbuzz the only explicit bucket
-            // we keep for these characters is `Script::Other` (ASCII
-            // digits / punctuation land in `Script::Latin`; combining
-            // marks inherit their cluster base's script via the
-            // `script_of` range table). So the only codepoints we
-            // still need to actively extend are the Unicode format
-            // characters (ZWJ/ZWNJ/LRM/RLM/ALM) plus any other char
-            // that `script_of` could not classify. Everything with a
-            // real script bucket attaches normally through the
-            // script-equality test below.
+            // `COMMON` / `INHERITED` / `UNKNOWN` extend the previous
+            // real-script run if one exists (their `script_of` bucket
+            // is `Script::Other`). Everything with a real script
+            // bucket attaches through the script-equality test
+            // below.
             let resolved = if is_common_or_inherited(ch) {
                 current.map_or(leading, |(s, _)| s)
             } else if is_hangul_tone_mark(ch) {
@@ -517,9 +516,8 @@ pub fn script_priority_for(script: Script) -> &'static [[u8; 4]] {
         Script::Cyrillic => CYRL_PRIORITY,
         Script::Greek => GREK_PRIORITY,
         Script::Han => HANI_PRIORITY,
-        // The buckets that take their code points from the Unicode
-        // Script property try their own tag. Scripts sigilbuzz has no
-        // bucket for fall back to DFLT.
+        // The buckets added in 0.22.0 try their own tag. Scripts
+        // sigilbuzz has no bucket for fall back to DFLT.
         other => other.table_script_priority().unwrap_or(DFLT_ONLY),
     }
 }
@@ -699,12 +697,8 @@ mod tests {
 
     #[test]
     fn script_runs_common_digits_stick_to_preceding_script() {
-        // "Price: ₪100 שלום": digits land in Script::Latin bucket
-        // via script_of, so they extend the Latin prefix. The shekel
-        // sign U+20AA falls outside our range table (Script::Other)
-        // but still extends the previous run because it is a COMMON
-        // codepoint in Unicode; sigilbuzz groups it with Latin here
-        // because `Script::Other` matches nothing-scripted neighbors.
+        // "Price: 100 שלום": the colon, the spaces, and the digits
+        // are COMMON, so they extend the Latin prefix.
         let mut b = Buffer::new();
         b.push_str("Price: 100 \u{05E9}\u{05DC}\u{05D5}\u{05DD}");
         let runs = b.script_runs();

src/buffer/flags.rs

@@ -250,31 +250,34 @@ impl SubAssign for BufferFlags {
 ///   clusters and a reordered glyph keeps its own offset, so clusters
 ///   can come out of order.
 ///
-/// HarfBuzz defaults to [`Self::MonotoneGraphemes`] (the C API does
-/// too). A Rust [`Buffer`] defaults to [`Self::MonotoneCharacters`],
-/// the level closest to what sigilbuzz produced before it supported
-/// cluster levels.
+/// The default is [`Self::MonotoneGraphemes`], as in HarfBuzz and the
+/// C API. Before 0.24.0 a Rust [`Buffer`] defaulted to
+/// [`Self::MonotoneCharacters`], which gives a combining mark (and the
+/// other characters a grapheme merges) a cluster of its own; call
+/// [`Buffer::set_cluster_level`] with it to keep those clusters.
 ///
 /// # Examples
 ///
 /// ```
 /// use sigilbuzz::{Buffer, ClusterLevel};
 ///
 /// let mut buffer = Buffer::new();
-/// assert_eq!(buffer.cluster_level(), ClusterLevel::MonotoneCharacters);
-/// buffer.set_cluster_level(ClusterLevel::MonotoneGraphemes);
+/// assert_eq!(buffer.cluster_level(), ClusterLevel::MonotoneGraphemes);
 /// assert!(buffer.cluster_level().is_monotone());
 /// assert!(buffer.cluster_level().is_graphemes());
+/// buffer.set_cluster_level(ClusterLevel::MonotoneCharacters);
+/// assert!(buffer.cluster_level().is_characters());
 /// ```
 #[derive(Debug, Default, Clone, Copy, PartialEq, Eq, Hash)]
 pub enum ClusterLevel {
     /// Characters merge into their grapheme, and clusters stay in
     /// order (`HB_BUFFER_CLUSTER_LEVEL_MONOTONE_GRAPHEMES`, HarfBuzz's
-    /// default).
+    /// default and sigilbuzz's).
+    #[default]
     MonotoneGraphemes,
     /// Every character starts with its own cluster, and clusters stay
-    /// in order (`HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS`).
-    #[default]
+    /// in order (`HB_BUFFER_CLUSTER_LEVEL_MONOTONE_CHARACTERS`). The
+    /// default of a Rust [`Buffer`] before 0.24.0.
     MonotoneCharacters,
     /// Every character keeps its own cluster, in whatever order
     /// shaping leaves them (`HB_BUFFER_CLUSTER_LEVEL_CHARACTERS`).
@@ -336,7 +339,8 @@ impl Buffer {
     }
 
     /// The cluster level set with [`Self::set_cluster_level`];
-    /// [`ClusterLevel::MonotoneCharacters`] until then.
+    /// [`ClusterLevel::MonotoneGraphemes`], HarfBuzz's default, until
+    /// then.
     #[must_use]
     pub const fn cluster_level(&self) -> ClusterLevel {
         self.cluster_level
@@ -357,14 +361,15 @@ impl Buffer {
     /// let mut buffer = Buffer::new();
     /// buffer.push_str("x\u{0301}");
     ///
-    /// // The combining acute keeps its own cluster ...
+    /// // The default grapheme level merges the combining acute into
+    /// // the cluster of its base ...
     /// let clusters = |b: &Buffer| -> Vec<u32> {
     ///     shape(&font, b, &[]).unwrap().glyphs.iter().map(|g| g.cluster).collect()
     /// };
-    /// assert_eq!(clusters(&buffer), [0, 1]);
-    /// // ... until the grapheme levels merge it into its base.
-    /// buffer.set_cluster_level(ClusterLevel::MonotoneGraphemes);
     /// assert_eq!(clusters(&buffer), [0, 0]);
+    /// // ... and the character levels give it its own.
+    /// buffer.set_cluster_level(ClusterLevel::MonotoneCharacters);
+    /// assert_eq!(clusters(&buffer), [0, 1]);
     /// # Ok::<(), sigilbuzz::Error>(())
     /// ```
     pub fn set_cluster_level(&mut self, level: ClusterLevel) {
@@ -468,7 +473,8 @@ mod tests {
     fn flags_and_level_survive_clear() {
         let mut b = Buffer::new();
         assert_eq!(b.flags(), BufferFlags::DEFAULT);
-        assert_eq!(b.cluster_level(), ClusterLevel::MonotoneCharacters);
+        assert_eq!(b.cluster_level(), ClusterLevel::MonotoneGraphemes);
+        assert_eq!(ClusterLevel::default(), ClusterLevel::MonotoneGraphemes);
         b.set_flags(BufferFlags::BOT | BufferFlags::REMOVE_DEFAULT_IGNORABLES);
         b.set_cluster_level(ClusterLevel::Characters);
         b.push_str("abc");

src/ot/mongolian.rs

@@ -28,9 +28,9 @@
 //!   initial" behavior.
 //! - **Vertical default**. Mongolian is written top-to-bottom by
 //!   default. While the caller has not chosen a direction, the
-//!   dispatcher in [`crate::shape`] lays a dominantly Mongolian run
-//!   out top to bottom: vertical metrics and the `vert` GSUB
-//!   feature. Consumers who want horizontal Mongolian set a
+//!   dispatcher in [`crate::shape`] lays text that starts with a
+//!   Mongolian character out top to bottom: vertical metrics and the
+//!   `vert` GSUB feature. Consumers who want horizontal Mongolian set a
 //!   direction explicitly with
 //!   [`crate::buffer::Buffer::set_direction`]: LTR keeps logical
 //!   order, RTL returns the run reversed like any RTL run.
@@ -109,8 +109,7 @@ pub fn assign_mongolian_forms_in_context(
 ///
 /// `codepoints` and `glyphs` start 1:1 (a glyph per codepoint, post
 /// cmap). After the call `glyphs` may have shrunk through ligature
-/// collapse. Clusters merge at the monotone characters level, the
-/// default of a Rust [`crate::Buffer`].
+/// collapse. Clusters merge at the monotone characters level.
 pub fn shape_mongolian(
     gsub: Option<&Gsub<'_>>,
     gdef: Option<&Gdef<'_>>,

src/ot/tibetan.rs

@@ -29,10 +29,10 @@ use crate::tables::Gsub;
 /// for fonts that only carry features in the default LangSys.
 pub const TIBT_SCRIPT_PRIORITY: &[[u8; 4]] = &[*b"tibt", *b"DFLT"];
 
-/// True for every codepoint that is part of the Tibetan block.
-/// Mirrors [`crate::unicode::script_of`]'s Tibetan arm so the
-/// dispatcher can use it as a fast pre-filter without going through
-/// the full script classifier.
+/// True for every codepoint that is part of the Tibetan block
+/// (U+0F00..U+0FFF), a fast block check. [`crate::unicode::script_of`]
+/// reads the Script property instead, which gives the Common symbols
+/// of the block (U+0FD5..U+0FD8) no Tibetan bucket.
 #[must_use]
 pub const fn is_tibetan(ch: char) -> bool {
     matches!(ch as u32, 0x0F00..=0x0FFF)
@@ -42,8 +42,7 @@ pub const fn is_tibetan(ch: char) -> bool {
 /// Engine, as HarfBuzz does, every GSUB feature included.
 /// `codepoints` and `glyphs` are 1:1 on entry. After the call `glyphs`
 /// may have shrunk (ligature) or grown (multiple substitution).
-/// Clusters merge at the monotone characters level, the default of a
-/// Rust [`crate::Buffer`].
+/// Clusters merge at the monotone characters level.
 pub fn shape_tibetan(
     gsub: Option<&Gsub<'_>>,
     gdef: Option<&Gdef<'_>>,

src/ot/use_shaper/scripts.rs

@@ -87,7 +87,7 @@ fn merge_jamo_syllables(codepoints: &[char], glyphs: &mut [Glyph], level: Cluste
 /// `nko ` script tags, whose `isol`, `init`, `medi`, and `fina` follow
 /// the Arabic-style joining forms of the letters, as in HarfBuzz
 /// (`setup_masks_arabic_plan`). Clusters merge at the monotone
-/// characters level, the default of a Rust [`crate::Buffer`].
+/// characters level.
 pub fn shape_nko(
     gsub: Option<&Gsub<'_>>,
     gdef: Option<&Gdef<'_>>,

src/shape.rs

@@ -81,8 +81,8 @@
 //!
 //! When the caller never set a direction
 //! ([`crate::Buffer::has_explicit_direction`] is false) the buffer
-//! shapes as LTR, except that a Mongolian-dominant run switches to
-//! vertical top-to-bottom layout. An explicit
+//! shapes as LTR, except that text that starts with a Mongolian
+//! character switches to vertical top-to-bottom layout. An explicit
 //! [`crate::Direction::Ltr`] keeps Mongolian horizontal.
 //!
 //! # Clusters and buffer flags

src/shape/native_direction.rs

@@ -13,12 +13,15 @@
 //! script counts as LTR.
 //!
 //! The buffer's script is its first script-bearing character's, as
-//! HarfBuzz guesses it. Scripts sigilbuzz has no bucket for take their
-//! direction from their first strong bidi class instead, so an RTL
-//! script without a sigilbuzz bucket (Thaana, Samaritan, ...) still counts
-//! as RTL. HarfBuzz reports no native direction for the bidirectional
-//! scripts (Old Hungarian, Old Italic, Runic, Tifinagh), so text in
-//! them shapes in the direction asked for, never reversed.
+//! HarfBuzz guesses it. Text with no such character (only Common,
+//! Inherited, and Unknown ones, such as Arabic marks before a tatweel)
+//! keeps HarfBuzz's invalid script, whose native direction is LTR.
+//! Scripts sigilbuzz has no bucket for take their direction from their
+//! first strong bidi class instead, so an RTL script without a
+//! sigilbuzz bucket (Thaana, Samaritan, ...) still counts as RTL.
+//! HarfBuzz reports no native direction for the bidirectional scripts
+//! (Old Hungarian, Old Italic, Runic, Tifinagh), so text in them shapes
+//! in the direction asked for, never reversed.
 //!
 //! A grapheme is a character and the continuation characters after it
 //! (see `cluster::continuations`). At
@@ -62,10 +65,15 @@ const fn has_no_native_direction(ch: char) -> bool {
 /// ([`Direction::horizontal_for_script`], `None` for Tifinagh, which
 /// HarfBuzz writes either way), or for text sigilbuzz has no script
 /// bucket for, the direction of its first strong character (left to
-/// right when there is none, as for HarfBuzz's Common script). `None`
-/// when that character belongs to a script HarfBuzz gives no direction.
+/// right when there is none). `None` when that character belongs to a
+/// script HarfBuzz gives no direction. Text with no script (`script`
+/// is `None`) is left to right, as HarfBuzz's invalid script is: its
+/// tatweel or Arabic punctuation does not make it right to left.
 fn native_horizontal(script: Option<Script>, cps: &[char]) -> Option<Direction> {
-    match script.and_then(Script::iso15924_tag) {
+    let Some(script) = script else {
+        return Some(Direction::Ltr);
+    };
+    match script.iso15924_tag() {
         Some(tag) => Direction::horizontal_for_script(tag),
         _ => cps
             .iter()
@@ -195,6 +203,32 @@ mod tests {
         assert_eq!(resolve(Direction::Rtl, None, &digits), Direction::Ltr);
     }
 
+    #[test]
+    fn text_without_a_script_is_left_to_right() {
+        // Arabic marks before a tatweel, and Arabic punctuation: Common
+        // and Inherited characters whose bidi class is right to left.
+        // HarfBuzz's buffer keeps an invalid script, which is LTR.
+        for text in ["\u{064F}\u{064B}\u{0640}", "\u{061F}\u{060C}", "\u{05FF}"] {
+            let cps: Vec<char> = text.chars().collect();
+            assert_eq!(
+                resolve(Direction::Ltr, None, &cps),
+                Direction::Ltr,
+                "{text:?}"
+            );
+            assert_eq!(
+                resolve(Direction::Rtl, None, &cps),
+                Direction::Ltr,
+                "{text:?}"
+            );
+        }
+        // A script without a bucket still reads its strong direction.
+        let tatweel: Vec<char> = "\u{0640}".chars().collect();
+        assert_eq!(
+            resolve(Direction::Ltr, Some(Script::Other), &tatweel),
+            Direction::Rtl
+        );
+    }
+
     /// Reverses `text` (glyph ids and clusters are the code point
     /// indices) and returns the code points, ids, clusters, and
     /// mirrored flags after.