From 90f47a17a0b63b5d759dab83eeb296f8714d8503 Mon Sep 17 00:00:00 2001 From: Colin Rofls Date: Tue, 19 Sep 2023 12:22:58 -0400 Subject: [PATCH] [write-fonts] Only use IUP in gvar when more compact This requires changing the API for the PackedDeltas type so that the caller does not pre-prune deltas that can be interpolated, since we need to know these values if we decide *not* to interpolate; this also requires changing the iup_delta_optimize method to not discard deltas that can be interpolated. In a quick test on GS this reduces the size of the gvar table from 3231 kb to 2829 kb. There is one other potentially significant optimization remaining, which is to share points between tuples where possible. --- write-fonts/src/tables/gvar.rs | 199 ++++++++++++++++++++++------- write-fonts/src/tables/gvar/iup.rs | 66 +++++++--- 2 files changed, 203 insertions(+), 62 deletions(-) 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 })),