Merge pull request #731 from googlefonts/split-kern-by-script

Split kerning by script

Cargo.toml

@@ -35,7 +35,6 @@ temp-env = "0.3.3"
 rstest = "0.18.2"
 
 [workspace]
-
 resolver = "2"
 
 members = [
@@ -52,3 +51,8 @@ members = [
     "layout-normalizer",
 ]
 
+# remove this when icu_properties 1.5 is released:
+# https://github.com/unicode-org/icu4x/issues?q=is%3Aopen+is%3Aissue+milestone%3A%221.5+Blocking+⟨P1⟩%22
+[patch.crates-io]
+icu_properties = { version = "1.4", git = "https://github.com/unicode-org/icu4x.git", rev = "728eb44" }
+tinystr = { version = "0.7.5", git = "https://github.com/unicode-org/icu4x.git", rev = "728eb44" }

fea-rs/src/common/glyph_class.rs

@@ -65,9 +65,15 @@ impl GlyphSet {
         self.0.iter().copied()
     }
 
-    pub(crate) fn len(&self) -> usize {
+    /// The number of glyphs in the set
+    pub fn len(&self) -> usize {
         self.0.len()
     }
+
+    /// Returns `true` if the set contains no glyphs
+    pub fn is_empty(&self) -> bool {
+        self.0.is_empty()
+    }
 }
 
 impl std::iter::FromIterator<GlyphId> for GlyphClass {

fea-rs/src/compile/lookups.rs

@@ -8,6 +8,7 @@ mod helpers;
 use std::{
     collections::{BTreeMap, HashMap},
     convert::TryInto,
+    fmt::Debug,
 };
 
 use smol_str::SmolStr;
@@ -182,7 +183,8 @@ pub(crate) struct LookupFlagInfo {
 }
 
 /// A feature associated with a particular script and language.
-#[derive(Clone, Copy, Debug, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[derive(Clone, Copy, PartialEq, Eq, Hash, PartialOrd, Ord)]
+#[cfg_attr(feature = "serde", derive(serde::Serialize, serde::Deserialize))]
 pub struct FeatureKey {
     pub(crate) feature: Tag,
     pub(crate) language: Tag,
@@ -1171,6 +1173,12 @@ impl FeatureKey {
     }
 }
 
+impl Debug for FeatureKey {
+    fn fmt(&self, f: &mut std::fmt::Formatter<'_>) -> std::fmt::Result {
+        write!(f, "{}: {}/{}", self.feature, self.script, self.language)
+    }
+}
+
 fn is_gpos_rule(kind: Kind) -> bool {
     matches!(
         kind,

fea-rs/src/compile/lookups/gpos_builders.rs

@@ -196,6 +196,17 @@ impl PairPosBuilder {
         self.pairs.0.is_empty() && self.classes.0.is_empty()
     }
 
+    /// The number of rules in the builder
+    pub fn len(&self) -> usize {
+        self.pairs.0.values().map(|vals| vals.len()).sum::<usize>()
+            + self
+                .classes
+                .0
+                .values()
+                .flat_map(|x| x.iter().map(|y| y.items.values().len()))
+                .sum::<usize>()
+    }
+
     /// Insert a new kerning pair
     pub fn insert_pair(
         &mut self,

fea-rs/src/compile/metrics.rs

@@ -61,6 +61,16 @@ impl ValueRecord {
         Default::default()
     }
 
+    /// Duplicates the x-advance value to x-placement, required for RTL rules.
+    ///
+    /// This is only necessary when a record was originally created without
+    /// knowledge of the writing direction, and then later needs to be modified.
+    pub fn make_rtl_compatible(&mut self) {
+        if self.x_placement.is_none() {
+            self.x_placement = self.x_advance.clone();
+        }
+    }
+
     // these methods just match the existing builder methods on `ValueRecord`
     /// Builder style method to set the default x_placement value
     pub fn with_x_placement(mut self, val: i16) -> Self {

fea-rs/src/token_tree/typed.rs

@@ -420,17 +420,20 @@ impl ContextualRuleNode for Gsub8 {}
 impl ContextualRuleNode for IgnoreRule {}
 
 impl Root {
-    pub(crate) fn statements(&self) -> impl Iterator<Item = &NodeOrToken> {
+    /// Iterate over all top-level statements
+    pub fn statements(&self) -> impl Iterator<Item = &NodeOrToken> {
         self.iter().filter(|t| !t.kind().is_trivia())
     }
 }
 
 impl LanguageSystem {
-    pub(crate) fn script(&self) -> Tag {
+    /// The script tag
+    pub fn script(&self) -> Tag {
         self.inner.iter_children().find_map(Tag::cast).unwrap()
     }
 
-    pub(crate) fn language(&self) -> Tag {
+    /// The language tag
+    pub fn language(&self) -> Tag {
         self.inner
             .iter_children()
             .skip_while(|t| t.kind() != Kind::Tag)
@@ -451,7 +454,8 @@ impl Tag {
         self.inner.text.parse()
     }
 
-    pub(crate) fn to_raw(&self) -> write_fonts::types::Tag {
+    /// Convert this AST tag into a raw `Tag`
+    pub fn to_raw(&self) -> write_fonts::types::Tag {
         self.parse().expect("tag is exactly 4 bytes")
     }
 }

fontbe/Cargo.toml

@@ -14,6 +14,8 @@ categories = ["text-processing", "parsing", "graphics"]
 fontdrasil = { version = "0.0.1", path = "../fontdrasil" }
 fontir = { version = "0.0.1", path = "../fontir" }
 fea-rs = { version = "0.18.0", path = "../fea-rs", features = ["serde"] }
+icu_properties = "1.4"
+tinystr = {version = "0.7.5", features = ["serde"] }
 
 serde.workspace = true
 bincode.workspace = true

fontbe/src/features.rs

@@ -41,6 +41,9 @@ use crate::{
 
 mod kern;
 mod marks;
+mod ot_tags;
+mod properties;
+
 pub use kern::{create_gather_ir_kerning_work, create_kern_segment_work, create_kerns_work};
 pub use marks::create_mark_work;
 
@@ -211,12 +214,19 @@ impl<'a> FeatureWriter<'a> {
         if self.kerning.is_empty() {
             return Ok(());
         }
-        let pairpos_subtables = self.kerning.lookups.clone();
+        // convert the lookups into lookup ids
+        let lookup_ids = self
+            .kerning
+            .lookups
+            .iter()
+            .map(|lookups| builder.add_lookup(LookupFlag::empty(), None, lookups.to_owned()))
+            .collect::<Vec<_>>();
 
-        // now we have a builder for the pairpos subtables, so we can make
-        // a lookup:
-        let lookups = vec![builder.add_lookup(LookupFlag::empty(), None, pairpos_subtables)];
-        builder.add_to_default_language_systems(Tag::new(b"kern"), &lookups);
+        for (feature, ids) in &self.kerning.features {
+            // get the generated lookup ids based on the stored lookup indices
+            let ids = ids.iter().map(|idx| lookup_ids[*idx]).collect();
+            builder.add_feature(*feature, ids);
+        }
 
         {
             self.timing