Repository: fschutt/printpdf (language: rust)

Issue:
Status of the rusttype dependency
`printpdf` currently uses `rusttype` 0.8 that is no longer maintained.  I experimented with updating `rusttype` to 0.9.  It seems to work fine, but I noticed that `ab_glyph` is `rusttype`’s more efficient successor (by the same author).  Then I noticed that `printpdf` doesn’t really need `rusttype` or `ab_glyph` because they mainly deal with font rendering and rasterization.  Instead it could directly use `ttf-parser`.

So my questions are:
- Is there a reason to stick with `rusttype` 0.8?
- If no, should `printpdf`
  - update to `rusttype` 0.9,
  - move to `ab_glyph` or
  - move to `ttf-parser`?


Discussion / hints:
> Is there a reason to stick with rusttype 0.8?

No, but see #28 - it's just that nobody has implemented it (yet) and upgrading to 0.9 was a bit difficult.

You could theoretically use ttf-parser, but that limits you to TTF fonts. I had originally hoped that rusttype would add support for OTF fonts. This hasn't happened.

If you want to work on the font system, just move all the functions that currently depend on rusttype into a trait first, so that the implementation (rusttype, allsorts, harfbuzz, freetype, ab_glyph, whatever) can be separate from the implementation.

Ideally I'd want something like `allsorts`, because it can parse **and shape** TTF, OTF, WOFF and other font formats and it's a pure-Rust library that is already being used in commercial products, so there is some money / maintainership behind it.
Let’s start with moving the font access into a helper trait, okay?

Do you want to allow users to choose the font implementation at run time or compile time, or would that trait only be used by `printpdf` internally to make it easier to replace the implementation in the future?
> > Is there a reason to stick with rusttype 0.8?
> 
> No, but see #28 - it's just that nobody has implemented it (yet) and upgrading to 0.9 was a bit difficult.
> 
> You could theoretically use ttf-parser, but that limits you to TTF fonts. I had originally hoped that rusttype would add support for OTF fonts. This hasn't happened.

ttf-parser supports OTF fonts. rusttype gained support with the switch to ttf-parser. 
```
The current capabilities of RustType:

    Reading OpenType formatted fonts and font collections. This includes *.ttf as well as *.otf font files.
```

@robinkrahl Ideally at runtime with a `Box<Trait>` - printpdf isn't an extremely high-performance library, I don't think you have to count microseconds when writing a PDF and using a boxed trait makes it easier to compare two different font implementations - so that in the same PDF you can write one text with harfbuzz, the other text with ttf-parser and compare the results directly.

@jrmuizel Yes, but as far as I know, it is / was limited to OTF fonts that are just wrappers around TTF fonts (may be wrong). And while ttf-parser supports kerning (I think), it's not a full font shaper that you'd need for more complex scripts. But anyway, this is why I want the font implementation outside of printpdf, so that the decision on which implementation is better isn't up to me.
Refactoring the font access is pretty straight-forward, see:
	https://github.com/robinkrahl/printpdf/tree/font-trait
The only problem is that we can’t clone a boxed trait object, so
ExternalFont could no longer implement Clone, cascading all the way down
to the PdfDocument.

Do you happen to have an idea how to solve this?  Otherwise I’ll take a
closer look later today or next week.

I think it should work if the trait `FontData` requires `Clone`:

```rust
pub trait FontData: Debug + Clone {
    // ...
}
```
That doesn’t work because it would require that FontData is Sized.  But
I have implemented a workaround and will open a PR soon!

i have good calculation (more or less good) of multi line wrap for regular  normal fonts, still somewhat wrong for size, but mostly ok. but bold goes bad. may be something with rusttype and need to update.

Fix patch:
diff --git a/Cargo.lock b/Cargo.lock
index f10264b..f7d380c 100644
--- a/Cargo.lock
+++ b/Cargo.lock
@@ -12,15 +12,6 @@ version = "1.2.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "aae1277d39aeec15cb388266ecc24b11c80469deae6067e17a1a7aa9e5c1f234"
 
