diff --git a/fontir/src/ir.rs b/fontir/src/ir.rs index 5553f9e30..c741b96a2 100644 --- a/fontir/src/ir.rs +++ b/fontir/src/ir.rs @@ -15,7 +15,7 @@ use font_types::NameId; use font_types::Tag; use fontdrasil::types::{GlyphName, GroupName}; use indexmap::IndexSet; -use kurbo::{Affine, BezPath, Point}; +use kurbo::{Affine, BezPath, PathEl, Point}; use log::warn; use ordered_float::OrderedFloat; use serde::{Deserialize, Serialize}; @@ -1028,18 +1028,21 @@ pub struct GlyphPathBuilder { glyph_name: GlyphName, offcurve: Vec, leading_offcurve: Vec, - path: BezPath, + path: Vec, first_oncurve: Option, } impl GlyphPathBuilder { - /// Create a new GlyphPathBuilder for a glyph with the given name. - pub fn new(glyph_name: GlyphName) -> GlyphPathBuilder { + pub fn new(glyph_name: GlyphName, estimated_num_elements: usize) -> GlyphPathBuilder { + let mut capacity = estimated_num_elements.next_power_of_two(); + if capacity == estimated_num_elements { + capacity += 4; // close path often adds a few + } GlyphPathBuilder { glyph_name, - offcurve: Vec::new(), + offcurve: Vec::with_capacity(2), leading_offcurve: Vec::new(), - path: BezPath::new(), + path: Vec::with_capacity(capacity), first_oncurve: None, } } @@ -1064,7 +1067,7 @@ impl GlyphPathBuilder { fn begin_path(&mut self, oncurve: OnCurve) -> Result<(), PathConversionError> { assert!(self.first_oncurve.is_none()); - self.path.move_to(*oncurve.point()); + self.path.push(PathEl::MoveTo(*oncurve.point())); self.first_oncurve = Some(oncurve); Ok(()) } @@ -1094,7 +1097,7 @@ impl GlyphPathBuilder { if self.first_oncurve.is_none() { self.begin_path(OnCurve::Line(p.into()))?; } else { - self.path.line_to(p); + self.path.push(PathEl::LineTo(p.into())); } Ok(()) } @@ -1124,10 +1127,11 @@ impl GlyphPathBuilder { let next = window[1]; // current offcurve to halfway to the next one let implied = Point::new((curr.x + next.x) / 2.0, (curr.y + next.y) / 2.0); - self.path.quad_to(curr, implied); + self.path.push(PathEl::QuadTo(curr, implied)); } // last but not least, the last offcurve to the provided point - self.path.quad_to(*self.offcurve.last().unwrap(), p.into()); + self.path + .push(PathEl::QuadTo(*self.offcurve.last().unwrap(), p.into())); self.offcurve.clear(); Ok(()) @@ -1142,11 +1146,13 @@ impl GlyphPathBuilder { pub fn curve_to(&mut self, p: impl Into) -> Result<(), PathConversionError> { if self.first_oncurve.is_some() { match self.offcurve.len() { - 0 => self.path.line_to(p), - 1 => self.path.quad_to(self.offcurve[0], p.into()), - 2 => self - .path - .curve_to(self.offcurve[0], self.offcurve[1], p.into()), + 0 => self.path.push(PathEl::LineTo(p.into())), + 1 => self.path.push(PathEl::QuadTo(self.offcurve[0], p.into())), + 2 => self.path.push(PathEl::CurveTo( + self.offcurve[0], + self.offcurve[1], + p.into(), + )), _ => self.check_num_offcurve(|v| v < 3)?, } self.offcurve.clear(); @@ -1205,7 +1211,7 @@ impl GlyphPathBuilder { OnCurve::Cubic(pt) => self.curve_to(pt)?, _ => unreachable!(), } - self.path.close_path(); + self.path.push(PathEl::ClosePath); } else if !self.offcurve.is_empty() { // special TrueType oncurve-less quadratic contour, we assume the path // starts at midpoint between the first and last offcurves @@ -1221,7 +1227,7 @@ impl GlyphPathBuilder { /// Builds the kurbo::BezPath from the accumulated points. pub fn build(mut self) -> Result { self.end_path()?; - Ok(self.path) + Ok(BezPath::from_vec(self.path)) } } @@ -1271,40 +1277,49 @@ mod tests { } #[test] - fn a_qcurve_with_no_offcurve_is_a_line() { - let mut builder = GlyphPathBuilder::new("test".into()); + fn a_qcurve_with_no_offcurve_is_a_line_open_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.move_to((2.0, 2.0)).unwrap(); // open contour builder.qcurve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 L4,2", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn a_qcurve_with_no_offcurve_is_a_line_closed_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.qcurve_to((2.0, 2.0)).unwrap(); // closed, ie not starting with 'move' builder.qcurve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 L4,2 L2,2 Z", builder.build().unwrap().to_svg()); } #[test] - fn a_curve_with_no_offcurve_is_a_line() { - let mut builder = GlyphPathBuilder::new("test".into()); - builder.move_to((2.0, 2.0)).unwrap(); // open + fn a_curve_with_no_offcurve_is_a_line_open_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); + builder.move_to((2.0, 2.0)).unwrap(); // open contour builder.curve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 L4,2", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn a_curve_with_no_offcurve_is_a_line_closed_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.curve_to((2.0, 2.0)).unwrap(); // closed builder.curve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 L4,2 L2,2 Z", builder.build().unwrap().to_svg()); } #[test] - fn a_curve_with_one_offcurve_is_a_single_quad() { - let mut builder = GlyphPathBuilder::new("test".into()); + fn a_curve_with_one_offcurve_is_a_single_quad_open_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.move_to((2.0, 2.0)).unwrap(); // open builder.offcurve((3.0, 0.0)).unwrap(); builder.curve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 Q3,0 4,2", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn a_curve_with_one_offcurve_is_a_single_quad_closed_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.curve_to((2.0, 2.0)).unwrap(); // closed builder.offcurve((3.0, 0.0)).unwrap(); builder.curve_to((4.0, 2.0)).unwrap(); @@ -1312,14 +1327,17 @@ mod tests { } #[test] - fn a_qcurve_with_one_offcurve_is_a_single_quad_to() { - let mut builder = GlyphPathBuilder::new("test".into()); - builder.move_to((2.0, 2.0)).unwrap(); // open + fn a_qcurve_with_one_offcurve_is_a_single_quad_to_open_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); + builder.move_to((2.0, 2.0)).unwrap(); builder.offcurve((3.0, 0.0)).unwrap(); builder.qcurve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 Q3,0 4,2", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn a_qcurve_with_one_offcurve_is_a_single_quad_to_closed_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.qcurve_to((2.0, 2.0)).unwrap(); // closed builder.offcurve((3.0, 0.0)).unwrap(); builder.qcurve_to((4.0, 2.0)).unwrap(); @@ -1327,15 +1345,18 @@ mod tests { } #[test] - fn a_qcurve_with_two_offcurve_is_two_quad_to() { - let mut builder = GlyphPathBuilder::new("test".into()); - builder.move_to((2.0, 2.0)).unwrap(); // open + fn a_qcurve_with_two_offcurve_is_two_quad_to_open_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); + builder.move_to((2.0, 2.0)).unwrap(); builder.offcurve((3.0, 0.0)).unwrap(); builder.offcurve((5.0, 4.0)).unwrap(); builder.qcurve_to((6.0, 2.0)).unwrap(); assert_eq!("M2,2 Q3,0 4,2 Q5,4 6,2", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn a_qcurve_with_two_offcurve_is_two_quad_to_closed_contour() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.qcurve_to((2.0, 2.0)).unwrap(); // closed builder.offcurve((3.0, 0.0)).unwrap(); builder.offcurve((5.0, 4.0)).unwrap(); @@ -1347,14 +1368,17 @@ mod tests { } #[test] - fn last_line_always_emit_implied_closing_line() { - let mut builder = GlyphPathBuilder::new("test".into()); + fn last_line_always_emits_implied_closing_line() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.line_to((2.0, 2.0)).unwrap(); builder.line_to((4.0, 2.0)).unwrap(); // a closing line is implied by Z, but emit it nonetheless assert_eq!("M2,2 L4,2 L2,2 Z", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn last_line_emits_nop_implied_closing_line() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.line_to((2.0, 2.0)).unwrap(); builder.line_to((4.0, 2.0)).unwrap(); // duplicate last point, not to be confused with the closing line implied by Z @@ -1363,15 +1387,18 @@ mod tests { } #[test] - fn last_curve_equals_move_no_closing_line() { + fn last_quad_equals_move_no_closing_line() { // if last curve point is equal to move, there's no need to disambiguate it from // the implicit closing line, so we don't emit one - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.offcurve((3.0, 0.0)).unwrap(); builder.qcurve_to((2.0, 2.0)).unwrap(); assert_eq!("M2,2 Q3,0 2,2 Z", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn last_cubic_equals_move_no_closing_line() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.offcurve((3.0, 0.0)).unwrap(); builder.offcurve((0.0, 3.0)).unwrap(); builder.curve_to((2.0, 2.0)).unwrap(); @@ -1379,15 +1406,18 @@ mod tests { } #[test] - fn last_curve_not_equal_move_do_emit_closing_line() { + fn last_quad_not_equal_move_do_emit_closing_line() { // if last point is different from move, then emit the implied closing line - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.line_to((2.0, 2.0)).unwrap(); builder.offcurve((3.0, 0.0)).unwrap(); builder.qcurve_to((4.0, 2.0)).unwrap(); assert_eq!("M2,2 Q3,0 4,2 L2,2 Z", builder.build().unwrap().to_svg()); + } - let mut builder = GlyphPathBuilder::new("test".into()); + #[test] + fn last_cubic_not_equal_move_do_emit_closing_line() { + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.line_to((2.0, 2.0)).unwrap(); builder.offcurve((3.0, 0.0)).unwrap(); builder.offcurve((0.0, 3.0)).unwrap(); @@ -1404,7 +1434,7 @@ mod tests { // to the same path, which begins/ends on the first on-curve point i.e. (2,2). let expected = "M2,2 C6,0 0,6 4,2 C3,0 0,3 2,2 Z"; - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.offcurve((3.0, 0.0)).unwrap(); builder.offcurve((0.0, 3.0)).unwrap(); builder.curve_to((2.0, 2.0)).unwrap(); @@ -1413,7 +1443,7 @@ mod tests { builder.curve_to((4.0, 2.0)).unwrap(); assert_eq!(expected, builder.build().unwrap().to_svg()); - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.offcurve((0.0, 3.0)).unwrap(); builder.curve_to((2.0, 2.0)).unwrap(); builder.offcurve((6.0, 0.0)).unwrap(); @@ -1422,7 +1452,7 @@ mod tests { builder.offcurve((3.0, 0.0)).unwrap(); assert_eq!(expected, builder.build().unwrap().to_svg()); - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.curve_to((2.0, 2.0)).unwrap(); builder.offcurve((6.0, 0.0)).unwrap(); builder.offcurve((0.0, 6.0)).unwrap(); @@ -1434,7 +1464,7 @@ mod tests { #[test] fn closed_quadratic_contour_without_oncurve_points() { - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); // builder.qcurve_to((0.0, 1.0)).unwrap(); // implied builder.offcurve((1.0, 1.0)).unwrap(); builder.offcurve((1.0, -1.0)).unwrap(); @@ -1449,7 +1479,7 @@ mod tests { #[test] fn invalid_move_after_first_point() { // A point of type 'move' must be the first point in an (open) contour. - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.move_to((2.0, 2.0)).unwrap(); builder.end_path().unwrap(); // move_to after ending the current subpath is OK @@ -1491,7 +1521,7 @@ mod tests { #[test] fn closed_path_with_trailing_cubic_offcurves() { - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.curve_to((10.0, 0.0)).unwrap(); builder.line_to((0.0, 10.0)).unwrap(); builder.offcurve((5.0, 10.0)).unwrap(); @@ -1504,7 +1534,7 @@ mod tests { #[test] fn closed_path_with_trailing_quadratic_offcurves() { - let mut builder = GlyphPathBuilder::new("test".into()); + let mut builder = GlyphPathBuilder::new("test".into(), 0); builder.qcurve_to((10.0, 0.0)).unwrap(); builder.line_to((0.0, 10.0)).unwrap(); builder.offcurve((5.0, 10.0)).unwrap(); diff --git a/glyphs2fontir/src/toir.rs b/glyphs2fontir/src/toir.rs index e356ef669..6d6c3fe66 100644 --- a/glyphs2fontir/src/toir.rs +++ b/glyphs2fontir/src/toir.rs @@ -16,7 +16,8 @@ pub(crate) fn to_ir_contours_and_components( glyph_name: GlyphName, shapes: &[Shape], ) -> Result<(Vec, Vec), WorkError> { - let mut contours = Vec::new(); + // For most glyphs in most fonts all the shapes are contours so it's a good guess + let mut contours = Vec::with_capacity(shapes.len()); let mut components = Vec::new(); for shape in shapes.iter() { @@ -44,19 +45,44 @@ fn to_ir_component(glyph_name: GlyphName, component: &Component) -> ir::Componen } } +fn add_to_path<'a>( + path_builder: &'a mut GlyphPathBuilder, + nodes: impl Iterator, +) -> Result<(), WorkError> { + // Walk through the remaining points, accumulating off-curve points until we see an on-curve + // https://github.com/googlefonts/glyphsLib/blob/24b4d340e4c82948ba121dcfe563c1450a8e69c9/Lib/glyphsLib/pens.py#L92 + for node in nodes { + // Smooth is only relevant to editors so ignore here + match node.node_type { + NodeType::Line | NodeType::LineSmooth => path_builder + .line_to((node.pt.x, node.pt.y)) + .map_err(WorkError::PathConversionError)?, + NodeType::Curve | NodeType::CurveSmooth => path_builder + .curve_to((node.pt.x, node.pt.y)) + .map_err(WorkError::PathConversionError)?, + NodeType::OffCurve => path_builder + .offcurve((node.pt.x, node.pt.y)) + .map_err(WorkError::PathConversionError)?, + NodeType::QCurve | NodeType::QCurveSmooth => path_builder + .qcurve_to((node.pt.x, node.pt.y)) + .map_err(WorkError::PathConversionError)?, + } + } + Ok(()) +} + fn to_ir_path(glyph_name: GlyphName, src_path: &Path) -> Result { // Based on https://github.com/googlefonts/glyphsLib/blob/24b4d340e4c82948ba121dcfe563c1450a8e69c9/Lib/glyphsLib/builder/paths.py#L20 // See also https://github.com/fonttools/ufoLib2/blob/4d8a9600148b670b0840120658d9aab0b38a9465/src/ufoLib2/pointPens/glyphPointPen.py#L16 - let mut path_builder = GlyphPathBuilder::new(glyph_name.clone()); if src_path.nodes.is_empty() { - return Ok(path_builder.build()?); + return Ok(BezPath::new()); } - let mut nodes: Vec<_> = src_path.nodes.clone(); + let mut path_builder = GlyphPathBuilder::new(glyph_name.clone(), src_path.nodes.len()); // First is a delicate butterfly if !src_path.closed { - let first = nodes.remove(0); + let first = src_path.nodes.first().unwrap(); if first.node_type == NodeType::OffCurve { return Err(WorkError::InvalidSourceGlyph { glyph_name, @@ -64,33 +90,20 @@ fn to_ir_path(glyph_name: GlyphName, src_path: &Path) -> Result path_builder - .line_to((node.pt.x, node.pt.y)) - .map_err(WorkError::PathConversionError)?, - NodeType::Curve | NodeType::CurveSmooth => path_builder - .curve_to((node.pt.x, node.pt.y)) - .map_err(WorkError::PathConversionError)?, - NodeType::OffCurve => path_builder - .offcurve((node.pt.x, node.pt.y)) - .map_err(WorkError::PathConversionError)?, - NodeType::QCurve | NodeType::QCurveSmooth => path_builder - .qcurve_to((node.pt.x, node.pt.y)) - .map_err(WorkError::PathConversionError)?, - } - } - let path = path_builder.build()?; trace!( "Built a {} entry path for {}", diff --git a/ufo2fontir/src/toir.rs b/ufo2fontir/src/toir.rs index a3fc16523..80bee02a1 100644 --- a/ufo2fontir/src/toir.rs +++ b/ufo2fontir/src/toir.rs @@ -28,11 +28,12 @@ pub(crate) fn to_design_location( } fn to_ir_contour(glyph_name: GlyphName, contour: &norad::Contour) -> Result { - let mut path_builder = GlyphPathBuilder::new(glyph_name.clone()); if contour.points.is_empty() { - return Ok(path_builder.build()?); + return Ok(BezPath::new()); } + let mut path_builder = GlyphPathBuilder::new(glyph_name.clone(), contour.points.len()); + // Walk through the remaining points, accumulating off-curve points until we see an on-curve for node in contour.points.iter() { match node.typ {