Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Split kerning by script #731

Merged
merged 6 commits into from
Mar 14, 2024
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 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,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" }
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they don't publish on crates.io?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

they do, but I had to fix a few things to get this to work, and those things aren't published yet. See unicode-org/icu4x#4681

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

https://docs.rs/icu_properties/latest/icu_properties/, maybe there's an unreleased feature we need? If so we should get them to publish.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

see my comment above: I had to PR a few things to icu_properties. They have a release planned, but it's a ways away.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Let's add a comment pointing at unicode-org/icu4x#4681 and noting that we should be able to drop this as of their 1.5 release.

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
2 changes: 2 additions & 0 deletions fontbe/Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
20 changes: 15 additions & 5 deletions fontbe/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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
Expand Down
Loading
Loading