From 4c071e6d931a27ad234a94a4ac708679ffea00e6 Mon Sep 17 00:00:00 2001 From: Chad Brokaw Date: Thu, 24 Aug 2023 02:18:21 -0400 Subject: [PATCH] [skrifa] return overlap status from scaler (#583) Changes `Scaler::outline` to return a new `ScalerMetrics` struct that contains the overlap flag. This also has optional fields to represent metrics that may be adjusted by hinting. Based on #582 which must be merged first. Fixes #581 --- read-fonts/src/tables/glyf.rs | 80 +++++++++++++++++---------------- skrifa/src/scale/glyf/scaler.rs | 19 +++++--- skrifa/src/scale/mod.rs | 29 +++++++++++- skrifa/src/scale/scaler.rs | 29 ++++++++++-- 4 files changed, 107 insertions(+), 50 deletions(-) diff --git a/read-fonts/src/tables/glyf.rs b/read-fonts/src/tables/glyf.rs index bb171d065..7119c6803 100644 --- a/read-fonts/src/tables/glyf.rs +++ b/read-fonts/src/tables/glyf.rs @@ -958,50 +958,54 @@ mod tests { ); } - #[test] - fn simple_glyph_overlapping_contour_flag() { - let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap(); + // Test helper to enumerate all TrueType glyphs in the given font + fn all_glyphs(font_data: &[u8]) -> impl Iterator> { + let font = FontRef::new(font_data).unwrap(); let loca = font.loca(None).unwrap(); let glyf = font.glyf().unwrap(); let glyph_count = font.maxp().unwrap().num_glyphs(); - for gid in 0..glyph_count { - let glyph = match loca.get_glyf(GlyphId::new(gid), &glyf) { - Ok(Some(Glyph::Simple(glyph))) => glyph, - _ => continue, - }; - if gid == 3 { - // Only GID 3 has the overlap bit set - assert!(glyph.has_overlapping_contours()) - } else { - assert!(!glyph.has_overlapping_contours()) - } - } + (0..glyph_count).map(move |gid| loca.get_glyf(GlyphId::new(gid), &glyf).unwrap()) } #[test] - fn composite_overlapping_contour_flag() { - let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap(); - let loca = font.loca(None).unwrap(); - let glyf = font.glyf().unwrap(); - let glyph_count = font.maxp().unwrap().num_glyphs(); - for gid in 0..glyph_count { - let glyph = match loca.get_glyf(GlyphId::new(gid), &glyf) { - Ok(Some(Glyph::Composite(glyph))) => glyph, - _ => continue, - }; - // Only GID 2, component 1 has the overlap bit set - for (component_ix, component) in glyph.components().enumerate() { - if gid == 2 && component_ix == 1 { - assert!(component - .flags - .contains(CompositeGlyphFlags::OVERLAP_COMPOUND)) - } else { - assert!(!component - .flags - .contains(CompositeGlyphFlags::OVERLAP_COMPOUND)) - } - } - } + fn simple_glyph_overlapping_contour_flag() { + let gids_with_overlap: Vec<_> = all_glyphs(font_test_data::VAZIRMATN_VAR) + .enumerate() + .filter_map(|(gid, glyph)| match glyph { + Some(Glyph::Simple(glyph)) if glyph.has_overlapping_contours() => Some(gid), + _ => None, + }) + .collect(); + // Only GID 3 has the overlap bit set + let expected_gids_with_overlap = vec![3]; + assert_eq!(expected_gids_with_overlap, gids_with_overlap); + } + + #[test] + fn composite_glyph_overlapping_contour_flag() { + let gids_components_with_overlap: Vec<_> = all_glyphs(font_test_data::VAZIRMATN_VAR) + .enumerate() + .filter_map(|(gid, glyph)| match glyph { + Some(Glyph::Composite(glyph)) => Some((gid, glyph)), + _ => None, + }) + .flat_map(|(gid, glyph)| { + glyph + .components() + .enumerate() + .filter_map(move |(comp_ix, comp)| { + comp.flags + .contains(CompositeGlyphFlags::OVERLAP_COMPOUND) + .then_some((gid, comp_ix)) + }) + }) + .collect(); + // Only GID 2, component 1 has the overlap bit set + let expected_gids_components_with_overlap = vec![(2, 1)]; + assert_eq!( + expected_gids_components_with_overlap, + gids_components_with_overlap + ); } #[test] diff --git a/skrifa/src/scale/glyf/scaler.rs b/skrifa/src/scale/glyf/scaler.rs index 190aad06a..b919caa56 100644 --- a/skrifa/src/scale/glyf/scaler.rs +++ b/skrifa/src/scale/glyf/scaler.rs @@ -746,11 +746,18 @@ mod tests { let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap(); let scaler = Scaler::new(&font).unwrap(); let glyph_count = font.maxp().unwrap().num_glyphs(); - for gid in 0..glyph_count { - let glyph = scaler.glyph(GlyphId::new(gid), false).unwrap(); - // GID 2 is a composite glyph with the overlap bit on a component - // GID 3 is a simple glyph with the overlap bit on the first flag - assert_eq!(glyph.has_overlaps, gid == 2 || gid == 3); - } + // GID 2 is a composite glyph with the overlap bit on a component + // GID 3 is a simple glyph with the overlap bit on the first flag + let expected_gids_with_overlap = vec![2, 3]; + assert_eq!( + expected_gids_with_overlap, + (0..glyph_count) + .filter_map(|gid| scaler + .glyph(GlyphId::new(gid), false) + .unwrap() + .has_overlaps + .then_some(gid)) + .collect::>() + ); } } diff --git a/skrifa/src/scale/mod.rs b/skrifa/src/scale/mod.rs index 9fc27e73a..876c8570d 100644 --- a/skrifa/src/scale/mod.rs +++ b/skrifa/src/scale/mod.rs @@ -155,7 +155,7 @@ mod scaler; pub use read_fonts::types::Pen; pub use error::{Error, Result}; -pub use scaler::{Scaler, ScalerBuilder}; +pub use scaler::{Scaler, ScalerBuilder, ScalerMetrics}; use super::{ font::UniqueId, @@ -217,7 +217,7 @@ impl Context { #[cfg(test)] mod tests { use super::{Context, Size}; - use read_fonts::{scaler_test, FontRef}; + use read_fonts::{scaler_test, types::GlyphId, FontRef, TableProvider}; #[test] fn vazirmatin_var() { @@ -246,6 +246,31 @@ mod tests { ); } + #[test] + fn overlap_flags() { + let font = FontRef::new(font_test_data::VAZIRMATN_VAR).unwrap(); + let mut cx = Context::new(); + let mut path = scaler_test::Path { + elements: vec![], + is_cff: false, + }; + let mut scaler = cx.new_scaler().build(&font); + let glyph_count = font.maxp().unwrap().num_glyphs(); + // GID 2 is a composite glyph with the overlap bit on a component + // GID 3 is a simple glyph with the overlap bit on the first flag + let expected_gids_with_overlap = vec![2, 3]; + assert_eq!( + expected_gids_with_overlap, + (0..glyph_count) + .filter_map(|gid| scaler + .outline(GlyphId::new(gid), &mut path) + .unwrap() + .has_overlaps + .then_some(gid)) + .collect::>() + ); + } + fn compare_glyphs(font_data: &[u8], expected_outlines: &str, is_cff: bool) { let font = FontRef::new(font_data).unwrap(); let outlines = scaler_test::parse_glyph_outlines(expected_outlines); diff --git a/skrifa/src/scale/scaler.rs b/skrifa/src/scale/scaler.rs index 1729ace30..e0f6bef5d 100644 --- a/skrifa/src/scale/scaler.rs +++ b/skrifa/src/scale/scaler.rs @@ -11,6 +11,20 @@ use read_fonts::{ TableProvider, }; +/// Information and adjusted metrics generated while scaling a glyph. +#[derive(Copy, Clone, Default, Debug)] +pub struct ScalerMetrics { + /// True if the underlying glyph contains flags indicating the + /// presence of overlapping contours or components. + pub has_overlaps: bool, + /// If present, an adjusted left side bearing value generated by the + /// scaler. + pub adjusted_lsb: Option, + /// If present, an adjusted advance width value generated by the + /// scaler. + pub adjusted_advance_width: Option, +} + /// Builder for configuring a glyph scaler. /// /// See the [module level documentation](crate::scale#building-a-scaler) @@ -206,7 +220,7 @@ impl<'a> Scaler<'a> { /// Loads a simple outline for the specified glyph identifier and invokes the functions /// in the given pen for the sequence of path commands that define the outline. - pub fn outline(&mut self, glyph_id: GlyphId, pen: &mut impl Pen) -> Result<()> { + pub fn outline(&mut self, glyph_id: GlyphId, pen: &mut impl Pen) -> Result { if let Some(outlines) = &mut self.outlines { outlines.outline(glyph_id, self.size, self.coords, pen) } else { @@ -230,7 +244,7 @@ impl<'a> Outlines<'a> { size: f32, coords: &'a [NormalizedCoord], pen: &mut impl Pen, - ) -> Result<()> { + ) -> Result { match self { Self::TrueType(scaler, buf) => { let glyph = scaler.glyph(glyph_id, false)?; @@ -242,14 +256,21 @@ impl<'a> Outlines<'a> { .memory_from_buffer(&mut buf[..]) .ok_or(Error::InsufficientMemory)?; let outline = scaler.outline(memory, &glyph, size, coords)?; - Ok(outline.to_path(pen)?) + outline.to_path(pen)?; + Ok(ScalerMetrics { + has_overlaps: glyph.has_overlaps, + ..Default::default() + }) } Self::PostScript(scaler, subfont) => { let subfont_index = scaler.subfont_index(glyph_id); if subfont_index != subfont.index() { *subfont = scaler.subfont(subfont_index, size, coords)?; } - Ok(scaler.outline(subfont, glyph_id, coords, false, pen)?) + scaler.outline(subfont, glyph_id, coords, false, pen)?; + // CFF does not have overlap flags and hinting never adjusts + // horizontal metrics + Ok(ScalerMetrics::default()) } } }