-[[package]]
-name = "approx"
-version = "0.3.2"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f0e60b75072ecd4168020818c0107f2857bb6c4e64252d8d3983f6263b40a5c3"
-dependencies = [
- "num-traits",
-]
-
 [[package]]
 name = "autocfg"
 version = "1.0.1"
@@ -432,12 +423,12 @@ dependencies = [
 ]
 
 [[package]]
-name = "ordered-float"
-version = "1.1.1"
+name = "owned_ttf_parser"
+version = "0.12.0"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "3305af35278dd29f46fcdd139e0b1fbfae2153f0e5928b39b035542dd31e37b7"
+checksum = "3a3c7a20e3f122223e68eef6ca58e39bc1ea8a1d83418ba4c2c1ba189d2ee355"
 dependencies = [
- "num-traits",
+ "ttf-parser",
 ]
 
 [[package]]
@@ -460,7 +451,7 @@ dependencies = [
  "js-sys",
  "log",
  "lopdf",
- "rusttype",
+ "owned_ttf_parser",
  "time",
 ]
 
@@ -522,17 +513,6 @@ dependencies = [
  "semver",
 ]
 
-[[package]]
-name = "rusttype"
-version = "0.8.3"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "9f61411055101f7b60ecf1041d87fb74205fb20b0c7a723f07ef39174cf6b4c0"
-dependencies = [
- "approx",
- "ordered-float",
- "stb_truetype",
-]
-
 [[package]]
 name = "ryu"
 version = "1.0.5"
@@ -609,15 +589,6 @@ dependencies = [
  "version_check",
 ]
 
-[[package]]
-name = "stb_truetype"
-version = "0.3.1"
-source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "f77b6b07e862c66a9f3e62a07588fee67cd90a9135a2b942409f195507b4fb51"
-dependencies = [
- "byteorder",
-]
-
 [[package]]
 name = "stdweb"
 version = "0.4.20"
@@ -727,6 +698,12 @@ dependencies = [
  "syn",
 ]
 
+[[package]]
+name = "ttf-parser"
+version = "0.12.2"
+source = "registry+https://github.com/rust-lang/crates.io-index"
+checksum = "1c56097738aec26a3f347edf99f5c84d9d4e3a4b8ce5513ebca85cb621fc7c50"
+
 [[package]]
 name = "unicode-xid"
 version = "0.2.1"
diff --git a/Cargo.toml b/Cargo.toml
index a03ade9..af60bb7 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -13,7 +13,7 @@ exclude = ["./assets/*", "./doc/*", "./examples/*"]
 
 [dependencies]
 lopdf = { version = "0.26", default-features = false }
-rusttype = { version = "0.8.2", default-features = false, features = ["std"] }
+owned_ttf_parser = "0.12"
 time = { version = "0.2.11", default-features = false, features = ["std"] }
 log = { version = "0.4.8", optional = true }
 
diff --git a/src/errors/mod.rs b/src/errors/mod.rs
index 2dc47c3..ac5a59b 100644
--- a/src/errors/mod.rs
+++ b/src/errors/mod.rs
@@ -2,7 +2,7 @@
 
 use std::error::Error as IError;
 use std::io::Error as IoError;
-use rusttype::Error as RusttypeError;
+use owned_ttf_parser::FaceParsingError;
 use std::fmt;
 
 /// error_chain and failure are certainly nice, but completely overengineered
