diff --git a/crates/rdocx-layout/src/engine.rs b/crates/rdocx-layout/src/engine.rs index e61e8dc..0a4270a 100644 --- a/crates/rdocx-layout/src/engine.rs +++ b/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, diff --git a/crates/rdocx-layout/src/font.rs b/crates/rdocx-layout/src/font.rs index 1c7467b..f625f5f 100644 --- a/crates/rdocx-layout/src/font.rs +++ b/crates/rdocx-layout/src/font.rs @@ -37,6 +37,13 @@ pub struct ShapedText { pub glyph_ids: Vec, /// Per-glyph advances in points. pub advances: Vec, + /// 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, /// 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, }) } diff --git a/crates/rdocx-layout/src/line.rs b/crates/rdocx-layout/src/line.rs index 4e9c6ff..5df3f3d 100644 --- a/crates/rdocx-layout/src/line.rs +++ b/crates/rdocx-layout/src/line.rs @@ -41,6 +41,10 @@ pub struct TextSegment { pub font_size: f64, pub glyph_ids: Vec, pub advances: Vec, + /// 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, pub width: f64, pub ascent: f64, pub descent: f64, @@ -399,32 +403,51 @@ 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 = 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, @@ -432,6 +455,7 @@ fn split_text_subsegment(seg: &TextSegment, byte_start: usize, byte_end: usize) 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 + ); + } } diff --git a/crates/rdocx-layout/src/paginator.rs b/crates/rdocx-layout/src/paginator.rs index 2b8581d..11e773d 100644 --- a/crates/rdocx-layout/src/paginator.rs +++ b/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, diff --git a/scripts/hash_baseline.json b/scripts/hash_baseline.json index 5ce7248..7cc0533 100644 --- a/scripts/hash_baseline.json +++ b/scripts/hash_baseline.json @@ -1,6 +1,6 @@ { "entries": { - "contract:page1.png": "504977f2017680736254709429370708554166410856873fd5c38a1d5283b98a", + "contract:page1.png": "5b25f35ca837a7632f201aa91e2441968b763587d4c66923830111b6764e77c9", "contract:word/document.xml": "1ecd9138e09122a5980ca80451ddd8f93d4e6d3af8bb80e582dc8cb697277dfe", "contract:word/numbering.xml": "01d6cb0aa0ecc30b3d2a77c4df062b1c727d70ff9cbc0824910f049daf878a26", "contract:word/styles.xml": "0dc0b047b6019b798b83bfe4d66eb91d14b03caea3f6bdb0934d48fbb863fba4", @@ -8,11 +8,11 @@ "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", @@ -20,14 +20,14 @@ "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." }