Switch from state to decorations for underline/strikethrough/overline

8 files changed, 139 insertions, 141 deletions

diff --git a/src/eval/state.rs b/src/eval/state.rs
index 05558915..d7ca014d 100644
--- a/src/eval/state.rs
+++ b/src/eval/state.rs
@@ -140,12 +140,6 @@ pub struct FontState {
     /// A list of font families with generic class definitions (the final
     /// family list also depends on `monospace`).
     pub families: Rc<FamilyState>,
-    /// The specifications for a strikethrough line, if any.
-    pub strikethrough: Option<Rc<LineState>>,
-    /// The specifications for a underline, if any.
-    pub underline: Option<Rc<LineState>>,
-    /// The specifications for a overline line, if any.
-    pub overline: Option<Rc<LineState>>,
 }
 
 impl FontState {
@@ -212,9 +206,6 @@ impl Default for FontState {
             top_edge: VerticalFontMetric::CapHeight,
             bottom_edge: VerticalFontMetric::Baseline,
             fill: Paint::Color(Color::Rgba(RgbaColor::BLACK)),
-            strikethrough: None,
-            underline: None,
-            overline: None,
         }
     }
 }
@@ -248,19 +239,3 @@ impl Default for FamilyState {
         }
     }
 }
-
-/// Defines a line that is positioned over, under or on top of text.
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
-pub struct LineState {
-    /// Stroke color of the line, defaults to the text color if `None`.
-    pub stroke: Option<Paint>,
-    /// Thickness of the line's strokes (dependent on scaled font size), read
-    /// from the font tables if `None`.
-    pub thickness: Option<Linear>,
-    /// Position of the line relative to the baseline (dependent on scaled font
-    /// size), read from the font tables if `None`.
-    pub offset: Option<Linear>,
-    /// Amount that the line will be longer or shorter than its associated text
-    /// (dependent on scaled font size).
-    pub extent: Linear,
-}
diff --git a/src/eval/template.rs b/src/eval/template.rs
index 0ab49d04..4fc6985a 100644
--- a/src/eval/template.rs
+++ b/src/eval/template.rs
@@ -8,7 +8,8 @@ use super::{State, Str};
 use crate::diag::StrResult;
 use crate::geom::{Align, Dir, Gen, GenAxis, Length, Linear, Sides, Size};
 use crate::layout::{
-    LayoutNode, LayoutTree, PadNode, PageRun, ParChild, ParNode, StackChild, StackNode,
+    Decoration, LayoutNode, LayoutTree, PadNode, PageRun, ParChild, ParNode, StackChild,
+    StackNode,
 };
 use crate::util::EcoString;
 
@@ -43,13 +44,6 @@ enum TemplateNode {
     Modify(Rc<dyn Fn(&mut State)>),
 }
 
