Skip to content

Commit

Permalink
[hvar] prune point axes from glyph locations lest they trigger sub-model
Browse files Browse the repository at this point in the history
Fixes #1256
  • Loading branch information
anthrotype committed Feb 13, 2025
1 parent a0d4725 commit 7716fb6
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 8 deletions.
25 changes: 18 additions & 7 deletions fontbe/src/hvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@ use write_fonts::{
hvar::Hvar,
variations::{ivs_builder::VariationStoreBuilder, DeltaSetIndexMap, VariationRegion},
},
types::Tag,
validate::Validate,
FontWrite, OtRound,
};
Expand Down Expand Up @@ -50,6 +51,8 @@ where
struct AdvanceWidthDeltas {
/// Variation axes
axes: Vec<Axis>,
/// Set of axis tags
axis_tags: BTreeSet<Tag>,
/// Sparse variation models, keyed by the set of locations they define
models: HashMap<BTreeSet<NormalizedLocation>, VariationModel>,
/// Glyph's advance width deltas sorted by glyph order
Expand All @@ -59,13 +62,25 @@ struct AdvanceWidthDeltas {
}

impl AdvanceWidthDeltas {
fn new(global_model: VariationModel, glyph_locations: HashSet<NormalizedLocation>) -> Self {
fn new<'a>(
global_model: VariationModel,
glyph_locations: impl IntoIterator<Item = &'a NormalizedLocation>,
) -> Self {
let axes = global_model.axes().cloned().collect::<Vec<_>>();
let axis_tags: BTreeSet<_> = axes.iter().map(|axis| axis.tag).collect();
// prune axes that are not in the global model (e.g. 'point' axes) which might
// be confused for a distinct sub-model
// https://github.com/googlefonts/fontc/issues/1256
let glyph_locations = glyph_locations
.into_iter()
.map(|loc| loc.subset_axes(&axis_tags))
.collect();
let global_locations = global_model.locations().cloned().collect::<BTreeSet<_>>();
let mut models = HashMap::new();
models.insert(global_locations, global_model);
AdvanceWidthDeltas {
axes,
axis_tags,
models,
deltas: Vec::new(),
glyph_locations,
Expand All @@ -78,7 +93,7 @@ impl AdvanceWidthDeltas {
.iter()
// widths must be rounded before the computing deltas to match fontmake
// https://github.com/googlefonts/fontc/issues/1043
.map(|(loc, src)| (loc.clone(), vec![src.width.ot_round()]))
.map(|(loc, src)| (loc.subset_axes(&self.axis_tags), vec![src.width.ot_round()]))
.collect();
let name = glyph.name.clone();
let i = self.deltas.len();
Expand Down Expand Up @@ -169,11 +184,7 @@ impl Work<Context, AnyWorkId, Error> for HvarWork {
.names()
.map(|name| context.ir.glyphs.get(&FeWorkId::Glyph(name.clone())))
.collect();
let glyph_locations: HashSet<_> = glyphs
.iter()
.flat_map(|glyph| glyph.sources().keys())
.cloned()
.collect();
let glyph_locations = glyphs.iter().flat_map(|glyph| glyph.sources().keys());

let mut glyph_width_deltas = AdvanceWidthDeltas::new(var_model.clone(), glyph_locations);
for glyph in glyphs.into_iter() {
Expand Down
18 changes: 17 additions & 1 deletion fontdrasil/src/coords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@
//! See <https://github.com/googlefonts/fontmake-rs/blob/main/resources/text/units.md>
use std::{
collections::{BTreeMap, HashMap},
collections::{BTreeMap, BTreeSet, HashMap},
fmt::{Debug, Write},
marker::PhantomData,
ops::Sub,
Expand Down Expand Up @@ -323,6 +323,22 @@ impl<Space> Location<Space> {
self.0.retain(pred);
}

/// Creates a new `Location` containing only the axis tags contained in the given set.
pub fn subset_axes(&self, axis_tags: &BTreeSet<Tag>) -> Self {
Self(
self.0
.iter()
.filter_map(|(tag, coord)| {
if axis_tags.contains(tag) {
Some((*tag, *coord))
} else {
None
}
})
.collect(),
)
}

pub fn convert<ToSpace>(&self, axes: &HashMap<Tag, &Axis>) -> Location<ToSpace>
where
Space: ConvertSpace<ToSpace>,
Expand Down

0 comments on commit 7716fb6

Please sign in to comment.