diff --git a/font-test-data/src/ift.rs b/font-test-data/src/ift.rs index 545bdd1e9..e369bcbd1 100644 --- a/font-test-data/src/ift.rs +++ b/font-test-data/src/ift.rs @@ -284,15 +284,15 @@ pub fn features_and_design_space_format2() -> BeBuffer { 0x02BC_0000u32, // end = 700 (Tag::new(b"wdth")), // tag = wdth - 0x0u32, // start = 0 - 0x8000, // end = 0 + 0x0u32, // start = 0.0 + 0x8000u32, // end = 0.5 (Tag::new(b"wdth")), // tag = wdth - 0x0002_0000, // start = 2.0 - 0x0002_8000, // end = 2.5 + 0x0002_0000u32, // start = 2.0 + 0x0002_8000u32, // end = 2.5 5u16, // bias = 5 - 0b00001101, 0b00000011, 0b00110001 // codepoints = [5..22] + [0b00001101, 0b00000011, 0b00110001u8] // codepoints = [5..22] }; let offset = buffer.offset_for("entries[0]") as u32; diff --git a/fuzz/fuzz_targets/fuzz_ift_patch_group.rs b/fuzz/fuzz_targets/fuzz_ift_patch_group.rs index 2eb4226e1..199aa2347 100644 --- a/fuzz/fuzz_targets/fuzz_ift_patch_group.rs +++ b/fuzz/fuzz_targets/fuzz_ift_patch_group.rs @@ -1,17 +1,18 @@ #![no_main] //! Fuzzes the incremental_font_transfer patch_group.rs API -use std::{ - collections::{BTreeSet, HashMap, HashSet}, - ops::RangeInclusive, -}; +use std::collections::{BTreeSet, HashMap, HashSet}; +use font_types::Fixed; use incremental_font_transfer::{ patch_group::{PatchGroup, UriStatus}, patchmap::SubsetDefinition, }; use libfuzzer_sys::{arbitrary, fuzz_target}; -use read_fonts::{collections::IntSet, types::Tag}; +use read_fonts::{ + collections::{IntSet, RangeSet}, + types::Tag, +}; use skrifa::FontRef; use write_fonts::FontBuilder; @@ -25,7 +26,7 @@ struct FuzzInput { // Parts of the target subset definition. codepoints: HashSet, features: HashSet, - design_space: HashMap>, + design_space: HashMap>, // Patches patches: HashMap>, @@ -51,12 +52,14 @@ impl FuzzInput { let feature_tags: BTreeSet = self.features.iter().copied().map(Tag::from_u32).collect(); - let design_space: HashMap>> = self + let design_space: HashMap> = self .design_space .iter() .map(|(tag, v)| { - let v: Vec> = - v.iter().map(|(start, end)| *start..=*end).collect(); + let v: RangeSet = v + .iter() + .map(|(start, end)| Fixed::from_i32(*start)..=Fixed::from_i32(*end)) + .collect(); (Tag::from_u32(*tag), v) }) .collect(); diff --git a/incremental-font-transfer/src/patchmap.rs b/incremental-font-transfer/src/patchmap.rs index ce0c99d7a..b86c81948 100644 --- a/incremental-font-transfer/src/patchmap.rs +++ b/incremental-font-transfer/src/patchmap.rs @@ -11,6 +11,7 @@ use std::io::Cursor; use std::io::Read; use std::ops::RangeInclusive; +use font_types::Fixed; use font_types::Tag; use data_encoding::BASE64URL; @@ -18,7 +19,7 @@ use data_encoding_macro::new_encoding; use read_fonts::FontRead; use read_fonts::{ - collections::IntSet, + collections::{IntSet, RangeSet}, tables::ift::{ CompatibilityId, EntryData, EntryFormatFlags, EntryMapRecord, Ift, PatchMapFormat1, PatchMapFormat2, @@ -32,8 +33,6 @@ use skrifa::charmap::Charmap; use uritemplate::UriTemplate; // TODO(garretrieger): implement support for building and compiling mapping tables. -// TODO(garretrieger): implement coalescing of patches by patch URI, ensuring that all entries with the same -// uri have matching encoding and compat id. /// Find the set of patches which intersect the specified subset definition. pub fn intersecting_patches( @@ -352,8 +351,10 @@ fn add_intersecting_format2_patches( if e.uri.encoding().is_invalidating() { // for invalidating keyed patches we need to record information about intersection size to use later // for patch selection. - e.uri.intersection_info = - IntersectionInfo::from_subset(e.intersection(subset_definition), order); + e.uri.intersection_info = IntersectionInfo::from_subset( + e.subset_definition.intersection(subset_definition), + order, + ); } patches.push(e.uri) @@ -444,7 +445,7 @@ fn decode_format2_entry<'a>( .design_space .entry(dss.axis_tag()) .or_default() - .push(dss.start().to_f64()..=dss.end().to_f64()); + .insert(dss.start()..=dss.end()); } } @@ -692,7 +693,7 @@ pub struct PatchUri { pub(crate) struct IntersectionInfo { intersecting_codepoints: u64, intersecting_layout_tags: usize, - // TODO(garretrieger): metric for design space intersection. + intersecting_design_space: BTreeMap, entry_order: usize, } @@ -720,6 +721,13 @@ impl Ord for IntersectionInfo { Ordering::Equal => {} ord => return ord, } + match self + .intersecting_design_space + .cmp(&other.intersecting_design_space) + { + Ordering::Equal => {} + ord => return ord, + } // We select the largest intersection info, and the spec requires in ties that the lowest entry order // is selected. So reverse the ordering of comparing entry_order. @@ -821,9 +829,24 @@ impl IntersectionInfo { IntersectionInfo { intersecting_codepoints: value.codepoints.len(), intersecting_layout_tags: value.feature_tags.len(), + intersecting_design_space: Self::design_space_size(value.design_space), entry_order: order, } } + + fn design_space_size(value: HashMap>) -> BTreeMap { + value + .into_iter() + .map(|(tag, ranges)| { + let total = ranges + .iter() + .map(|range| *range.end() - *range.start()) + .fold(Fixed::ZERO, |acc, x| acc + x); + + (tag, total) + }) + .collect() + } } /// Stores a description of a font subset over codepoints, feature tags, and design space. @@ -831,7 +854,7 @@ impl IntersectionInfo { pub struct SubsetDefinition { codepoints: IntSet, feature_tags: BTreeSet, - design_space: HashMap>>, + design_space: HashMap>, } impl SubsetDefinition { @@ -846,7 +869,7 @@ impl SubsetDefinition { pub fn new( codepoints: IntSet, feature_tags: BTreeSet, - design_space: HashMap>>, + design_space: HashMap>, ) -> SubsetDefinition { SubsetDefinition { codepoints, @@ -864,9 +887,44 @@ impl SubsetDefinition { self.design_space .entry(*tag) .or_default() - .extend(segments.clone()); + .extend(segments.iter()); } } + + fn intersection(&self, other: &Self) -> Self { + let mut result: SubsetDefinition = self.clone(); + + result.codepoints.intersect(&other.codepoints); + + result.feature_tags = self + .feature_tags + .intersection(&other.feature_tags) + .copied() + .collect(); + + result.design_space = self.design_space_intersection(&other.design_space); + + result + } + + fn design_space_intersection( + &self, + design_space: &HashMap>, + ) -> HashMap> { + let mut result: HashMap> = Default::default(); + for (tag, input_segments) in design_space { + let Some(entry_segments) = self.design_space.get(tag) else { + continue; + }; + + let ranges: RangeSet = input_segments.intersection(entry_segments).collect(); + if !ranges.is_empty() { + result.insert(*tag, ranges); + } + } + + result + } } /// Stores a materialized version of an IFT patchmap entry. @@ -934,45 +992,14 @@ impl Entry { || self.design_space_intersects(&subset_definition.design_space) } - fn intersection(&self, subset_definition: &SubsetDefinition) -> SubsetDefinition { - let mut result: SubsetDefinition = subset_definition.clone(); - - if !self.subset_definition.codepoints.is_empty() { - result - .codepoints - .intersect(&self.subset_definition.codepoints); - } - - if !self.subset_definition.feature_tags.is_empty() { - result.feature_tags = subset_definition - .feature_tags - .intersection(&self.subset_definition.feature_tags) - .copied() - .collect(); - } - - // TODO(garretrieger): design space. - - result - } - - fn design_space_intersects( - &self, - design_space: &HashMap>>, - ) -> bool { + fn design_space_intersects(&self, design_space: &HashMap>) -> bool { for (tag, input_segments) in design_space { let Some(entry_segments) = self.subset_definition.design_space.get(tag) else { continue; }; - // TODO(garretrieger): this is inefficient (O(n^2)). If we keep the ranges sorted by start then - // this can be implemented much more efficiently. - for a in input_segments { - for b in entry_segments { - if a.start() <= b.end() && b.start() <= a.end() { - return true; - } - } + if input_segments.intersection(entry_segments).next().is_some() { + return true; } } @@ -1004,6 +1031,21 @@ mod tests { IntersectionInfo { intersecting_codepoints: codepoints, intersecting_layout_tags: features, + intersecting_design_space: Default::default(), + entry_order: order, + } + } + + pub(crate) fn from_design_space( + codepoints: u64, + features: usize, + design_space: [(Tag, Fixed); N], + order: usize, + ) -> Self { + IntersectionInfo { + intersecting_codepoints: codepoints, + intersecting_layout_tags: features, + intersecting_design_space: BTreeMap::from(design_space), entry_order: order, } } @@ -1095,7 +1137,7 @@ mod tests { font: &FontRef, codepoints: [u32; M], tags: [Tag; N], - design_space: [(Tag, Vec>); O], + design_space: [(Tag, RangeSet); O], expected_entries: [ExpectedEntry; P], ) { let patches = intersecting_patches( @@ -1759,7 +1801,12 @@ mod tests { &font, [0x02], [Tag::new(b"rlig")], - [(Tag::new(b"wdth"), vec![0.7..=0.8])], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(0.7)..=Fixed::from_f64(0.8)] + .into_iter() + .collect(), + )], [e2], ); @@ -1767,21 +1814,48 @@ mod tests { &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![0.7..=0.8])], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(0.7)..=Fixed::from_f64(0.8)] + .into_iter() + .collect(), + )], [e1], ); test_design_space_intersection( &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![0.2..=0.3])], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(0.2)..=Fixed::from_f64(0.3)] + .into_iter() + .collect(), + )], [e3], ); + test_design_space_intersection( + &font, + [0x55], + [Tag::new(b"smcp")], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(0.2)..=Fixed::from_f64(0.3)] + .into_iter() + .collect(), + )], + [], + ); test_design_space_intersection( &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![1.2..=1.3])], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(1.2)..=Fixed::from_f64(1.3)] + .into_iter() + .collect(), + )], [], ); @@ -1789,21 +1863,42 @@ mod tests { &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![0.2..=0.3, 0.7..=0.8])], + [( + Tag::new(b"wdth"), + [ + Fixed::from_f64(0.2)..=Fixed::from_f64(0.3), + Fixed::from_f64(0.7)..=Fixed::from_f64(0.8), + ] + .into_iter() + .collect(), + )], [e1, e3], ); test_design_space_intersection( &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![2.2..=2.3])], + [( + Tag::new(b"wdth"), + [Fixed::from_f64(2.2)..=Fixed::from_f64(2.3)] + .into_iter() + .collect(), + )], [e3], ); test_design_space_intersection( &font, [0x05], [Tag::new(b"smcp")], - [(Tag::new(b"wdth"), vec![2.2..=2.3, 1.2..=1.3])], + [( + Tag::new(b"wdth"), + [ + Fixed::from_f64(2.2)..=Fixed::from_f64(2.3), + Fixed::from_f64(1.2)..=Fixed::from_f64(1.3), + ] + .into_iter() + .collect(), + )], [e3], ); } @@ -1845,7 +1940,12 @@ mod tests { &SubsetDefinition::new( IntSet::from([10, 15, 22]), BTreeSet::from([Tag::new(b"rlig"), Tag::new(b"liga"), Tag::new(b"smcp")]), - HashMap::from([(Tag::new(b"wght"), vec![300f64..=400f64])]), + HashMap::from([( + Tag::new(b"wght"), + [Fixed::from_i32(505)..=Fixed::from_i32(800)] + .into_iter() + .collect(), + )]), ), ) .unwrap(); @@ -1860,7 +1960,12 @@ mod tests { patch_with_intersection( map.offset_for("entries[2]") * 8 + 3, 3, - IntersectionInfo::new(3, 1, 2), + IntersectionInfo::from_design_space( + 3, + 1, + [(Tag::new(b"wght"), Fixed::from_i32(195))], + 2 + ), ), ] ); @@ -1889,7 +1994,12 @@ mod tests { &font, [], [Tag::new(b"rlig")], - [(Tag::new(b"wght"), vec![500.0..=500.0])], + [( + Tag::new(b"wght"), + [Fixed::from_i32(500)..=Fixed::from_i32(500)] + .into_iter() + .collect(), + )], [e3, e6], ); @@ -1897,7 +2007,12 @@ mod tests { &font, [0x05], [Tag::new(b"rlig")], - [(Tag::new(b"wght"), vec![500.0..=500.0])], + [( + Tag::new(b"wght"), + [Fixed::from_i32(500)..=Fixed::from_i32(500)] + .into_iter() + .collect(), + )], [e3, e5, e6, e7, e8, e9], ); } @@ -2151,4 +2266,204 @@ mod tests { assert_eq!(v3.cmp(&v5), Ordering::Less); assert_eq!(v5.cmp(&v3), Ordering::Greater); } + + #[test] + fn intersection_info_ordering_with_design_space() { + let aaaa = Tag::new(b"aaaa"); + let bbbb = Tag::new(b"bbbb"); + + // these are in the correct order + let v1 = IntersectionInfo::from_design_space(1, 0, [(aaaa, 4i32.into())], 0); + let v2 = IntersectionInfo::from_design_space( + 1, + 0, + [(aaaa, 5i32.into()), (bbbb, 2i32.into())], + 0, + ); + let v3 = IntersectionInfo::from_design_space(1, 0, [(aaaa, 6i32.into())], 0); + let v4 = IntersectionInfo::from_design_space( + 1, + 0, + [(aaaa, 6i32.into()), (bbbb, 2i32.into())], + 0, + ); + let v5 = IntersectionInfo::from_design_space(1, 0, [(bbbb, 1i32.into())], 0); + let v6 = IntersectionInfo::from_design_space(2, 0, [(aaaa, 1i32.into())], 0); + + assert_eq!(v1.cmp(&v1), Ordering::Equal); + + assert_eq!(v1.cmp(&v2), Ordering::Less); + assert_eq!(v2.cmp(&v1), Ordering::Greater); + + assert_eq!(v2.cmp(&v3), Ordering::Less); + assert_eq!(v3.cmp(&v2), Ordering::Greater); + + assert_eq!(v3.cmp(&v4), Ordering::Less); + assert_eq!(v4.cmp(&v3), Ordering::Greater); + + assert_eq!(v4.cmp(&v5), Ordering::Less); + assert_eq!(v5.cmp(&v4), Ordering::Greater); + + assert_eq!(v5.cmp(&v6), Ordering::Less); + assert_eq!(v6.cmp(&v5), Ordering::Greater); + + assert_eq!(v3.cmp(&v5), Ordering::Less); + assert_eq!(v5.cmp(&v3), Ordering::Greater); + } + + #[test] + fn entry_design_codepoints_intersection() { + let uri = PatchUri::from_index( + "", + 0, + IftTableTag::Ift(CompatibilityId::from_u32s([0, 0, 0, 0])), + 0, + PatchEncoding::GlyphKeyed, + IntersectionInfo::default(), + ); + + let s1 = SubsetDefinition::codepoints([3, 5, 7].into_iter().collect()); + let s2 = SubsetDefinition::codepoints([13, 15, 17].into_iter().collect()); + let s3 = SubsetDefinition::codepoints([7, 13].into_iter().collect()); + + let e1 = Entry { + subset_definition: s1.clone(), + uri: uri.clone(), + ignored: false, + }; + let e2 = Entry { + subset_definition: Default::default(), + uri: uri.clone(), + ignored: false, + }; + + assert!(e1.intersects(&s1)); + assert_eq!(s1.intersection(&s1), s1); + + assert!(!e1.intersects(&s2)); + assert_eq!(s1.intersection(&s2), Default::default()); + + assert!(e1.intersects(&s3)); + assert_eq!( + s1.intersection(&s3), + SubsetDefinition::codepoints([7].into_iter().collect()) + ); + + assert!(e2.intersects(&s1)); + assert_eq!( + SubsetDefinition::default().intersection(&s1), + Default::default() + ); + } + + #[test] + fn entry_design_space_intersection() { + let uri = PatchUri::from_index( + "", + 0, + IftTableTag::Ift(CompatibilityId::from_u32s([0, 0, 0, 0])), + 0, + PatchEncoding::GlyphKeyed, + IntersectionInfo::default(), + ); + + let s1 = SubsetDefinition::new( + Default::default(), + Default::default(), + HashMap::from([ + ( + Tag::new(b"aaaa"), + [Fixed::from_i32(100)..=Fixed::from_i32(200)] + .into_iter() + .collect(), + ), + ( + Tag::new(b"bbbb"), + [ + Fixed::from_i32(300)..=Fixed::from_i32(600), + Fixed::from_i32(700)..=Fixed::from_i32(900), + ] + .into_iter() + .collect(), + ), + ]), + ); + let s2 = SubsetDefinition::new( + Default::default(), + Default::default(), + HashMap::from([ + ( + Tag::new(b"bbbb"), + [Fixed::from_i32(100)..=Fixed::from_i32(200)] + .into_iter() + .collect(), + ), + ( + Tag::new(b"cccc"), + [Fixed::from_i32(300)..=Fixed::from_i32(600)] + .into_iter() + .collect(), + ), + ]), + ); + let s3 = SubsetDefinition::new( + Default::default(), + Default::default(), + HashMap::from([ + ( + Tag::new(b"bbbb"), + [Fixed::from_i32(500)..=Fixed::from_i32(800)] + .into_iter() + .collect(), + ), + ( + Tag::new(b"cccc"), + [Fixed::from_i32(300)..=Fixed::from_i32(600)] + .into_iter() + .collect(), + ), + ]), + ); + let s4 = SubsetDefinition::new( + Default::default(), + Default::default(), + HashMap::from([( + Tag::new(b"bbbb"), + [ + Fixed::from_i32(500)..=Fixed::from_i32(600), + Fixed::from_i32(700)..=Fixed::from_i32(800), + ] + .into_iter() + .collect(), + )]), + ); + + let e1 = Entry { + subset_definition: s1.clone(), + uri: uri.clone(), + ignored: false, + }; + let e2 = Entry { + subset_definition: Default::default(), + uri: uri.clone(), + ignored: false, + }; + + assert!(e1.intersects(&s1)); + assert_eq!(s1.intersection(&s1), s1.clone()); + + assert!(!e1.intersects(&s2)); + assert_eq!(s1.intersection(&s2), Default::default()); + + assert!(e1.intersects(&s3)); + assert_eq!(s1.intersection(&s3), s4.clone()); + + assert!(e2.intersects(&s1)); + assert_eq!( + SubsetDefinition::default().intersection(&s1), + SubsetDefinition::default() + ); + } + + // TODO(garretrieger): test for design space union of SubsetDefinition. } diff --git a/read-fonts/src/collections/range_set.rs b/read-fonts/src/collections/range_set.rs index e9b2252d2..e4e8592b4 100644 --- a/read-fonts/src/collections/range_set.rs +++ b/read-fonts/src/collections/range_set.rs @@ -31,6 +31,11 @@ impl RangeSet where T: Ord + Copy + OrdAdjacency, { + // Returns true if there are no members in this set currently. + pub fn is_empty(&self) -> bool { + self.ranges.is_empty() + } + /// Insert a range into this set, automatically merging with existing ranges as needed. pub fn insert(&mut self, range: RangeInclusive) { if range.end() < range.start() {