@@ -28,8 +28,8 @@ macro_rules! impl_from {
 pub enum Error {
     /// External: std::io::Error
     Io(IoError),
-    /// External: rusttype::Error
-    Rusttype(RusttypeError),
+    /// External: owned_ttf_parser::FaceParsingError
+    FaceParsing(FaceParsingError),
     /// PDF error
     Pdf(PdfError),
     /// Indexing error (please report if this happens, shouldn't happen)
@@ -70,7 +70,7 @@ impl fmt::Display for IndexError {
 impl IError for IndexError {}
 
 impl_from!(IoError, Error::Io);
-impl_from!(RusttypeError, Error::Rusttype);
+impl_from!(FaceParsingError, Error::FaceParsing);
 impl_from!(PdfError, Error::Pdf);
 impl_from!(IndexError, Error::Index);
 
@@ -79,7 +79,7 @@ impl fmt::Display for Error {
         use self::Error::*;
         match *self {
             Io(ref e) => write!(f, "{}", e),
-            Rusttype(ref e) => write!(f, "{}", e),
+            FaceParsing(ref e) => write!(f, "{}", e),
             Pdf(ref e) => write!(f, "{}", e),
             Index(ref e) => write!(f, "{}", e),
         }
diff --git a/src/lib.rs b/src/lib.rs
index 2330563..e68acbb 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -334,7 +334,7 @@
 #[macro_use] pub extern crate log;
 
 pub extern crate lopdf;
-extern crate rusttype;
+extern crate owned_ttf_parser;
 extern crate time;
 #[cfg(all(target_arch = "wasm32", target_os = "unknown"))]
 extern crate js_sys;
@@ -356,7 +356,6 @@ pub use self::errors::Error;
 pub use self::errors::PdfError;
 pub use date::*;
 pub use self::errors::IndexError;
-pub use rusttype::Error as RusttypeError;
 
 pub use self::scale::{Mm, Pt, Px};
 pub use self::types::pdf_conformance::{CustomPdfConformance, PdfConformance};
diff --git a/src/types/plugins/graphics/two_dimensional/font.rs b/src/types/plugins/graphics/two_dimensional/font.rs
index be8b409..323f8fb 100644
--- a/src/types/plugins/graphics/two_dimensional/font.rs
+++ b/src/types/plugins/graphics/two_dimensional/font.rs
@@ -4,11 +4,10 @@
 use lopdf;
 use lopdf::{Stream as LoStream, Dictionary as LoDictionary};
 use lopdf::StringFormat;
+use owned_ttf_parser::{AsFaceRef as _, Face, OwnedFace};
 use std::collections::BTreeMap;
 use std::iter::FromIterator;
-use Error;
-
-use rusttype::FontCollection;
+use {Error, PdfError};
 
 /// The font
 #[derive(Debug, Clone, PartialEq)]
@@ -132,10 +131,10 @@ impl ExternalFont {
 
     /// Creates a new font. The `index` is used for naming / identifying the font
     ///
-    /// This method uses [`rusttype`][] to parse the font data.  If you want to use a different
+    /// This method uses [`owned_ttf_parser`][] to parse the font data.  If you want to use a different
     /// font backend, use the [`with_font_data`][] method instead.
     ///
-    /// [`rusttype`]: https://docs.rs/rusttype/latest/rusttype/
+    /// [`owned_ttf_parser`]: https://docs.rs/owned_ttf_parser/latest/owned_ttf_parser/
     /// [`with_font_data`]: #method.with_font_data
     pub fn new<R>(mut font_stream: R, font_index: usize)
     -> Result<Self, Error> where R: ::std::io::Read
@@ -144,8 +143,7 @@ impl ExternalFont {
         let mut buf = Vec::<u8>::new();
         font_stream.read_to_end(&mut buf)?;
 
-        let collection = FontCollection::from_bytes(buf.clone())?;
-        let font = collection.clone().into_font().or_else(|_| collection.font_at(0))?;
+        let font = TtfFace::from_vec(buf.clone())?;
 
         Ok(Self::with_font_data(buf, font_index, Box::new(font)))
     }
@@ -490,10 +488,10 @@ pub struct GlyphMetrics {
 
 /// Provides access to font metrics.
 ///
-/// Per default, printpdf uses [`rusttype`][] to extract the font data.  You can implement this
-/// trait for other types if you want to use a different font backend.
+/// Per default, printpdf uses [`owned_ttf_parser`][] to extract the font data.  You can implement
+/// this trait for other types if you want to use a different font backend.
 ///
-/// [`rusttype`]: https://docs.rs/rusttype/latest/rusttype/
+/// [`owned_ttf_parser`]: https://docs.rs/owned_ttf_parser/latest/owned_ttf_parser/
 pub trait FontData: FontDataClone + std::fmt::Debug {
     /// Returns the unscaled metrics for this font.
     fn font_metrics(&self) -> FontMetrics;
@@ -508,43 +506,74 @@ pub trait FontData: FontDataClone + std::fmt::Debug {
     fn glyph_metrics(&self, glyph_id: u16) -> Option<GlyphMetrics>;
 }
 
-impl FontData for rusttype::Font<'static> {
+/// Wrapper struct for `owned_ttf_parser::OwnedFace` that implements `Clone` and that makes sure
+/// that the font is scalable.
+#[derive(Clone, Debug)]
+struct TtfFace {
+    inner: std::sync::Arc<OwnedFace>,
+    units_per_em: u16,
+}
+
+impl TtfFace {
+    pub fn from_vec(v: Vec<u8>) -> Result<Self, Error> {
+        let face = OwnedFace::from_vec(v, 0)?;
+        if let Some(units_per_em) = face.as_face_ref().units_per_em() {
+            Ok(Self {
+                inner: std::sync::Arc::new(face),
+                units_per_em,
+            })
+        } else {
+            Err(PdfError::FontFaceError.into())
+        }
+    }
+
+    fn face(&self) -> &Face<'_> {
+        self.inner.as_face_ref()
+    }
+}
+
+impl FontData for TtfFace {
     fn font_metrics(&self) -> FontMetrics {
-        let metrics = self.v_metrics_unscaled();
         FontMetrics {
-            ascent: metrics.ascent as i16,
-            descent: metrics.descent as i16,
-            units_per_em: self.units_per_em(),
+            ascent: self.face().ascender(),
+            descent: self.face().descender(),
+            units_per_em: self.units_per_em,
         }
     }
 
     fn glyph_id(&self, c: char) -> Option<u16> {
-        let glyph_id = self.glyph(c).id().0;
-        if glyph_id == 0 {
-            None
-        } else {
-            Some(glyph_id as u16)
-        }
+        self.face()
+            .glyph_index(c)
+            .map(|id| id.0)
     }
 
     fn glyph_ids(&self) -> std::collections::HashMap<u16, char> {
-        let mut map = std::collections::HashMap::with_capacity(self.glyph_count());
-        for c in char::default()..std::char::MAX {
-            if let Some(glyph_id) = self.glyph_id(c) {
-                map.insert(glyph_id, c);
-            }
+        let subtables = self
+            .face()
+            .character_mapping_subtables()
+            .filter(|s| s.is_unicode());
+        let mut map = std::collections::HashMap::with_capacity(self.face().number_of_glyphs().into());
+        for subtable in subtables {
+            subtable.codepoints(|c| {
+                use std::convert::TryFrom as _;
+
+                if let Ok(ch) = char::try_from(c) {
+                    if let Some(idx) = subtable.glyph_index(c).filter(|idx| idx.0 > 0) {
+                        map.entry(idx.0).or_insert(ch);
+                    }
+                }
+            })
         }
         map
     }
 
     fn glyph_metrics(&self, glyph_id: u16) -> Option<GlyphMetrics> {
-        let glyph_id = rusttype::GlyphId(u32::from(glyph_id));
-        if let Some(glyph_metrics) = self.glyph(glyph_id).standalone().get_data() {
-            let width = glyph_metrics.unit_h_metrics.advance_width as u32;
-            let height = glyph_metrics.extents.and_then(|extents| {
-                let descent = self.v_metrics_unscaled().descent as i32;
-                Some((extents.max.y - extents.min.y - descent) as u32)
-            }).unwrap_or(1000);
+        let glyph_id = owned_ttf_parser::GlyphId(glyph_id);
+        if let Some(width) = self.face().glyph_hor_advance(glyph_id) {
+            let width = width as u32;
+            let height = self.face().glyph_bounding_box(glyph_id).map(|bbox| {
+                bbox.y_max - bbox.y_min - self.face().descender()
+            }).unwrap_or(1000) as u32;
             Some(GlyphMetrics { width, height })
         } else {
             None


Test patch: