From 61d461f0800c22afd2c279786d326733f679a36d Mon Sep 17 00:00:00 2001 From: Laurenz Date: Sat, 12 Oct 2024 14:01:32 +0200 Subject: [PATCH] Refactor and fix bounds metric (#5186) --- crates/typst/src/layout/inline/shaping.rs | 18 +++---- crates/typst/src/math/equation.rs | 17 ++++--- crates/typst/src/text/deco.rs | 31 ++++++------ crates/typst/src/text/font/mod.rs | 57 ++++++++++++++++++++++- crates/typst/src/text/mod.rs | 46 +----------------- tests/suite/layout/measure.typ | 7 +++ 6 files changed, 97 insertions(+), 79 deletions(-) diff --git a/crates/typst/src/layout/inline/shaping.rs b/crates/typst/src/layout/inline/shaping.rs index 9d83a9a02..5b95dadb7 100644 --- a/crates/typst/src/layout/inline/shaping.rs +++ b/crates/typst/src/layout/inline/shaping.rs @@ -16,7 +16,7 @@ use crate::foundations::{Smart, StyleChain}; use crate::layout::{Abs, Dir, Em, Frame, FrameItem, Point, Size}; use crate::text::{ decorate, families, features, is_default_ignorable, variant, Font, FontVariant, - Glyph, Lang, Region, TextElem, TextItem, + Glyph, Lang, Region, TextEdgeBounds, TextElem, TextItem, }; use crate::utils::SliceExt; use crate::World; @@ -338,9 +338,10 @@ impl<'a> ShapedText<'a> { let bottom_edge = TextElem::bottom_edge_in(self.styles); // Expand top and bottom by reading the font's vertical metrics. - let mut expand = |font: &Font, bbox: Option| { - top.set_max(top_edge.resolve(self.size, font, bbox)); - bottom.set_max(-bottom_edge.resolve(self.size, font, bbox)); + let mut expand = |font: &Font, bounds: TextEdgeBounds| { + let (t, b) = font.edges(top_edge, bottom_edge, self.size, bounds); + top.set_max(t); + bottom.set_max(b); }; if self.glyphs.is_empty() { @@ -353,18 +354,13 @@ impl<'a> ShapedText<'a> { .select(family, self.variant) .and_then(|id| world.font(id)) { - expand(&font, None); + expand(&font, TextEdgeBounds::Zero); break; } } } else { for g in self.glyphs.iter() { - let bbox = if top_edge.is_bounds() || bottom_edge.is_bounds() { - g.font.ttf().glyph_bounding_box(ttf_parser::GlyphId(g.glyph_id)) - } else { - None - }; - expand(&g.font, bbox); + expand(&g.font, TextEdgeBounds::Glyph(g.glyph_id)); } } diff --git a/crates/typst/src/math/equation.rs b/crates/typst/src/math/equation.rs index babcbb8b0..6f0290d45 100644 --- a/crates/typst/src/math/equation.rs +++ b/crates/typst/src/math/equation.rs @@ -20,7 +20,8 @@ use crate::math::{ use crate::model::{Numbering, Outlinable, ParElem, ParLine, Refable, Supplement}; use crate::syntax::Span; use crate::text::{ - families, variant, Font, FontFamily, FontList, FontWeight, LocalName, TextElem, + families, variant, Font, FontFamily, FontList, FontWeight, LocalName, TextEdgeBounds, + TextElem, }; use crate::utils::{NonZeroExt, Numeric}; use crate::World; @@ -290,12 +291,16 @@ fn layout_equation_inline( let font_size = scaled_font_size(&ctx, styles); let slack = ParElem::leading_in(styles) * 0.7; - let top_edge = TextElem::top_edge_in(styles).resolve(font_size, &font, None); - let bottom_edge = - -TextElem::bottom_edge_in(styles).resolve(font_size, &font, None); - let ascent = top_edge.max(frame.ascent() - slack); - let descent = bottom_edge.max(frame.descent() - slack); + let (t, b) = font.edges( + TextElem::top_edge_in(styles), + TextElem::bottom_edge_in(styles), + font_size, + TextEdgeBounds::Frame(frame), + ); + + let ascent = t.max(frame.ascent() - slack); + let descent = b.max(frame.descent() - slack); frame.translate(Point::with_y(ascent - frame.baseline())); frame.size_mut().y = ascent + descent; } diff --git a/crates/typst/src/text/deco.rs b/crates/typst/src/text/deco.rs index ce819993c..ce98348d7 100644 --- a/crates/typst/src/text/deco.rs +++ b/crates/typst/src/text/deco.rs @@ -10,7 +10,8 @@ use crate::layout::{ }; use crate::syntax::Span; use crate::text::{ - BottomEdge, BottomEdgeMetric, TextElem, TextItem, TopEdge, TopEdgeMetric, + BottomEdge, BottomEdgeMetric, TextEdgeBounds, TextElem, TextItem, TopEdge, + TopEdgeMetric, }; use crate::visualize::{styled_rect, Color, FixedStroke, Geometry, Paint, Stroke}; @@ -422,7 +423,7 @@ pub(crate) fn decorate( &deco.line { let (top, bottom) = determine_edges(text, *top_edge, *bottom_edge); - let size = Size::new(width + 2.0 * deco.extent, top - bottom); + let size = Size::new(width + 2.0 * deco.extent, top + bottom); let rects = styled_rect(size, radius, fill.clone(), stroke); let origin = Point::new(pos.x - deco.extent, pos.y - top - shift); frame.prepend_multiple( @@ -540,22 +541,20 @@ fn determine_edges( top_edge: TopEdge, bottom_edge: BottomEdge, ) -> (Abs, Abs) { - let mut bbox = None; - if top_edge.is_bounds() || bottom_edge.is_bounds() { - let ttf = text.font.ttf(); - bbox = text - .glyphs - .iter() - .filter_map(|g| ttf.glyph_bounding_box(ttf_parser::GlyphId(g.id))) - .reduce(|a, b| ttf_parser::Rect { - y_max: a.y_max.max(b.y_max), - y_min: a.y_min.min(b.y_min), - ..a - }); + let mut top = Abs::zero(); + let mut bottom = Abs::zero(); + + for g in text.glyphs.iter() { + let (t, b) = text.font.edges( + top_edge, + bottom_edge, + text.size, + TextEdgeBounds::Glyph(g.id), + ); + top.set_max(t); + bottom.set_max(b); } - let top = top_edge.resolve(text.size, &text.font, bbox); - let bottom = bottom_edge.resolve(text.size, &text.font, bbox); (top, bottom) } diff --git a/crates/typst/src/text/font/mod.rs b/crates/typst/src/text/font/mod.rs index 701118136..09837312a 100644 --- a/crates/typst/src/text/font/mod.rs +++ b/crates/typst/src/text/font/mod.rs @@ -9,6 +9,7 @@ mod variant; pub use self::book::{Coverage, FontBook, FontFlags, FontInfo}; pub use self::variant::{FontStretch, FontStyle, FontVariant, FontWeight}; +use std::cell::OnceCell; use std::fmt::{self, Debug, Formatter}; use std::hash::{Hash, Hasher}; use std::sync::Arc; @@ -17,7 +18,8 @@ use ttf_parser::GlyphId; use self::book::find_name; use crate::foundations::{Bytes, Cast}; -use crate::layout::Em; +use crate::layout::{Abs, Em, Frame}; +use crate::text::{BottomEdge, TopEdge}; /// An OpenType font. /// @@ -125,6 +127,48 @@ impl Font { // internal 'static lifetime. &self.0.rusty } + + /// Resolve the top and bottom edges of text. + pub fn edges( + &self, + top_edge: TopEdge, + bottom_edge: BottomEdge, + font_size: Abs, + bounds: TextEdgeBounds, + ) -> (Abs, Abs) { + let cell = OnceCell::new(); + let bbox = |gid, f: fn(ttf_parser::Rect) -> i16| { + cell.get_or_init(|| self.ttf().glyph_bounding_box(GlyphId(gid))) + .map(|bbox| self.to_em(f(bbox)).at(font_size)) + .unwrap_or_default() + }; + + let top = match top_edge { + TopEdge::Metric(metric) => match metric.try_into() { + Ok(metric) => self.metrics().vertical(metric).at(font_size), + Err(_) => match bounds { + TextEdgeBounds::Zero => Abs::zero(), + TextEdgeBounds::Frame(frame) => frame.ascent(), + TextEdgeBounds::Glyph(gid) => bbox(gid, |b| b.y_max), + }, + }, + TopEdge::Length(length) => length.at(font_size), + }; + + let bottom = match bottom_edge { + BottomEdge::Metric(metric) => match metric.try_into() { + Ok(metric) => -self.metrics().vertical(metric).at(font_size), + Err(_) => match bounds { + TextEdgeBounds::Zero => Abs::zero(), + TextEdgeBounds::Frame(frame) => frame.descent(), + TextEdgeBounds::Glyph(gid) => -bbox(gid, |b| b.y_min), + }, + }, + BottomEdge::Length(length) => -length.at(font_size), + }; + + (top, bottom) + } } impl Hash for Font { @@ -249,3 +293,14 @@ pub enum VerticalFontMetric { /// The font's ascender, which typically exceeds the depth of all glyphs. Descender, } + +/// Defines how to resolve a `Bounds` text edge. +#[derive(Debug, Copy, Clone)] +pub enum TextEdgeBounds<'a> { + /// Set the bounds to zero. + Zero, + /// Use the bounding box of the given glyph for the bounds. + Glyph(u16), + /// Use the dimension of the given frame for the bounds. + Frame(&'a Frame), +} diff --git a/crates/typst/src/text/mod.rs b/crates/typst/src/text/mod.rs index 2e53b11a1..53163e186 100644 --- a/crates/typst/src/text/mod.rs +++ b/crates/typst/src/text/mod.rs @@ -37,7 +37,7 @@ use icu_provider_blob::BlobDataProvider; use once_cell::sync::Lazy; use rustybuzz::Feature; use smallvec::SmallVec; -use ttf_parser::{Rect, Tag}; +use ttf_parser::Tag; use crate::diag::{bail, warning, HintedStrResult, SourceResult}; use crate::engine::Engine; @@ -891,28 +891,6 @@ pub enum TopEdge { Length(Length), } -impl TopEdge { - /// Determine if the edge is specified from bounding box info. - pub fn is_bounds(&self) -> bool { - matches!(self, Self::Metric(TopEdgeMetric::Bounds)) - } - - /// Resolve the value of the text edge given a font's metrics. - pub fn resolve(self, font_size: Abs, font: &Font, bbox: Option) -> Abs { - match self { - TopEdge::Metric(metric) => { - if let Ok(metric) = metric.try_into() { - font.metrics().vertical(metric).at(font_size) - } else { - bbox.map(|bbox| (font.to_em(bbox.y_max)).at(font_size)) - .unwrap_or_default() - } - } - TopEdge::Length(length) => length.at(font_size), - } - } -} - cast! { TopEdge, self => match self { @@ -961,28 +939,6 @@ pub enum BottomEdge { Length(Length), } -impl BottomEdge { - /// Determine if the edge is specified from bounding box info. - pub fn is_bounds(&self) -> bool { - matches!(self, Self::Metric(BottomEdgeMetric::Bounds)) - } - - /// Resolve the value of the text edge given a font's metrics. - pub fn resolve(self, font_size: Abs, font: &Font, bbox: Option) -> Abs { - match self { - BottomEdge::Metric(metric) => { - if let Ok(metric) = metric.try_into() { - font.metrics().vertical(metric).at(font_size) - } else { - bbox.map(|bbox| (font.to_em(bbox.y_min)).at(font_size)) - .unwrap_or_default() - } - } - BottomEdge::Length(length) => length.at(font_size), - } - } -} - cast! { BottomEdge, self => match self { diff --git a/tests/suite/layout/measure.typ b/tests/suite/layout/measure.typ index 5429c611e..66e6461fa 100644 --- a/tests/suite/layout/measure.typ +++ b/tests/suite/layout/measure.typ @@ -92,3 +92,10 @@ table(columns: 5, u(17), it, u(1), it, u(5)) [#size.width] // 17pt } + +--- issue-5180-measure-inline-math-bounds --- +#context { + let height = measure(text(top-edge: "bounds", $x$)).height + assert(height > 4pt) + assert(height < 5pt) +}