Better line spacing calculations ↕

- Only add line spacing between lines. Previously, line spacing was added below
  every line, making `#box[word]` higher than just `word`.
- Compute box height of text as `ascender - descender` so that the full word is
  contained in the box.

src/exec/state.rs

@@ -93,7 +93,7 @@ impl Default for ParState {
     fn default() -> Self {
         Self {
             word_spacing: Relative::new(0.25).into(),
-            line_spacing: Relative::new(0.2).into(),
+            line_spacing: Linear::ZERO,
             par_spacing: Relative::new(0.5).into(),
         }
     }

src/export/pdf.rs

@@ -195,7 +195,7 @@ impl<'a> PdfExporter<'a> {
                 }
 
                 let x = pos.x.to_pt() as f32;
-                let y = (page.size.height - pos.y - size).to_pt() as f32;
+                let y = (page.size.height - pos.y).to_pt() as f32;
                 text.matrix(1.0, 0.0, 0.0, 1.0, x, y);
                 text.show(&shaped.encode_glyphs_be());
             }

src/layout/par.rs

@@ -163,13 +163,17 @@ impl<'a> ParLayouter<'a> {
             output.push_frame(pos, frame);
         }
 
+        // Add line spacing, but only between lines.
+        if !self.lines.is_empty() {
+            self.lines_size.main += self.par.line_spacing;
+            *self.areas.current.get_mut(self.main) -= self.par.line_spacing;
+        }
+
         // Update metrics of the whole paragraph.
         self.lines.push((self.lines_size.main, output, self.line_ruler));
         self.lines_size.main += full_size.main;
-        self.lines_size.main += self.par.line_spacing;
         self.lines_size.cross = self.lines_size.cross.max(full_size.cross);
         *self.areas.current.get_mut(self.main) -= full_size.main;
-        *self.areas.current.get_mut(self.main) -= self.par.line_spacing;
 
         // Reset metrics for the single line.
         self.line_size = Gen::ZERO;

src/shaping.rs

