From 6d699b484fd844ca3f33d6d33a23d8b17decea78 Mon Sep 17 00:00:00 2001
From: Harrison Booth <73434116+Harrison-Booth@users.noreply.github.com>
Date: Wed, 28 Oct 2026 13:37:24 -0500
Subject: [PATCH] M1.2: bound collection face enumeration

---
 crates/fonts/src/fontdb.rs | 59 +++++++++++++++++++++++++++++++-------
 1 file changed, 48 insertions(+), 11 deletions(-)

diff --git a/crates/fonts/src/fontdb.rs b/crates/fonts/src/fontdb.rs
index 373f4c32..1b04e0f3 100644
--- a/crates/fonts/src/fontdb.rs
+++ b/crates/fonts/src/fontdb.rs
@@ -606,26 +606,51 @@ type Found = (u32, String, String, Vec<([u8; 4], f32)>);
 
 /// Maximum number of faces we enumerate from a single font collection.
 const MAX_COLLECTION_FACES: u32 = 256;
+/// Maximum collection entries inspected while looking for distinct table directories.
+const MAX_COLLECTION_ENTRIES_SCANNED: u32 = 16_384;
+/// Maximum named instances read from a single face.
+const MAX_NAMED_INSTANCES_PER_FACE: usize = 1024;
 
-fn capped_face_count(count: u32) -> u32 {
-    count.min(MAX_COLLECTION_FACES)
+fn distinct_face_indices(entries: impl IntoIterator<Item = (u32, usize)>) -> Vec<u32> {
+    let mut directories = std::collections::HashSet::new();
+    let mut indices = Vec::with_capacity(MAX_COLLECTION_FACES as usize);
+    for (index, directory) in entries.into_iter().take(MAX_COLLECTION_ENTRIES_SCANNED as usize) {
+        if directories.insert(directory) {
+            indices.push(index);
+            if indices.len() == MAX_COLLECTION_FACES as usize {
+                break;
+            }
+        }
+    }
+    indices
+}
+
+fn capped_named_instance_count(count: usize) -> usize {
+    count.min(MAX_NAMED_INSTANCES_PER_FACE)
 }
 
 /// Parse every face in `data` (a font file or collection); a variable font yields one face per
 /// named instance (InDesign lists them as styles).
 fn enumerate_faces(data: &[u8]) -> Vec<Found> {
-    let count = match FileRef::new(data) {
-        Ok(FileRef::Font(_)) => 1,
-        Ok(FileRef::Collection(c)) => capped_face_count(c.len()),
-        Err(_) => 0,
+    let indices = match FileRef::new(data) {
+        Ok(FileRef::Font(_)) => vec![0],
+        Ok(FileRef::Collection(c)) => {
+            let entries = (0..c.len().min(MAX_COLLECTION_ENTRIES_SCANNED)).filter_map(|i| {
+                let f = skrifa::FontRef::from_index(data, i).ok()?;
+                Some((i, f.table_directory().table_records().as_ptr() as usize))
+            });
+            distinct_face_indices(entries)
+        }
+        Err(_) => Vec::new(),
     };
     let mut out = Vec::new();
-    for i in 0..count {
+    for i in indices {
         let Ok(f) = skrifa::FontRef::from_index(data, i) else { continue };
         let Some((family, style)) = face_names(&f) else { continue };
         let axes = f.axes();
         let mut named = Vec::new();
-        for ni in f.named_instances().iter() {
+        let instances = f.named_instances();
+        for ni in instances.iter().take(capped_named_instance_count(instances.len())) {
             let Some(style) = name(&f, &[ni.subfamily_name_id()]) else { continue };
             if named.iter().any(|(_, s, _): &(u32, String, _)| s.eq_ignore_ascii_case(&style)) {
                 continue;
@@ -1443,9 +1468,21 @@ mod font_dir_tests {
     use super::*;
 
     #[test]
-    fn font_collection_face_count_is_capped() {
-        assert_eq!(capped_face_count(MAX_COLLECTION_FACES), MAX_COLLECTION_FACES);
-        assert_eq!(capped_face_count(MAX_COLLECTION_FACES + 1), MAX_COLLECTION_FACES);
+    fn duplicate_collection_offsets_do_not_consume_face_cap() {
+        let mut entries = vec![(0, 7); MAX_COLLECTION_FACES as usize];
+        entries.extend((0..MAX_COLLECTION_FACES).map(|i| (i + MAX_COLLECTION_FACES, i as usize + 8)));
+        let faces = distinct_face_indices(entries);
+        assert_eq!(faces.len(), MAX_COLLECTION_FACES as usize);
+        assert_eq!(faces.first(), Some(&0));
+        assert_eq!(faces.get(1), Some(&MAX_COLLECTION_FACES));
+        assert_eq!(faces.last(), Some(&(MAX_COLLECTION_FACES * 2 - 2)));
+    }
+
+    #[test]
+    fn named_instance_limit_is_bounded_per_face() {
+        assert_eq!(capped_named_instance_count(0), 0);
+        assert_eq!(capped_named_instance_count(MAX_NAMED_INSTANCES_PER_FACE), MAX_NAMED_INSTANCES_PER_FACE);
+        assert_eq!(capped_named_instance_count(MAX_NAMED_INSTANCES_PER_FACE + 1), MAX_NAMED_INSTANCES_PER_FACE);
     }
 
     #[test]