Restore underline and strikethrough in render_text (#763)

Another regression from #754 ... Needed for #762 --------- Co-authored-by: Tom Churchman <[email protected]>
linebender · Nov 28, 2024 · d981f0d · d981f0d
1 parent 14f3dc2
commit d981f0d
Show file tree

Hide file tree

Showing 6 changed files with 104 additions and 6 deletions.
diff --git a/.typos.toml b/.typos.toml
@@ -17,7 +17,8 @@ extend-ignore-re = [
 # is treated as always incorrect.
 
 [default.extend-identifiers]
-wdth = "wdth"           # Variable font parameter
+wdth = "wdth" # Variable font parameter
+Tpyo = "Tpyo" # Intentional typo for a strikethrough test
 
 # Case insensitive
 [default.extend-words]

diff --git a/masonry/src/text/render_text.rs b/masonry/src/text/render_text.rs
@@ -4,7 +4,7 @@
 //! Helper functions for working with text in Masonry.
 
 use parley::{Layout, PositionedLayoutItem};
-use vello::kurbo::Affine;
+use vello::kurbo::{Affine, Line, Stroke};
 use vello::peniko::{Brush, Fill};
 use vello::Scene;
 
@@ -24,6 +24,39 @@ pub fn render_text(
             let PositionedLayoutItem::GlyphRun(glyph_run) = item else {
                 continue;
             };
+            let style = glyph_run.style();
+            // We draw underlines under the text, then the strikethrough on top, following:
+            // https://drafts.csswg.org/css-text-decor/#painting-order
+            if let Some(underline) = &style.underline {
+                let underline_brush = &brushes[underline.brush.0];
+                let run_metrics = glyph_run.run().metrics();
+                let offset = match underline.offset {
+                    Some(offset) => offset,
+                    None => run_metrics.underline_offset,
+                };
+                let width = match underline.size {
+                    Some(size) => size,
+                    None => run_metrics.underline_size,
+                };
+                // The `offset` is the distance from the baseline to the top of the underline
+                // so we move the line down by half the width
+                // Remember that we are using a y-down coordinate system
+                // If there's a custom width, because this is an underline, we want the custom
+                // width to go down from the default expectation
+                let y = glyph_run.baseline() - offset + width / 2.;
+
+                let line = Line::new(
+                    (glyph_run.offset() as f64, y as f64),
+                    ((glyph_run.offset() + glyph_run.advance()) as f64, y as f64),
+                );
+                scene.stroke(
+                    &Stroke::new(width.into()),
+                    transform,
+                    underline_brush,
+                    None,
+                    &line,
+                );
+            }
             let mut x = glyph_run.offset();
             let y = glyph_run.baseline();
             let run = glyph_run.run();
@@ -38,7 +71,7 @@ pub fn render_text(
                 .iter()
                 .map(|coord| vello::skrifa::instance::NormalizedCoord::from_bits(*coord))
                 .collect::<Vec<_>>();
-            let brush = &brushes[glyph_run.style().brush.0];
+            let brush = &brushes[style.brush.0];
             scene
                 .draw_glyphs(font)
                 .brush(brush)
@@ -60,6 +93,36 @@ pub fn render_text(
                         }
                     }),
                 );
+
+            if let Some(strikethrough) = &style.strikethrough {
+                let strikethrough_brush = &brushes[strikethrough.brush.0];
+                let run_metrics = glyph_run.run().metrics();
+                let offset = match strikethrough.offset {
+                    Some(offset) => offset,
+                    None => run_metrics.strikethrough_offset,
+                };
+                let width = match strikethrough.size {
+                    Some(size) => size,
+                    None => run_metrics.strikethrough_size,
+                };
+                // The `offset` is the distance from the baseline to the *top* of the strikethrough
+                // so we calculate the middle y-position of the strikethrough based on the font's
+                // standard strikethrough width.
+                // Remember that we are using a y-down coordinate system
+                let y = glyph_run.baseline() - offset + run_metrics.strikethrough_size / 2.;
+
+                let line = Line::new(
+                    (glyph_run.offset() as f64, y as f64),
+                    ((glyph_run.offset() + glyph_run.advance()) as f64, y as f64),
+                );
+                scene.stroke(
+                    &Stroke::new(width.into()),
+                    transform,
+                    strikethrough_brush,
+                    None,
+                    &line,
+                );
+            }
         }
     }
 }
