More robust glyph drawing (#5159)

laurmaedje · laurmaedje · commit b060dd7f61f2 · 2024-10-15T15:06:36.000+02:00
diff --git a/crates/typst-pdf/src/color_font.rs b/crates/typst-pdf/src/color_font.rs
@@ -12,10 +12,11 @@ use indexmap::IndexMap;
 use pdf_writer::types::UnicodeCmap;
 use pdf_writer::writers::WMode;
 use pdf_writer::{Filter, Finish, Name, Rect, Ref};
-use typst::diag::SourceResult;
+use typst::diag::{bail, error, SourceDiagnostic, SourceResult};
+use typst::foundations::Repr;
 use typst::layout::Em;
-use typst::text::color::frame_for_glyph;
-use typst::text::Font;
+use typst::text::color::glyph_frame;
+use typst::text::{Font, Glyph, TextItemView};
 
 use crate::content;
 use crate::font::{base_font_name, write_font_descriptor, CMAP_NAME, SYSTEM_INFO};
@@ -211,9 +212,10 @@ impl ColorFontMap<()> {
     pub fn get(
         &mut self,
         options: &PdfOptions,
-        font: &Font,
-        gid: u16,
+        text: &TextItemView,
+        glyph: &Glyph,
     ) -> SourceResult<(usize, u8)> {
+        let font = &text.item.font;
         let color_font = self.map.entry(font.clone()).or_insert_with(|| {
             let global_bbox = font.ttf().global_bounding_box();
             let bbox = Rect::new(
@@ -230,7 +232,7 @@ impl ColorFontMap<()> {
             }
         });
 
-        Ok(if let Some(index_of_glyph) = color_font.glyph_indices.get(&gid) {
+        Ok(if let Some(index_of_glyph) = color_font.glyph_indices.get(&glyph.id) {
             // If we already know this glyph, return it.
             (color_font.slice_ids[index_of_glyph / 256], *index_of_glyph as u8)
         } else {
@@ -242,18 +244,22 @@ impl ColorFontMap<()> {
                 self.total_slice_count += 1;
             }
 
-            let frame = frame_for_glyph(font, gid);
-            let width =
-                font.advance(gid).unwrap_or(Em::new(0.0)).get() * font.units_per_em();
+            let (frame, tofu) = glyph_frame(font, glyph.id);
+            if options.standards.pdfa && tofu {
+                bail!(failed_to_convert(text, glyph));
+            }
+
+            let width = font.advance(glyph.id).unwrap_or(Em::new(0.0)).get()
+                * font.units_per_em();
             let instructions = content::build(
                 options,
                 &mut self.resources,
                 &frame,
                 None,
                 Some(width as f32),
             )?;
-            color_font.glyphs.push(ColorGlyph { gid, instructions });
-            color_font.glyph_indices.insert(gid, index);
+            color_font.glyphs.push(ColorGlyph { gid: glyph.id, instructions });
+            color_font.glyph_indices.insert(glyph.id, index);
 
             (color_font.slice_ids[index / 256], index as u8)
         })
@@ -321,3 +327,19 @@ pub struct ColorFontSlice {
     /// represent the subset of the TTF font we are interested in.
     pub subfont: usize,
 }
+
+/// The error when the glyph could not be converted.
+#[cold]
+fn failed_to_convert(text: &TextItemView, glyph: &Glyph) -> SourceDiagnostic {
+    let mut diag = error!(
+        glyph.span.0,
+        "the glyph for {} could not be exported",
+        text.glyph_text(glyph).repr()
+    );
+
+    if text.item.font.ttf().tables().cff2.is_some() {
+        diag.hint("CFF2 fonts are not currently supported");
+    }
+
+    diag
+}
diff --git a/crates/typst-pdf/src/content.rs b/crates/typst-pdf/src/content.rs
@@ -10,15 +10,15 @@ use pdf_writer::types::{
 };
 use pdf_writer::writers::PositionedItems;
 use pdf_writer::{Content, Finish, Name, Rect, Str};
-use typst::diag::{bail, SourceResult};
+use typst::diag::{bail, error, SourceDiagnostic, SourceResult};
 use typst::foundations::Repr;
 use typst::layout::{
     Abs, Em, Frame, FrameItem, GroupItem, Point, Ratio, Size, Transform,
 };
 use typst::model::Destination;
 use typst::syntax::Span;
-use typst::text::color::is_color_glyph;
-use typst::text::{Font, TextItem, TextItemView};
+use typst::text::color::should_outline;
+use typst::text::{Font, Glyph, TextItem, TextItemView};
 use typst::utils::{Deferred, Numeric, SliceExt};
 use typst::visualize::{
     FillRule, FixedStroke, Geometry, Image, LineCap, LineJoin, Paint, Path, PathItem,
@@ -418,46 +418,27 @@ fn write_group(ctx: &mut Builder, pos: Point, group: &GroupItem) -> SourceResult
 
 /// Encode a text run into the content stream.
 fn write_text(ctx: &mut Builder, pos: Point, text: &TextItem) -> SourceResult<()> {
-    if ctx.options.standards.pdfa {
-        let last_resort = text.font.info().is_last_resort();
-        for g in &text.glyphs {
-            if last_resort || g.id == 0 {
-                bail!(
-                    g.span.0,
-                    "the text {} could not be displayed with any font",
-                    TextItemView::full(text).glyph_text(g).repr(),
-                );
-            }
-        }
-    }
-
-    let ttf = text.font.ttf();
-    let tables = ttf.tables();
-
-    // If the text run contains either only color glyphs (used for emojis for
-    // example) or normal text we can render it directly
-    let has_color_glyphs = tables.sbix.is_some()
-        || tables.cbdt.is_some()
-        || tables.svg.is_some()
-        || tables.colr.is_some();
-    if !has_color_glyphs {
-        write_normal_text(ctx, pos, TextItemView::full(text))?;
-        return Ok(());
+    if ctx.options.standards.pdfa && text.font.info().is_last_resort() {
+        bail!(
+            Span::find(text.glyphs.iter().map(|g| g.span.0)),
+            "the text {} could not be displayed with any font",
+            &text.text,
+        );
     }
 
-    let color_glyph_count =
-        text.glyphs.iter().filter(|g| is_color_glyph(&text.font, g)).count();
+    let outline_glyphs =
+        text.glyphs.iter().filter(|g| should_outline(&text.font, g)).count();
 
-    if color_glyph_count == text.glyphs.len() {
-        write_color_glyphs(ctx, pos, TextItemView::full(text))?;
-    } else if color_glyph_count == 0 {
+    if outline_glyphs == text.glyphs.len() {
         write_normal_text(ctx, pos, TextItemView::full(text))?;
+    } else if outline_glyphs == 0 {
+        write_complex_glyphs(ctx, pos, TextItemView::full(text))?;
     } else {
-        // Otherwise we need to split it in smaller text runs
+        // Otherwise we need to split it into smaller text runs.
         let mut offset = 0;
         let mut position_in_run = Abs::zero();
-        for (color, sub_run) in
-            text.glyphs.group_by_key(|g| is_color_glyph(&text.font, g))
+        for (should_outline, sub_run) in
+            text.glyphs.group_by_key(|g| should_outline(&text.font, g))
         {
             let end = offset + sub_run.len();
 
@@ -468,11 +449,12 @@ fn write_text(ctx: &mut Builder, pos: Point, text: &TextItem) -> SourceResult<()
             let pos = pos + Point::new(position_in_run, Abs::zero());
             position_in_run += text_item_view.width();
             offset = end;
-            // Actually write the sub text-run
-            if color {
-                write_color_glyphs(ctx, pos, text_item_view)?;
-            } else {
+
+            // Actually write the sub text-run.
+            if should_outline {
                 write_normal_text(ctx, pos, text_item_view)?;
+            } else {
+                write_complex_glyphs(ctx, pos, text_item_view)?;
             }
         }
     }
@@ -534,6 +516,10 @@ fn write_normal_text(
 
     // Write the glyphs with kerning adjustments.
     for glyph in text.glyphs() {
+        if ctx.options.standards.pdfa && glyph.id == 0 {
+            bail!(tofu(&text, glyph));
+        }
+
         adjustment += glyph.x_offset;
 
         if !adjustment.is_zero() {
@@ -596,7 +582,7 @@ fn show_text(items: &mut PositionedItems, encoded: &[u8]) {
 }
 
 /// Encodes a text run made only of color glyphs into the content stream
-fn write_color_glyphs(
+fn write_complex_glyphs(
     ctx: &mut Builder,
     pos: Point,
     text: TextItemView,
@@ -621,12 +607,17 @@ fn write_color_glyphs(
         .or_default();
 
     for glyph in text.glyphs() {
+        if ctx.options.standards.pdfa && glyph.id == 0 {
+            bail!(tofu(&text, glyph));
+        }
+
         // Retrieve the Type3 font reference and the glyph index in the font.
         let color_fonts = ctx
             .resources
             .color_fonts
             .get_or_insert_with(|| Box::new(ColorFontMap::new()));
-        let (font, index) = color_fonts.get(ctx.options, &text.item.font, glyph.id)?;
+
+        let (font, index) = color_fonts.get(ctx.options, &text, glyph)?;
 
         if last_font != Some(font) {
             ctx.content.set_font(
@@ -824,3 +815,13 @@ fn to_pdf_line_join(join: LineJoin) -> LineJoinStyle {
         LineJoin::Bevel => LineJoinStyle::BevelJoin,
     }
 }
+
+/// The error when there is a tofu glyph.
+#[cold]
+fn tofu(text: &TextItemView, glyph: &Glyph) -> SourceDiagnostic {
+    error!(
+        glyph.span.0,
+        "the text {} could not be displayed with any font",
+        text.glyph_text(glyph).repr(),
+    )
+}
diff --git a/crates/typst-render/src/text.rs b/crates/typst-render/src/text.rs
@@ -4,7 +4,7 @@ use pixglyph::Bitmap;
 use tiny_skia as sk;
 use ttf_parser::{GlyphId, OutlineBuilder};
 use typst::layout::{Abs, Axes, Point, Size};
-use typst::text::color::{frame_for_glyph, is_color_glyph};
+use typst::text::color::{glyph_frame, should_outline};
 use typst::text::{Font, TextItem};
 use typst::visualize::{FixedStroke, Paint};
 
@@ -18,20 +18,19 @@ pub fn render_text(canvas: &mut sk::Pixmap, state: State, text: &TextItem) {
         let id = GlyphId(glyph.id);
         let offset = x + glyph.x_offset.at(text.size).to_f32();
 
-        if is_color_glyph(&text.font, glyph) {
+        if should_outline(&text.font, glyph) {
+            let state =
+                state.pre_translate(Point::new(Abs::raw(offset as _), Abs::raw(0.0)));
+            render_outline_glyph(canvas, state, text, id);
+        } else {
             let upem = text.font.units_per_em();
             let text_scale = Abs::raw(text.size.to_raw() / upem);
             let state = state
                 .pre_translate(Point::new(Abs::raw(offset as _), -text.size))
                 .pre_scale(Axes::new(text_scale, text_scale));
 
-            let glyph_frame = frame_for_glyph(&text.font, glyph.id);
-
+            let (glyph_frame, _) = glyph_frame(&text.font, glyph.id);
             crate::render_frame(canvas, state, &glyph_frame);
-        } else {
-            let state =
-                state.pre_translate(Point::new(Abs::raw(offset as _), Abs::raw(0.0)));
-            render_outline_glyph(canvas, state, text, id);
         }
 
         x += glyph.x_advance.at(text.size).to_f32();
diff --git a/crates/typst-syntax/src/span.rs b/crates/typst-syntax/src/span.rs
@@ -92,6 +92,13 @@ impl Span {
         }
     }
 
+    /// Find the first non-detached span in the iterator.
+    pub fn find(iter: impl IntoIterator<Item = Self>) -> Self {
+        iter.into_iter()
+            .find(|span| !span.is_detached())
+            .unwrap_or(Span::detached())
+    }
+
     /// Resolve a file location relative to this span's source.
     pub fn resolve_path(self, path: &str) -> Result<FileId, EcoString> {
         let Some(file) = self.id() else {
diff --git a/crates/typst/src/realize.rs b/crates/typst/src/realize.rs
@@ -1241,9 +1241,5 @@ fn destruct_space(buf: &mut [Pair], end: &mut usize, state: &mut SpaceState) {
 
 /// Finds the first non-detached span in the list.
 fn select_span(children: &[Pair]) -> Span {
-    children
-        .iter()
-        .map(|(c, _)| c.span())
-        .find(|span| !span.is_detached())
-        .unwrap_or(Span::detached())
+    Span::find(children.iter().map(|(c, _)| c.span()))
 }
diff --git a/crates/typst/src/text/font/color.rs b/crates/typst/src/text/font/color.rs

Original file line number	Diff line number	Diff line change
`@@ -92,6 +92,13 @@ impl Span {`
`92`	`92`	`}`
`93`	`93`	`}`
`94`	`94`
	`95`	`+ /// Find the first non-detached span in the iterator.`
	`96`	`+ pub fn find(iter: impl IntoIterator<Item = Self>) -> Self {`
	`97`	`+ iter.into_iter()`
	`98`	`+ .find(\|span\| !span.is_detached())`
	`99`	`+ .unwrap_or(Span::detached())`
	`100`	`+ }`
	`101`	`+`
`95`	`102`	`/// Resolve a file location relative to this span's source.`
`96`	`103`	`pub fn resolve_path(self, path: &str) -> Result<FileId, EcoString> {`
`97`	`104`	`let Some(file) = self.id() else {`