Skip to content

Commit

Permalink
Merge pull request #789 from googlefonts/marks-marks-marks
Browse files Browse the repository at this point in the history 
More marks progress
  • Loading branch information
@cmyr
cmyr authored May 9, 2024
2 parents 09306f6 + ab6f2f9 commit 1322b0d
Show file tree
Hide file tree
Showing 4 changed files with 149 additions and 37 deletions.
3 changes: 3 additions & 0 deletions fontbe/src/features.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -51,6 +51,9 @@ mod properties;
pub use kern::{create_gather_ir_kerning_work, create_kern_segment_work, create_kerns_work};
pub use marks::create_mark_work;

const DFLT_SCRIPT: Tag = Tag::new(b"DFLT");
const DFLT_LANG: Tag = Tag::new(b"dflt");

#[derive(Debug)]
pub struct FeatureParsingWork {}

Expand Down
4 changes: 1 addition & 3 deletions fontbe/src/features/kern.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,7 @@ use crate::{
},
};

use super::{properties::CharMap, PendingLookup};
use super::{properties::CharMap, PendingLookup, DFLT_LANG, DFLT_SCRIPT};

/// On Linux it took ~0.01 ms per loop, try to get enough to make fan out worthwhile
/// based on empirical testing
Expand All @@ -53,8 +53,6 @@ const KERN: Tag = Tag::new(b"kern");
// we don't currently compile this feature, but we will, and it is referenced
// in places because our impl is based on fonttools.
const DIST: Tag = Tag::new(b"dist");
const DFLT_SCRIPT: Tag = Tag::new(b"DFLT");
const DFLT_LANG: Tag = Tag::new(b"dflt");

/// Accumulation of all the kerning from IR
#[derive(Debug)]
Expand Down
175 changes: 141 additions & 34 deletions fontbe/src/features/marks.rs
View file
Original file line number Diff line number Diff line change
@@ -1,8 +1,12 @@
//! Generates a [FeaRsMarks] datastructure to be fed to fea-rs

use std::collections::{BTreeMap, BTreeSet, HashMap, HashSet};
use std::collections::{BTreeMap, BTreeSet, HashSet};

