diff --git a/src/attrs.rs b/src/attrs.rs index d7129d0..da1a345 100644 --- a/src/attrs.rs +++ b/src/attrs.rs @@ -276,6 +276,9 @@ pub struct GlyphDecorationData { pub underline_metrics: DecorationMetrics, /// Strikethrough offset and thickness from the font pub strikethrough_metrics: DecorationMetrics, + /// Font ascent in EM units (ascent / upem). + /// Used for overline positioning + pub ascent: f32, } /// Text attributes diff --git a/src/render.rs b/src/render.rs index c5a2004..4348381 100644 --- a/src/render.rs +++ b/src/render.rs @@ -115,9 +115,8 @@ fn draw_decoration_span( let thickness = (deco.underline_metrics.thickness * font_size) .max(1.0) .ceil(); - //TODO: this should be run.line_y - ascent, but we don't have per-glyph ascent - // in LayoutGlyph. Using line_top as an approximation for now. - let y = run.line_top; + // clamped so it doesn't go above the line top. + let y = (run.line_y - deco.ascent * font_size).max(run.line_top); renderer.rectangle(x_start as i32, y as i32, w, thickness as u32, color); } } diff --git a/src/shape.rs b/src/shape.rs index 28f9a85..82e30a9 100644 --- a/src/shape.rs +++ b/src/shape.rs @@ -612,11 +612,15 @@ impl ShapeGlyph { } } -fn decoration_metrics(font: &Font) -> (DecorationMetrics, DecorationMetrics) { +fn decoration_metrics(font: &Font) -> (DecorationMetrics, DecorationMetrics, f32) { let metrics = font.metrics(); let upem = metrics.units_per_em as f32; if upem == 0.0 { - return (DecorationMetrics::default(), DecorationMetrics::default()); + return ( + DecorationMetrics::default(), + DecorationMetrics::default(), + 0.0, + ); } ( DecorationMetrics { @@ -627,10 +631,11 @@ fn decoration_metrics(font: &Font) -> (DecorationMetrics, DecorationMetrics) { offset: metrics.strikeout.map_or(0.3, |d| d.offset / upem), thickness: metrics.strikeout.map_or(1.0 / 14.0, |d| d.thickness / upem), }, + metrics.ascent / upem, ) } -/// span index used in VlRange to indicate this range is the ellipsis. +/// span index used in `VlRange` to indicate this range is the ellipsis. const ELLIPSIS_SPAN: usize = usize::MAX; fn shape_ellipsis( @@ -1046,7 +1051,7 @@ impl ShapeSpan { .map(|font| decoration_metrics(&font)) }); - if let Some((ul_metrics, st_metrics)) = primary_metrics { + if let Some((ul_metrics, st_metrics, ascent)) = primary_metrics { // Track which sub-ranges of span_range are covered by explicit spans let mut covered_end = span_range.start; @@ -1068,6 +1073,7 @@ impl ShapeSpan { text_decoration: default_attrs.text_decoration, underline_metrics: ul_metrics, strikethrough_metrics: st_metrics, + ascent, }, )); } @@ -1082,6 +1088,7 @@ impl ShapeSpan { text_decoration: attrs.text_decoration, underline_metrics: ul_metrics, strikethrough_metrics: st_metrics, + ascent, }, )); } @@ -1097,6 +1104,7 @@ impl ShapeSpan { text_decoration: default_attrs.text_decoration, underline_metrics: ul_metrics, strikethrough_metrics: st_metrics, + ascent, }, )); } @@ -2081,7 +2089,7 @@ impl ShapeLine { .map_or(0.0, |s| s.words.iter().map(|w| w.width(font_size)).sum()) } - /// Creates a VlRange for the ellipsis with the given BiDi level. + /// Creates a `VlRange` for the ellipsis with the give`BiDi`Di level. fn ellipsis_vlrange(&self, level: unicode_bidi::Level) -> VlRange { VlRange { span: ELLIPSIS_SPAN, @@ -2091,7 +2099,7 @@ impl ShapeLine { } } - /// Determines the appropriate BiDi level for the ellipsis based on the + /// Determines the appropriate `BiDi` level for the ellipsis based on the /// adjacent ranges, following UAX#9 N1/N2 rules for neutral characters. fn ellipsis_level_between( &self, @@ -2860,11 +2868,8 @@ impl ShapeLine { } glyphs.push(layout_glyph); - // Advance (or reset) the decoration cursor to find - // the span covering this glyph's byte position. - // Resets for RTL/BiDi where byte order reverses. - if deco_cursor < deco_spans.len() - && glyph.start < deco_spans[deco_cursor].0.start + if deco_cursor >= deco_spans.len() + || glyph.start < deco_spans[deco_cursor].0.start { deco_cursor = 0; } diff --git a/tests/images/text_decoration_bidi.png b/tests/images/text_decoration_bidi.png index 78d35a1..53d76d8 100644 --- a/tests/images/text_decoration_bidi.png +++ b/tests/images/text_decoration_bidi.png @@ -1,3 +1,3 @@ version https://git-lfs.github.com/spec/v1 -oid sha256:7c856de2313bef83d36dd8776dfcba6f6312238d983f97603fc9a76fa53fea2c -size 15386 +oid sha256:d83b58ede597fede243ebb042c31668c0847069a3843dc9b279c3624e1387728 +size 15146 diff --git a/tests/images/text_decoration_multiline_bidi.png b/tests/images/text_decoration_multiline_bidi.png new file mode 100644 index 0000000..4da8a28 --- /dev/null +++ b/tests/images/text_decoration_multiline_bidi.png @@ -0,0 +1,3 @@ +version https://git-lfs.github.com/spec/v1 +oid sha256:6e332dc9edc0d8c2fd6dd7d9361dc1441b4c979023e79571ac26b7f4f1f1b274 +size 15305 diff --git a/tests/text_decorations.rs b/tests/text_decorations.rs index 5be8e74..e59daaa 100644 --- a/tests/text_decorations.rs +++ b/tests/text_decorations.rs @@ -50,7 +50,6 @@ fn test_text_decorations() { .validate_text_rendering(); } - #[test] fn test_text_decorations_rtl() { let base = base_attrs(); @@ -125,3 +124,41 @@ fn test_text_decorations_bidi() { .canvas(600, 50) .validate_text_rendering(); } + +/// Multiline Bidi test +#[test] +fn test_text_decorations_multiline_bidi() { + let base = base_attrs(); + let red = Color::rgb(0xFF, 0x00, 0x00); + let cyan = Color::rgb(0x00, 0xFF, 0xFF); + + DrawTestCfg::new("text_decoration_multiline_bidi") + .font_size(20., 26.) + .font_attrs(base.clone()) + .rich_text(vec![ + ("زیرخط ", base.clone().underline(UnderlineStyle::Single)), + ("Double ", base.clone().underline(UnderlineStyle::Double)), + ("خط ", base.clone().strikethrough()), + ("Over \n", base.clone().overline()), + ( + "Red زیر خط ", + base.clone() + .underline(UnderlineStyle::Single) + .underline_color(red), + ), + ( + "CyanSt ", + base.clone().strikethrough().strikethrough_color(cyan), + ), + ( + "All", + base.clone() + .underline(UnderlineStyle::Single) + .strikethrough() + .overline(), + ), + (" Plain", base), + ]) + .canvas(400, 80) + .validate_text_rendering(); +}