diff --git a/masonry/src/widget/label.rs b/masonry/src/widget/label.rs
@@ -187,7 +187,10 @@ impl Label {
 
     /// Shared logic between `with_style` and `insert_style`
     fn insert_style_inner(&mut self, property: StyleProperty) -> Option<StyleProperty> {
-        if let StyleProperty::Brush(idx @ BrushIndex(1..)) = &property {
+        if let StyleProperty::Brush(idx @ BrushIndex(1..))
+        | StyleProperty::UnderlineBrush(Some(idx @ BrushIndex(1..)))
+        | StyleProperty::StrikethroughBrush(Some(idx @ BrushIndex(1..))) = &property
+        {
             debug_panic!(
                 "Can't set a non-zero brush index ({idx:?}) on a `Label`, as it only supports global styling."
             );
@@ -443,7 +446,7 @@ impl Widget for Label {
 mod tests {
     use insta::assert_debug_snapshot;
     use parley::style::GenericFamily;
-    use parley::FontFamily;
+    use parley::{FontFamily, StyleProperty};
 
     use super::*;
     use crate::assert_render_snapshot;
@@ -475,6 +478,28 @@ mod tests {
         assert_render_snapshot!(harness, "styled_label");
     }
 
+    #[test]
+    fn underline_label() {
+        let label = Label::new("Emphasis")
+            .with_line_break_mode(LineBreaking::WordWrap)
+            .with_style(StyleProperty::Underline(true));
+
+        let mut harness = TestHarness::create_with_size(label, Size::new(100.0, 20.));
+
+        assert_render_snapshot!(harness, "underline_label");
+    }
+    #[test]
+    fn strikethrough_label() {
+        let label = Label::new("Tpyo")
+            .with_line_break_mode(LineBreaking::WordWrap)
+            .with_style(StyleProperty::Strikethrough(true))
+            .with_style(StyleProperty::StrikethroughSize(Some(4.)));
+
+        let mut harness = TestHarness::create_with_size(label, Size::new(100.0, 20.));
+
+        assert_render_snapshot!(harness, "strikethrough_label");
+    }
+
     #[test]
     /// A wrapping label's alignment should be respected, regardkess of
     /// its parent's alignment.

diff --git a/...y/src/widget/screenshots/masonry__widget__label__tests__strikethrough_label.png b/...y/src/widget/screenshots/masonry__widget__label__tests__strikethrough_label.png
diff --git a/masonry/src/widget/screenshots/masonry__widget__label__tests__underline_label.png b/masonry/src/widget/screenshots/masonry__widget__label__tests__underline_label.png
diff --git a/masonry/src/widget/text_area.rs b/masonry/src/widget/text_area.rs
@@ -277,7 +277,10 @@ impl<const EDITABLE: bool> TextArea<EDITABLE> {
     /// Shared logic between `with_style` and `insert_style`
     #[track_caller]
     fn insert_style_inner(&mut self, property: StyleProperty) -> Option<StyleProperty> {
-        if let StyleProperty::Brush(idx @ BrushIndex(1..)) = &property {
+        if let StyleProperty::Brush(idx @ BrushIndex(1..))
+        | StyleProperty::UnderlineBrush(Some(idx @ BrushIndex(1..)))
+        | StyleProperty::StrikethroughBrush(Some(idx @ BrushIndex(1..))) = &property
+        {
             debug_panic!(
                 "Can't set a non-zero brush index ({idx:?}) on a `TextArea`, as it only supports global styling.\n\
                 To modify the active brush, use `set_brush` or `with_brush` instead"