-/// A template node decoration.
-#[derive(Debug, Clone, Eq, PartialEq, Hash)]
-pub enum Decoration {
-    /// A link.
-    Link(EcoString),
-}
-
 impl Template {
     /// Create a new, empty template.
     pub fn new() -> Self {
@@ -114,7 +108,7 @@ impl Template {
         self.make_mut().push(TemplateNode::Spacing(axis, spacing));
     }
 
-    /// Add a decoration to the last template node.
+    /// Add a decoration to all contained nodes.
     pub fn decorate(&mut self, deco: Decoration) {
         for node in self.make_mut() {
             let decos = match node {
diff --git a/src/export/pdf.rs b/src/export/pdf.rs
index d613efc3..ee2b026c 100644
--- a/src/export/pdf.rs
+++ b/src/export/pdf.rs
@@ -50,7 +50,7 @@ impl<'a> PdfExporter<'a> {
         for frame in frames {
             for (_, element) in frame.elements() {
                 match *element {
-                    Element::Text(ref shaped) => font_map.insert(shaped.face_id),
+                    Element::Text(ref text) => font_map.insert(text.face_id),
                     Element::Geometry(_, _) => {}
                     Element::Image(id, _) => {
                         let img = ctx.images.get(id);
@@ -168,35 +168,35 @@ impl<'a> PdfExporter<'a> {
             let y = (page.size.h - pos.y).to_pt() as f32;
 
             match *element {
-                Element::Text(ref shaped) => {
-                    if fill != Some(shaped.fill) {
-                        write_fill(&mut content, shaped.fill);
-                        fill = Some(shaped.fill);
+                Element::Text(ref text) => {
+                    if fill != Some(text.fill) {
+                        write_fill(&mut content, text.fill);
+                        fill = Some(text.fill);
                     }
 
-                    let mut text = content.text();
+                    let mut text_writer = content.text();
 
                     // Then, also check if we need to issue a font switching
                     // action.
-                    if face_id != Some(shaped.face_id) || shaped.size != size {
-                        face_id = Some(shaped.face_id);
-                        size = shaped.size;
+                    if face_id != Some(text.face_id) || text.size != size {
+                        face_id = Some(text.face_id);
+                        size = text.size;
 
-                        let name = format!("F{}", self.font_map.map(shaped.face_id));
-                        text.font(Name(name.as_bytes()), size.to_pt() as f32);
+                        let name = format!("F{}", self.font_map.map(text.face_id));
+                        text_writer.font(Name(name.as_bytes()), size.to_pt() as f32);
                     }
 
-                    let face = self.fonts.get(shaped.face_id);
+                    let face = self.fonts.get(text.face_id);
 
                     // Position the text.
-                    text.matrix(1.0, 0.0, 0.0, 1.0, x, y);
+                    text_writer.matrix(1.0, 0.0, 0.0, 1.0, x, y);
 
-                    let mut positioned = text.show_positioned();
+                    let mut positioned = text_writer.show_positioned();
                     let mut adjustment = Em::zero();
                     let mut encoded = vec![];
 
                     // Write the glyphs with kerning adjustments.
-                    for glyph in &shaped.glyphs {
+                    for glyph in &text.glyphs {
                         adjustment += glyph.x_offset;
 
                         if !adjustment.is_zero() {
diff --git a/src/font.rs b/src/font.rs
index 690884f4..633a1a0e 100644
--- a/src/font.rs
+++ b/src/font.rs
@@ -163,6 +163,7 @@ pub struct Face {
 }
 
 /// Metrics for a decorative line.
+#[derive(Debug, Copy, Clone)]
 pub struct LineMetrics {
     pub strength: Em,
     pub position: Em,
diff --git a/src/layout/frame.rs b/src/layout/frame.rs
index 15ef541b..2c8ba3d8 100644
--- a/src/layout/frame.rs
+++ b/src/layout/frame.rs
@@ -131,6 +131,8 @@ pub struct Text {
     pub face_id: FaceId,
     /// The font size.
     pub size: Length,
+    /// The width of the text run.
+    pub width: Length,
     /// Glyph color.
     pub fill: Paint,
     /// The glyphs.
diff --git a/src/layout/par.rs b/src/layout/par.rs
index e92e5a18..d7fbde16 100644
--- a/src/layout/par.rs
+++ b/src/layout/par.rs
@@ -4,7 +4,7 @@ use unicode_bidi::{BidiInfo, Level};
 use xi_unicode::LineBreakIterator;
 
 use super::*;
-use crate::eval::{Decoration, FontState};
+use crate::eval::FontState;
 use crate::util::{EcoString, RangeExt, SliceExt};
 
 type Range = std::ops::Range<usize>;
@@ -368,7 +368,7 @@ impl<'a> LineStack<'a> {
         let mut first = true;
 
         for line in self.lines.drain(..) {
-            let frame = line.build(ctx, self.size.w);
+            let frame = line.build(self.size.w);
 
             let pos = Point::new(Length::zero(), offset);
             if first {
@@ -380,21 +380,14 @@ impl<'a> LineStack<'a> {
             output.merge_frame(pos, frame);
         }
 
-        // For each frame, we look if any decorations apply.
-        for i in 0 .. output.children.len() {
-            let &(point, ref child) = &output.children[i];
-            if let &FrameChild::Frame(Some(frame_idx), ref frame) = child {
-                let size = frame.size;
-                for deco in match &self.children[frame_idx] {
+        for (_, child) in &mut output.children {
+            if let FrameChild::Frame(Some(frame_idx), frame) = child {
+                for deco in match &self.children[*frame_idx] {
                     ParChild::Spacing(_) => continue,
                     ParChild::Text(.., decos) => decos,
                     ParChild::Any(.., decos) => decos,
                 } {
-                    match deco {
-                        Decoration::Link(href) => {
-                            output.push(point, Element::Link(href.to_string(), size));
-                        }
-                    }
+                    deco.apply(ctx, Rc::make_mut(frame));
                 }
             }
         }
@@ -528,7 +521,7 @@ impl<'a> LineLayout<'a> {
     }
 
     /// Build the line's frame.
-    fn build(&self, ctx: &LayoutContext, width: Length) -> Frame {
+    fn build(&self, width: Length) -> Frame {
         let size = Size::new(self.size.w.max(width), self.size.h);
         let free = size.w - self.size.w;
 
@@ -544,7 +537,7 @@ impl<'a> LineLayout<'a> {
                 }
                 ParItem::Text(ref shaped, align, _) => {
                     ruler = ruler.max(align);
-                    Rc::new(shaped.build(ctx))
+                    Rc::new(shaped.build())
                 }
                 ParItem::Frame(ref frame, align, _) => {
                     ruler = ruler.max(align);
@@ -618,6 +611,96 @@ impl<'a> LineLayout<'a> {
     }
 }
 
+/// A decoration for a paragraph child.
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+pub enum Decoration {
+    /// A link.
+    Link(EcoString),
+    /// An underline/strikethrough/overline decoration.
+    Line(LineDecoration),
+}
+
+/// Defines a line that is positioned over, under or on top of text.
+#[derive(Debug, Clone, Eq, PartialEq, Hash)]
+pub struct LineDecoration {
+    /// The kind of line.
+    pub kind: LineKind,
+    /// Stroke color of the line, defaults to the text color if `None`.
+    pub stroke: Option<Paint>,
+    /// Thickness of the line's strokes (dependent on scaled font size), read
+    /// from the font tables if `None`.
+    pub thickness: Option<Linear>,
+    /// Position of the line relative to the baseline (dependent on scaled font
+    /// size), read from the font tables if `None`.
+    pub offset: Option<Linear>,
+    /// Amount that the line will be longer or shorter than its associated text
+    /// (dependent on scaled font size).
+    pub extent: Linear,
+}
+
+/// The kind of line decoration.
+#[derive(Debug, Copy, Clone, Eq, PartialEq, Hash)]
+pub enum LineKind {
+    /// A line under text.
+    Underline,
+    /// A line through text.
+    Strikethrough,
+    /// A line over text.
+    Overline,
+}
+
+impl Decoration {
+    /// Apply a decoration to a child's frame.
+    pub fn apply(&self, ctx: &LayoutContext, frame: &mut Frame) {
+        match self {
+            Decoration::Link(href) => {
+                let link = Element::Link(href.to_string(), frame.size);
+                frame.push(Point::zero(), link);
+            }
+            Decoration::Line(line) => {
+                line.apply(ctx, frame);
+            }
+        }
+    }
+}
+
+impl LineDecoration {
+    /// Apply a line decoration to a all text elements in a frame.
+    pub fn apply(&self, ctx: &LayoutContext, frame: &mut Frame) {
+        for i in 0 .. frame.children.len() {
+            let (pos, child) = &frame.children[i];
+            if let FrameChild::Element(Element::Text(text)) = child {
+                let face = ctx.fonts.get(text.face_id);
+                let metrics = match self.kind {
+                    LineKind::Underline => face.underline,
+                    LineKind::Strikethrough => face.strikethrough,
+                    LineKind::Overline => face.overline,
+                };
+
+                let stroke = self.stroke.unwrap_or(text.fill);
+
+                let thickness = self
+                    .thickness
+                    .map(|s| s.resolve(text.size))
+                    .unwrap_or(metrics.strength.to_length(text.size));
+
+                let offset = self
+                    .offset
+                    .map(|s| s.resolve(text.size))
+                    .unwrap_or(-metrics.position.to_length(text.size));
+
+                let extent = self.extent.resolve(text.size);
+
+                let subpos = Point::new(pos.x - extent, pos.y + offset);
+                let vector = Point::new(text.width + 2.0 * extent, Length::zero());
+                let line = Geometry::Line(vector, thickness);
+
+                frame.push(subpos, Element::Geometry(line, stroke));
+            }
+        }
+    }
+}
+
 /// Additional methods for BiDi levels.
 trait LevelExt: Sized {
     fn from_dir(dir: Dir) -> Option<Self>;
diff --git a/src/layout/shaping.rs b/src/layout/shaping.rs
index efee1591..eebeacbf 100644
--- a/src/layout/shaping.rs
+++ b/src/layout/shaping.rs
@@ -4,10 +4,9 @@ use std::ops::Range;
 use rustybuzz::UnicodeBuffer;
 
 use super::{Element, Frame, Glyph, LayoutContext, Text};
-use crate::eval::{FontState, LineState};
-use crate::font::{Face, FaceId, FontVariant, LineMetrics};
+use crate::eval::FontState;
+use crate::font::{Face, FaceId, FontVariant};
 use crate::geom::{Dir, Em, Length, Point, Size};
-use crate::layout::Geometry;
 use crate::util::SliceExt;
 
 /// Shape text into [`ShapedText`].
@@ -85,7 +84,7 @@ pub struct ShapedGlyph {
 
 impl<'a> ShapedText<'a> {
     /// Build the shaped text's frame.
-    pub fn build(&self, ctx: &LayoutContext) -> Frame {
+    pub fn build(&self) -> Frame {
         let mut frame = Frame::new(self.size, self.baseline);
         let mut offset = Length::zero();
 
@@ -95,24 +94,22 @@ impl<'a> ShapedText<'a> {
             let mut text = Text {
                 face_id,
                 size: self.state.size,
+                width: Length::zero(),
                 fill: self.state.fill,
                 glyphs: vec![],
             };
 
-            let mut width = Length::zero();
             for glyph in group {
                 text.glyphs.push(Glyph {
                     id: glyph.glyph_id,
                     x_advance: glyph.x_advance,
                     x_offset: glyph.x_offset,
                 });
-                width += glyph.x_advance.to_length(text.size);
+                text.width += glyph.x_advance.to_length(text.size);
             }
 
+            offset += text.width;
             frame.push(pos, Element::Text(text));
-            decorate(ctx, &mut frame, pos, width, face_id, &self.state);
-
-            offset += width;
         }
 
         frame
@@ -371,48 +368,3 @@ fn measure(
 
     (Size::new(width, top + bottom), top)
 }
-
-/// Add underline, strikthrough and overline decorations.
-fn decorate(
-    ctx: &LayoutContext,
-    frame: &mut Frame,
-    pos: Point,
-    width: Length,
-    face_id: FaceId,
-    state: &FontState,
-) {
-    let mut apply = |substate: &LineState, metrics: fn(&Face) -> &LineMetrics| {
-        let metrics = metrics(ctx.fonts.get(face_id));
-
-        let stroke = substate.stroke.unwrap_or(state.fill);
-
-        let thickness = substate
-            .thickness
-            .map(|s| s.resolve(state.size))
-            .unwrap_or(metrics.strength.to_length(state.size));
-
-        let offset = substate
-            .offset
-            .map(|s| s.resolve(state.size))
-            .unwrap_or(-metrics.position.to_length(state.size));
-
-        let extent = substate.extent.resolve(state.size);
-
-        let pos = Point::new(pos.x - extent, pos.y + offset);
-        let target = Point::new(width + 2.0 * extent, Length::zero());
-        let element = Element::Geometry(Geometry::Line(target, thickness), stroke);
-        frame.push(pos, element);
-    };
-
-    if let Some(strikethrough) = &state.strikethrough {
-        apply(strikethrough, |face| &face.strikethrough);
-    }
-
-    if let Some(underline) = &state.underline {
-        apply(underline, |face| &face.underline);
-    }
-
-    if let Some(overline) = &state.overline {
-        apply(overline, |face| &face.overline);
-    }
-}
diff --git a/src/library/text.rs b/src/library/text.rs
index cfd2de99..f3e086c8 100644
--- a/src/library/text.rs
+++ b/src/library/text.rs
@@ -1,5 +1,4 @@
-use crate::eval::{Decoration, FontState, LineState};
-use crate::layout::Paint;
+use crate::layout::{Decoration, LineDecoration, LineKind, Paint};
 
 use super::*;
 
@@ -155,47 +154,39 @@ fn lang_dir(iso: &str) -> Dir {
 
 /// `strike`: Set striken-through text.
 pub fn strike(ctx: &mut EvalContext, args: &mut Arguments) -> TypResult<Value> {
-    line_impl(ctx, args, |font| &mut font.strikethrough)
+    line_impl(ctx, args, LineKind::Strikethrough)
 }
 
 /// `underline`: Set underlined text.
 pub fn underline(ctx: &mut EvalContext, args: &mut Arguments) -> TypResult<Value> {
-    line_impl(ctx, args, |font| &mut font.underline)
+    line_impl(ctx, args, LineKind::Underline)
 }
 
 /// `overline`: Set text with an overline.
 pub fn overline(ctx: &mut EvalContext, args: &mut Arguments) -> TypResult<Value> {
-    line_impl(ctx, args, |font| &mut font.overline)
+    line_impl(ctx, args, LineKind::Overline)
 }
 
 fn line_impl(
     _: &mut EvalContext,
     args: &mut Arguments,
-    substate: fn(&mut FontState) -> &mut Option<Rc<LineState>>,
+    kind: LineKind,
 ) -> TypResult<Value> {
     let stroke = args.named("stroke")?.or_else(|| args.eat());
     let thickness = args.named::<Linear>("thickness")?.or_else(|| args.eat());
     let offset = args.named("offset")?;
     let extent = args.named("extent")?.unwrap_or_default();
-    let body = args.expect("body")?;
-
-    // Suppress any existing strikethrough if strength is explicitly zero.
-    let line = thickness.map_or(true, |s| !s.is_zero()).then(|| {
-        Rc::new(LineState {
-            stroke: stroke.map(Paint::Color),
-            thickness,
-            offset,
-            extent,
-        })
-    });
 
-    let mut template = Template::new();
-    template.save();
-    template.modify(move |state| *substate(state.font_mut()) = line.clone());
-    template += body;
-    template.restore();
+    let mut body: Template = args.expect("body")?;
+    body.decorate(Decoration::Line(LineDecoration {
+        kind,
+        stroke: stroke.map(Paint::Color),
+        thickness,
+        offset,
+        extent,
+    }));
 
-    Ok(Value::Template(template))
+    Ok(Value::Template(body))
 }
 
 /// `link`: Set a link.