diff --git a/write-fonts/src/tables/gvar.rs b/write-fonts/src/tables/gvar.rs index 9cb68c40a..d86328520 100644 --- a/write-fonts/src/tables/gvar.rs +++ b/write-fonts/src/tables/gvar.rs @@ -26,7 +26,24 @@ pub struct GlyphDeltas { // start and end tuples of optional intermediate region intermediate_region: Option<(Tuple, Tuple)>, // (x, y) deltas or None for do not encode. One entry per point in the glyph. - deltas: Vec>, + deltas: Vec, +} + +/// A delta for a single value in a glyph. +/// +/// This includes a flag indicating whether or not this delta is required (i.e +/// it cannot be interpolated from neighbouring deltas and coordinates). +/// This is only relevant for simple glyphs; interpolatable points may be omitted +/// in the final binary when doing so saves space. +/// See +/// for more information. +#[derive(Clone, Copy, Debug, PartialEq, Eq)] +#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] +pub struct GlyphDelta { + pub x: i16, + pub y: i16, + /// This delta must be included, i.e. cannot be interpolated + pub required: bool, } /// An error representing invalid input when building a gvar table @@ -198,6 +215,23 @@ impl GlyphVariations { } } +impl GlyphDelta { + /// Create a new delta value. + pub fn new(x: i16, y: i16, required: bool) -> Self { + Self { x, y, required } + } + + /// Create a new delta value that must be encoded (cannot be interpolated) + pub fn required(x: i16, y: i16) -> Self { + Self::new(x, y, true) + } + + /// Create a new delta value that may be omitted (can be interpolated) + pub fn optional(x: i16, y: i16) -> Self { + Self::new(x, y, false) + } +} + impl GlyphDeltas { /// Create a new set of deltas. /// @@ -205,7 +239,7 @@ impl GlyphDeltas { /// it isn't required. pub fn new( peak_tuple: Tuple, - deltas: Vec>, + deltas: Vec, intermediate_region: Option<(Tuple, Tuple)>, ) -> Self { if let Some((start, end)) = intermediate_region.as_ref() { @@ -226,43 +260,69 @@ impl GlyphDeltas { shared_tuple_map: &HashMap<&Tuple, u16>, _shared_points: &PackedPointNumbers, ) -> (TupleVariationHeader, GlyphTupleVariationData) { + // build variation data for all points, even if they could be interp'd + fn build_non_sparse(deltas: &[GlyphDelta]) -> GlyphTupleVariationData { + let (x_deltas, y_deltas) = deltas.iter().map(|delta| (delta.x, delta.y)).unzip(); + GlyphTupleVariationData { + private_point_numbers: None, + x_deltas: PackedDeltas::new(x_deltas), + y_deltas: PackedDeltas::new(y_deltas), + } + } + + // build sparse variation data, omitting interpolatable values + fn build_sparse(deltas: &[GlyphDelta]) -> GlyphTupleVariationData { + let mut x_deltas = Vec::with_capacity(deltas.len()); + let mut y_deltas = Vec::with_capacity(deltas.len()); + let mut points = Vec::with_capacity(deltas.len()); + for (i, (x, y)) in deltas + .iter() + .enumerate() + .filter_map(|(i, delta)| delta.required.then_some((i, (delta.x, delta.y)))) + { + x_deltas.push(x); + y_deltas.push(y); + points.push(i as u16); + } + GlyphTupleVariationData { + private_point_numbers: Some(PackedPointNumbers::Some(points)), + x_deltas: PackedDeltas::new(x_deltas), + y_deltas: PackedDeltas::new(y_deltas), + } + } + let GlyphDeltas { peak_tuple, intermediate_region, deltas, } = self; - // over-capacity here isn't a big deal - let mut x_deltas = Vec::with_capacity(deltas.len()); - let mut y_deltas = Vec::with_capacity(deltas.len()); - - for delta in deltas.iter().filter(|d| d.is_some()) { - let (x, y) = delta.unwrap(); - x_deltas.push(x); - y_deltas.push(y); - } - let private_point_numbers = if x_deltas.len() < deltas.len() { - Some(PackedPointNumbers::Some( - (0..deltas.len()) - .filter(|i| deltas[*i].is_some()) - .map(|v| v as u16) - .collect::>(), - )) - } else { - None - }; - let has_private_points = private_point_numbers.is_some(); - let data = GlyphTupleVariationData { - private_point_numbers, - x_deltas: PackedDeltas::new(x_deltas), - y_deltas: PackedDeltas::new(y_deltas), - }; - let data_size = data.compute_size(); let (idx, peak_tuple) = match shared_tuple_map.get(&peak_tuple) { Some(idx) => (Some(*idx), None), None => (None, Some(peak_tuple)), }; + // just because some points may be interpolatable does not mean that the + // sparse representation is more efficient, since it requires us to + // also explicitly include point numbers; so we try both packings and + // pick the best. + + let dense_data = build_non_sparse(&deltas); + let dense_size = dense_data.compute_size(); + let has_interpolatable_points = deltas.iter().any(|d| !d.required); + let (data, data_size, has_private_points) = if has_interpolatable_points { + let sparse_data = build_sparse(&deltas); + let sparse_size = sparse_data.compute_size(); + log::trace!("gvar tuple variation size (dense, sparse): ({dense_size}, {sparse_size})"); + if sparse_size < dense_size { + (sparse_data, sparse_size, true) + } else { + (dense_data, dense_size, false) + } + } else { + (dense_data, dense_size, false) + }; + let header = TupleVariationHeader::new( data_size, idx, @@ -472,11 +532,11 @@ mod tests { vec![GlyphDeltas::new( Tuple::new(vec![F2Dot14::from_f32(1.0), F2Dot14::from_f32(1.0)]), vec![ - Some((30, 31)), - Some((40, 41)), - Some((-50, -49)), - Some((101, 102)), - Some((10, 11)), + GlyphDelta::required(30, 31), + GlyphDelta::required(40, 41), + GlyphDelta::required(-50, -49), + GlyphDelta::required(101, 102), + GlyphDelta::required(10, 11), ], None, )], @@ -487,22 +547,22 @@ mod tests { GlyphDeltas::new( Tuple::new(vec![F2Dot14::from_f32(1.0), F2Dot14::from_f32(1.0)]), vec![ - Some((11, -20)), - Some((69, -41)), - Some((-69, 49)), - Some((168, 101)), - Some((1, 2)), + GlyphDelta::required(11, -20), + GlyphDelta::required(69, -41), + GlyphDelta::required(-69, 49), + GlyphDelta::required(168, 101), + GlyphDelta::required(1, 2), ], None, ), GlyphDeltas::new( Tuple::new(vec![F2Dot14::from_f32(0.8), F2Dot14::from_f32(1.0)]), vec![ - Some((3, -200)), - Some((4, -500)), - Some((5, -800)), - Some((6, -1200)), - Some((7, -1500)), + GlyphDelta::required(3, -200), + GlyphDelta::required(4, -500), + GlyphDelta::required(5, -800), + GlyphDelta::required(6, -1200), + GlyphDelta::required(7, -1500), ], None, ), @@ -544,13 +604,22 @@ mod tests { } #[test] - fn not_all_your_points_are_belong_to_us() { + fn use_iup_when_appropriate() { + // IFF iup provides space savings, we should prefer it. + let _ = env_logger::builder().is_test(true).try_init(); let gid = GlyphId::new(0); let table = Gvar::new(vec![GlyphVariations::new( gid, vec![GlyphDeltas::new( Tuple::new(vec![F2Dot14::from_f32(1.0), F2Dot14::from_f32(1.0)]), - vec![Some((30, 31)), None, None, Some((101, 102)), Some((10, 11))], + vec![ + GlyphDelta::required(30, 31), + GlyphDelta::optional(30, 31), + GlyphDelta::optional(30, 31), + GlyphDelta::required(101, 102), + GlyphDelta::required(10, 11), + GlyphDelta::optional(10, 11), + ], None, )], )]) @@ -579,4 +648,44 @@ mod tests { .collect(); assert_eq!(points, vec![(30, 31), (101, 102), (10, 11)]); } + + #[test] + fn disregard_iup_when_appropriate() { + // if the cost of encoding the list of points is greater than the savings + // from omitting some deltas, we should just encode explicit zeros + let points = vec![ + GlyphDelta::required(1, 2), + GlyphDelta::required(3, 4), + GlyphDelta::required(5, 6), + GlyphDelta::optional(5, 6), + GlyphDelta::required(7, 8), + ]; + let gid = GlyphId::new(0); + let table = Gvar::new(vec![GlyphVariations::new( + gid, + vec![GlyphDeltas::new( + Tuple::new(vec![F2Dot14::from_f32(1.0), F2Dot14::from_f32(1.0)]), + points, + None, + )], + )]) + .unwrap(); + let bytes = crate::dump_table(&table).unwrap(); + let gvar = read_fonts::tables::gvar::Gvar::read(FontData::new(&bytes)).unwrap(); + assert_eq!(gvar.version(), MajorMinor::VERSION_1_0); + assert_eq!(gvar.shared_tuple_count(), 0); + assert_eq!(gvar.glyph_count(), 1); + + let g1 = gvar.glyph_variation_data(gid).unwrap(); + let g1tup = g1.tuples().collect::>(); + assert_eq!(g1tup.len(), 1); + let tuple_variation = &g1tup[0]; + + assert!(tuple_variation.has_deltas_for_all_points()); + let points: Vec<_> = tuple_variation + .deltas() + .map(|d| (d.x_delta, d.y_delta)) + .collect(); + assert_eq!(points, vec![(1, 2), (3, 4), (5, 6), (5, 6), (7, 8)]); + } } diff --git a/write-fonts/src/tables/gvar/iup.rs b/write-fonts/src/tables/gvar/iup.rs index 5de34a93a..14fd8466e 100644 --- a/write-fonts/src/tables/gvar/iup.rs +++ b/write-fonts/src/tables/gvar/iup.rs @@ -9,7 +9,8 @@ use std::collections::HashSet; -use crate::util::WrappingGet; +use super::GlyphDelta; +use crate::{round::OtRound, util::WrappingGet}; use kurbo::{Point, Vec2}; @@ -18,8 +19,8 @@ const NUM_PHANTOM_POINTS: usize = 4; /// For the outline given in `coords`, with contour endpoints given /// `ends`, optimize a set of delta values `deltas` within error `tolerance`. /// -/// Returns delta vector that has most number of None items instead of -/// the input delta. +/// For each delta in the input, returns an [`GlyphDelta`] with a flag +/// indicating whether or not it can be interpolated. /// /// See: /// * @@ -29,7 +30,7 @@ pub fn iup_delta_optimize( coords: Vec, tolerance: f64, contour_ends: &[usize], -) -> Result>, IupError> { +) -> Result, IupError> { let num_coords = coords.len(); if num_coords < NUM_PHANTOM_POINTS { return Err(IupError::NotEnoughCoords(num_coords)); @@ -404,12 +405,15 @@ fn iup_contour_optimize_dp( /// For contour with coordinates `coords`, optimize a set of delta values `deltas` within error `tolerance`. /// /// Returns delta vector that has most number of None items instead of the input delta. +/// Returns a vector of [`GlyphDelta`]s with the maximal number marked as +/// 'optional'. +/// /// fn iup_contour_optimize( deltas: &mut [Vec2], coords: &mut [Point], tolerance: f64, -) -> Result>, IupError> { +) -> Result, IupError> { if deltas.len() != coords.len() { return Err(IupError::DeltaCoordLengthMismatch { num_deltas: deltas.len(), @@ -426,11 +430,17 @@ fn iup_contour_optimize( return Ok(Vec::new()); }; if deltas.iter().all(|d| d == first_delta) { - let mut result = vec![None; n]; - if first_delta.x != 0.0 || first_delta.y != 0.0 { - result[0] = Some(*first_delta); + if *first_delta == Vec2::ZERO { + return Ok(vec![GlyphDelta::optional(0, 0); n]); } - return Ok(result); + + let (x, y) = first_delta.to_point().ot_round(); + // if all deltas are equal than the first is explict and the rest + // are interpolatable + return Ok(std::iter::once(GlyphDelta::required(x, y)) + .chain(std::iter::repeat(GlyphDelta::optional(x, y))) + .take(n) + .collect()); } // Solve the general problem using Dynamic Programming @@ -543,12 +553,15 @@ fn iup_contour_optimize( encode }; - Ok((0..n) - .map(|i| { + Ok(deltas + .iter() + .enumerate() + .map(|(i, delta)| { + let (x, y) = delta.to_point().ot_round(); if encode.contains(&i) { - Some(deltas[i]) + GlyphDelta::required(x, y) } else { - None + GlyphDelta::optional(x, y) } }) .collect()) @@ -966,6 +979,22 @@ mod tests { iup_scenario8().assert_optimize_contour(); } + // a helper to let us match the existing test format + fn make_vec_of_options(deltas: &[GlyphDelta]) -> Vec<(usize, Option)> { + deltas + .iter() + .enumerate() + .map(|(i, delta)| { + ( + i, + delta + .required + .then_some(Vec2::new(delta.x as _, delta.y as _)), + ) + }) + .collect() + } + #[test] fn iup_delta_optimize_oswald_glyph_two() { // https://github.com/googlefonts/fontations/issues/564 @@ -1055,10 +1084,12 @@ mod tests { // a single contour, minus the phantom points let contour_ends = vec![coords.len() - 1 - 4]; - let result = iup_delta_optimize(deltas, coords, tolerance, &contour_ends).unwrap(); + let result = iup_delta_optimize(deltas.clone(), coords, tolerance, &contour_ends).unwrap(); + assert_eq!(result.len(), deltas.len()); + let result = make_vec_of_options(&result); assert_eq!( - result.into_iter().enumerate().collect::>(), + result, // this is what fonttools iup_delta_optimize returns and what we want to match vec![ (0, None), @@ -1198,10 +1229,11 @@ mod tests { // a single contour, minus the phantom points let contour_ends = vec![coords.len() - 1 - 4]; - let result = iup_delta_optimize(deltas, coords, tolerance, &contour_ends).unwrap(); + let result = iup_delta_optimize(deltas.clone(), coords, tolerance, &contour_ends).unwrap(); + let result = make_vec_of_options(&result); assert_eq!( - result.into_iter().enumerate().collect::>(), + result, vec![ (0, None), (1, Some(Vec2 { x: 4.0, y: 0.0 })), diff --git a/write-fonts/src/tables/variations.rs b/write-fonts/src/tables/variations.rs index 7f70a402e..e8ce667a7 100644 --- a/write-fonts/src/tables/variations.rs +++ b/write-fonts/src/tables/variations.rs @@ -81,6 +81,8 @@ pub enum PackedPointNumbers { #[derive(Clone, Debug, Default, PartialEq, Eq, PartialOrd, Ord, Hash)] #[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))] pub struct PackedDeltas { + // NOTE: these are the 'raw' values, e.g. the actual delta value, without packing. + // The deltas will be packed during compilation. deltas: Vec, }