From e97b85cf1bc8419481a523dbd2cc80342a68ad29 Mon Sep 17 00:00:00 2001 From: "alonso.torres" Date: Wed, 28 Oct 2026 11:24:45 +0000 Subject: [PATCH] :bug: Attach horizontal annotations to the ideographic em box Zhuyin, Pinyin and emphasis marks over horizontal text were placed against an em box bottom-aligned to Skia's tight glyph rect, which spans the hhea ascent and descent. In CJK faces the descent reaches past the em box, so Zhuyin columns sat about 0.17 em below their base, and over annotations relied on a hand-tuned clearance. Place them against the ideographic em box (OS/2 typo metrics) of the font that shaped each base, centred on the line baseline, the same metrics vertical text uses for upright glyphs. AI-assisted-by: claude-opus-5-5 --- .../src/shapes/text_horizontal/emphasis.rs | 18 ++++--- render-wasm/src/shapes/text_horizontal/mod.rs | 29 ++++++++--- .../src/shapes/text_horizontal/ruby.rs | 34 ++++++++++--- .../src/shapes/text_horizontal/tests.rs | 50 +++++++++++++++---- render-wasm/src/shapes/text_paragraph.rs | 15 ++++++ .../src/shapes/text_vertical/font_tables.rs | 2 +- render-wasm/src/shapes/text_vertical/mod.rs | 1 + .../src/shapes/text_vertical/test_support.rs | 2 +- 8 files changed, 120 insertions(+), 31 deletions(-) diff --git a/render-wasm/src/shapes/text_horizontal/emphasis.rs b/render-wasm/src/shapes/text_horizontal/emphasis.rs index 23840a5f60..08cfea70a4 100644 --- a/render-wasm/src/shapes/text_horizontal/emphasis.rs +++ b/render-wasm/src/shapes/text_horizontal/emphasis.rs @@ -9,6 +9,8 @@ pub(crate) struct HorizontalEmphasisPlacement { pub(crate) mark: char, /// Base character rect in the laid-out paragraph. pub(crate) rect: skia::Rect, + /// Top of the base character's em box (`horizontal_base_em_top`). + pub(crate) em_top: f32, } /// Place one emphasis mark beside each eligible character, from the laid-out @@ -19,6 +21,7 @@ pub(crate) fn horizontal_emphasis_placements( laid_out: &LayoutParagraph, ) -> Vec { let offset_map = &offsets.offset_map; + let lines = laid_out.get_line_metrics(); let mut placements = Vec::new(); for range in offsets.ranges.iter().filter(|range| !range.warichu) { @@ -57,6 +60,13 @@ pub(crate) fn horizontal_emphasis_placements( range: local_utf16..next_utf16, mark, rect, + em_top: horizontal_base_em_top( + laid_out, + &lines, + builder_start, + span.font_size, + rect, + ), }); } } @@ -74,9 +84,7 @@ pub(crate) fn horizontal_emphasis_mark_box( ) -> skia::Rect { let size = span.font_size * EMPHASIS_FONT_SCALE; let top = if span.emphasis_side(false) == RubySide::Over { - horizontal_annotation_over_top(placement.rect, span.font_size) - - span.emphasis_outside_offset(false) - - size + placement.em_top - span.emphasis_outside_offset(false) - size } else { placement.rect.bottom() + span.emphasis_outside_offset(false) }; @@ -137,9 +145,7 @@ pub(crate) fn paint_horizontal_emphasis( { let mark_x = x + placement.rect.center_x() - mark_width / 2.0; let mark_y = y + if span.emphasis_side(false) == RubySide::Over { - horizontal_annotation_over_top(placement.rect, span.font_size) - - mark_ink_bottom - - span.emphasis_outside_offset(false) + placement.em_top - mark_ink_bottom - span.emphasis_outside_offset(false) } else { placement.rect.bottom() - mark_ink_top + span.emphasis_outside_offset(false) }; diff --git a/render-wasm/src/shapes/text_horizontal/mod.rs b/render-wasm/src/shapes/text_horizontal/mod.rs index 44096e9a32..970eefd4dd 100644 --- a/render-wasm/src/shapes/text_horizontal/mod.rs +++ b/render-wasm/src/shapes/text_horizontal/mod.rs @@ -455,11 +455,26 @@ pub(crate) fn horizontal_span_style( .map(|(_, metric)| metric.text_style.clone()) } -/// Top of the horizontal base em inside SkParagraph's typographic rect, less -/// a 3/14-em clearance (12 px at 56 px). The tight rect includes ascender-side -/// padding, so anchoring an over annotation to its top leaves a gap above CJK -/// ink; the em is bottom-aligned to the rect. -pub(crate) fn horizontal_annotation_over_top(rect: skia::Rect, font_size: f32) -> f32 { - let font_size = font_size.max(0.0); - rect.top.max(rect.bottom - font_size) - font_size * (3.0 / 14.0) +/// Top of the em box of a horizontal base laid out in `rect`, whose builder +/// text starts at UTF-16 `base_start`. Annotations attach to this box: the +/// ideographic em box (OS/2 typo metrics) of the font that shaped the base, +/// centred on the line baseline. Skia's tight rect spans the hhea ascent and +/// descent instead, which in CJK faces reach past the em box. +pub(crate) fn horizontal_base_em_top( + laid_out: &LayoutParagraph, + lines: &[skia::textlayout::LineMetrics], + base_start: usize, + font_size: f32, + rect: skia::Rect, +) -> f32 { + let middle = rect.center_y(); + let Some(line) = lines.iter().find(|line| { + let baseline = line.baseline as f32; + middle >= baseline - line.ascent as f32 && middle <= baseline + line.descent as f32 + }) else { + return rect.top.max(rect.bottom - font_size); + }; + let font = laid_out.font_at_utf16(base_start.max(line.start_index)); + let (ascent, descent) = super::text_vertical::upright_centre_metrics(&font); + line.baseline as f32 + (ascent + descent - font_size) / 2.0 } diff --git a/render-wasm/src/shapes/text_horizontal/ruby.rs b/render-wasm/src/shapes/text_horizontal/ruby.rs index 60942d6142..b6205208ac 100644 --- a/render-wasm/src/shapes/text_horizontal/ruby.rs +++ b/render-wasm/src/shapes/text_horizontal/ruby.rs @@ -4,10 +4,16 @@ use std::collections::HashMap; use super::*; -pub(crate) fn horizontal_zhuyin_origin(span: &TextSpan, rect: skia::Rect) -> skia::Point { +/// Top-left of the Zhuyin strip beside a horizontal base whose em box starts +/// at `em_top`. +pub(crate) fn horizontal_zhuyin_origin( + span: &TextSpan, + rect: skia::Rect, + em_top: f32, +) -> skia::Point { skia::Point::new( rect.right() - span.font_size * super::super::text_zhuyin::STRIP_WIDTH, - rect.top.max(rect.bottom - span.font_size), + em_top, ) } @@ -21,8 +27,10 @@ pub(crate) fn horizontal_zhuyin_entries( } let provider = get_resources().fonts.font_provider(); let fallbacks = get_fallback_fonts(); + let lines = laid_out.get_line_metrics(); let mut entries = Vec::new(); for (index, range) in horizontal_ruby_targets(paragraph, offsets) { + let base_start = range.start; let span = ¶graph.children()[index]; let Some(syllable) = super::super::text_zhuyin::syllable(span) else { continue; @@ -32,7 +40,9 @@ pub(crate) fn horizontal_zhuyin_entries( let Some(rect) = rects.first() else { continue; }; - let origin = horizontal_zhuyin_origin(span, rect.rect); + let em_top = + horizontal_base_em_top(laid_out, &lines, base_start, span.font_size, rect.rect); + let origin = horizontal_zhuyin_origin(span, rect.rect, em_top); let size = span.ruby_font_size(); for glyph in super::super::text_zhuyin::shape(span, &syllable, provider, fallbacks) { entries.push(crate::shapes::PositionData { @@ -451,6 +461,7 @@ pub(crate) fn paint_horizontal_ruby( for (span_index, span_range) in horizontal_ruby_targets(paragraph, offsets) { let span = ¶graph.children()[span_index]; + let base_start = span_range.start; let paint = horizontal_ruby_paint( laid_out, &offsets.ranges[span_index], @@ -463,6 +474,9 @@ pub(crate) fn paint_horizontal_ruby( ); if let Some(syllable) = super::super::text_zhuyin::syllable(span) { if let Some(rect) = rects.first() { + let em_top = + horizontal_base_em_top(laid_out, &lines, base_start, span.font_size, rect.rect); + let origin = horizontal_zhuyin_origin(span, rect.rect, em_top); for glyph in super::super::text_zhuyin::shape( span, &syllable, @@ -472,7 +486,6 @@ pub(crate) fn paint_horizontal_ruby( if let Some(blob) = single_glyph_blob(&glyph.run.font, glyph.run.glyphs[glyph.glyph]) { - let origin = horizontal_zhuyin_origin(span, rect.rect); canvas.draw_text_blob( &blob, (x + origin.x + glyph.origin.x, y + origin.y + glyph.origin.y), @@ -516,6 +529,13 @@ pub(crate) fn paint_horizontal_ruby( shaped: &shaped, glyphs: slice, base: rect_box.rect, + em_top: horizontal_base_em_top( + laid_out, + &lines, + base_start, + span.font_size, + rect_box.rect, + ), leading_gap, room, }; @@ -531,6 +551,8 @@ pub(super) struct RubyLine<'a> { pub glyphs: &'a [RubyGlyphRef], /// Base rect in the laid-out paragraph. pub base: skia::Rect, + /// Top of the base em box (`horizontal_base_em_top`). + pub em_top: f32, /// Spacing before the base, which sits on the previous character. pub leading_gap: f32, /// Overhang room (before, after) inside the line. @@ -581,9 +603,7 @@ impl RubyLine<'_> { let over = self.span.ruby_side == RubySide::Over; let edge = ruby_line_ink_edge(self.shaped, self.glyphs, over); if over { - y + horizontal_annotation_over_top(self.base, self.span.font_size) - - edge - - self.span.ruby_decoration_offset(false) + y + self.em_top - edge - self.span.ruby_decoration_offset(false) } else { y + self.base.bottom() - edge + self.span.ruby_decoration_offset(false) } diff --git a/render-wasm/src/shapes/text_horizontal/tests.rs b/render-wasm/src/shapes/text_horizontal/tests.rs index 33d8ec7f43..763aa4c6f6 100644 --- a/render-wasm/src/shapes/text_horizontal/tests.rs +++ b/render-wasm/src/shapes/text_horizontal/tests.rs @@ -884,13 +884,6 @@ fn horizontal_ruby_is_atomic() { ); } -#[test] -fn horizontal_annotation_uses_the_base_em_not_typographic_leading() { - let rect = skia::Rect::from_xywh(0.0, 66.0, 56.0, 90.0); - - assert_eq!(horizontal_annotation_over_top(rect, 56.0), 88.0); -} - #[test] fn horizontal_warichu_collapses_to_one_builder_position() { init_state(); @@ -1200,6 +1193,7 @@ fn horizontal_emphasis_mark_stacks_outside_auto_clearance_ruby() { range: 0..1, mark: '•', rect: skia::Rect::from_xywh(10.0, 40.0, 16.0, 20.0), + em_top: 40.0, }; let plain = make_span("漢", 0.0); let mut stacked = make_span("漢", 0.0); @@ -1322,6 +1316,7 @@ fn horizontal_ruby_glyphs_share_one_baseline() { shaped: &shaped, glyphs, base, + em_top: base.top(), leading_gap: 0.0, room: (0.0, 0.0), } @@ -1381,6 +1376,7 @@ fn latin_readings_share_one_baseline_whatever_their_letters() { shaped: &shaped, glyphs: &glyphs, base, + em_top: base.top(), leading_gap: 0.0, room: (0.0, 0.0), }; @@ -1398,7 +1394,7 @@ fn latin_readings_share_one_baseline_whatever_their_letters() { .font .get_bounds(&shaped[0].glyphs[..1], &mut bounds, None); assert!( - descender + bounds[0].bottom <= horizontal_annotation_over_top(base, EM) + 0.01, + descender + bounds[0].bottom <= base.top() + 0.01, "the descender of y reaches into the base" ); } @@ -1412,6 +1408,7 @@ fn emphasis_position_overrides_the_language_and_stacks_on_its_own_side() { range: 0..1, mark: '•', rect: skia::Rect::from_xywh(0.0, 20.0, 20.0, 20.0), + em_top: 20.0, }; for (language, position, under) in [ ("ja", TextEmphasisPosition::Auto, false), @@ -1523,6 +1520,7 @@ fn below_emphasis_clears_an_underline_on_the_same_side() { range: 0..1, mark: '•', rect: skia::Rect::from_xywh(0.0, 20.0, 16.0, 18.0), + em_top: 20.0, }; let mark = super::emphasis::horizontal_emphasis_mark_box(&span, &placement); assert!(mark.top() > placement.rect.bottom() + span.font_size / 9.0); @@ -1540,6 +1538,7 @@ fn chinese_horizontal_emphasis_defaults_below_the_base() { range: 0..1, mark: '•', rect: skia::Rect::from_xywh(0.0, 20.0, 16.0, 18.0), + em_top: 20.0, }; let mark = super::emphasis::horizontal_emphasis_mark_box(&span, &placement); assert!(mark.top() >= placement.rect.bottom()); @@ -1587,6 +1586,7 @@ fn horizontal_pinyin_tones_keep_canonical_shaping_and_group_alignment() { shaped: &shaped, glyphs: &glyphs, base, + em_top: base.top(), leading_gap: 0.0, room: (0.0, 0.0), } @@ -1640,6 +1640,7 @@ fn horizontal_western_ruby_keeps_its_word_whole() { shaped: &shaped, glyphs: &glyphs, base, + em_top: base.top(), leading_gap: 0.0, room: (0.0, 0.0), } @@ -1835,12 +1836,43 @@ fn horizontal_zhuyin_reserves_its_right_strip_without_changing_line_leading() { assert_eq!(spacing.adjustments[0], vec![(0, span.font_size * 0.6)]); assert_eq!(span.annotation_room_em(), 0.0); let rect = skia::Rect::from_xywh(0.0, 0.0, span.font_size * 1.6, span.font_size); - let origin = horizontal_zhuyin_origin(&span, rect); + let origin = horizontal_zhuyin_origin(&span, rect, 0.0); assert!((origin.x - span.font_size).abs() < 0.001); assert_eq!(origin.y, 0.0); } } +#[test] +fn horizontal_annotations_attach_to_the_ideographic_em_box() { + use crate::shapes::text_vertical::test_support::{provider, EM, VMTX_TEST_FONT}; + init_state(); + // Noto metrics: hhea ascent/descent 1.16/0.288 em, em box 0.88/0.12 em. + let mut fonts = skia::textlayout::FontCollection::new(); + fonts.set_asset_font_manager(Some(provider(VMTX_TEST_FONT).into())); + // The em box depends on the base alone; a reading would shape it through + // the global font resources. + let mut span = make_span("中", 0.0); + span.font_size = EM; + span.text_emphasis = TextEmphasis::FilledDot; + let paragraph = make_paragraph(vec![span], 0.0); + let (plan, layout) = cjk_layout_paragraph(¶graph, EM * 10.0, &fonts); + let range = &plan.offsets.ranges[0]; + let rect = layout.get_rects_for_range( + range.builder_start..range.builder_end, + RectHeightStyle::Tight, + RectWidthStyle::Tight, + )[0] + .rect; + let lines = layout.get_line_metrics(); + let em_top = horizontal_base_em_top(&layout, &lines, range.builder_start, EM, rect); + let baseline = lines[0].baseline as f32; + assert!((em_top - (baseline - 0.88 * EM)).abs() < 0.01); + + let placements = horizontal_emphasis_placements(¶graph, &plan.offsets, &layout); + assert_eq!(placements.len(), 1); + assert!((placements[0].em_top - em_top).abs() < 0.01); +} + #[test] fn horizontal_indivisible_punctuation_unit_overflows_a_narrow_box() { use crate::shapes::text_policy::KoreanWrap; diff --git a/render-wasm/src/shapes/text_paragraph.rs b/render-wasm/src/shapes/text_paragraph.rs index 4134a5f951..4de36df72e 100644 --- a/render-wasm/src/shapes/text_paragraph.rs +++ b/render-wasm/src/shapes/text_paragraph.rs @@ -233,6 +233,21 @@ impl LayoutParagraph { self.inner.ideographic_baseline() + self.line_shift(0) } + /// Font that shaped the text at UTF-16 `offset`, fallbacks included. + pub fn font_at_utf16(&self, offset: usize) -> skia_safe::Font { + let mut utf16 = 0; + let byte = self + .text + .char_indices() + .find(|(_, ch)| { + let start = utf16; + utf16 += ch.len_utf16(); + start >= offset + }) + .map_or(self.text.len(), |(byte, _)| byte); + self.inner.get_font_at(byte) + } + pub fn get_line_metrics(&self) -> Vec> { let mut lines = self.inner.get_line_metrics(); for (index, line) in lines.iter_mut().enumerate() { diff --git a/render-wasm/src/shapes/text_vertical/font_tables.rs b/render-wasm/src/shapes/text_vertical/font_tables.rs index f7a6cdf32c..c9f7498d06 100644 --- a/render-wasm/src/shapes/text_vertical/font_tables.rs +++ b/render-wasm/src/shapes/text_vertical/font_tables.rs @@ -170,7 +170,7 @@ impl TypoMetrics { /// (ascent negative, descent positive) at the font's size. Uses the OS/2 /// typographic metrics, which match the browser's vertical central baseline /// in the SVG/foreignObject export, or `Font::metrics` without an OS/2 table. -pub(super) fn upright_centre_metrics(font: &Font) -> (f32, f32) { +pub(crate) fn upright_centre_metrics(font: &Font) -> (f32, f32) { if let Some(typo) = cached_table(&TYPO_METRICS, font, TypoMetrics::from_font) { let size = font.size(); return (-typo.ascender * size, -typo.descender * size); diff --git a/render-wasm/src/shapes/text_vertical/mod.rs b/render-wasm/src/shapes/text_vertical/mod.rs index 6417c8535f..2fc3de0c73 100644 --- a/render-wasm/src/shapes/text_vertical/mod.rs +++ b/render-wasm/src/shapes/text_vertical/mod.rs @@ -41,6 +41,7 @@ mod shaping; #[cfg(test)] pub(crate) mod test_support; +pub(crate) use font_tables::upright_centre_metrics; pub use layout::{block_axis_offset, layout_for_rect, VerticalLayout}; pub(crate) use orientation::{is_emoji_char, is_upright_char}; pub use paint::{ diff --git a/render-wasm/src/shapes/text_vertical/test_support.rs b/render-wasm/src/shapes/text_vertical/test_support.rs index 9204a66861..105290f9d4 100644 --- a/render-wasm/src/shapes/text_vertical/test_support.rs +++ b/render-wasm/src/shapes/text_vertical/test_support.rs @@ -20,7 +20,7 @@ pub(crate) const TEST_FONT: &[u8] = include_bytes!("../../fonts/sourcesanspro-re pub(crate) const PINYIN_TEST_FONT: &[u8] = include_bytes!("../../fonts/pinyin-test.ttf"); /// Noto Sans JP subset with `vmtx`/`vhea`: 〱 advances 2em vertically, あ/く 1em. -pub(super) const VMTX_TEST_FONT: &[u8] = include_bytes!("../../fonts/notosansjp-vmtx-test.ttf"); +pub(crate) const VMTX_TEST_FONT: &[u8] = include_bytes!("../../fonts/notosansjp-vmtx-test.ttf"); /// Noto Sans JP subset with GSUB `vert` and GPOS `vpal` for 、。「」あく. pub(crate) const VPAL_TEST_FONT: &[u8] = include_bytes!("../../fonts/notosansjp-vpal-test.ttf");