From 49b8b6eea86ba0d2e8e2eac362de962c858ee13a Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 13:08:03 +0000 Subject: [PATCH 01/17] Add Font::kerning_lookup_slow --- src/font.rs | 46 ++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 46 insertions(+) diff --git a/src/font.rs b/src/font.rs index 627df40a..da38d19a 100644 --- a/src/font.rs +++ b/src/font.rs @@ -604,6 +604,52 @@ impl Font { pub fn guidelines_mut(&mut self) -> &mut Vec { self.font_info.guidelines.get_or_insert_with(Default::default) } + + pub fn kerning_lookup_slow(&self, first: &str, second: &str) -> Option { + let basic_lookup = |first: &str, second: &str| { + self.kerning.get(first).and_then(|first| first.get(second)).copied() + }; + + // glyph name glyph name + if let Some(kern) = basic_lookup(first, second) { + return Some(kern); + } + + // group name glyph name + let first_group = + self.groups.iter().filter(|(name, _)| name.starts_with("public.kern1.")).find_map( + |(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) + }, + ); + if let Some(first_group) = first_group { + if let Some(kern) = basic_lookup(first_group.as_str(), second) { + return Some(kern); + } + } + + // glyph name group name + let second_group = + self.groups.iter().filter(|(name, _)| name.starts_with("public.kern2.")).find_map( + |(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) + }, + ); + if let Some(second_group) = second_group { + if let Some(kern) = basic_lookup(first, second_group.as_str()) { + return Some(kern); + } + } + + // group name group name + if let Some((first_group, second_group)) = first_group.zip(second_group) { + if let Some(kern) = basic_lookup(first_group.as_str(), second_group.as_str()) { + return Some(kern); + } + } + + None + } } fn load_lib(lib_path: &Path) -> Result { From a5a0efa9292a1fff580cae0b691bdcad2c1417bc Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 13:18:58 +0000 Subject: [PATCH 02/17] Add Font::kerning_lookup & ReverseGroupsLookup --- src/font.rs | 55 +++++++++++++++++++++++++++++++++++++++++--------- src/kerning.rs | 46 +++++++++++++++++++++++++++++++++++++++-- 2 files changed, 90 insertions(+), 11 deletions(-) diff --git a/src/font.rs b/src/font.rs index da38d19a..03913a79 100644 --- a/src/font.rs +++ b/src/font.rs @@ -15,7 +15,7 @@ use crate::fontinfo::FontInfo; use crate::glyph::Glyph; use crate::groups::{validate_groups, Groups}; use crate::guideline::Guideline; -use crate::kerning::Kerning; +use crate::kerning::{Kerning, ReverseGroupsLookup}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; @@ -604,14 +604,47 @@ impl Font { pub fn guidelines_mut(&mut self) -> &mut Vec { self.font_info.guidelines.get_or_insert_with(Default::default) } + + #[inline] + pub fn get_reverse_groups_lookup(&self) -> ReverseGroupsLookup { + ReverseGroupsLookup::from(&self.groups) + } - pub fn kerning_lookup_slow(&self, first: &str, second: &str) -> Option { - let basic_lookup = |first: &str, second: &str| { - self.kerning.get(first).and_then(|first| first.get(second)).copied() - }; + pub fn kerning_lookup(&self, lookup: &ReverseGroupsLookup, first: &str, second: &str) -> Option { + // glyph name glyph name + if let Some(kern) = self.kerning_lookup_dumb(first, second) { + return Some(kern); + } + + // group name glyph name + let first_group = lookup.get_first(first); + if let Some(first_group) = &first_group { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { + return Some(kern); + } + } + + // glyph name group name + let second_group = lookup.get_second(second); + if let Some(second_group) = &second_group { + if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { + return Some(kern); + } + } + + // group name group name + if let Some((first_group, second_group)) = first_group.zip(second_group) { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) { + return Some(kern); + } + } + + None + } + pub fn kerning_lookup_slow(&self, first: &str, second: &str) -> Option { // glyph name glyph name - if let Some(kern) = basic_lookup(first, second) { + if let Some(kern) = self.kerning_lookup_dumb(first, second) { return Some(kern); } @@ -623,7 +656,7 @@ impl Font { }, ); if let Some(first_group) = first_group { - if let Some(kern) = basic_lookup(first_group.as_str(), second) { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { return Some(kern); } } @@ -636,20 +669,24 @@ impl Font { }, ); if let Some(second_group) = second_group { - if let Some(kern) = basic_lookup(first, second_group.as_str()) { + if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { return Some(kern); } } // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = basic_lookup(first_group.as_str(), second_group.as_str()) { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) { return Some(kern); } } None } + + fn kerning_lookup_dumb(&self, first: &str, second: &str) -> Option { + self.kerning.get(first).and_then(|first| first.get(second)).copied() + } } fn load_lib(lib_path: &Path) -> Result { diff --git a/src/kerning.rs b/src/kerning.rs index ed2c6ea5..cf558bdf 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -1,9 +1,9 @@ -use std::collections::BTreeMap; +use std::collections::{BTreeMap, HashMap}; use serde::ser::{SerializeMap, Serializer}; use serde::Serialize; -use crate::Name; +use crate::{Groups, Name}; /// A map of kerning pairs. /// @@ -14,6 +14,48 @@ use crate::Name; /// We use a [`BTreeMap`] because we need sorting for serialization. pub type Kerning = BTreeMap>; +#[derive(Debug)] +pub struct ReverseGroupsLookup { + first: HashMap, + second: HashMap, +} + +impl ReverseGroupsLookup { + #[inline] + pub fn get_first(&self, glyph_name: &str) -> Option { + self.first.get(glyph_name).cloned() + } + + #[inline] + pub fn get_second(&self, glyph_name: &str) -> Option { + self.second.get(glyph_name).cloned() + } +} + +impl From<&Groups> for ReverseGroupsLookup { + fn from(groups: &Groups) -> Self { + let first = groups + .iter() + .filter(|(group_name, _)| group_name.starts_with("public.kern1.")) + .flat_map(|(group_name, members)| { + members + .iter() + .map(|member| (member.clone(), group_name.clone())) + }) + .collect(); + let second = groups + .iter() + .filter(|(group_name, _)| group_name.starts_with("public.kern2.")) + .flat_map(|(group_name, members)| { + members + .iter() + .map(|member| (member.clone(), group_name.clone())) + }) + .collect(); + Self { first, second } + } +} + /// A helper for serializing kerning values. /// /// `KerningSerializer` is a crutch to serialize kerning values as integers if they are From 98fa6b98ef10f883272c63425e7e7a56ff1bcb03 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 13:23:24 +0000 Subject: [PATCH 03/17] Use constants for kerning group prefixes --- src/font.rs | 8 ++++---- src/groups.rs | 5 +++-- src/kerning.rs | 8 ++++++-- src/upconversion.rs | 12 +++++++----- 4 files changed, 20 insertions(+), 13 deletions(-) diff --git a/src/font.rs b/src/font.rs index 03913a79..267a547e 100644 --- a/src/font.rs +++ b/src/font.rs @@ -15,7 +15,7 @@ use crate::fontinfo::FontInfo; use crate::glyph::Glyph; use crate::groups::{validate_groups, Groups}; use crate::guideline::Guideline; -use crate::kerning::{Kerning, ReverseGroupsLookup}; +use crate::kerning::{Kerning, ReverseGroupsLookup, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; @@ -604,7 +604,7 @@ impl Font { pub fn guidelines_mut(&mut self) -> &mut Vec { self.font_info.guidelines.get_or_insert_with(Default::default) } - + #[inline] pub fn get_reverse_groups_lookup(&self) -> ReverseGroupsLookup { ReverseGroupsLookup::from(&self.groups) @@ -650,7 +650,7 @@ impl Font { // group name glyph name let first_group = - self.groups.iter().filter(|(name, _)| name.starts_with("public.kern1.")).find_map( + self.groups.iter().filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)).find_map( |(name, members)| { members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) }, @@ -663,7 +663,7 @@ impl Font { // glyph name group name let second_group = - self.groups.iter().filter(|(name, _)| name.starts_with("public.kern2.")).find_map( + self.groups.iter().filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)).find_map( |(name, members)| { members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) }, diff --git a/src/groups.rs b/src/groups.rs index 2a76c8c9..02a0309d 100644 --- a/src/groups.rs +++ b/src/groups.rs @@ -1,6 +1,7 @@ use std::collections::{BTreeMap, HashSet}; use crate::error::GroupsValidationError; +use crate::kerning::{FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::Name; /// A map of group name to a list of glyph names. @@ -18,7 +19,7 @@ pub(crate) fn validate_groups(groups_map: &Groups) -> Result<(), GroupsValidatio return Err(GroupsValidationError::InvalidName); } - if group_name.starts_with("public.kern1.") { + if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { if group_name.len() == 13 { // Prefix but no actual name. return Err(GroupsValidationError::InvalidName); @@ -31,7 +32,7 @@ pub(crate) fn validate_groups(groups_map: &Groups) -> Result<(), GroupsValidatio }); } } - } else if group_name.starts_with("public.kern2.") { + } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { if group_name.len() == 13 { // Prefix but no actual name. return Err(GroupsValidationError::InvalidName); diff --git a/src/kerning.rs b/src/kerning.rs index cf558bdf..69c1e0b7 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -5,6 +5,10 @@ use serde::Serialize; use crate::{Groups, Name}; +pub const FIRST_KERNING_GROUP_PREFIX: &str = "public.kern1."; +pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; + + /// A map of kerning pairs. /// /// This is represented as a map of first half of a kerning pair (glyph name or group name) @@ -36,7 +40,7 @@ impl From<&Groups> for ReverseGroupsLookup { fn from(groups: &Groups) -> Self { let first = groups .iter() - .filter(|(group_name, _)| group_name.starts_with("public.kern1.")) + .filter(|(group_name, _)| group_name.starts_with(FIRST_KERNING_GROUP_PREFIX)) .flat_map(|(group_name, members)| { members .iter() @@ -45,7 +49,7 @@ impl From<&Groups> for ReverseGroupsLookup { .collect(); let second = groups .iter() - .filter(|(group_name, _)| group_name.starts_with("public.kern2.")) + .filter(|(group_name, _)| group_name.starts_with(SECOND_KERNING_GROUP_PREFIX)) .flat_map(|(group_name, members)| { members .iter() diff --git a/src/upconversion.rs b/src/upconversion.rs index 9914b52f..236c4dd8 100644 --- a/src/upconversion.rs +++ b/src/upconversion.rs @@ -7,7 +7,7 @@ use crate::error::FontLoadError; use crate::font::LIB_FILE; use crate::fontinfo::FontInfo; use crate::groups::Groups; -use crate::kerning::Kerning; +use crate::kerning::{Kerning, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::names::NameList; use crate::Name; @@ -31,14 +31,14 @@ pub(crate) fn upconvert_kerning( for (first, seconds) in kerning { if groups.contains_key(first) && !glyph_set.contains(first) - && !first.starts_with("public.kern1.") + && !first.starts_with(FIRST_KERNING_GROUP_PREFIX) { groups_first.insert(first.clone()); } for second in seconds.keys() { if groups.contains_key(second) && !glyph_set.contains(second) - && !second.starts_with("public.kern2.") + && !second.starts_with(SECOND_KERNING_GROUP_PREFIX) { groups_second.insert(second.clone()); } @@ -51,7 +51,8 @@ pub(crate) fn upconvert_kerning( let mut groups_first_old_to_new: HashMap = HashMap::new(); for first in &groups_first { let first_new = make_unique_group_name( - Name::new(&format!("public.kern1.{}", first.replace("@MMK_L_", ""))).unwrap(), + Name::new(&format!("{FIRST_KERNING_GROUP_PREFIX}{}", first.replace("@MMK_L_", ""))) + .unwrap(), &groups_new, ); groups_first_old_to_new.insert(first.clone(), first_new.clone()); @@ -60,7 +61,8 @@ pub(crate) fn upconvert_kerning( let mut groups_second_old_to_new: HashMap = HashMap::new(); for second in &groups_second { let second_new = make_unique_group_name( - Name::new(&format!("public.kern2.{}", second.replace("@MMK_R_", ""))).unwrap(), + Name::new(&format!("{SECOND_KERNING_GROUP_PREFIX}{}", second.replace("@MMK_R_", ""))) + .unwrap(), &groups_new, ); groups_second_old_to_new.insert(second.clone(), second_new.clone()); From 6a413640f79a25288ef088dac2891b09b7afcc4e Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 13:24:04 +0000 Subject: [PATCH 04/17] Format --- src/font.rs | 45 +++++++++++++++++++++++++++++---------------- src/kerning.rs | 9 ++------- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/src/font.rs b/src/font.rs index 267a547e..48fb3e95 100644 --- a/src/font.rs +++ b/src/font.rs @@ -15,7 +15,9 @@ use crate::fontinfo::FontInfo; use crate::glyph::Glyph; use crate::groups::{validate_groups, Groups}; use crate::guideline::Guideline; -use crate::kerning::{Kerning, ReverseGroupsLookup, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; +use crate::kerning::{ + Kerning, ReverseGroupsLookup, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX, +}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; @@ -610,7 +612,12 @@ impl Font { ReverseGroupsLookup::from(&self.groups) } - pub fn kerning_lookup(&self, lookup: &ReverseGroupsLookup, first: &str, second: &str) -> Option { + pub fn kerning_lookup( + &self, + lookup: &ReverseGroupsLookup, + first: &str, + second: &str, + ) -> Option { // glyph name glyph name if let Some(kern) = self.kerning_lookup_dumb(first, second) { return Some(kern); @@ -634,7 +641,9 @@ impl Font { // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) { + if let Some(kern) = + self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) + { return Some(kern); } } @@ -649,12 +658,13 @@ impl Font { } // group name glyph name - let first_group = - self.groups.iter().filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)).find_map( - |(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) - }, - ); + let first_group = self + .groups + .iter() + .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) + .find_map(|(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) + }); if let Some(first_group) = first_group { if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { return Some(kern); @@ -662,12 +672,13 @@ impl Font { } // glyph name group name - let second_group = - self.groups.iter().filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)).find_map( - |(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) - }, - ); + let second_group = self + .groups + .iter() + .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) + .find_map(|(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) + }); if let Some(second_group) = second_group { if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { return Some(kern); @@ -676,7 +687,9 @@ impl Font { // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) { + if let Some(kern) = + self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) + { return Some(kern); } } diff --git a/src/kerning.rs b/src/kerning.rs index 69c1e0b7..805babcd 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -8,7 +8,6 @@ use crate::{Groups, Name}; pub const FIRST_KERNING_GROUP_PREFIX: &str = "public.kern1."; pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; - /// A map of kerning pairs. /// /// This is represented as a map of first half of a kerning pair (glyph name or group name) @@ -42,18 +41,14 @@ impl From<&Groups> for ReverseGroupsLookup { .iter() .filter(|(group_name, _)| group_name.starts_with(FIRST_KERNING_GROUP_PREFIX)) .flat_map(|(group_name, members)| { - members - .iter() - .map(|member| (member.clone(), group_name.clone())) + members.iter().map(|member| (member.clone(), group_name.clone())) }) .collect(); let second = groups .iter() .filter(|(group_name, _)| group_name.starts_with(SECOND_KERNING_GROUP_PREFIX)) .flat_map(|(group_name, members)| { - members - .iter() - .map(|member| (member.clone(), group_name.clone())) + members.iter().map(|member| (member.clone(), group_name.clone())) }) .collect(); Self { first, second } From 361e3ed9752ff76f855f95eb66b244f8a92b5ed9 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 16:27:40 +0000 Subject: [PATCH 05/17] Fix lookup order Match the spec: https://unifiedfontobject.org/versions/ufo3/kerning.plist/#exception-conflict-resolution --- src/font.rs | 40 ++++++++++++++++++++-------------------- 1 file changed, 20 insertions(+), 20 deletions(-) diff --git a/src/font.rs b/src/font.rs index 48fb3e95..072239ec 100644 --- a/src/font.rs +++ b/src/font.rs @@ -623,14 +623,6 @@ impl Font { return Some(kern); } - // group name glyph name - let first_group = lookup.get_first(first); - if let Some(first_group) = &first_group { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { - return Some(kern); - } - } - // glyph name group name let second_group = lookup.get_second(second); if let Some(second_group) = &second_group { @@ -639,6 +631,14 @@ impl Font { } } + // group name glyph name + let first_group = lookup.get_first(first); + if let Some(first_group) = &first_group { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { + return Some(kern); + } + } + // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { if let Some(kern) = @@ -657,30 +657,30 @@ impl Font { return Some(kern); } - // group name glyph name - let first_group = self + // glyph name group name + let second_group = self .groups .iter() - .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) + .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) + members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) }); - if let Some(first_group) = first_group { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { + if let Some(second_group) = second_group { + if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { return Some(kern); } } - // glyph name group name - let second_group = self + // group name glyph name + let first_group = self .groups .iter() - .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) + .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) + members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) }); - if let Some(second_group) = second_group { - if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { + if let Some(first_group) = first_group { + if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { return Some(kern); } } From aab44e38ed4ed098a826f8c17dbed5083a79a16b Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 16:38:36 +0000 Subject: [PATCH 06/17] Test kerning lookup methods --- src/kerning.rs | 53 ++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 53 insertions(+) diff --git a/src/kerning.rs b/src/kerning.rs index 805babcd..2b9f440a 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -102,6 +102,7 @@ impl Serialize for KerningInnerSerializer<'_> { #[cfg(test)] mod tests { use super::*; + use crate::Font; use maplit::btreemap; use serde_test::{assert_ser_tokens, Token}; @@ -136,4 +137,56 @@ mod tests { ], ); } + + #[test] + fn test_kerning_resolution() { + // Test data taken from https://unifiedfontobject.org/versions/ufo3/kerning.plist/#exceptions + let font = Font { + groups: btreemap! { + Name::new_raw("public.kern1.O") => vec![ + Name::new_raw("O"), + Name::new_raw("D"), + Name::new_raw("Q"), + ], + Name::new_raw("public.kern2.E") => vec![ + Name::new_raw("E"), + Name::new_raw("F"), + ], + }, + kerning: btreemap! { + Name::new_raw("public.kern1.O") => btreemap! { + Name::new_raw("public.kern2.E") => -100f64, + Name::new_raw("F") => -200f64, + }, + Name::new_raw("Q") => btreemap! { + Name::new_raw("public.kern2.E") => -250f64, + }, + Name::new_raw("D") => btreemap! { + Name::new_raw("F") => -300f64, + }, + }, + ..Default::default() + }; + + let lookup = font.get_reverse_groups_lookup(); + for (left, right, expected) in [ + ("O", "E", -100f64), + ("O", "F", -200f64), + ("D", "E", -100f64), + ("D", "F", -300f64), + ("Q", "E", -250f64), + ("Q", "F", -250f64), + ] { + assert_eq!( + font.kerning_lookup(&lookup, left, right), + Some(expected), + "kerning_lookup incorrect for /{left}/{right}" + ); + assert_eq!( + font.kerning_lookup_slow(left, right), + Some(expected), + "kerning_lookup_slow incorrect for /{left}/{right}" + ); + } + } } From ab569f7f5c4b5e9706aa2d5684d77d14fae814a2 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 17:12:43 +0000 Subject: [PATCH 07/17] Document new methods on Font --- src/font.rs | 34 ++++++++++++++++++++++++++++++++++ 1 file changed, 34 insertions(+) diff --git a/src/font.rs b/src/font.rs index 072239ec..d0e39b28 100644 --- a/src/font.rs +++ b/src/font.rs @@ -607,11 +607,31 @@ impl Font { self.font_info.guidelines.get_or_insert_with(Default::default) } + /// Builds a [`ReverseGroupsLookup`], used to determine what group a glyph + /// is in. + /// + /// Effectively the inverse of `groups.plist`. + /// + /// Used by [`Font::kerning_lookup`]. #[inline] pub fn get_reverse_groups_lookup(&self) -> ReverseGroupsLookup { ReverseGroupsLookup::from(&self.groups) } + /// Retrieve the kerning value (if any) between a pair of elements. + /// + /// The elments can be either individual glyphs (by name) or kerning groups + /// (by name), or any combination of the two. + // ^ note: this works without any special consideration in the code + // because glyph names are forbidden from using the group prefix, + // thus meaning the group name lookup will always fail if a group + // was passed in + /// + /// This method is accelerated by the [`ReverseGroupsLookup`], which can be + /// obtained from [`Font::get_reverse_groups_lookup`]. Using it is highly + /// recommended if doing numerous lookups, but if not you can use + /// [`Font::kerning_lookup_slow`], which is the same method without needing + /// the pre-computed groups lookup. pub fn kerning_lookup( &self, lookup: &ReverseGroupsLookup, @@ -651,6 +671,20 @@ impl Font { None } + /// Retrieve the kerning value (if any) between a pair of elements. + /// + /// The elments can be either individual glyphs (by name) or kerning groups + /// (by name), or any combination of the two. + // ^ note: this works without any special consideration in the code + // because glyph names are forbidden from using the group prefix, + // thus meaning the group name lookup will always fail if a group + // was passed in + /// + /// ⚠️ This method forgoes pre-computing a reverse glyph groups lookup for the + /// use case where you're only after a couple of kerns and the overhead of + /// generating the lookup is not worth it. But in cases where you're + /// querying many kerns, you should definitely be using + /// [`Font::kerning_lookup`] over this method. pub fn kerning_lookup_slow(&self, first: &str, second: &str) -> Option { // glyph name glyph name if let Some(kern) = self.kerning_lookup_dumb(first, second) { From dfceb24306d7965493e213e171c964a538d679c7 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 17:17:09 +0000 Subject: [PATCH 08/17] Make kerning module public and document its members Retains re-export of Kerning type alias --- src/kerning.rs | 9 +++++++++ src/lib.rs | 2 +- 2 files changed, 10 insertions(+), 1 deletion(-) diff --git a/src/kerning.rs b/src/kerning.rs index 2b9f440a..46f1b096 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -1,3 +1,5 @@ +//! Helper types & constants for working with groups & kerning. + use std::collections::{BTreeMap, HashMap}; use serde::ser::{SerializeMap, Serializer}; @@ -5,7 +7,9 @@ use serde::Serialize; use crate::{Groups, Name}; +/// The UFO standard group prefix for kerns in the first position. pub const FIRST_KERNING_GROUP_PREFIX: &str = "public.kern1."; +/// The UFO standard group prefix for kerns in the second position. pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; /// A map of kerning pairs. @@ -17,6 +21,7 @@ pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; /// We use a [`BTreeMap`] because we need sorting for serialization. pub type Kerning = BTreeMap>; +/// Maps glyph names to group names; the inverse of a `groups.plist` file. #[derive(Debug)] pub struct ReverseGroupsLookup { first: HashMap, @@ -24,11 +29,15 @@ pub struct ReverseGroupsLookup { } impl ReverseGroupsLookup { + /// Get the group (if any) for the glyph name when it's first in a kerning + /// pair. #[inline] pub fn get_first(&self, glyph_name: &str) -> Option { self.first.get(glyph_name).cloned() } + /// Get the group (if any) for the glyph name when it's second in a + /// kerning pair. #[inline] pub fn get_second(&self, glyph_name: &str) -> Option { self.second.get(glyph_name).cloned() diff --git a/src/lib.rs b/src/lib.rs index 56b06864..d1bc1e01 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -75,7 +75,7 @@ mod glyph; mod groups; mod guideline; mod identifier; -mod kerning; +pub mod kerning; mod layer; mod name; mod names; From b56e3f2f05e23fa90e67b569ac811b3f305a9345 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Fri, 20 Mar 2026 17:18:24 +0000 Subject: [PATCH 09/17] Fix typos --- src/font.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/font.rs b/src/font.rs index d0e39b28..0ed09d86 100644 --- a/src/font.rs +++ b/src/font.rs @@ -620,7 +620,7 @@ impl Font { /// Retrieve the kerning value (if any) between a pair of elements. /// - /// The elments can be either individual glyphs (by name) or kerning groups + /// The elements can be either individual glyphs (by name) or kerning groups /// (by name), or any combination of the two. // ^ note: this works without any special consideration in the code // because glyph names are forbidden from using the group prefix, @@ -673,7 +673,7 @@ impl Font { /// Retrieve the kerning value (if any) between a pair of elements. /// - /// The elments can be either individual glyphs (by name) or kerning groups + /// The elements can be either individual glyphs (by name) or kerning groups /// (by name), or any combination of the two. // ^ note: this works without any special consideration in the code // because glyph names are forbidden from using the group prefix, From 36c06ecb653c5a8986ebe580ff9f4d25c1b8c5e1 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Mon, 23 Mar 2026 10:37:14 +0000 Subject: [PATCH 10/17] Use constants for length --- src/groups.rs | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/src/groups.rs b/src/groups.rs index 02a0309d..1320424b 100644 --- a/src/groups.rs +++ b/src/groups.rs @@ -20,7 +20,7 @@ pub(crate) fn validate_groups(groups_map: &Groups) -> Result<(), GroupsValidatio } if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { - if group_name.len() == 13 { + if group_name.len() == FIRST_KERNING_GROUP_PREFIX.len() { // Prefix but no actual name. return Err(GroupsValidationError::InvalidName); } @@ -33,7 +33,7 @@ pub(crate) fn validate_groups(groups_map: &Groups) -> Result<(), GroupsValidatio } } } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { - if group_name.len() == 13 { + if group_name.len() == SECOND_KERNING_GROUP_PREFIX.len() { // Prefix but no actual name. return Err(GroupsValidationError::InvalidName); } From 7a2a1aeafcb5a9e5b15fd2e11b4ee8db0db8da53 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Mon, 23 Mar 2026 10:40:35 +0000 Subject: [PATCH 11/17] Move kerning group prefixes to groups & make mod pub --- src/font.rs | 8 ++++---- src/groups.rs | 8 +++++++- src/kerning.rs | 8 ++------ src/lib.rs | 2 +- src/upconversion.rs | 4 ++-- 5 files changed, 16 insertions(+), 14 deletions(-) diff --git a/src/font.rs b/src/font.rs index 0ed09d86..bccdeeda 100644 --- a/src/font.rs +++ b/src/font.rs @@ -13,11 +13,11 @@ use crate::datastore::{DataStore, ImageStore}; use crate::error::{FontLoadError, FontWriteError}; use crate::fontinfo::FontInfo; use crate::glyph::Glyph; -use crate::groups::{validate_groups, Groups}; -use crate::guideline::Guideline; -use crate::kerning::{ - Kerning, ReverseGroupsLookup, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX, +use crate::groups::{ + validate_groups, Groups, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX, }; +use crate::guideline::Guideline; +use crate::kerning::{Kerning, ReverseGroupsLookup}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; diff --git a/src/groups.rs b/src/groups.rs index 1320424b..7cf1e427 100644 --- a/src/groups.rs +++ b/src/groups.rs @@ -1,9 +1,15 @@ +//! Helper types & constants for working with groups. + use std::collections::{BTreeMap, HashSet}; use crate::error::GroupsValidationError; -use crate::kerning::{FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::Name; +/// The UFO standard group prefix for kerns in the first position. +pub const FIRST_KERNING_GROUP_PREFIX: &str = "public.kern1."; +/// The UFO standard group prefix for kerns in the second position. +pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; + /// A map of group name to a list of glyph names. /// /// We use a [`BTreeMap`] because we need sorting for serialization. diff --git a/src/kerning.rs b/src/kerning.rs index 46f1b096..a258ed66 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -1,17 +1,13 @@ -//! Helper types & constants for working with groups & kerning. +//! Helper types for working with kerning. use std::collections::{BTreeMap, HashMap}; use serde::ser::{SerializeMap, Serializer}; use serde::Serialize; +use crate::groups::{FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::{Groups, Name}; -/// The UFO standard group prefix for kerns in the first position. -pub const FIRST_KERNING_GROUP_PREFIX: &str = "public.kern1."; -/// The UFO standard group prefix for kerns in the second position. -pub const SECOND_KERNING_GROUP_PREFIX: &str = "public.kern2."; - /// A map of kerning pairs. /// /// This is represented as a map of first half of a kerning pair (glyph name or group name) diff --git a/src/lib.rs b/src/lib.rs index d1bc1e01..3b3ca224 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -72,7 +72,7 @@ pub mod error; mod font; pub mod fontinfo; mod glyph; -mod groups; +pub mod groups; mod guideline; mod identifier; pub mod kerning; diff --git a/src/upconversion.rs b/src/upconversion.rs index 236c4dd8..72cb310d 100644 --- a/src/upconversion.rs +++ b/src/upconversion.rs @@ -6,8 +6,8 @@ use serde::Deserialize; use crate::error::FontLoadError; use crate::font::LIB_FILE; use crate::fontinfo::FontInfo; -use crate::groups::Groups; -use crate::kerning::{Kerning, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; +use crate::groups::{Groups, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; +use crate::kerning::Kerning; use crate::names::NameList; use crate::Name; From d25941175437505e00ee328b80ba3811c3dd81fb Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Wed, 25 Mar 2026 12:03:14 +0000 Subject: [PATCH 12/17] Make ReverseGroupsLookup creation more efficient Only iterate through groups once --- src/kerning.rs | 26 +++++++++++--------------- 1 file changed, 11 insertions(+), 15 deletions(-) diff --git a/src/kerning.rs b/src/kerning.rs index a258ed66..e8f50430 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -42,21 +42,17 @@ impl ReverseGroupsLookup { impl From<&Groups> for ReverseGroupsLookup { fn from(groups: &Groups) -> Self { - let first = groups - .iter() - .filter(|(group_name, _)| group_name.starts_with(FIRST_KERNING_GROUP_PREFIX)) - .flat_map(|(group_name, members)| { - members.iter().map(|member| (member.clone(), group_name.clone())) - }) - .collect(); - let second = groups - .iter() - .filter(|(group_name, _)| group_name.starts_with(SECOND_KERNING_GROUP_PREFIX)) - .flat_map(|(group_name, members)| { - members.iter().map(|member| (member.clone(), group_name.clone())) - }) - .collect(); - Self { first, second } + groups.iter().fold(ReverseGroupsLookup { first: HashMap::new(), second: HashMap::new() }, |mut rgl, (group_name, members)| { + let inverted = members.iter().map(|member| (member.clone(), group_name.clone())); + if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { + rgl.first.extend(inverted); + } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { + rgl.second.extend(inverted); + } else { + unreachable!("group didn't start with {FIRST_KERNING_GROUP_PREFIX} or {SECOND_KERNING_GROUP_PREFIX}"); + } + rgl + }) } } From 9c37bed2a086062b65f512a34cc8f116645b3f43 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Wed, 25 Mar 2026 13:50:20 +0000 Subject: [PATCH 13/17] Remove incorrect panic --- src/kerning.rs | 23 ++++++++++++----------- 1 file changed, 12 insertions(+), 11 deletions(-) diff --git a/src/kerning.rs b/src/kerning.rs index e8f50430..59ebf1cc 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -42,17 +42,18 @@ impl ReverseGroupsLookup { impl From<&Groups> for ReverseGroupsLookup { fn from(groups: &Groups) -> Self { - groups.iter().fold(ReverseGroupsLookup { first: HashMap::new(), second: HashMap::new() }, |mut rgl, (group_name, members)| { - let inverted = members.iter().map(|member| (member.clone(), group_name.clone())); - if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { - rgl.first.extend(inverted); - } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { - rgl.second.extend(inverted); - } else { - unreachable!("group didn't start with {FIRST_KERNING_GROUP_PREFIX} or {SECOND_KERNING_GROUP_PREFIX}"); - } - rgl - }) + groups.iter().fold( + ReverseGroupsLookup { first: HashMap::new(), second: HashMap::new() }, + |mut rgl, (group_name, members)| { + let inverted = members.iter().map(|member| (member.clone(), group_name.clone())); + if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { + rgl.first.extend(inverted); + } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { + rgl.second.extend(inverted); + } + rgl + }, + ) } } From 8438f435f5d815609b082b3e31a8e2c39e0cd9f7 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Tue, 31 Mar 2026 12:03:41 +0100 Subject: [PATCH 14/17] Avoid searching through group members if arg is a group name --- src/font.rs | 48 +++++++++++++++++++++++++----------------------- src/kerning.rs | 2 ++ 2 files changed, 27 insertions(+), 23 deletions(-) diff --git a/src/font.rs b/src/font.rs index bccdeeda..d0b456c9 100644 --- a/src/font.rs +++ b/src/font.rs @@ -675,10 +675,6 @@ impl Font { /// /// The elements can be either individual glyphs (by name) or kerning groups /// (by name), or any combination of the two. - // ^ note: this works without any special consideration in the code - // because glyph names are forbidden from using the group prefix, - // thus meaning the group name lookup will always fail if a group - // was passed in /// /// ⚠️ This method forgoes pre-computing a reverse glyph groups lookup for the /// use case where you're only after a couple of kerns and the overhead of @@ -692,38 +688,44 @@ impl Font { } // glyph name group name - let second_group = self - .groups - .iter() - .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) - .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) - }); + let second_group = if second.starts_with(SECOND_KERNING_GROUP_PREFIX) { + Some(second) + } else { + self.groups + .iter() + .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) + .find_map(|(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) + }) + .map(Name::as_str) + }; if let Some(second_group) = second_group { - if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { + if let Some(kern) = self.kerning_lookup_dumb(first, second_group) { return Some(kern); } } // group name glyph name - let first_group = self - .groups - .iter() - .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) - .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) - }); + let first_group = if first.starts_with(FIRST_KERNING_GROUP_PREFIX) { + Some(first) + } else { + self.groups + .iter() + .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) + .find_map(|(name, members)| { + members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) + }) + .map(Name::as_str) + }; if let Some(first_group) = first_group { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { + if let Some(kern) = self.kerning_lookup_dumb(first_group, second) { return Some(kern); } } // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = - self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) - { + if let Some(kern) = self.kerning_lookup_dumb(first_group, second_group) { return Some(kern); } } diff --git a/src/kerning.rs b/src/kerning.rs index 59ebf1cc..4b5c4b51 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -1,4 +1,6 @@ //! Helper types for working with kerning. +//! +//! To find the kerning value for a glyph/group pair, see [`Font::kerning_lookup`](crate::Font::kerning_lookup). use std::collections::{BTreeMap, HashMap}; From fdf191eea9e6974ab344250b4a579f680ddade45 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Tue, 7 Apr 2026 17:06:22 +0100 Subject: [PATCH 15/17] Refactor based on feedback Remove slow version of method Rename ReverseGroupsLookup to ReverseGroups Make ReverseGroups hold references so its lifetime depends on the Font, also preventing edits without dropping it impl Borrow for &Name --- src/font.rs | 105 +++++++++---------------------------------------- src/kerning.rs | 33 +++++++--------- src/name.rs | 6 +++ 3 files changed, 38 insertions(+), 106 deletions(-) diff --git a/src/font.rs b/src/font.rs index d0b456c9..07c1058c 100644 --- a/src/font.rs +++ b/src/font.rs @@ -13,11 +13,9 @@ use crate::datastore::{DataStore, ImageStore}; use crate::error::{FontLoadError, FontWriteError}; use crate::fontinfo::FontInfo; use crate::glyph::Glyph; -use crate::groups::{ - validate_groups, Groups, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX, -}; +use crate::groups::{validate_groups, Groups}; use crate::guideline::Guideline; -use crate::kerning::{Kerning, ReverseGroupsLookup}; +use crate::kerning::{Kerning, ReverseGroups}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; @@ -607,15 +605,15 @@ impl Font { self.font_info.guidelines.get_or_insert_with(Default::default) } - /// Builds a [`ReverseGroupsLookup`], used to determine what group a glyph + /// Builds a [`ReverseGroups`], used to determine what group a glyph /// is in. /// /// Effectively the inverse of `groups.plist`. /// /// Used by [`Font::kerning_lookup`]. #[inline] - pub fn get_reverse_groups_lookup(&self) -> ReverseGroupsLookup { - ReverseGroupsLookup::from(&self.groups) + pub fn get_reverse_groups_lookup(&'_ self) -> ReverseGroups<'_> { + ReverseGroups::from(&self.groups) } /// Retrieve the kerning value (if any) between a pair of elements. @@ -627,115 +625,48 @@ impl Font { // thus meaning the group name lookup will always fail if a group // was passed in /// - /// This method is accelerated by the [`ReverseGroupsLookup`], which can be - /// obtained from [`Font::get_reverse_groups_lookup`]. Using it is highly - /// recommended if doing numerous lookups, but if not you can use - /// [`Font::kerning_lookup_slow`], which is the same method without needing - /// the pre-computed groups lookup. + /// This method is requires a [`ReverseGroups`], which can be obtained from + /// [`Font::get_reverse_groups_lookup`]. pub fn kerning_lookup( &self, - lookup: &ReverseGroupsLookup, + resolver: &ReverseGroups, first: &str, second: &str, ) -> Option { + let kerning_lookup = |first: &str, second: &str| { + self.kerning.get(first).and_then(|first| first.get(second)).copied() + }; + // glyph name glyph name - if let Some(kern) = self.kerning_lookup_dumb(first, second) { + if let Some(kern) = kerning_lookup(first, second) { return Some(kern); } // glyph name group name - let second_group = lookup.get_second(second); + let second_group = resolver.get_second(second); if let Some(second_group) = &second_group { - if let Some(kern) = self.kerning_lookup_dumb(first, second_group.as_str()) { + if let Some(kern) = kerning_lookup(first, second_group.as_str()) { return Some(kern); } } // group name glyph name - let first_group = lookup.get_first(first); + let first_group = resolver.get_first(first); if let Some(first_group) = &first_group { - if let Some(kern) = self.kerning_lookup_dumb(first_group.as_str(), second) { + if let Some(kern) = kerning_lookup(first_group.as_str(), second) { return Some(kern); } } // group name group name if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = - self.kerning_lookup_dumb(first_group.as_str(), second_group.as_str()) - { + if let Some(kern) = kerning_lookup(first_group.as_str(), second_group.as_str()) { return Some(kern); } } None } - - /// Retrieve the kerning value (if any) between a pair of elements. - /// - /// The elements can be either individual glyphs (by name) or kerning groups - /// (by name), or any combination of the two. - /// - /// ⚠️ This method forgoes pre-computing a reverse glyph groups lookup for the - /// use case where you're only after a couple of kerns and the overhead of - /// generating the lookup is not worth it. But in cases where you're - /// querying many kerns, you should definitely be using - /// [`Font::kerning_lookup`] over this method. - pub fn kerning_lookup_slow(&self, first: &str, second: &str) -> Option { - // glyph name glyph name - if let Some(kern) = self.kerning_lookup_dumb(first, second) { - return Some(kern); - } - - // glyph name group name - let second_group = if second.starts_with(SECOND_KERNING_GROUP_PREFIX) { - Some(second) - } else { - self.groups - .iter() - .filter(|(name, _)| name.starts_with(SECOND_KERNING_GROUP_PREFIX)) - .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == second).then_some(name) - }) - .map(Name::as_str) - }; - if let Some(second_group) = second_group { - if let Some(kern) = self.kerning_lookup_dumb(first, second_group) { - return Some(kern); - } - } - - // group name glyph name - let first_group = if first.starts_with(FIRST_KERNING_GROUP_PREFIX) { - Some(first) - } else { - self.groups - .iter() - .filter(|(name, _)| name.starts_with(FIRST_KERNING_GROUP_PREFIX)) - .find_map(|(name, members)| { - members.iter().any(|glyph_name| glyph_name.as_str() == first).then_some(name) - }) - .map(Name::as_str) - }; - if let Some(first_group) = first_group { - if let Some(kern) = self.kerning_lookup_dumb(first_group, second) { - return Some(kern); - } - } - - // group name group name - if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = self.kerning_lookup_dumb(first_group, second_group) { - return Some(kern); - } - } - - None - } - - fn kerning_lookup_dumb(&self, first: &str, second: &str) -> Option { - self.kerning.get(first).and_then(|first| first.get(second)).copied() - } } fn load_lib(lib_path: &Path) -> Result { diff --git a/src/kerning.rs b/src/kerning.rs index 4b5c4b51..8a53c8b4 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -2,10 +2,10 @@ //! //! To find the kerning value for a glyph/group pair, see [`Font::kerning_lookup`](crate::Font::kerning_lookup). -use std::collections::{BTreeMap, HashMap}; - use serde::ser::{SerializeMap, Serializer}; use serde::Serialize; +use std::collections::{BTreeMap, HashMap}; +use std::iter; use crate::groups::{FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; use crate::{Groups, Name}; @@ -21,33 +21,33 @@ pub type Kerning = BTreeMap>; /// Maps glyph names to group names; the inverse of a `groups.plist` file. #[derive(Debug)] -pub struct ReverseGroupsLookup { - first: HashMap, - second: HashMap, +pub struct ReverseGroups<'font> { + first: HashMap<&'font Name, &'font Name>, + second: HashMap<&'font Name, &'font Name>, } -impl ReverseGroupsLookup { +impl ReverseGroups<'_> { /// Get the group (if any) for the glyph name when it's first in a kerning /// pair. #[inline] - pub fn get_first(&self, glyph_name: &str) -> Option { - self.first.get(glyph_name).cloned() + pub fn get_first(&self, glyph_name: &str) -> Option<&Name> { + self.first.get(glyph_name).copied() } /// Get the group (if any) for the glyph name when it's second in a /// kerning pair. #[inline] - pub fn get_second(&self, glyph_name: &str) -> Option { - self.second.get(glyph_name).cloned() + pub fn get_second(&self, glyph_name: &str) -> Option<&Name> { + self.second.get(glyph_name).copied() } } -impl From<&Groups> for ReverseGroupsLookup { - fn from(groups: &Groups) -> Self { +impl<'font> From<&'font Groups> for ReverseGroups<'font> { + fn from(groups: &'font Groups) -> Self { groups.iter().fold( - ReverseGroupsLookup { first: HashMap::new(), second: HashMap::new() }, + ReverseGroups { first: HashMap::new(), second: HashMap::new() }, |mut rgl, (group_name, members)| { - let inverted = members.iter().map(|member| (member.clone(), group_name.clone())); + let inverted = members.iter().zip(iter::repeat(group_name)); if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { rgl.first.extend(inverted); } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { @@ -186,11 +186,6 @@ mod tests { Some(expected), "kerning_lookup incorrect for /{left}/{right}" ); - assert_eq!( - font.kerning_lookup_slow(left, right), - Some(expected), - "kerning_lookup_slow incorrect for /{left}/{right}" - ); } } } diff --git a/src/name.rs b/src/name.rs index 22c7a907..319b09ec 100644 --- a/src/name.rs +++ b/src/name.rs @@ -93,6 +93,12 @@ impl std::borrow::Borrow for Name { } } +impl std::borrow::Borrow for &Name { + fn borrow(&self) -> &str { + self.0.as_ref() + } +} + impl FromStr for Name { type Err = NamingError; From 1926b2a925e9656ef0b3bdf7f493600a781ea516 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Wed, 8 Apr 2026 11:49:11 +0100 Subject: [PATCH 16/17] Refactor: KerningResolver holds kerning and does lookups Rename ReverseGroups to KerningResolver KerningResolver holds a reference to the kerning Move kerning lookup method to KerningResolver Go back to using owned Names within KerningResolver --- src/font.rs | 92 +++++++++++++++----------------------------------- src/kerning.rs | 90 ++++++++++++++++++++++++++++++++---------------- 2 files changed, 88 insertions(+), 94 deletions(-) diff --git a/src/font.rs b/src/font.rs index 07c1058c..a0847126 100644 --- a/src/font.rs +++ b/src/font.rs @@ -2,8 +2,9 @@ #![deny(rustdoc::broken_intra_doc_links)] -use std::fs; +use std::collections::HashMap; use std::path::{Path, PathBuf}; +use std::{fs, iter}; use serde::{Deserialize, Serialize}; use serde_repr::{Deserialize_repr, Serialize_repr}; @@ -13,9 +14,11 @@ use crate::datastore::{DataStore, ImageStore}; use crate::error::{FontLoadError, FontWriteError}; use crate::fontinfo::FontInfo; use crate::glyph::Glyph; -use crate::groups::{validate_groups, Groups}; +use crate::groups::{ + validate_groups, Groups, FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX, +}; use crate::guideline::Guideline; -use crate::kerning::{Kerning, ReverseGroups}; +use crate::kerning::{Kerning, KerningResolver}; use crate::layer::{Layer, LayerContents, LAYER_CONTENTS_FILE}; use crate::name::Name; use crate::names::NameList; @@ -605,67 +608,28 @@ impl Font { self.font_info.guidelines.get_or_insert_with(Default::default) } - /// Builds a [`ReverseGroups`], used to determine what group a glyph - /// is in. - /// - /// Effectively the inverse of `groups.plist`. - /// - /// Used by [`Font::kerning_lookup`]. - #[inline] - pub fn get_reverse_groups_lookup(&'_ self) -> ReverseGroups<'_> { - ReverseGroups::from(&self.groups) - } - - /// Retrieve the kerning value (if any) between a pair of elements. - /// - /// The elements can be either individual glyphs (by name) or kerning groups - /// (by name), or any combination of the two. - // ^ note: this works without any special consideration in the code - // because glyph names are forbidden from using the group prefix, - // thus meaning the group name lookup will always fail if a group - // was passed in - /// - /// This method is requires a [`ReverseGroups`], which can be obtained from - /// [`Font::get_reverse_groups_lookup`]. - pub fn kerning_lookup( - &self, - resolver: &ReverseGroups, - first: &str, - second: &str, - ) -> Option { - let kerning_lookup = |first: &str, second: &str| { - self.kerning.get(first).and_then(|first| first.get(second)).copied() - }; - - // glyph name glyph name - if let Some(kern) = kerning_lookup(first, second) { - return Some(kern); - } - - // glyph name group name - let second_group = resolver.get_second(second); - if let Some(second_group) = &second_group { - if let Some(kern) = kerning_lookup(first, second_group.as_str()) { - return Some(kern); - } - } - - // group name glyph name - let first_group = resolver.get_first(first); - if let Some(first_group) = &first_group { - if let Some(kern) = kerning_lookup(first_group.as_str(), second) { - return Some(kern); - } - } - - // group name group name - if let Some((first_group, second_group)) = first_group.zip(second_group) { - if let Some(kern) = kerning_lookup(first_group.as_str(), second_group.as_str()) { - return Some(kern); - } - } - - None + /// Builds a [`KerningResolver`], which can do full kerning lookups, + /// including group resolution. + /// + /// Note: creating a [`KerningResolver`] will prevent you from mutating + /// the font until it is dropped. + pub fn kerning_resolver(&'_ self) -> KerningResolver<'_> { + self.groups.iter().fold( + KerningResolver { + kerning: &self.kerning, + first: HashMap::new(), + second: HashMap::new(), + }, + |mut kr, (group_name, members)| { + let inverted = members.iter().cloned().zip(iter::repeat(group_name.clone())); + if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { + kr.first.extend(inverted); + } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { + kr.second.extend(inverted); + } + kr + }, + ) } } diff --git a/src/kerning.rs b/src/kerning.rs index 8a53c8b4..97fc9c2a 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -1,14 +1,14 @@ //! Helper types for working with kerning. //! -//! To find the kerning value for a glyph/group pair, see [`Font::kerning_lookup`](crate::Font::kerning_lookup). +//! To find the kerning value for a glyph/group pair, see +//! [`Font::kerning_resolver`](crate::Font::kerning_resolver) and then +//! [`KerningResolver::get`]. use serde::ser::{SerializeMap, Serializer}; use serde::Serialize; use std::collections::{BTreeMap, HashMap}; -use std::iter; -use crate::groups::{FIRST_KERNING_GROUP_PREFIX, SECOND_KERNING_GROUP_PREFIX}; -use crate::{Groups, Name}; +use crate::Name; /// A map of kerning pairs. /// @@ -19,43 +19,73 @@ use crate::{Groups, Name}; /// We use a [`BTreeMap`] because we need sorting for serialization. pub type Kerning = BTreeMap>; -/// Maps glyph names to group names; the inverse of a `groups.plist` file. +/// A utility struct to facilitate kerning lookups, including resolving group membership. +/// +/// Created by calling [`Font::kerning_resolver`](crate::Font::kerning_resolver). #[derive(Debug)] -pub struct ReverseGroups<'font> { - first: HashMap<&'font Name, &'font Name>, - second: HashMap<&'font Name, &'font Name>, +pub struct KerningResolver<'font> { + pub(crate) kerning: &'font Kerning, + pub(crate) first: HashMap, + pub(crate) second: HashMap, } -impl ReverseGroups<'_> { +impl KerningResolver<'_> { /// Get the group (if any) for the glyph name when it's first in a kerning /// pair. #[inline] - pub fn get_first(&self, glyph_name: &str) -> Option<&Name> { - self.first.get(glyph_name).copied() + pub fn get_first_group(&self, glyph_name: &str) -> Option { + self.first.get(glyph_name).cloned() } /// Get the group (if any) for the glyph name when it's second in a /// kerning pair. #[inline] - pub fn get_second(&self, glyph_name: &str) -> Option<&Name> { - self.second.get(glyph_name).copied() + pub fn get_second_group(&self, glyph_name: &str) -> Option { + self.second.get(glyph_name).cloned() } -} -impl<'font> From<&'font Groups> for ReverseGroups<'font> { - fn from(groups: &'font Groups) -> Self { - groups.iter().fold( - ReverseGroups { first: HashMap::new(), second: HashMap::new() }, - |mut rgl, (group_name, members)| { - let inverted = members.iter().zip(iter::repeat(group_name)); - if group_name.starts_with(FIRST_KERNING_GROUP_PREFIX) { - rgl.first.extend(inverted); - } else if group_name.starts_with(SECOND_KERNING_GROUP_PREFIX) { - rgl.second.extend(inverted); - } - rgl - }, - ) + /// Retrieve the kerning value (if any) between a pair of elements. + /// + /// The elements can be either individual glyphs (by name) or kerning groups + /// (by name), or any combination of the two. + // ^ note: this works without any special consideration in the code + // because glyph names are forbidden from using the group prefix, + // thus meaning the group name lookup will always fail if a group + // was passed in + pub fn get(&self, first: &str, second: &str) -> Option { + let kerning_lookup = |first: &str, second: &str| { + self.kerning.get(first).and_then(|first| first.get(second)).copied() + }; + + // glyph name glyph name + if let Some(kern) = kerning_lookup(first, second) { + return Some(kern); + } + + // glyph name group name + let second_group = self.get_second_group(second); + if let Some(second_group) = &second_group { + if let Some(kern) = kerning_lookup(first, second_group.as_str()) { + return Some(kern); + } + } + + // group name glyph name + let first_group = self.get_first_group(first); + if let Some(first_group) = &first_group { + if let Some(kern) = kerning_lookup(first_group.as_str(), second) { + return Some(kern); + } + } + + // group name group name + if let Some((first_group, second_group)) = first_group.zip(second_group) { + if let Some(kern) = kerning_lookup(first_group.as_str(), second_group.as_str()) { + return Some(kern); + } + } + + None } } @@ -172,7 +202,7 @@ mod tests { ..Default::default() }; - let lookup = font.get_reverse_groups_lookup(); + let resolver = font.kerning_resolver(); for (left, right, expected) in [ ("O", "E", -100f64), ("O", "F", -200f64), @@ -182,7 +212,7 @@ mod tests { ("Q", "F", -250f64), ] { assert_eq!( - font.kerning_lookup(&lookup, left, right), + resolver.get(left, right), Some(expected), "kerning_lookup incorrect for /{left}/{right}" ); From b038a01f2de8b50dd5ff29ae8ebe4b09f4c6a511 Mon Sep 17 00:00:00 2001 From: Ricky Atkins Date: Wed, 8 Apr 2026 11:58:59 +0100 Subject: [PATCH 17/17] Add examples to documentation --- src/font.rs | 22 ++++++++++++++++++++++ src/kerning.rs | 22 ++++++++++++++++++++++ 2 files changed, 44 insertions(+) diff --git a/src/font.rs b/src/font.rs index a0847126..0703b7fe 100644 --- a/src/font.rs +++ b/src/font.rs @@ -613,6 +613,28 @@ impl Font { /// /// Note: creating a [`KerningResolver`] will prevent you from mutating /// the font until it is dropped. + /// + /// ``` + /// # use norad::{Font, Name}; + /// use maplit::btreemap; + /// + /// let mut font = Font::new(); + /// font.groups = btreemap! { + /// Name::new("public.kern1.A").unwrap() => vec![ + /// Name::new("A").unwrap(), + /// ], + /// }; + /// font.kerning = btreemap! { + /// Name::new("public.kern1.A").unwrap() => btreemap! { + /// Name::new("V").unwrap() => -15.0, + /// }, + /// }; + /// let resolver = font.kerning_resolver(); + /// assert_eq!( + /// resolver.get("A", "V"), + /// Some(-15.0), + /// ); + /// ``` pub fn kerning_resolver(&'_ self) -> KerningResolver<'_> { self.groups.iter().fold( KerningResolver { diff --git a/src/kerning.rs b/src/kerning.rs index 97fc9c2a..dd28aab7 100644 --- a/src/kerning.rs +++ b/src/kerning.rs @@ -22,6 +22,28 @@ pub type Kerning = BTreeMap>; /// A utility struct to facilitate kerning lookups, including resolving group membership. /// /// Created by calling [`Font::kerning_resolver`](crate::Font::kerning_resolver). +/// +/// ``` +/// # use norad::{Font, Name}; +/// use maplit::btreemap; +/// +/// let mut font = Font::new(); +/// font.groups = btreemap! { +/// Name::new("public.kern1.A").unwrap() => vec![ +/// Name::new("A").unwrap(), +/// ], +/// }; +/// font.kerning = btreemap! { +/// Name::new("public.kern1.A").unwrap() => btreemap! { +/// Name::new("V").unwrap() => -15.0, +/// }, +/// }; +/// let resolver = font.kerning_resolver(); +/// assert_eq!( +/// resolver.get("A", "V"), +/// Some(-15.0), +/// ); +/// ``` #[derive(Debug)] pub struct KerningResolver<'font> { pub(crate) kerning: &'font Kerning,