Skip to content

Commit

Permalink
[kerning] Split kern rules by script & directionality
Browse files Browse the repository at this point in the history
This is very closely based on the code in the KernFeatureWriter, in
ufo2ft. With this patch we very nearly match fontmake's output for
kerning in oswald, with a few lingering differences.

It feels like there is room for polish here, but I also think it's worth
checkpointing here, for further iteration.
  • Loading branch information
cmyr committed Mar 12, 2024
1 parent 0eaaffb commit 02aaf77
Show file tree
Hide file tree
Showing 12 changed files with 1,088 additions and 59 deletions.
4 changes: 3 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -35,7 +35,6 @@ temp-env = "0.3.3"
rstest = "0.18.2"

[workspace]

resolver = "2"

members = [
Expand All @@ -52,3 +51,6 @@ members = [
"layout-normalizer",
]

[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" }
8 changes: 7 additions & 1 deletion fea-rs/src/common/glyph_class.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
10 changes: 9 additions & 1 deletion fea-rs/src/compile/lookups.rs
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,7 @@ mod helpers;
use std::{
collections::{BTreeMap, HashMap},
convert::TryInto,
fmt::Debug,
};

use smol_str::SmolStr;
Expand Down Expand Up @@ -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,
Expand Down Expand Up @@ -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,
Expand Down
11 changes: 11 additions & 0 deletions fea-rs/src/compile/lookups/gpos_builders.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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,
Expand Down
10 changes: 10 additions & 0 deletions fea-rs/src/compile/metrics.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down
12 changes: 8 additions & 4 deletions fea-rs/src/token_tree/typed.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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")
}
}
Expand Down
6 changes: 2 additions & 4 deletions fontbe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +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"
icu_properties = { version = "1.4", path = "../../extern/icu4x/components/properties" }
#tinystr = "0.7.5"
tinystr = {version = "0.7.5", path = "../../extern/icu4x/utils/tinystr" }
icu_properties = "1.4"
tinystr = {version = "0.7.5", features = ["serde"] }

serde.workspace = true
bincode.workspace = true
Expand Down
17 changes: 12 additions & 5 deletions fontbe/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -213,12 +213,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
Expand Down
Loading

0 comments on commit 02aaf77

Please sign in to comment.