Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Restore underline and strikethrough in render_text #763

Merged
merged 5 commits into from
Nov 28, 2024
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
3 changes: 2 additions & 1 deletion .typos.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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]
Expand Down
62 changes: 60 additions & 2 deletions masonry/src/text/render_text.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -38,7 +38,8 @@ 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 style = glyph_run.style();
let brush = &brushes[style.brush.0];
scene
.draw_glyphs(font)
.brush(brush)
Expand All @@ -60,6 +61,63 @@ pub fn render_text(
}
}),
);
// Draw the underline/strikethrough on top of the
DJMcNab marked this conversation as resolved.
Show resolved Hide resolved
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
let y = glyph_run.baseline() - offset + width / 2.;
DJMcNab marked this conversation as resolved.
Show resolved Hide resolved

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,
);
}
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 move the line down by half the width
// Remember that we are using a y-down coordinate system
let y = glyph_run.baseline() - offset + width / 2.;
DJMcNab marked this conversation as resolved.
Show resolved Hide resolved

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,
);
}
}
}
}
29 changes: 27 additions & 2 deletions masonry/src/widget/label.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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."
);
Expand Down Expand Up @@ -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;
Expand Down Expand Up @@ -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.
Expand Down
DJMcNab marked this conversation as resolved.
Show resolved Hide resolved
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
Loading
Sorry, something went wrong. Reload?
Sorry, we cannot display this file.
Sorry, this file is invalid so it cannot be displayed.
5 changes: 4 additions & 1 deletion masonry/src/widget/text_area.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down