From 075129ae399afe3d15dfeb7c90a85b06976b5699 Mon Sep 17 00:00:00 2001
From: Chad Brokaw <cbrokaw@gmail.com>
Date: Sat, 13 Jan 2024 13:12:31 -0500
Subject: [PATCH 1/2] [read-fonts] fix ClassDefFormat2::get()

The conditional logic for selecting the appropriate class record was not quite right leading to incorrect class values being returned. This implements the lookup using a binary search and adds a test for correct behavior.
---
 read-fonts/src/tables/layout.rs | 46 ++++++++++++++++++++++++++++-----
 1 file changed, 39 insertions(+), 7 deletions(-)

diff --git a/read-fonts/src/tables/layout.rs b/read-fonts/src/tables/layout.rs
index 437247818..daaf4b91e 100644
--- a/read-fonts/src/tables/layout.rs
+++ b/read-fonts/src/tables/layout.rs
@@ -196,13 +196,17 @@ impl<'a> ClassDefFormat1<'a> {
 impl<'a> ClassDefFormat2<'a> {
     /// Get the class for this glyph id
     pub fn get(&self, gid: GlyphId) -> u16 {
-        self.class_range_records()
-            .iter()
-            .find_map(|record| {
-                (record.start_glyph_id() >= gid && record.end_glyph_id() <= gid)
-                    .then_some(record.class())
-            })
-            .unwrap_or(0)
+        let records = self.class_range_records();
+        let ix = match records.binary_search_by(|rec| rec.start_glyph_id().cmp(&gid)) {
+            Ok(ix) => ix,
+            Err(ix) => ix.saturating_sub(1),
+        };
+        if let Some(record) = records.get(ix) {
+            if gid >= record.start_glyph_id() && gid <= record.end_glyph_id() {
+                return record.class();
+            }
+        }
+        0
     }
 
     /// Iterate over each glyph and its class.
@@ -315,6 +319,34 @@ mod tests {
         assert_eq!(coverage.get(GlyphId::new(40)), None);
     }
 
+    #[test]
+    fn classdef_get_format2() {
+        let classdef = ClassDef::read(FontData::new(
+            font_test_data::gdef::MARKATTACHCLASSDEF_TABLE,
+        ))
+        .unwrap();
+        assert!(matches!(classdef, ClassDef::Format2(..)));
+        let gid_class_pairs = [
+            (616, 1),
+            (617, 1),
+            (618, 1),
+            (624, 1),
+            (625, 1),
+            (626, 1),
+            (652, 2),
+            (653, 2),
+            (654, 2),
+            (655, 2),
+            (661, 2),
+        ];
+        for (gid, class) in gid_class_pairs {
+            assert_eq!(classdef.get(GlyphId::new(gid)), class);
+        }
+        for (gid, class) in classdef.iter() {
+            assert_eq!(classdef.get(gid), class);
+        }
+    }
+
     #[test]
     fn delta_decode() {
         // these examples come from the spec

From 32e1f8864f2c0427c38e66153666b818009ac069 Mon Sep 17 00:00:00 2001
From: Chad Brokaw <cbrokaw@gmail.com>
Date: Mon, 15 Jan 2024 07:57:43 -0500
Subject: [PATCH 2/2] use `Range::contains`

---
 read-fonts/src/tables/layout.rs | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/read-fonts/src/tables/layout.rs b/read-fonts/src/tables/layout.rs
index daaf4b91e..671274446 100644
--- a/read-fonts/src/tables/layout.rs
+++ b/read-fonts/src/tables/layout.rs
@@ -202,7 +202,7 @@ impl<'a> ClassDefFormat2<'a> {
             Err(ix) => ix.saturating_sub(1),
         };
         if let Some(record) = records.get(ix) {
-            if gid >= record.start_glyph_id() && gid <= record.end_glyph_id() {
+            if (record.start_glyph_id()..=record.end_glyph_id()).contains(&gid) {
                 return record.class();
             }
         }