diff --git a/read-fonts/src/tables/gvar.rs b/read-fonts/src/tables/gvar.rs index 132d4944b..4d43c748e 100644 --- a/read-fonts/src/tables/gvar.rs +++ b/read-fonts/src/tables/gvar.rs @@ -38,7 +38,8 @@ impl FontReadWithArgs<'_> for U16Or32 { } impl U16Or32 { - fn get(self) -> u32 { + #[inline] + pub fn get(self) -> u32 { self.0 } } diff --git a/write-fonts/src/tables/gvar.rs b/write-fonts/src/tables/gvar.rs index b5f544933..bc787e7ce 100644 --- a/write-fonts/src/tables/gvar.rs +++ b/write-fonts/src/tables/gvar.rs @@ -123,8 +123,16 @@ impl Gvar { } fn compute_flags(&self) -> GvarFlags { - //TODO: use short offsets sometimes - GvarFlags::LONG_OFFSETS + let max_offset = self + .glyph_variation_data_offsets + .iter() + .fold(0, |acc, val| acc + val.length + val.length % 2); + + if max_offset / 2 <= (u16::MAX as u32) { + GvarFlags::default() + } else { + GvarFlags::LONG_OFFSETS + } } fn compute_glyph_count(&self) -> u16 { @@ -254,11 +262,15 @@ impl GlyphVariations { .map(|tup| tup.build(shared_tuple_map, shared_points.as_ref())) .unzip(); - GlyphVariationData { + let mut temp = GlyphVariationData { tuple_variation_headers: tuple_headers, shared_point_numbers: shared_points, per_tuple_data: tuple_data, - } + length: 0, + }; + + temp.length = temp.compute_size(); + temp } } @@ -417,6 +429,11 @@ pub struct GlyphVariationData { // optional; present if multiple variations have the same point numbers shared_point_numbers: Option, per_tuple_data: Vec, + /// calculated length required to store this data + /// + /// we compute this once up front because we need to know it in a bunch + /// of different places (u32 because offsets are max u32) + length: u32, } /// The serializable representation of a single glyph tuple variation data @@ -457,19 +474,37 @@ struct GlyphDataWriter<'a> { impl FontWrite for GlyphDataWriter<'_> { fn write_into(&self, writer: &mut TableWriter) { - assert!(self.long_offsets, "short offset logic not implemented"); - let mut last = 0u32; - last.write_into(writer); + if self.long_offsets { + let mut last = 0u32; + last.write_into(writer); - // write all the offsets - for glyph in self.data { - last += glyph.compute_size(); + // write all the offsets + for glyph in self.data { + last += glyph.compute_size(); + last.write_into(writer); + } + } else { + // for short offsets we divide the real offset by two; this means + // we will have to add padding if necessary + let mut last = 0u16; last.write_into(writer); + + // write all the offsets + for glyph in self.data { + let size = glyph.compute_size(); + // ensure we're always rounding up to the next 2 + let short_size = (size / 2) + size % 2; + last += short_size as u16; + last.write_into(writer); + } } // then write the actual data for glyph in self.data { if !glyph.is_empty() { glyph.write_into(writer); + if !self.long_offsets { + writer.pad_to_2byte_aligned(); + } } } } @@ -934,4 +969,107 @@ mod tests { Some(PackedPointNumbers::Some(vec![1, 3])) ); } + + // when using short offsets we store (real offset / 2), so all offsets must + // be even, which means when we have an odd number of bytes we have to pad. + fn make_31_bytes_of_variation_data() -> Vec { + vec![ + GlyphDeltas::new( + Tuple::new(vec![F2Dot14::from_f32(-1.0), F2Dot14::from_f32(-1.0)]), + vec![ + GlyphDelta::optional(0, 0), + GlyphDelta::required(35, 0), + GlyphDelta::optional(0, 0), + GlyphDelta::required(-24, 0), + GlyphDelta::optional(0, 0), + GlyphDelta::optional(0, 0), + ], + None, + ), + GlyphDeltas::new( + Tuple::new(vec![F2Dot14::from_f32(1.0), F2Dot14::from_f32(1.0)]), + vec![ + GlyphDelta::optional(0, 0), + GlyphDelta::required(26, 15), + GlyphDelta::optional(0, 0), + GlyphDelta::required(46, 0), + GlyphDelta::optional(0, 0), + GlyphDelta::required(1, 0), + ], + None, + ), + ] + } + + // sanity checking my input data for two subsequent tests + #[test] + fn who_tests_the_testers() { + let variations = GlyphVariations::new(GlyphId::NOTDEF, make_31_bytes_of_variation_data()); + let mut tupl_map = HashMap::new(); + + // without shared tuples this would be 39 bytes + assert_eq!(variations.clone().build(&tupl_map).length, 39); + + // to get our real size we need to mock up the shared tuples: + tupl_map.insert(&variations.variations[0].peak_tuple, 1); + tupl_map.insert(&variations.variations[1].peak_tuple, 2); + + let built = variations.clone().build(&tupl_map); + // we need an odd number to test impact of padding + assert_eq!(built.length, 31); + } + + fn assert_test_offset_packing(n_glyphs: u16, should_be_short: bool) { + let (offset_len, data_len, expected_flags) = if should_be_short { + // when using short offset we need to pad data to ensure offset is even + (u16::RAW_BYTE_LEN, 32, GvarFlags::empty()) + } else { + (u32::RAW_BYTE_LEN, 31, GvarFlags::LONG_OFFSETS) + }; + + let test_data = make_31_bytes_of_variation_data(); + let a_small_number_of_variations = (0..n_glyphs) + .map(|i| GlyphVariations::new(GlyphId::new(i), test_data.clone())) + .collect(); + + let gvar = Gvar::new(a_small_number_of_variations).unwrap(); + assert_eq!(gvar.compute_flags(), expected_flags); + + let writer = gvar.compile_variation_data(); + let mut sink = TableWriter::default(); + writer.write_into(&mut sink); + + let bytes = sink.into_data().bytes; + let expected_len = (n_glyphs + 1) as usize * offset_len // offsets + + data_len * n_glyphs as usize; // rounded size of each glyph + assert_eq!(bytes.len(), expected_len); + + let dumped = crate::dump_table(&gvar).unwrap(); + let loaded = read_fonts::tables::gvar::Gvar::read(FontData::new(&dumped)).unwrap(); + + assert_eq!(loaded.glyph_count(), n_glyphs); + assert_eq!(loaded.flags(), expected_flags); + assert!(loaded + .glyph_variation_data_offsets() + .iter() + .map(|off| off.unwrap().get()) + .enumerate() + .all(|(i, off)| off as usize == i * data_len)); + } + + #[test] + fn prefer_short_offsets() { + let _ = env_logger::builder().is_test(true).try_init(); + assert_test_offset_packing(5, true); + } + + #[test] + fn use_long_offsets_when_necessary() { + // 2**16 * 2 / (31 + 1 padding) (bytes per tuple) = 4096 should be the first + // overflow + let _ = env_logger::builder().is_test(true).try_init(); + assert_test_offset_packing(4095, true); + assert_test_offset_packing(4096, false); + assert_test_offset_packing(4097, false); + } }