Slice shaped text by cluster instead of guessing a range

crates/rdocx-layout/src/engine.rs

@@ -586,6 +586,7 @@ pub fn layout_paragraph(
                     font_id,
                     font_size: marker_font_size,
                     glyph_ids: shaped.glyph_ids,
+                    clusters: shaped.clusters,
                     advances: shaped.advances,
                     width: shaped.width,
                     ascent: metrics.ascent,
@@ -718,6 +719,7 @@ pub fn layout_paragraph(
                         font_id,
                         font_size,
                         glyph_ids: shaped.glyph_ids,
+                        clusters: shaped.clusters,
                         advances: shaped.advances,
                         width: shaped.width,
                         ascent: metrics.ascent,
@@ -768,6 +770,7 @@ pub fn layout_paragraph(
                         font_id,
                         font_size,
                         glyph_ids: shaped.glyph_ids,
+                        clusters: shaped.clusters,
                         advances: shaped.advances,
                         width: shaped.width,
                         ascent: metrics.ascent,
@@ -797,6 +800,7 @@ pub fn layout_paragraph(
                         font_id,
                         font_size: sup_size,
                         glyph_ids: shaped.glyph_ids,
+                        clusters: shaped.clusters,
                         advances: shaped.advances,
                         width: shaped.width,
                         ascent: sup_metrics.ascent,

crates/rdocx-layout/src/font.rs

@@ -37,6 +37,13 @@ pub struct ShapedText {
     pub glyph_ids: Vec<u16>,
     /// Per-glyph advances in points.
     pub advances: Vec<f64>,
+    /// Byte offset into the shaped text that each glyph came from.
+    ///
+    /// Shaping is not one glyph per character. A ligature turns several
+    /// characters into one glyph, so glyph index and character index drift
+    /// apart. This is the shaper's own mapping back to the source, and it is
+    /// the only reliable way to slice a shaped run.
+    pub clusters: Vec<u32>,
     /// Total width in points.
     pub width: f64,
 }
@@ -545,6 +552,7 @@ impl FontManager {
             return Ok(ShapedText {
                 glyph_ids: Vec::new(),
                 advances: Vec::new(),
+                clusters: Vec::new(),
                 width: 0.0,
             });
         }
@@ -571,10 +579,12 @@ impl FontManager {
 
         let mut glyph_ids = Vec::with_capacity(infos.len());
         let mut advances = Vec::with_capacity(positions.len());
+        let mut clusters = Vec::with_capacity(infos.len());
         let mut total_width = 0.0;
 
         for (info, pos) in infos.iter().zip(positions.iter()) {
             glyph_ids.push(info.glyph_id as u16);
+            clusters.push(info.cluster);
             let advance = pos.x_advance as f64 * scale;
             advances.push(advance);
             total_width += advance;
@@ -583,6 +593,7 @@ impl FontManager {
         Ok(ShapedText {
             glyph_ids,
             advances,
+            clusters,
             width: total_width,
         })
     }

crates/rdocx-layout/src/line.rs

@@ -41,6 +41,10 @@ pub struct TextSegment {
     pub font_size: f64,
     pub glyph_ids: Vec<u16>,
     pub advances: Vec<f64>,
+    /// Byte offset in `text` that each glyph came from, straight from the
+    /// shaper. Required to slice this segment correctly, because shaping is
+    /// not one glyph per character.
+    pub clusters: Vec<u32>,
     pub width: f64,
     pub ascent: f64,
     pub descent: f64,
@@ -399,39 +403,59 @@ fn split_text_subsegment(seg: &TextSegment, byte_start: usize, byte_end: usize)
     }
 
     let sub_text = seg.text[byte_start..byte_end].to_string();
-    let total_chars = seg.text.chars().count();
-    let char_start = seg.text[..byte_start].chars().count();
-    let char_count = sub_text.chars().count();
-
-    let (sub_glyphs, sub_advances, sub_width) = if seg.glyph_ids.len() == total_chars {
-        // 1:1 char-to-glyph mapping (common for Latin text)
-        let end = (char_start + char_count).min(seg.glyph_ids.len());
-        let glyphs = seg.glyph_ids[char_start..end].to_vec();
-        let advances = seg.advances[char_start..end].to_vec();
-        let width: f64 = advances.iter().sum();
-        (glyphs, advances, width)
-    } else if seg.glyph_ids.is_empty() || seg.text.is_empty() {
-        // No glyphs, or no text to apportion them across
-        (vec![], vec![], 0.0)
-    } else {
-        // Non-1:1 mapping (ligatures, complex scripts) — proportional estimate
-        let byte_frac = (byte_end - byte_start) as f64 / seg.text.len() as f64;
-        let est_glyphs = (seg.glyph_ids.len() as f64 * byte_frac).round() as usize;
-        let glyph_start = (seg.glyph_ids.len() as f64 * byte_start as f64 / seg.text.len() as f64)
-            .round() as usize;
-        let glyph_end = (glyph_start + est_glyphs).min(seg.glyph_ids.len());
-        let glyphs = seg.glyph_ids[glyph_start..glyph_end].to_vec();
-        let advances = seg.advances[glyph_start..glyph_end].to_vec();
-        let width: f64 = advances.iter().sum();
-        (glyphs, advances, width)
-    };
+
+    // Select glyphs by the shaper's own cluster values, which say the byte
+    // each glyph came from. Slicing by character index instead only works
+    // when shaping happens to be one glyph per character, and estimating a
+    // proportional range when it is not made neighbouring chunks overlap, so
+    // a glyph was drawn twice and a letter appeared doubled.
+    let (sub_glyphs, sub_advances, sub_width) =
+        if seg.clusters.len() == seg.glyph_ids.len() && !seg.clusters.is_empty() {
+            let mut glyphs = Vec::new();
+            let mut advances = Vec::new();
+            for (i, &cluster) in seg.clusters.iter().enumerate() {
+                let c = cluster as usize;
+                if c >= byte_start && c < byte_end {
+                    glyphs.push(seg.glyph_ids[i]);
+                    advances.push(seg.advances[i]);
+                }
+            }
+            let width: f64 = advances.iter().sum();
+            (glyphs, advances, width)
+        } else if seg.glyph_ids.is_empty() || seg.text.is_empty() {
+            // No glyphs, or no text to apportion them across
+            (vec![], vec![], 0.0)
+        } else {
+            // No cluster data, which should not happen for anything the shaper
+            // produced. Fall back to a 1:1 slice rather than guessing a range.
+            let char_start = seg.text[..byte_start].chars().count();
+            let char_count = sub_text.chars().count();
+            let end = (char_start + char_count).min(seg.glyph_ids.len());
+            let start = char_start.min(end);
+            let glyphs = seg.glyph_ids[start..end].to_vec();
+            let advances = seg.advances[start..end].to_vec();
+            let width: f64 = advances.iter().sum();
+            (glyphs, advances, width)
+        };
+
+    // A cluster is attributed to the chunk containing its starting byte, so
+    // a ligature spanning a break lands wholly in the first chunk. Its glyph
+    // is drawn once, which is the point.
+    let sub_clusters: Vec<u32> = seg
+        .clusters
+        .iter()
+        .copied()
+        .filter(|&c| (c as usize) >= byte_start && (c as usize) < byte_end)
+        .map(|c| c - byte_start as u32)
+        .collect();
 
     InlineItem::Text(TextSegment {
         text: sub_text,
         font_id: seg.font_id,
         font_size: seg.font_size,
         glyph_ids: sub_glyphs,
         advances: sub_advances,
+        clusters: sub_clusters,
         width: sub_width,
         ascent: seg.ascent,
         descent: seg.descent,
@@ -608,6 +632,9 @@ fn shape_leader(
         font_size,
         glyph_ids,
         advances,
+        // Leader glyphs are generated by repetition, not shaped from the
+        // text, so there is no cluster mapping to carry.
+        clusters: Vec::new(),
         width: tab_width, // fill the entire tab gap
         ascent: metrics.ascent,
         descent: metrics.descent,
@@ -719,6 +746,7 @@ mod tests {
             font_size: 12.0,
             glyph_ids: vec![],
             advances: vec![],
+            clusters: vec![],
             width,
             ascent: 10.0,
             descent: 3.0,
@@ -851,4 +879,97 @@ mod tests {
         assert!((w - 300.0).abs() < 0.01);
         assert_eq!(leader, Some('.'));
     }
+
+    /// Build a segment whose shaping is not one glyph per character, the way
+    /// a font with an "fi" ligature shapes "financial": 9 characters, 8
+    /// glyphs, and the first glyph covering two characters.
+    fn ligature_segment() -> TextSegment {
+        let text = "financial";
+        // cluster = byte the glyph came from. The fi ligature covers bytes 0
+        // and 1, so no glyph reports cluster 1.
+        let clusters = vec![0, 2, 3, 4, 5, 6, 7, 8];
+        let mut seg = make_text_segment(text, 90.0);
+        seg.glyph_ids = (0..clusters.len() as u16).map(|i| i + 100).collect();
+        seg.advances = vec![10.0; clusters.len()];
+        seg.clusters = clusters;
+        seg
+    }
+
+    /// Splitting a shaped segment must never emit the same glyph twice.
+    ///
+    /// Glyphs used to be selected by slicing on character index, with a
+    /// proportional guess when shaping was not one glyph per character.
+    /// Neighbouring chunks then overlapped and a glyph was drawn in both, so
+    /// a letter appeared doubled on the page.
+    #[test]
+    fn splitting_a_shaped_segment_never_repeats_a_glyph() {
+        let seg = ligature_segment();
+        let total = seg.glyph_ids.len();
+
+        // Split at every byte boundary and collect what each half emits.
+        for cut in 1..seg.text.len() {
+            if !seg.text.is_char_boundary(cut) {
+                continue;
+            }
+            let mut seen = Vec::new();
+            for (start, end) in [(0, cut), (cut, seg.text.len())] {
+                if let InlineItem::Text(part) = split_text_subsegment(&seg, start, end) {
+                    seen.extend(part.glyph_ids);
+                }
+            }
+            let mut sorted = seen.clone();
+            sorted.sort_unstable();
+            sorted.dedup();
+            assert_eq!(
+                sorted.len(),
+                seen.len(),
+                "cut at {cut} drew a glyph twice: {seen:?}"
+            );
+            assert_eq!(
+                seen.len(),
+                total,
+                "cut at {cut} lost or gained a glyph: {seen:?}"
+            );
+        }
+    }
+
+    /// A ligature spanning a break belongs to one side of it, not both.
+    #[test]
+    fn a_ligature_is_not_split_across_chunks() {
+        let seg = ligature_segment();
+        // Cut between the two characters the ligature covers.
+        let InlineItem::Text(first) = split_text_subsegment(&seg, 0, 1) else {
+            panic!("expected text");
+        };
+        let InlineItem::Text(second) = split_text_subsegment(&seg, 1, seg.text.len()) else {
+            panic!("expected text");
+        };
+        assert_eq!(
+            first.glyph_ids,
+            vec![100],
+            "the ligature glyph belongs to the chunk holding its first byte"
+        );
+        assert!(
+            !second.glyph_ids.contains(&100),
+            "and must not be repeated in the next chunk"
+        );
+    }
+
+    /// Clusters are rebased onto the sub-segment, so a chunk can be split
+    /// again without the offsets pointing outside its own text.
+    #[test]
+    fn sub_segment_clusters_are_relative_to_the_sub_segment() {
+        let seg = ligature_segment();
+        let InlineItem::Text(part) = split_text_subsegment(&seg, 3, 7) else {
+            panic!("expected text");
+        };
+        assert!(
+            part.clusters
+                .iter()
+                .all(|&c| (c as usize) < part.text.len()),
+            "clusters {:?} must index into {:?}",
+            part.clusters,
+            part.text
+        );
+    }
 }

crates/rdocx-layout/src/paginator.rs

@@ -1412,6 +1412,7 @@ mod tests {
             font_size: 12.0,
             glyph_ids: vec![1, 2, 3],
             advances: vec![6.0, 6.0, 6.0],
+            clusters: vec![0, 1, 2],
             width: 40.0,
             ascent: height * 0.77,
             descent: height * 0.23,
@@ -1510,6 +1511,7 @@ mod tests {
             font_size: 12.0,
             glyph_ids: vec![1],
             advances: vec![10.0],
+            clusters: vec![],
             width: 20.0,
             ascent: 10.0,
             descent: 3.0,
@@ -1685,6 +1687,7 @@ mod tests {
             font_size: 12.0,
             glyph_ids: vec![1; text.len()],
             advances: vec![seg_width / text.len() as f64; text.len()],
+            clusters: vec![],
             width: seg_width,
             ascent: 10.0,
             descent: 3.0,
@@ -1722,6 +1725,7 @@ mod tests {
             font_size: 12.0,
             glyph_ids: vec![1, 2, 3],
             advances: vec![8.0, 8.0, 8.0],
+            clusters: vec![],
             width: 60.0,
             ascent: 10.0,
             descent: 3.0,

scripts/hash_baseline.json

@@ -1,33 +1,33 @@
 {
   "entries": {
-    "contract:page1.png": "504977f2017680736254709429370708554166410856873fd5c38a1d5283b98a",
+    "contract:page1.png": "5b25f35ca837a7632f201aa91e2441968b763587d4c66923830111b6764e77c9",
     "contract:word/document.xml": "1ecd9138e09122a5980ca80451ddd8f93d4e6d3af8bb80e582dc8cb697277dfe",
     "contract:word/numbering.xml": "01d6cb0aa0ecc30b3d2a77c4df062b1c727d70ff9cbc0824910f049daf878a26",
     "contract:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4",
     "feature_showcase:page1.png": "f4ad22ffa12cf20c667aa5de661555b7c34c362e327fbccdd854c5dd225022e5",
     "feature_showcase:word/document.xml": "b5c4efbb49263060558242bad0cca79d4b03a43b62d72feed66789b4926b44f5",
     "feature_showcase:word/numbering.xml": "2aa0599486d98573be0b7febb04ec63c08d3dfc5341b0f722b93cae113803bbc",
     "feature_showcase:word/styles.xml": "b815acd04cc89189d5b2b0caf822435d941c8cc70077f34932e4a2b3ce1b6595",
-    "invoice:page1.png": "7693aab43aaf6971edc2b0fab6b50e978db1192c18811a8e2649113339348d49",
+    "invoice:page1.png": "850572f788d246ac8beceec0a68ac5da63d4904327315a712140cea977fe1bcd",
     "invoice:word/document.xml": "bb2d71711f6a613044dfabc166ae340354345666a98687e02df815efa220c655",
     "invoice:word/numbering.xml": null,
     "invoice:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4",
-    "letter:page1.png": "d3a00cf6c37a1eb6c1897e36b062c10e8b0fdc8c2a6f26c1034026f83578d1c0",
+    "letter:page1.png": "79b23d965cd43bbdbb3cf2128df8f51b5d2d53d4a8934aaa34ed00b2aef8821e",
     "letter:word/document.xml": "54f74a133887b1b93da1070517f9d7f687dbdc27b1307f1696c4b41f66d5fc05",
     "letter:word/numbering.xml": "c6511604704117eb00ad2faffb9173e4f48d22f62557ca72a51d60b7907c8058",
     "letter:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4",
     "proposal:page1.png": "d92257f65b7f149b68f39ace59aec2613924112fe0dc1584ac9dbbd8d3beda34",
     "proposal:word/document.xml": "1f8222a2d0785f6cc33b868274aaf53796ccbcea254f62b102dc51cc075dee6b",
     "proposal:word/numbering.xml": "061e20beb3409ce0f3feda1a99793956124a2967dd23afe1b528dd8fdeb82182",
     "proposal:word/styles.xml": "dae1d6c1083bba6bb4b29ba2ab748a713f5da60b8c2cd4d52224c6f0a8f21bef",
-    "quote:page1.png": "6f69877633a4d09e80cb9172ba8f2d4d220cf7776260fcaa21c5be84362be103",
+    "quote:page1.png": "e969347efaf9973367195736ed1eeeec2f61e600bd5c55363ea6ef398569e01b",
     "quote:word/document.xml": "dd3b7c8bae85d8e22a8c08f12fc4a1ec4829b00a84fbe8cf13f9204213b4cf68",
     "quote:word/numbering.xml": "c6511604704117eb00ad2faffb9173e4f48d22f62557ca72a51d60b7907c8058",
     "quote:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4",
-    "report:page1.png": "9bfaa63b0b14482ad765d9e6528077e96e574a037e701e3d60ea32bdd96d7d32",
+    "report:page1.png": "792f1fdc032c7ec1b6a3e451aa7f94fc45e8ea810d12b9815ced82d2fa7fbd82",
     "report:word/document.xml": "5a834b2ebe01156c082f25ea05483c135d3718beae7d80b2224e0a02f9b93365",
     "report:word/numbering.xml": "2aa0599486d98573be0b7febb04ec63c08d3dfc5341b0f722b93cae113803bbc",
     "report:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4"
   },
-  "reason": "List levels now apply their own w:pPr indent (#13), so page-one PNGs for letter and contract shift. Verified by rendering both before and after: the only visual change is list markers moving to their level indent with correct hanging indent. No other content moved."
+  "reason": "Glyphs are now selected by shaper cluster instead of a proportional guess (#23), so neighbouring chunks no longer overlap and letters are no longer drawn twice. Page-one PNGs change for letter, contract, invoice, quote and report. Verified by rendering letter and invoice before and after: doubled letters such as SSuite, fifinancial and 778701 are gone and nothing else moved."
 }