An AlignPointElem has a non-zero height

[Leedehai]

Description

A line of - - should have a very small height, as shown in the left two boxes' rendering; but after inserting & to this line, the line gains a visible height, as shown in the right two boxes.

#box($ - - $, fill: silver)
#box($ a \ - - $, fill: silver)
#box($ - &- $, fill: silver)
#box($ a \ - &- $, fill: silver)

However, the height of a MathFragment::Align (inserted by impl LayoutMath for Packed<AlignPointElem>) is supposed to be zero, according to

typst/crates/typst/src/math/fragment.rs

Lines 45 to 52 in 4f8bbb0

	pub fn height(&self) -> Abs {
	match self {
	Self::Glyph(glyph) => glyph.height(),
	Self::Variant(variant) => variant.frame.height(),
	Self::Frame(fragment) => fragment.frame.height(),
	_ => Abs::zero(),
	}
	}

Reproduction URL

No response

Operating system

Web app, Linux

Typst version

• I am using the latest version of Typst

[Leedehai]

This is because in MathRun::into_line_frame(), the line's frame height is calculated by adding the line's MathRun::ascent() and MathRun::descent(), which are the maximum ascent() and maximum descent() among all MathFragments in this line (MathRun).

In a line with - (a MathFragment::Glyph) only, every fragment has ascent 2.7pt and descent -2.3pt (negative), so the line's height is 2.7pt + (-2.3pt) = 0.4pt. If the line contains a MathFragment::Align, that fragment's ascent and descent are 0pt, so the max ascent of all fragments in this line is still 2.7pt, but the max descent of all fragments in this line becomes 0pt (instead of -2.3pt), so the line's height becomes 2.7pt + 0pt = 2.7pt.

[laurmaedje]

I think ascent and descent should be clamped to zero there.

[Leedehai]

Clamping to zero may lead to unintended results, because a glyph's lowest point could indeed be higher than the baseline, and therefore having a negative descent. For example, if MathRun::descent() is modified like this

    pub fn descent(&self) -> Abs {
        self.iter()
-           .map(MathFragment::descent)
+           .map(|e| e.descent().max(Abs::zero()))
            .max()
            .unwrap_or_default()
    }

then one of the broken tests,math-underover-brackets, shows

More obvious: $ overbrace(equiv) $
Output: , Reference:

[Leedehai]

Instead, I suggest this change below (all tests passing).

The idea is that some MathFragments are not real fragments to be layouted, but are in-band control signals to instruct how fragments shall be layouted. Therefore, these control fragments are "shapeless"/"sizeless" -- it is not the same as "having a zero size"; the notion of "shape"/"size" itself is absent for these fragments.

diff --git a/crates/typst/src/math/fragment.rs b/crates/typst/src/math/fragment.rs
index ef865b38..596c3c16 100644
--- a/crates/typst/src/math/fragment.rs
+++ b/crates/typst/src/math/fragment.rs
@@ -139,6 +139,10 @@ impl MathFragment {
         }
     }

+    pub fn is_shapeless(&self) -> bool {
+        matches!(self, MathFragment::Align | MathFragment::Linebreak)
+    }
+
     pub fn italics_correction(&self) -> Abs {
         match self {
             Self::Glyph(glyph) => glyph.italics_correction,
diff --git a/crates/typst/src/math/row.rs b/crates/typst/src/math/row.rs
index 6454f491..6577abf5 100644
--- a/crates/typst/src/math/row.rs
+++ b/crates/typst/src/math/row.rs
@@ -117,11 +117,17 @@ impl MathRun {
     }

     pub fn ascent(&self) -> Abs {
-        self.iter().map(MathFragment::ascent).max().unwrap_or_default()
+        self.iter()
+            .filter_map(|e| (!e.is_shapeless()).then(|| e.ascent()))
+            .max()
+            .unwrap_or_default()
     }

     pub fn descent(&self) -> Abs {
-        self.iter().map(MathFragment::descent).max().unwrap_or_default()
+        self.iter()
+            .filter_map(|e| (!e.is_shapeless()).then(|| e.descent()))
+            .max()
+            .unwrap_or_default()
     }

---

Edit:

Alternatively, following the same idea of control fragments being "shapeless"/"sizeless", a better change is letting MathFragment::height()/MathFragment::width() return Option<Abs> (status quo is Abs), instead of adding this is_shapeless() method -- because if is_shapeless() should return true, it makes more sense if height()/width() return None instead of Abs::zero(). The downside is there are numerous other call sites to modify.