diff --git a/src/cff2.rs b/src/cff2.rs index 3b92fd12..b722ca57 100644 --- a/src/cff2.rs +++ b/src/cff2.rs @@ -6,6 +6,7 @@ use std::borrow::Cow; /// CFF2 fonts will currently be converted into TTF fonts. pub fn subset(ctx: &mut Context) -> crate::Result<()> { let mut maxp_data = MaxpData::default(); + let mut hmtx_data = Vec::new(); glyf::subset_with(ctx, |old_gid, ctx| { let data = match &ctx.interjector { @@ -13,7 +14,10 @@ pub fn subset(ctx: &mut Context) -> crate::Result<()> { Interjector::Dummy(_) => unreachable!(), #[cfg(feature = "variable-fonts")] Interjector::Skrifa(s) => { - Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?) + let (advance, lsb, data) = + s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?; + hmtx_data.push((advance, lsb)); + Cow::Owned(data) } }; @@ -21,5 +25,7 @@ pub fn subset(ctx: &mut Context) -> crate::Result<()> { })?; ctx.custom_maxp_data = Some(maxp_data); + ctx.custom_hmtx_data = Some(hmtx_data); + Ok(()) } diff --git a/src/glyf.rs b/src/glyf.rs index e13259d4..2c2a6c6d 100644 --- a/src/glyf.rs +++ b/src/glyf.rs @@ -50,6 +50,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> { #[allow(unused_mut)] let mut maxp_data = MaxpData::default(); + #[allow(unused_mut)] + let mut hmtx_data = Vec::new(); subset_with(ctx, |old_gid, ctx| { let data = match &ctx.interjector { @@ -58,7 +60,10 @@ pub fn subset(ctx: &mut Context) -> Result<()> { } #[cfg(feature = "variable-fonts")] Interjector::Skrifa(s) => { - Cow::Owned(s.glyph_data(&mut maxp_data, old_gid).ok_or(MalformedFont)?) + let (advance, lsb, data) = + s.interject(&mut maxp_data, old_gid).ok_or(MalformedFont)?; + hmtx_data.push((advance, lsb)); + Cow::Owned(data) } }; @@ -67,6 +72,7 @@ pub fn subset(ctx: &mut Context) -> Result<()> { if ctx.interjector.is_skrifa() { ctx.custom_maxp_data = Some(maxp_data); + ctx.custom_hmtx_data = Some(hmtx_data); } Ok(()) diff --git a/src/hmtx.rs b/src/hmtx.rs index 1eb927d8..c0e380c6 100644 --- a/src/hmtx.rs +++ b/src/hmtx.rs @@ -22,10 +22,8 @@ pub fn subset(ctx: &mut Context) -> Result<()> { match &ctx.interjector { Interjector::Dummy(_) => extract_metrics(hmtx, &mut new_metrics, ctx)?, #[cfg(feature = "variable-fonts")] - Interjector::Skrifa(s) => { - for old_gid in ctx.mapper.remapped_gids() { - new_metrics.push(s.horizontal_metrics(old_gid).ok_or(MalformedFont)?); - } + Interjector::Skrifa(_) => { + new_metrics = ctx.custom_hmtx_data.take().ok_or(Error::SubsetError)? } } diff --git a/src/interjector.rs b/src/interjector.rs index 3d0d3cc5..bf26dd5d 100644 --- a/src/interjector.rs +++ b/src/interjector.rs @@ -48,32 +48,19 @@ pub(crate) mod skrifa { } impl<'a> SkrifaInterjector<'a> { - /// Return the advance width and left side bearing of the glyph. - pub(crate) fn horizontal_metrics(&self, glyph: u16) -> Option<(u16, i16)> { - let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location); - - let adv = metrics.advance_width(GlyphId::new(glyph as u32))?; - // Note that for variable fonts, our left side bearing points don't seem to - // match the ones from fonttools (they use some different technique for deriving - // it which isn't reflected in skrifa's API), but I _think_ that this shouldn't - // really be relevant in the context of PDF. - let lsb = metrics.left_side_bearing(GlyphId::new(glyph as u32))?; - - Some((adv.round() as u16, lsb.round() as i16)) - } - /// Return the glyph description in the `glyf` outline format. - pub(crate) fn glyph_data<'b>( + pub(crate) fn interject<'b>( &'b self, maxp_data: &'b mut MaxpData, glyph: u16, - ) -> Option> { + ) -> Option<(u16, i16, Vec)> { let outlines = self.font_ref.outline_glyphs(); + let metrics = self.font_ref.glyph_metrics(Size::unscaled(), &self.location); let mut outline_builder = OutlinePath::new(); - let glyph = GlyphId::new(glyph as u32); + let glyph_id = GlyphId::new(glyph as u32); - if let Some(outline_glyph) = outlines.get(glyph) { + if let Some(outline_glyph) = outlines.get(glyph_id) { outline_glyph .draw( DrawSettings::unhinted(Size::unscaled(), &self.location), @@ -84,12 +71,25 @@ pub(crate) mod skrifa { let path = outline_builder.path; + let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?; + let advance = metrics.advance_width(glyph_id)?.round() as u16; + + // We derive the LSB from the resulting bounding box rather than + // from the font's metrics, because the latter does not always agree + // with the `xMin` of the fresh outline we've generated. + // + // The OpenType spec heavily advises xMin and LSB to match (it + // actually requires it for variable fonts or when `head.flags` bit + // 1 is set). + // + // If `LSB != xMin`, glyphs get repositioned by PDF readers and the + // kerning gets very wonky. + let lsb = simple_glyph.bbox.x_min; + if path.is_empty() { - return Some(vec![]); + return Some((advance, lsb, vec![])); } - let simple_glyph = SimpleGlyph::from_bezpath(&path).ok()?; - maxp_data.max_points = maxp_data .max_points .max(simple_glyph.contours.iter().map(|c| c.len() as u16).sum()); @@ -98,8 +98,9 @@ pub(crate) mod skrifa { let mut writer = TableWriter::default(); simple_glyph.write_into(&mut writer); + let data = dump_table(&simple_glyph).ok()?; - dump_table(&simple_glyph).ok() + Some((advance, lsb, data)) } } diff --git a/src/lib.rs b/src/lib.rs index dae99fdd..4fe6964d 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -200,6 +200,7 @@ fn prepare_context<'a>( mapper: gid_remapper, interjector, custom_maxp_data: None, + custom_hmtx_data: None, flavor, tables: vec![], long_loca: false, @@ -383,6 +384,7 @@ struct Context<'a> { /// Custom data that should be used for writing the `maxp` table. Only needed for CFF2, /// where we need to synthesize a V1 table after converting. pub(crate) custom_maxp_data: Option, + pub(crate) custom_hmtx_data: Option>, /// Whether the long loca format was chosen. long_loca: bool, } diff --git a/tests/ttx/Cantarell-VF_1.ttx b/tests/ttx/Cantarell-VF_1.ttx index 1064f105..5008c186 100644 --- a/tests/ttx/Cantarell-VF_1.ttx +++ b/tests/ttx/Cantarell-VF_1.ttx @@ -22,7 +22,7 @@ - + @@ -79,7 +79,7 @@ - + diff --git a/tests/ttx/Cantarell-VF_2.ttx b/tests/ttx/Cantarell-VF_2.ttx index 883a7634..146b1c60 100644 --- a/tests/ttx/Cantarell-VF_2.ttx +++ b/tests/ttx/Cantarell-VF_2.ttx @@ -22,7 +22,7 @@ - + @@ -79,19 +79,19 @@ - - - - - - - - - - - - - + + + + + + + + + + + + + diff --git a/tests/ttx/NotoSans-Regular_var_2.ttx b/tests/ttx/NotoSans-Regular_var_2.ttx index f09ac0c9..b0a9199a 100644 --- a/tests/ttx/NotoSans-Regular_var_2.ttx +++ b/tests/ttx/NotoSans-Regular_var_2.ttx @@ -23,7 +23,7 @@ - + @@ -80,20 +80,20 @@ - - - - - - + + + + + + - - - - + + + + - + diff --git a/tests/ttx/NotoSans-Regular_var_3.ttx b/tests/ttx/NotoSans-Regular_var_3.ttx index e5cd26f6..70a97a48 100644 --- a/tests/ttx/NotoSans-Regular_var_3.ttx +++ b/tests/ttx/NotoSans-Regular_var_3.ttx @@ -23,7 +23,7 @@ - + @@ -80,20 +80,20 @@ - - - - - - + + + + + + - - - - + + + + - +