Improve positioning of multiple accents and attachments (#3059)

10 files changed, 92 insertions, 56 deletions

diff --git a/crates/typst/src/math/accent.rs b/crates/typst/src/math/accent.rs
index 06fd6bab..8f88c5b8 100644
--- a/crates/typst/src/math/accent.rs
+++ b/crates/typst/src/math/accent.rs
@@ -1,9 +1,8 @@
-use ttf_parser::GlyphId;
 use unicode_math_class::MathClass;
 
 use crate::diag::{bail, SourceResult};
 use crate::foundations::{cast, elem, Content, NativeElement, Resolve, Smart, Value};
-use crate::layout::{Abs, Em, Frame, Length, Point, Rel, Size};
+use crate::layout::{Em, Frame, Length, Point, Rel, Size};
 use crate::math::{
     FrameFragment, GlyphFragment, LayoutMath, MathContext, MathFragment, Scaled,
 };
@@ -72,12 +71,7 @@ impl LayoutMath for AccentElem {
 
         // Preserve class to preserve automatic spacing.
         let base_class = base.class().unwrap_or(MathClass::Normal);
-        let base_attach = match &base {
-            MathFragment::Glyph(base) => {
-                attachment(ctx, base.id, base.italics_correction)
-            }
-            _ => (base.width() + base.italics_correction()) / 2.0,
-        };
+        let base_attach = base.accent_attach();
 
         let width = self
             .size(ctx.styles())
@@ -92,10 +86,7 @@ impl LayoutMath for AccentElem {
         let short_fall = ACCENT_SHORT_FALL.scaled(ctx);
         let variant = glyph.stretch_horizontal(ctx, width, short_fall);
         let accent = variant.frame;
-        let accent_attach = match variant.id {
-            Some(id) => attachment(ctx, id, variant.italics_correction),
-            None => accent.width() / 2.0,
-        };
+        let accent_attach = variant.accent_attach;
 
         // Descent is negative because the accent's ink bottom is above the
         // baseline. Therefore, the default gap is the accent's negated descent
@@ -106,8 +97,14 @@ impl LayoutMath for AccentElem {
         let size = Size::new(base.width(), accent.height() + gap + base.height());
         let accent_pos = Point::with_x(base_attach - accent_attach);
         let base_pos = Point::with_y(accent.height() + gap);
-        let base_ascent = base.ascent();
         let baseline = base_pos.y + base.ascent();
+        let base_italics_correction = base.italics_correction();
+        let base_text_like = base.is_text_like();
+
+        let base_ascent = match &base {
+            MathFragment::Frame(frame) => frame.base_ascent,
+            _ => base.ascent(),
+        };
 
         let mut frame = Frame::soft(size);
         frame.set_baseline(baseline);
@@ -116,26 +113,16 @@ impl LayoutMath for AccentElem {
         ctx.push(
             FrameFragment::new(ctx, frame)
                 .with_class(base_class)
-                .with_base_ascent(base_ascent),
+                .with_base_ascent(base_ascent)
+                .with_italics_correction(base_italics_correction)
+                .with_accent_attach(base_attach)
+                .with_text_like(base_text_like),
         );
 
         Ok(())
     }
 }
 
-/// The horizontal attachment position for the given glyph.
-fn attachment(ctx: &MathContext, id: GlyphId, italics_correction: Abs) -> Abs {
-    ctx.table
-        .glyph_info
-        .and_then(|info| info.top_accent_attachments)
-        .and_then(|attachments| attachments.get(id))
-        .map(|record| record.value.scaled(ctx))
-        .unwrap_or_else(|| {
-            let advance = ctx.ttf.glyph_hor_advance(id).unwrap_or_default();
-            (advance.scaled(ctx) + italics_correction) / 2.0
-        })
-}
-
 /// An accent character.
 #[derive(Debug, Default, Copy, Clone, Eq, PartialEq, Hash)]
 pub struct Accent(char);
diff --git a/crates/typst/src/math/attach.rs b/crates/typst/src/math/attach.rs
index 67f77001..a6f92afb 100644
--- a/crates/typst/src/math/attach.rs
+++ b/crates/typst/src/math/attach.rs
@@ -2,7 +2,7 @@ use unicode_math_class::MathClass;
 
 use crate::diag::SourceResult;
 use crate::foundations::{elem, Content, StyleChain};
-use crate::layout::{Abs, Frame, FrameItem, Point, Size};
+use crate::layout::{Abs, Frame, Point, Size};
 use crate::math::{
     FrameFragment, LayoutMath, MathContext, MathFragment, MathSize, Scaled,
 };
@@ -382,7 +382,7 @@ fn compute_shifts_up_and_down(
 
     let mut shift_up = Abs::zero();
     let mut shift_down = Abs::zero();
-    let is_char_box = is_character_box(base);
+    let is_text_like = base.is_text_like();
 
     if tl.is_some() || tr.is_some() {
         let ascent = match &base {
@@ -391,7 +391,7 @@ fn compute_shifts_up_and_down(
         };
         shift_up = shift_up
             .max(sup_shift_up)
-            .max(if is_char_box { Abs::zero() } else { ascent - sup_drop_max })
+            .max(if is_text_like { Abs::zero() } else { ascent - sup_drop_max })
             .max(sup_bottom_min + measure!(tl, descent))
             .max(sup_bottom_min + measure!(tr, descent));
     }
@@ -399,7 +399,7 @@ fn compute_shifts_up_and_down(
     if bl.is_some() || br.is_some() {
         shift_down = shift_down
             .max(sub_shift_down)
-            .max(if is_char_box { Abs::zero() } else { base.descent() + sub_drop_min })
+            .max(if is_text_like { Abs::zero() } else { base.descent() + sub_drop_min })
             .max(measure!(bl, ascent) - sub_top_max)
             .max(measure!(br, ascent) - sub_top_max);
     }
@@ -429,24 +429,3 @@ fn compute_shifts_up_and_down(
 fn is_integral_char(c: char) -> bool {
     ('∫'..='∳').contains(&c) || ('⨋'..='⨜').contains(&c)
 }
-
-/// Whether the fragment consists of a single character or atomic piece of text.
-fn is_character_box(fragment: &MathFragment) -> bool {
-    match fragment {
-        MathFragment::Glyph(_) | MathFragment::Variant(_) => {
-            fragment.class() != Some(MathClass::Large)
-        }
-        MathFragment::Frame(fragment) => is_atomic_text_frame(&fragment.frame),
-        _ => false,
-    }
-}
-
-/// Handles e.g. "sin", "log", "exp", "CustomOperator".
-fn is_atomic_text_frame(frame: &Frame) -> bool {
-    // Meta information isn't visible or renderable, so we exclude it.
-    let mut iter = frame
-        .items()
-        .map(|(_, item)| item)
-        .filter(|item| !matches!(item, FrameItem::Meta(_, _)));
-    matches!(iter.next(), Some(FrameItem::Text(_))) && iter.next().is_none()
-}
diff --git a/crates/typst/src/math/cancel.rs b/crates/typst/src/math/cancel.rs
index bf1e6d75..5fa34d7d 100644
--- a/crates/typst/src/math/cancel.rs
+++ b/crates/typst/src/math/cancel.rs
@@ -109,8 +109,12 @@ impl LayoutMath for CancelElem {
     #[typst_macros::time(name = "math.cancel", span = self.span())]
     fn layout_math(&self, ctx: &mut MathContext) -> SourceResult<()> {
         let body = ctx.layout_fragment(self.body())?;
-        // Use the same math class as the body, in order to preserve automatic spacing around it.
+        // Preserve properties of body.
         let body_class = body.class().unwrap_or(MathClass::Special);
+        let body_italics = body.italics_correction();
+        let body_attach = body.accent_attach();
+        let body_text_like = body.is_text_like();
+
         let mut body = body.into_frame();
 
         let styles = ctx.styles();
@@ -150,7 +154,13 @@ impl LayoutMath for CancelElem {
             body.push_frame(center, second_line);
         }
 
-        ctx.push(FrameFragment::new(ctx, body).with_class(body_class));
+        ctx.push(
+            FrameFragment::new(ctx, body)
+                .with_class(body_class)
+                .with_italics_correction(body_italics)
+                .with_accent_attach(body_attach)
+                .with_text_like(body_text_like),
+        );
 
         Ok(())
     }
diff --git a/crates/typst/src/math/ctx.rs b/crates/typst/src/math/ctx.rs
index 73cb8707..b93d7cc2 100644
--- a/crates/typst/src/math/ctx.rs
+++ b/crates/typst/src/math/ctx.rs
@@ -226,7 +226,7 @@ impl<'a, 'b, 'v> MathContext<'a, 'b, 'v> {
                 fragments.push(GlyphFragment::new(self, c, span).into());
             }
             let frame = MathRow::new(fragments).into_frame(self);
-            FrameFragment::new(self, frame).into()
+            FrameFragment::new(self, frame).with_text_like(true).into()
         } else {
             // Anything else is handled by Typst's standard text layout.
             let mut style = self.style;
@@ -286,6 +286,7 @@ impl<'a, 'b, 'v> MathContext<'a, 'b, 'v> {
 
         Ok(FrameFragment::new(self, frame)
             .with_class(MathClass::Alphabetic)
+            .with_text_like(true)
             .with_spaced(spaced))
     }
 
diff --git a/crates/typst/src/math/fragment.rs b/crates/typst/src/math/fragment.rs
index 6131f663..cad79c67 100644
--- a/crates/typst/src/math/fragment.rs
+++ b/crates/typst/src/math/fragment.rs
@@ -142,14 +142,32 @@ impl MathFragment {
         }
     }
 
+    pub fn is_text_like(&self) -> bool {
+        match self {
+            Self::Glyph(_) | Self::Variant(_) => self.class() != Some(MathClass::Large),
+            MathFragment::Frame(frame) => frame.text_like,
+            _ => false,
+        }
+    }
+
     pub fn italics_correction(&self) -> Abs {
         match self {
             Self::Glyph(glyph) => glyph.italics_correction,
             Self::Variant(variant) => variant.italics_correction,
+            Self::Frame(fragment) => fragment.italics_correction,
             _ => Abs::zero(),
         }
     }
 
+    pub fn accent_attach(&self) -> Abs {
+        match self {
+            Self::Glyph(glyph) => glyph.accent_attach,
+            Self::Variant(variant) => variant.accent_attach,
+            Self::Frame(fragment) => fragment.accent_attach,
+            _ => self.width() / 2.0,
+        }
+    }
+
     pub fn into_frame(self) -> Frame {
         match self {
             Self::Glyph(glyph) => glyph.into_frame(),
@@ -199,6 +217,7 @@ pub struct GlyphFragment {
     pub ascent: Abs,
     pub descent: Abs,
     pub italics_correction: Abs,
+    pub accent_attach: Abs,
     pub style: MathStyle,
     pub font_size: Abs,
     pub class: Option<MathClass>,
@@ -241,6 +260,7 @@ impl GlyphFragment {
             descent: Abs::zero(),
             limits: Limits::for_char(c),
             italics_correction: Abs::zero(),
+            accent_attach: Abs::zero(),
             class,
             span,
             meta: MetaElem::data_in(ctx.styles()),
@@ -271,6 +291,8 @@ impl GlyphFragment {
         });
 
         let mut width = advance.scaled(ctx);
+        let accent_attach = accent_attach(ctx, id).unwrap_or((width + italics) / 2.0);
+
         if !is_extended_shape(ctx, id) {
             width += italics;
         }
@@ -280,6 +302,7 @@ impl GlyphFragment {
         self.ascent = bbox.y_max.scaled(ctx);
         self.descent = -bbox.y_min.scaled(ctx);
         self.italics_correction = italics;
+        self.accent_attach = accent_attach;
     }
 
     pub fn height(&self) -> Abs {
@@ -293,6 +316,7 @@ impl GlyphFragment {
             style: self.style,
             font_size: self.font_size,
             italics_correction: self.italics_correction,
+            accent_attach: self.accent_attach,
             class: self.class,
             span: self.span,
             limits: self.limits,
@@ -356,6 +380,7 @@ pub struct VariantFragment {
     pub c: char,
     pub id: Option<GlyphId>,
     pub italics_correction: Abs,
+    pub accent_attach: Abs,
     pub frame: Frame,
     pub style: MathStyle,
     pub font_size: Abs,
@@ -389,11 +414,15 @@ pub struct FrameFragment {
     pub limits: Limits,
     pub spaced: bool,
     pub base_ascent: Abs,
+    pub italics_correction: Abs,
+    pub accent_attach: Abs,
+    pub text_like: bool,
 }
 
 impl FrameFragment {
     pub fn new(ctx: &MathContext, mut frame: Frame) -> Self {
         let base_ascent = frame.ascent();
+        let accent_attach = frame.width() / 2.0;
         frame.meta(ctx.styles(), false);
         Self {
             frame,
@@ -403,6 +432,9 @@ impl FrameFragment {
             limits: Limits::Never,
             spaced: false,
             base_ascent,
+            italics_correction: Abs::zero(),
+            accent_attach,
+            text_like: false,
         }
     }
 
@@ -421,6 +453,18 @@ impl FrameFragment {
     pub fn with_base_ascent(self, base_ascent: Abs) -> Self {
         Self { base_ascent, ..self }
     }
+
+    pub fn with_italics_correction(self, italics_correction: Abs) -> Self {
+        Self { italics_correction, ..self }
+    }
+
+    pub fn with_accent_attach(self, accent_attach: Abs) -> Self {
+        Self { accent_attach, ..self }
+    }
+
+    pub fn with_text_like(self, text_like: bool) -> Self {
+        Self { text_like, ..self }
+    }
 }
 
 /// Look up the italics correction for a glyph.
@@ -428,6 +472,11 @@ fn italics_correction(ctx: &MathContext, id: GlyphId) -> Option<Abs> {
     Some(ctx.table.glyph_info?.italic_corrections?.get(id)?.scaled(ctx))
 }
 
+/// Loop up the top accent attachment position for a glyph.
+fn accent_attach(ctx: &MathContext, id: GlyphId) -> Option<Abs> {
+    Some(ctx.table.glyph_info?.top_accent_attachments?.get(id)?.scaled(ctx))
+}
+
 /// Look up the script/scriptscript alternates for a glyph
 fn script_alternatives<'a>(
     ctx: &MathContext<'a, '_, '_>,
@@ -438,7 +487,7 @@ fn script_alternatives<'a>(
     })
 }
 
-/// Look up the italics correction for a glyph.
+/// Look up whether a glyph is an extended shape.
 fn is_extended_shape(ctx: &MathContext, id: GlyphId) -> bool {
     ctx.table
         .glyph_info
diff --git a/crates/typst/src/math/op.rs b/crates/typst/src/math/op.rs
index bf455903..e1ac2b72 100644
--- a/crates/typst/src/math/op.rs
+++ b/crates/typst/src/math/op.rs
@@ -37,9 +37,16 @@ impl LayoutMath for OpElem {
     #[typst_macros::time(name = "math.op", span = self.span())]
     fn layout_math(&self, ctx: &mut MathContext) -> SourceResult<()> {
         let fragment = ctx.layout_fragment(self.text())?;
+        let italics = fragment.italics_correction();
+        let accent_attach = fragment.accent_attach();
+        let text_like = fragment.is_text_like();
+
         ctx.push(
             FrameFragment::new(ctx, fragment.into_frame())
                 .with_class(MathClass::Large)
+                .with_italics_correction(italics)
+                .with_accent_attach(accent_attach)
+                .with_text_like(text_like)
                 .with_limits(if self.limits(ctx.styles()) {
                     Limits::Display
                 } else {
diff --git a/crates/typst/src/math/stretch.rs b/crates/typst/src/math/stretch.rs
index bfeb80ef..90ed03b7 100644
--- a/crates/typst/src/math/stretch.rs
+++ b/crates/typst/src/math/stretch.rs
@@ -177,6 +177,8 @@ fn assemble(
         offset += advance;
     }
 
+    let accent_attach = if horizontal { frame.width() / 2.0 } else { base.accent_attach };
+
     VariantFragment {
         c: base.c,
         id: None,
@@ -184,6 +186,7 @@ fn assemble(
         style: base.style,
         font_size: base.font_size,
         italics_correction: Abs::zero(),
+        accent_attach,
         class: base.class,
         span: base.span,
         limits: base.limits,
diff --git a/tests/ref/math/accent.png b/tests/ref/math/accent.png
index b955b5ad..72fa420a 100644
--- a/tests/ref/math/accent.png
+++ b/tests/ref/math/accent.png
diff --git a/tests/ref/math/attach-p1.png b/tests/ref/math/attach-p1.png
index fa610729..45c465ce 100644
--- a/tests/ref/math/attach-p1.png
+++ b/tests/ref/math/attach-p1.png
diff --git a/tests/ref/math/frac.png b/tests/ref/math/frac.png
index 3af9d33c..3e08f7e5 100644
--- a/tests/ref/math/frac.png
+++ b/tests/ref/math/frac.png