Make glyph writing more robust (#22)

Cargo.toml

@@ -20,6 +20,7 @@ parley = {git = "https://github.com/linebender/parley", rev = "66979c5"}
 flate2 = "1.0.30"
 fontdb = "0.20.0"
 yoke = { version = "0.7.4", features = ["derive"] }
+float-cmp = "0.9.0"
 
 [patch.crates-io]
 tiny-skia-path = {git = "https://github.com/RazrFalcon/tiny-skia", rev="eec0a47"}

src/font/cosmic_text.rs

@@ -5,17 +5,18 @@ mod tests {
     use crate::serialize::SerializeSettings;
     use crate::stream::Glyph;
 
+    use crate::util::SliceExt;
     use crate::Fill;
     use cosmic_text::{Attrs, Buffer, FontSystem, Metrics, Shaping};
     use fontdb::Source;
     use skrifa::GlyphId;
     use std::sync::Arc;
 
-    #[test]
     #[ignore]
+    #[test]
     fn cosmic_text_integration() {
         let mut font_system = FontSystem::new_with_fonts([Source::Binary(Arc::new(std::fs::read("/Users/lstampfl/Programming/GitHub/resvg/crates/resvg/tests/fonts/NotoSans-Regular.ttf").unwrap()))]);
-        let metrics = Metrics::new(14.0, 20.0);
+        let metrics = Metrics::new(18.0, 20.0);
         let mut buffer = Buffer::new(&mut font_system, metrics);
         buffer.set_size(&mut font_system, Some(200.0), None);
         let attrs = Attrs::new();
@@ -24,7 +25,7 @@ mod tests {
         buffer.shape_until_scroll(&mut font_system, false);
 
         let page_size = tiny_skia_path::Size::from_wh(200.0, 400.0).unwrap();
-        let mut document_builder = Document::new(SerializeSettings::default());
+        let mut document_builder = Document::new(SerializeSettings::default_test());
         let mut builder = document_builder.start_page(page_size);
         let mut surface = builder.surface();
 
@@ -33,21 +34,38 @@ mod tests {
         // Inspect the output runs
         for run in buffer.layout_runs() {
             let y_offset = run.line_y;
-            let glyphs = run
+
+            let segmented = run
                 .glyphs
-                .iter()
-                .map(|g| {
-                    Glyph::new(
-                        font_map.get(&g.font_id).unwrap().clone(),
-                        GlyphId::new(g.glyph_id as u32),
-                        g.w,
-                        g.x_offset,
-                        g.font_size,
-                        g.start..g.end,
-                    )
-                })
-                .collect::<Vec<_>>();
-            surface.draw_glyph_run(0.0, y_offset, Fill::<Rgb>::default(), &glyphs, run.text);
+                .group_by_key(|g| font_map.get(&g.font_id).unwrap().clone());
+
+            let mut x = 0.0;
+            for (font, glyphs) in segmented {
+                let start_x = x;
+                let glyphs = glyphs
+                    .iter()
+                    .map(|glyph| {
+                        x += glyph.w;
+                        Glyph::new(
+                            GlyphId::new(glyph.glyph_id as u32),
+                            glyph.w,
+                            glyph.x_offset,
+                            glyph.y_offset,
+                            glyph.start..glyph.end,
+                            glyph.font_size,
+                        )
+                    })
+                    .collect::<Vec<_>>();
+
+                surface.draw_glyph_run(
+                    start_x,
+                    y_offset,
+                    Fill::<Rgb>::default(),
+                    &glyphs,
+                    font,
+                    run.text,
+                );
+            }
         }
 
         surface.finish();

src/font/mod.rs

@@ -1,6 +1,8 @@
-use crate::serialize::SvgSettings;
+use crate::serialize::{Object, SerializerContext, SvgSettings};
 use crate::surface::Surface;
+use crate::type3_font::Type3ID;
 use crate::util::Prehashed;
+use pdf_writer::{Chunk, Ref};
 use skrifa::instance::Location;
 use skrifa::outline::OutlinePen;
 use skrifa::prelude::{LocationRef, Size};
@@ -63,7 +65,6 @@ pub struct FontInfo {
     pub(crate) units_per_em: u16,
     global_bbox: Rect,
     postscript_name: Option<String>,
-    is_type3_font: bool,
     ascent: FiniteF32,
     descent: FiniteF32,
     cap_height: Option<FiniteF32>,
@@ -136,17 +137,6 @@ impl FontInfo {
             }
         };
 
-        // Right now, we decide whether to embed a font as a Type3 font solely based on whether one of these
-        // tables exist. This is not the most "efficient" method, because it is possible a font has a `COLR` table,
-        // but there are still some glyphs which are not in COLR but still in `glyf` or `CFF`. In this case,
-        // we would still choose a Type3 font for the outlines, even though they could be embedded as a CID font.
-        // For now, we make the simplifying assumption that a font is either mapped to a series of Type3 fonts
-        // or to a single CID font, but not a mix of both.
-        let is_type3_font = font_ref.svg().is_ok()
-            || font_ref.colr().is_ok()
-            || font_ref.sbix().is_ok()
-            || font_ref.cff2().is_ok();
-
         Some(FontInfo {
             index,
             checksum,
@@ -160,7 +150,6 @@ impl FontInfo {
             weight,
             italic_angle,
             global_bbox,
-            is_type3_font,
         })
     }
 }
@@ -175,6 +164,15 @@ impl Font {
         data: Arc<dyn AsRef<[u8]> + Send + Sync>,
         index: u32,
         location: Location,
+    ) -> Option<Self> {
+        let font_info = FontInfo::new(data.as_ref().as_ref(), index, location)?;
+
+        Font::new_with_info(data, Arc::new(font_info))
+    }
+
+    pub(crate) fn new_with_info(
+        data: Arc<dyn AsRef<[u8]> + Send + Sync>,
+        font_info: Arc<FontInfo>,
     ) -> Option<Self> {
         let font_ref_yoke =
             Yoke::<FontRefWrapper<'static>, Arc<dyn AsRef<[u8]> + Send + Sync>>::attach_to_cart(
@@ -184,11 +182,9 @@ impl Font {
                 },
             );
 
-        let font_info = FontInfo::new(data.as_ref().as_ref(), index, location)?;
-
         Some(Font(Arc::new(Prehashed::new(Repr {
             font_ref_yoke,
-            font_info: Arc::new(font_info),
+            font_info,
         }))))
     }
 
@@ -228,8 +224,8 @@ impl Font {
         self.0.font_info.italic_angle.get()
     }
 
-    pub fn units_per_em(&self) -> u16 {
-        self.0.font_info.units_per_em
+    pub fn units_per_em(&self) -> f32 {
+        self.0.font_info.units_per_em as f32
     }
 
     pub fn bbox(&self) -> Rect {
@@ -240,10 +236,6 @@ impl Font {
         (&self.0.font_info.location).into()
     }
 
-    pub fn is_type3_font(&self) -> bool {
-        self.0.font_info.is_type3_font
-    }
-
     pub fn font_ref(&self) -> &FontRef {
         &self.0.font_ref_yoke.get().font_ref
     }
@@ -288,6 +280,33 @@ pub fn draw_glyph(
     glyph_type
 }
 
+#[derive(Clone, Debug, Hash, Eq, PartialEq)]
+pub struct CIDIdentifer(pub Font);
+#[derive(Clone, Debug, Hash, Eq, PartialEq)]
+pub struct Type3Identifier(pub Font, pub Type3ID);
+
+#[derive(Clone, Debug, Hash, Eq, PartialEq)]
+pub enum FontIdentifier {
+    Cid(CIDIdentifer),
+    Type3(Type3Identifier),
+}
+
+impl FontIdentifier {
+    pub fn font(&self) -> Font {
+        match self {
+            FontIdentifier::Cid(cid) => cid.0.clone(),
+            FontIdentifier::Type3(t3) => t3.0.clone(),
+        }
+    }
+}
+
+// TODO: Remove?
+impl Object for FontIdentifier {
+    fn serialize_into(&self, _: &mut SerializerContext, _: Ref) -> Chunk {
+        unreachable!()
+    }
+}
+
 #[cfg(test)]
 fn draw(font_data: Arc<Vec<u8>>, glyphs: Option<Vec<(GlyphId, String)>>, name: &str) {
     use crate::document::Document;
@@ -354,14 +373,8 @@ fn draw(font_data: Arc<Vec<u8>>, glyphs: Option<Vec<(GlyphId, String)>>, name: &
             0.0,
             0.0,
             crate::Fill::<Rgb>::default(),
-            &[Glyph::new(
-                font.clone(),
-                i,
-                0.0,
-                0.0,
-                size as f32,
-                0..text.len(),
-            )],
+            &[Glyph::new(i, 0.0, 0.0, 0.0, 0..text.len(), size as f32)],
+            font.clone(),
             &text,
         );
         // let res = single_glyph(&font, GlyphId::new(i), &mut builder);

src/font/simple_shape.rs

@@ -27,6 +27,24 @@ mod tests {
                 Direction::LeftToRight,
                 14.0,
             ),
+            (
+                "DejaVuSansMono.ttf",
+                "Here with a mono font, some longer text.",
+                Direction::LeftToRight,
+                16.0,
+            ),
+            (
+                "NotoSans-Regular.ttf",
+                "z͈̤̭͖̉͑́a̳ͫ́̇͑̽͒ͯlͨ͗̍̀̍̔̀ģ͔̫̫̄o̗̠͔̦̳͆̏̓͢",
+                Direction::LeftToRight,
+                14.0,
+            ),
+            (
+                "NotoSans-Regular.ttf",
+                " birth\u{ad}day ",
+                Direction::LeftToRight,
+                14.0,
+            ),
             (
                 "NotoSansCJKsc-Regular.otf",
                 "你好世界，这是一段很长的测试文章",
@@ -40,22 +58,18 @@ mod tests {
                 14.0,
             ),
         ];
-        let page_size = tiny_skia_path::Size::from_wh(200.0, 200.0).unwrap();
+        let page_size = tiny_skia_path::Size::from_wh(200.0, 300.0).unwrap();
         let mut document_builder = Document::new(SerializeSettings::default_test());
         let mut builder = document_builder.start_page(page_size);
         let mut surface = builder.surface();
 
         for (font, text, dir, size) in data {
             let font_data = load_font(font);
             let font = Font::new(Arc::new(font_data), 0, Location::default()).unwrap();
-            let glyphs = simple_shape(text, dir, font, size);
-
-            // eprintln!("{:?}", glyphs.iter().map(|g| g.glyph_id).collect::<Vec<_>>());
-            // panic!();
-
-            surface.draw_glyph_run(0.0, y, Fill::<Rgb>::default(), &glyphs, text);
+            let glyphs = simple_shape(text, dir, font.clone(), size);
+            surface.draw_glyph_run(0.0, y, Fill::<Rgb>::default(), &glyphs, font, text);
 
-            y += size * 1.5;
+            y += size * 2.0;
         }
 
         surface.finish();

src/lib.rs

@@ -159,12 +159,12 @@ pub(crate) mod test_utils {
             .map_or(text.len(), |info| info.cluster as usize);
 
             glyphs.push(Glyph::new(
-                font.clone(),
                 GlyphId::new(start_info.glyph_id),
                 (pos.x_advance as f32 / font.units_per_em() as f32) * size,
                 (pos.x_offset as f32 / font.units_per_em() as f32) * size,
-                size,
+                (pos.y_offset as f32 / font.units_per_em() as f32) * size,
                 start..end,
+                size,
             ));
         }