diff --git a/write-fonts/src/tables/cmap.rs b/write-fonts/src/tables/cmap.rs index 64695190a..0dee2aa42 100644 --- a/write-fonts/src/tables/cmap.rs +++ b/write-fonts/src/tables/cmap.rs @@ -164,6 +164,35 @@ impl CmapSubtable { } } +#[derive(Debug, Clone, PartialEq, Eq)] +pub enum MappingError { + /// When a char is mapped to multiple distinct GlyphIds. + /// gid1 is less than gid2. + /// If there are multiple conflicting mappings, one is chosen arbitrarily. + ConflictingMappings { + ch: char, + gid1: GlyphId, + gid2: GlyphId, + }, +} + +impl std::fmt::Display for MappingError { + fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result { + match self { + MappingError::ConflictingMappings { ch, gid1, gid2 } => { + let ch32 = *ch as u32; + write!( + f, + "Cannot map {ch:?} (U+{ch32:04X}) to two different glyph ids: {gid1} and {gid2}" + ) + } + } + } +} + +#[cfg(feature = "std")] +impl std::error::Error for MappingError {} + impl Cmap { /// Generates a [cmap](https://learn.microsoft.com/en-us/typography/opentype/spec/cmap) that is expected to work in most modern environments. /// @@ -172,9 +201,27 @@ impl Cmap { /// subtables, respectively for the Basic Multilingual Plane and Full Unicode Repertoire. /// /// Also see: - pub fn from_mappings(mappings: impl IntoIterator) -> Cmap { + pub fn from_mappings( + mappings: impl IntoIterator, + ) -> Result { let mut mappings: Vec<_> = mappings.into_iter().collect(); - mappings.sort(); + + let mut error = None; + mappings.sort_by(|(a, gid1), (b, gid2)| { + let ord = a.cmp(b); + if ord == std::cmp::Ordering::Equal && gid1 != gid2 { + let (gid1, gid2) = if gid1 > gid2 { + (*gid2, *gid1) + } else { + (*gid1, *gid2) + }; + error = Some(MappingError::ConflictingMappings { ch: *a, gid1, gid2 }) + } + ord + }); + if let Some(err) = error { + return Err(err); + } let mut uni_records = Vec::new(); // platform 0 let mut win_records = Vec::new(); // platform 3 @@ -221,7 +268,9 @@ impl Cmap { // - Unicode (0), full repertoire (4) // - Windows (3), BMP (1) // - Windows (3), full repertoire (10) - Cmap::new(uni_records.into_iter().chain(win_records).collect()) + Ok(Cmap::new( + uni_records.into_iter().chain(win_records).collect(), + )) } } @@ -236,13 +285,13 @@ mod tests { use crate::{ dump_table, tables::cmap::{ - self as write, UNICODE_BMP_ENCODING, UNICODE_FULL_REPERTOIRE_ENCODING, + self as write, MappingError, UNICODE_BMP_ENCODING, UNICODE_FULL_REPERTOIRE_ENCODING, WINDOWS_BMP_ENCODING, WINDOWS_FULL_REPERTOIRE_ENCODING, }, }; fn assert_generates_simple_cmap(mappings: Vec<(char, GlyphId)>) { - let cmap = write::Cmap::from_mappings(mappings); + let cmap = write::Cmap::from_mappings(mappings).unwrap(); let bytes = dump_table(&cmap).unwrap(); let font_data = FontData::new(&bytes); @@ -324,7 +373,7 @@ mod tests { let gid = GlyphId::new(153); mappings.push((codepoint, gid)); - let cmap = write::Cmap::from_mappings(mappings); + let cmap = write::Cmap::from_mappings(mappings).unwrap(); let bytes = dump_table(&cmap).unwrap(); let font_data = FontData::new(&bytes); @@ -336,7 +385,7 @@ mod tests { fn bytes_are_reused() { // We emit extra encoding records assuming it's cheap. Make sure. let mappings = simple_cmap_mappings(); - let cmap_both = write::Cmap::from_mappings(mappings); + let cmap_both = write::Cmap::from_mappings(mappings).unwrap(); assert_eq!(2, cmap_both.encoding_records.len(), "{cmap_both:?}"); let bytes_for_both = dump_table(&cmap_both).unwrap().len(); @@ -360,8 +409,8 @@ mod tests { ('\u{1f133}', GlyphId::new(484)), // gid 485 skipped, starts third group ('\u{1f134}', GlyphId::new(486)), - // char 0x1f135 skipped, starts fourth group; 0x1f136 defined twice, last map prevails - ('\u{1f136}', GlyphId::new(487)), + // char 0x1f135 skipped, starts fourth group. identical duplicate bindings are fine + ('\u{1f136}', GlyphId::new(488)), ('\u{1f136}', GlyphId::new(488)), ] } @@ -395,7 +444,7 @@ mod tests { fn generate_cmap4_and_12() { let mappings = bmp_and_non_bmp_cmap_mappings(); - let cmap = write::Cmap::from_mappings(mappings); + let cmap = write::Cmap::from_mappings(mappings).unwrap(); let bytes = dump_table(&cmap).unwrap(); let font_data = FontData::new(&bytes); @@ -444,7 +493,7 @@ mod tests { fn generate_cmap12_only() { let mappings = non_bmp_cmap_mappings(); - let cmap = write::Cmap::from_mappings(mappings); + let cmap = write::Cmap::from_mappings(mappings).unwrap(); let bytes = dump_table(&cmap).unwrap(); let font_data = FontData::new(&bytes); @@ -473,4 +522,20 @@ mod tests { assert_cmap12_groups(font_data, &cmap, 0, &expected_groups); assert_cmap12_groups(font_data, &cmap, 1, &expected_groups); } + + #[test] + fn multiple_mappings_fails() { + let mut mappings = non_bmp_cmap_mappings(); + // add an additional mapping to a different glyphId + let (ch, gid1) = mappings[0]; + let gid2 = GlyphId::new(gid1.to_u16() + 1); + mappings.push((ch, gid2)); + + let result = write::Cmap::from_mappings(mappings); + + assert_eq!( + result, + Err(MappingError::ConflictingMappings { ch, gid1, gid2 }) + ) + } }