@@ -70,6 +70,8 @@ pub fn shape(
     let mut frame = Frame::new(Size::new(Length::ZERO, font_size));
     let mut shaped = Shaped::new(FaceId::MAX, font_size);
     let mut offset = Length::ZERO;
+    let mut ascender = Length::ZERO;
+    let mut descender = Length::ZERO;
 
     // Create an iterator with conditional direction.
     let mut forwards = text.chars();
@@ -84,47 +86,56 @@ pub fn shape(
         let query = FaceQuery { fallback: fallback.iter(), variant, c };
         if let Some(id) = loader.query(query) {
             let face = loader.face(id).get();
-            let (glyph, width) = match lookup_glyph(face, c, font_size) {
+            let (glyph, width) = match lookup_glyph(face, c) {
                 Some(v) => v,
                 None => continue,
             };
 
-            // Flush the buffer if we change the font face.
-            if shaped.face != id && !shaped.text.is_empty() {
-                let pos = Point::new(frame.size.width, Length::ZERO);
-                frame.push(pos, Element::Text(shaped));
-                frame.size.width += offset;
-                shaped = Shaped::new(FaceId::MAX, font_size);
+            let units_per_em = f64::from(face.units_per_em().unwrap_or(1000));
+            let convert = |units| units / units_per_em * font_size;
+
+            // Flush the buffer and reset the metrics if we use a new font face.
+            if shaped.face != id {
+                place(&mut frame, shaped, offset, ascender, descender);
+
+                shaped = Shaped::new(id, font_size);
                 offset = Length::ZERO;
+                ascender = convert(f64::from(face.ascender()));
+                descender = convert(f64::from(face.descender()));
             }
 
-            shaped.face = id;
             shaped.text.push(c);
             shaped.glyphs.push(glyph);
             shaped.offsets.push(offset);
-            offset += width;
+            offset += convert(f64::from(width));
         }
     }
 
     // Flush the last buffered parts of the word.
-    if !shaped.text.is_empty() {
-        let pos = Point::new(frame.size.width, Length::ZERO);
-        frame.push(pos, Element::Text(shaped));
-        frame.size.width += offset;
-    }
+    place(&mut frame, shaped, offset, ascender, descender);
 
     frame
 }
 
-/// Looks up the glyph for `c` and returns its index alongside its width at the
-/// given `size`.
-fn lookup_glyph(face: &Face, c: char, size: Length) -> Option<(GlyphId, Length)> {
+/// Look up the glyph for `c` and returns its index alongside its advance width.
+fn lookup_glyph(face: &Face, c: char) -> Option<(GlyphId, u16)> {
     let glyph = face.glyph_index(c)?;
-
-    // Determine the width of the char.
-    let units_per_em = face.units_per_em().unwrap_or(1000) as f64;
-    let width_units = face.glyph_hor_advance(glyph)? as f64;
-    let width = width_units / units_per_em * size;
-
+    let width = face.glyph_hor_advance(glyph)?;
     Some((glyph, width))
 }
+
+/// Place shaped text into a frame.
+fn place(
+    frame: &mut Frame,
+    shaped: Shaped,
+    offset: Length,
+    ascender: Length,
+    descender: Length,
+) {
+    if !shaped.text.is_empty() {
+        let pos = Point::new(frame.size.width, ascender);
+        frame.push(pos, Element::Text(shaped));
+        frame.size.width += offset;
+        frame.size.height = frame.size.height.max(ascender - descender);
+    }
+}

tests/typ/full/coma.typ

@@ -1,5 +1,5 @@
 // Configuration with `page` and `font` functions.
-#page(width: 450pt, height: 380pt, margins: 1cm)
+#page(width: 450pt, margins: 1cm)
 
 // There are variables and they can take normal values like strings, ...
 #let city = "Berlin"

tests/typ/library/box.typ

@@ -1,15 +1,15 @@
 // Test the box function.
 
 ---
-#page("a7", flip: true)
+#page("a8", flip: true)
 
 // Box with fixed width, should have text height.
-#box(width: 2cm, color: #9650D6)[A]
+#box(width: 2cm, color: #9650D6)[Legal]
 
 Sometimes there is no box.
 
 // Box with fixed height, should span line.
-#box(height: 2cm, width: 100%, color: #734CED)[B]
+#box(height: 1cm, width: 100%, color: #734CED)[B]
 
 // Empty box with fixed width and height.
 #box(width: 6cm, height: 12pt, color: #CB4CED)
@@ -18,6 +18,6 @@ Sometimes there is no box.
 #box(width: 2in, color: #ff0000)
 
 // These are in a row!
-#box(width: 1in, height: 10pt, color: #D6CD67)
-#box(width: 1in, height: 10pt, color: #EDD466)
-#box(width: 1in, height: 10pt, color: #E3BE62)
+#box(width: 0.5in, height: 10pt, color: #D6CD67)
+#box(width: 0.5in, height: 10pt, color: #EDD466)
+#box(width: 0.5in, height: 10pt, color: #E3BE62)

tests/typ/repr.typ

@@ -32,9 +32,8 @@
 {12e1pt} \
 {2.5rad} \
 {45deg} \
-
 // Not in monospace via repr.
-#repr(45deg)
+#repr(45deg) \
 
 ---
 // Colors.

tests/typeset.rs

@@ -393,7 +393,7 @@ fn draw_text(env: &Env, canvas: &mut Canvas, pos: Point, shaped: &Shaped) {
         let units_per_em = face.units_per_em().unwrap_or(1000);
 
         let x = (pos.x + offset).to_pt() as f32;
-        let y = (pos.y + shaped.font_size).to_pt() as f32;
+        let y = pos.y.to_pt() as f32;
         let scale = (shaped.font_size / units_per_em as f64).to_pt() as f32;
 
         let mut builder = WrappedPathBuilder(PathBuilder::new());