From cc648b95d4fd509dc46a3f4c902419b85cd5a36c Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Tue, 24 Oct 2023 18:35:44 +0100 Subject: [PATCH] [ivs_builder] add test for all-zero delta sets not being de-duplicated MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit this test currently fails because the VarStore ends up with two identical [0] delta-sets instead of one. The builder is treating glyphs without any variations (add_deltas([]) and glyphs with explicitly defined 0 deltas (add_deltas([0]) as different, whereas it should treat them the same and have all point to the same no-op delta-set row. --- .../src/tables/variations/ivs_builder.rs | 41 +++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/write-fonts/src/tables/variations/ivs_builder.rs b/write-fonts/src/tables/variations/ivs_builder.rs index 9c910ac1c..29c07339b 100644 --- a/write-fonts/src/tables/variations/ivs_builder.rs +++ b/write-fonts/src/tables/variations/ivs_builder.rs @@ -1086,4 +1086,45 @@ mod tests { .iter() .all(|(key, idx)| *key == idx.delta_set_inner_index as u32)) } + + #[test] + fn no_duplicate_zero_delta_sets() { + let r0 = VariationRegion::new(vec![reg_coords(0.0, 1.0, 1.0)]); + let mut builder = VariationStoreBuilder::new(); + let varidxes = vec![ + // first glyph has no variations (e.g. .notdef only defined at default location) + // but we still need to add it to the variation store to reserve an index so + // we add an empty delta set + builder.add_deltas::(Vec::new()), + builder.add_deltas(vec![(r0.clone(), 100)]), + // this glyph has an explicit master that is the same as the default (delta is 0); + // we expect the builder to reuse the same no-op delta set as the first glyph + builder.add_deltas(vec![(r0.clone(), 0)]), + // this glyph repeats the same delta set as the second glyph, thus we expect + // the builder to map it to the same delta set index + builder.add_deltas(vec![(r0.clone(), 100)]), + ]; + let (store, key_map) = builder.build(); + + let varidx_map: Vec = varidxes + .into_iter() + .map(|idx| key_map.get(idx).unwrap().into()) + .collect::>(); + + println!("{:#?}", store); + println!("{:#?}", varidx_map); + + assert_eq!(store.variation_region_list.variation_regions.len(), 1); + assert_eq!(store.item_variation_data.len(), 1); + + let var_data = store.item_variation_data[0].as_ref().unwrap(); + + assert_eq!(var_data.item_count, 2); + assert_eq!(var_data.word_delta_count, 0); + assert_eq!(var_data.region_indexes, vec![0]); + assert_eq!(var_data.delta_sets, vec![0, 100],); + // glyph 0 and 2 should map to the same no-op 0 delta, while + // glyph 1 and 3 should map to delta 100 + assert_eq!(varidx_map, vec![0, 1, 0, 1]); + } }