use fea_rs::compile::{MarkToBaseBuilder, MarkToMarkBuilder};
use fea_rs::{
compile::{MarkToBaseBuilder, MarkToMarkBuilder, NopFeatureProvider, NopVariationInfo},
typed::{AstNode, LanguageSystem},
Opts, ParseTree,
};
use fontdrasil::{
orchestration::{Access, AccessBuilder, Work},
types::GlyphName,
Expand All @@ -17,13 +21,15 @@ use write_fonts::{

use crate::{
error::Error,
orchestration::{AnyWorkId, BeWork, Context, FeaRsMarks, WorkId},
orchestration::{AnyWorkId, BeWork, Context, FeaAst, FeaRsMarks, WorkId},
};
use fontir::{
ir::{self, AnchorKind, GlyphAnchors, GlyphOrder, StaticMetadata},
ir::{self, Anchor, AnchorKind, GlyphAnchors, GlyphOrder, StaticMetadata},
orchestration::WorkId as FeWorkId,
};

use super::DFLT_SCRIPT;

#[derive(Debug)]
struct MarkWork {}

Expand All @@ -34,18 +40,20 @@ pub fn create_mark_work() -> Box<BeWork> {
/// The canonical name shared for a given mark/base pair, e.g. `top` for `top`/`_top`
type MarkGroupName = SmolStr;

#[allow(dead_code)] // a few currently unused fields
struct MarkLookupBuilder<'a> {
gdef_classes: Option<HashMap<GlyphId, GlyphClassDef>>,
// extracted from public.openTypeCatgories/GlyphData.xml or FEA
gdef_classes: BTreeMap<GlyphName, GlyphClassDef>,
// pruned, only the anchors we are using
anchors: BTreeMap<GlyphName, Vec<&'a ir::Anchor>>,
anchor_lists: BTreeMap<GlyphName, Vec<&'a ir::Anchor>>,
glyph_order: &'a GlyphOrder,
static_metadata: &'a StaticMetadata,
fea_scripts: HashSet<Tag>,
// we don't currently use this, because just adding lookups to all scripts works?
_fea_scripts: HashSet<Tag>,
mark_glyphs: BTreeSet<GlyphName>,
}

/// The bases and marks in a particular group, e.g. "top" or "bottom"
#[derive(Default, Clone, PartialEq)]
#[derive(Default, Debug, Clone, PartialEq)]
struct MarkGroup<'a> {
bases: Vec<(GlyphName, &'a ir::Anchor)>,
marks: Vec<(GlyphName, &'a ir::Anchor)>,
Expand Down Expand Up @@ -83,27 +91,45 @@ impl<'a> MarkLookupBuilder<'a> {
fn new(
anchors: Vec<&'a GlyphAnchors>,
glyph_order: &'a GlyphOrder,
gdef_classes: Option<HashMap<GlyphId, GlyphClassDef>>,
gdef_classes: BTreeMap<GlyphName, GlyphClassDef>,
static_metadata: &'a StaticMetadata,
fea_scripts: HashSet<Tag>,
) -> Self {
// first we want to narrow our input down to only anchors that are participating.
// in pythonland this is https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66a/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L380
let mut pruned = BTreeMap::new();
let mut base_names = HashSet::new();
let mut mark_names = HashSet::new();
let mut base_groups = HashSet::new();
let mut mark_groups = HashSet::new();
let include = gdef_classes
.iter()
.filter_map(|(name, cls)| {
matches!(
*cls,
GlyphClassDef::Base | GlyphClassDef::Mark | GlyphClassDef::Ligature
)
.then_some(name)
})
.collect::<HashSet<_>>();
for anchors in anchors {
// skip anchors for non-export glyphs
if !glyph_order.contains(&anchors.glyph_name) {
continue;
}
// skip glyphs that are not mark/lig/base, if we have any defined categories
if !include.is_empty() && !include.contains(&anchors.glyph_name) {
continue;
}
for anchor in &anchors.anchors {
match &anchor.kind {
AnchorKind::Base(group) => {
base_names.insert(group);
AnchorKind::Base(group)
| AnchorKind::Ligature {
group_name: group, ..
} => {
base_groups.insert(group);
}
AnchorKind::Mark(group) => {
mark_names.insert(group);
mark_groups.insert(group);
}
//TODO: ligatures
// skip non base/mark anchors
_ => continue,
}
Expand All @@ -114,7 +140,9 @@ impl<'a> MarkLookupBuilder<'a> {
}
}

let used_groups = base_names.intersection(&mark_names).collect::<HashSet<_>>();
let used_groups = base_groups
.intersection(&mark_groups)
.collect::<HashSet<_>>();
// we don't care about marks with no corresponding bases, & vice-versa see:
// <https://github.com/googlefonts/ufo2ft/blob/6787e37e63530/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L359>
pruned.retain(|_, anchors| {
Expand All @@ -126,12 +154,15 @@ impl<'a> MarkLookupBuilder<'a> {
});
!anchors.is_empty()
});

let mark_glyphs = find_mark_glyphs(&pruned, &gdef_classes);
Self {
gdef_classes,
anchors: pruned,
anchor_lists: pruned,
glyph_order,
fea_scripts: Default::default(),
_fea_scripts: fea_scripts,
static_metadata,
gdef_classes,
mark_glyphs,
}
}

Expand All @@ -154,12 +185,8 @@ impl<'a> MarkLookupBuilder<'a> {
) -> Result<Vec<T>, Error> {
groups
.into_iter()
.filter(|(_, group)| !(group.bases.is_empty() || group.marks.is_empty()))
.map(|(group_name, group)| {
assert!(
!group.bases.is_empty() && !group.marks.is_empty(),
"prechecked"
);

let mut builder = T::default();
for (mark_name, anchor) in group.marks {
// we already filtered to only things in glyph order
Expand All @@ -181,15 +208,21 @@ impl<'a> MarkLookupBuilder<'a> {

fn make_mark_to_base_groups(&self) -> BTreeMap<MarkGroupName, MarkGroup<'a>> {
let mut groups = BTreeMap::<_, MarkGroup>::new();
for (glyph_name, anchors) in &self.anchors {
for (glyph_name, anchors) in &self.anchor_lists {
let is_mark = self.mark_glyphs.contains(glyph_name);
// if we have explicit gdef classes and this is not an expilcit base
let is_not_base = !self.gdef_classes.is_empty()
&& (self.gdef_classes.get(glyph_name)) != Some(&GlyphClassDef::Base);

let treat_as_base = !(is_mark | is_not_base);
for anchor in anchors {
match &anchor.kind {
fontir::ir::AnchorKind::Base(group) => groups
fontir::ir::AnchorKind::Base(group) if treat_as_base => groups
.entry(group.clone())
.or_default()
.bases
.push((glyph_name.clone(), anchor)),
fontir::ir::AnchorKind::Mark(group) => groups
fontir::ir::AnchorKind::Mark(group) if is_mark => groups
.entry(group.clone())
.or_default()
.marks
Expand All @@ -204,12 +237,13 @@ impl<'a> MarkLookupBuilder<'a> {
fn make_mark_to_mark_groups(&self) -> BTreeMap<MarkGroupName, MarkGroup<'a>> {
// first find the set of glyphs that are marks, i.e. have any mark attachment point.
let (mark_glyphs, mark_anchors): (BTreeSet<_>, BTreeSet<_>) = self
.anchors
.anchor_lists
.iter()
.filter(|(name, _)| self.mark_glyphs.contains(*name))
.flat_map(|(name, anchors)| {
anchors.iter().filter_map(|anchor| {
anchors.iter().filter_map(move |anchor| {
if let AnchorKind::Mark(group) = &anchor.kind {
Some((name.clone(), group.clone()))
Some((name, group))
} else {
None
}
Expand All @@ -220,7 +254,7 @@ impl<'a> MarkLookupBuilder<'a> {
// then iterate again, looking for glyphs that we have identified as marks,
// but which also have participating base anchors
let mut result = BTreeMap::<MarkGroupName, MarkGroup>::new();
for (glyph, glyph_anchors) in &self.anchors {
for (glyph, glyph_anchors) in &self.anchor_lists {
if !mark_glyphs.contains(glyph) {
continue;
}
Expand All @@ -241,7 +275,7 @@ impl<'a> MarkLookupBuilder<'a> {

// then add the anchors for the mark glyphs, if they exist
for mark in mark_glyphs {
let anchors = self.anchors.get(&mark).unwrap();
let anchors = self.anchor_lists.get(mark).unwrap();
for anchor in anchors {
if let AnchorKind::Mark(group) = &anchor.kind {
// only add mark glyph if there is at least one base
Expand All @@ -264,6 +298,7 @@ impl Work<Context, AnyWorkId, Error> for MarkWork {
AccessBuilder::new()
.variant(FeWorkId::StaticMetadata)
.variant(FeWorkId::GlyphOrder)
.variant(WorkId::FeaturesAst)
.variant(FeWorkId::ALL_ANCHORS)
.build()
}
Expand All @@ -273,13 +308,24 @@ impl Work<Context, AnyWorkId, Error> for MarkWork {
let static_metadata = context.ir.static_metadata.get();
let glyph_order = context.ir.glyph_order.get();
let raw_anchors = context.ir.anchors.all();
let ast = context.fea_ast.get();
let gdef_classes = get_gdef_classes(&static_metadata, &ast, &glyph_order);

let anchors = raw_anchors
.iter()
.map(|(_, anchors)| anchors.as_ref())
.collect::<Vec<_>>();

let ctx = MarkLookupBuilder::new(anchors, &glyph_order, None, &static_metadata);
let fea_scripts = get_fea_scripts(&ast.ast);
// this code is roughly equivalent to what in pythonland happens in
// setContext: https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66a/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L322
let ctx = MarkLookupBuilder::new(
anchors,
&glyph_order,
gdef_classes,
&static_metadata,
fea_scripts,
);
let all_marks = ctx.build()?;

context.fea_rs_marks.set(all_marks);
Expand All @@ -288,6 +334,67 @@ impl Work<Context, AnyWorkId, Error> for MarkWork {
}
}

// in py this is set during _groupMarkGlyphsByAnchor; we try to match that logic
// https://github.com/googlefonts/ufo2ft/blob/8e9e6eb66/Lib/ufo2ft/featureWriters/markFeatureWriter.py#L412
fn find_mark_glyphs(
anchors: &BTreeMap<GlyphName, Vec<&Anchor>>,
gdef_classes: &BTreeMap<GlyphName, GlyphClassDef>,
) -> BTreeSet<GlyphName> {
anchors
.iter()
.filter(|(name, anchors)| {
(!gdef_classes.is_empty() && gdef_classes.get(*name) == Some(&GlyphClassDef::Mark))
&& anchors.iter().any(|a| a.is_mark())
})
.map(|(name, _)| name.to_owned())
.collect()
}

fn get_fea_scripts(ast: &ParseTree) -> HashSet<Tag> {
ast.typed_root()
.statements()
.filter_map(LanguageSystem::cast)
.map(|langsys| langsys.script().to_raw())
.filter(|tag| *tag != DFLT_SCRIPT)
.collect()
}

fn get_gdef_classes(
meta: &StaticMetadata,
ast: &FeaAst,
glyph_order: &GlyphOrder,
) -> BTreeMap<GlyphName, GlyphClassDef> {
let glyph_map = glyph_order.iter().cloned().collect();
// if we prefer classes defined in fea, compile the fea and see if we have any
if meta.gdef_categories.prefer_gdef_categories_in_fea {
if let Some(gdef_classes) =
fea_rs::compile::compile::<NopVariationInfo, NopFeatureProvider>(
&ast.ast,
&glyph_map,
None,
None,
Opts::new().compile_gpos(false),
)
.ok()
.and_then(|mut comp| comp.0.gdef_classes.take())
{
return gdef_classes
.into_iter()
.filter_map(|(g, cls)| {
glyph_order
.glyph_name(g.to_u16() as _)
.cloned()
.map(|name| (name, cls))
})
.collect();
}
}
// otherwise (we don't care about FEA or nothing is defined) we use the
// classes in StaticMetadata (which are from public.openTypeCategories or
// GlyphData.xml)
meta.gdef_categories.categories.clone()
}

fn resolve_anchor(
anchor: &fontir::ir::Anchor,
static_metadata: &StaticMetadata,
Expand Down
4 changes: 4 additions & 0 deletions fontir/src/ir.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1112,6 +1112,10 @@ impl Anchor {
_ => None,
}
}

pub fn is_mark(&self) -> bool {
matches!(self.kind, AnchorKind::Mark(_))
}
}

/// A type for building glyph anchors, reused by different backends
Expand Down

0 comments on commit 1322b0d

Please sign in to comment.