Skip to content

Commit

Permalink
Merge pull request #569 from googlefonts/virtual-masters
Browse files Browse the repository at this point in the history
Add support for 'Virtual Master' Glyphs.app's custom parameter
  • Loading branch information
anthrotype authored Nov 14, 2023
2 parents f7fd464 + 1d9e338 commit 10d79dc
Show file tree
Hide file tree
Showing 50 changed files with 1,406 additions and 718 deletions.
40 changes: 28 additions & 12 deletions fontc/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1883,7 +1883,7 @@ mod tests {
let font = FontRef::new(&buf).unwrap();

assert_eq!(
vec!["wght"],
vec!["CPHT", "wght"],
font.fvar()
.unwrap()
.axes()
Expand All @@ -1902,26 +1902,44 @@ mod tests {
Rect::new(191.0, 0.0, 410.0, 761.0),
],
vec![
cbox_of_char(0x69, &font, vec![0.0]), // wght=400
cbox_of_char(0x69, &font, vec![0.6]), // wght=700
cbox_of_char(0x69, &font, vec![1.0]), // wght=900
cbox_of_char(0x69, &font, vec![0.0, 0.0]), // CPHT=700, wght=400
cbox_of_char(0x69, &font, vec![0.0, 0.6]), // CPHT=700, wght=700
cbox_of_char(0x69, &font, vec![0.0, 1.0]), // CPHT=700, wght=900
]
);

// glyph "I" (char 0x49) only defines two masters at wght=400 and wght=900,
// so wght=700 should be interpolated between them.
// glyph "I" (char 0x49) only defines two masters along wght at 400 and 900
// (and default CPHT=700) so wght=700 should be interpolated between them.
assert_eq!(
vec![
Rect::new(231.0, 0.0, 364.0, 700.0),
Rect::new(195.0, 0.0, 400.0, 700.0),
Rect::new(171.0, 0.0, 424.0, 700.0),
],
vec![
cbox_of_char(0x49, &font, vec![0.0]), // wght=400
cbox_of_char(0x49, &font, vec![0.6]), // wght=700
cbox_of_char(0x49, &font, vec![1.0]), // wght=900
cbox_of_char(0x49, &font, vec![0.0, 0.0]), // CPHT=700, wght=400
cbox_of_char(0x49, &font, vec![0.0, 0.6]), // CPHT=700, wght=700
cbox_of_char(0x49, &font, vec![0.0, 1.0]), // CPHT=700, wght=900
]
);

// glyph "I" also defines two additional ('virtual') masters along CPHT at 600 and 800
// (and default wght=400), check that the cap-height (bbox.yMax) matches that throughout
// the wght axis.
assert_eq!(
vec![
Rect::new(231.0, 0.0, 364.0, 600.0),
Rect::new(171.0, 0.0, 424.0, 600.0),
Rect::new(231.0, 0.0, 364.0, 800.0),
Rect::new(171.0, 0.0, 424.0, 800.0),
],
vec![
cbox_of_char(0x49, &font, vec![-1.0, 0.0]), // CPHT=600, wght=400
cbox_of_char(0x49, &font, vec![-1.0, 1.0]), // CPHT=600, wght=900
cbox_of_char(0x49, &font, vec![1.0, 0.0]), // CPHT=800, wght=400
cbox_of_char(0x49, &font, vec![1.0, 1.0]), // CPHT=800, wght=900
]
)
}

#[test]
Expand All @@ -1939,9 +1957,7 @@ mod tests {
#[test]
fn intermediate_layer_in_designspace() {
// https://github.com/googlefonts/fontc/issues/400
assert_intermediate_layer(
"designspace_from_glyphs/IntermediateLayer/IntermediateLayer.designspace",
);
assert_intermediate_layer("designspace_from_glyphs/IntermediateLayer.designspace");
}

#[test]
Expand Down
60 changes: 52 additions & 8 deletions fontdrasil/src/coords.rs
Original file line number Diff line number Diff line change
Expand Up @@ -101,14 +101,14 @@ impl CoordConverter {

/// Initialize a converter from just min/default/max user coords, e.g. a source with no mapping
pub fn unmapped(min: UserCoord, default: UserCoord, max: UserCoord) -> CoordConverter {
CoordConverter::new(
vec![
(min, DesignCoord::new(min.into_inner())),
(default, DesignCoord::new(default.into_inner())),
(max, DesignCoord::new(max.into_inner())),
],
1,
)
let mut mappings = vec![
(min, DesignCoord::new(min.into_inner())),
(default, DesignCoord::new(default.into_inner())),
(max, DesignCoord::new(max.into_inner())),
];
mappings.dedup();
let default_idx = mappings.iter().position(|(u, _)| *u == default).unwrap();
CoordConverter::new(mappings, default_idx)
}

/// Walk the vertices of the mappings, viewing the user/design/normalized value at each stop.
Expand Down Expand Up @@ -550,4 +550,48 @@ mod tests {
.into_inner()
);
}

#[test]
fn unmapped_coords_get_deduped() {
// min==default==max
assert_eq!(
CoordConverter::unmapped(
UserCoord(100.0.into()),
UserCoord(100.0.into()),
UserCoord(100.0.into()),
)
.default_idx,
0
);
// min==default<max
assert_eq!(
CoordConverter::unmapped(
UserCoord(0.0.into()),
UserCoord(0.0.into()),
UserCoord(100.0.into()),
)
.default_idx,
0
);
// min<default==max
assert_eq!(
CoordConverter::unmapped(
UserCoord(0.0.into()),
UserCoord(100.0.into()),
UserCoord(100.0.into()),
)
.default_idx,
1
);
// min<default<max
assert_eq!(
CoordConverter::unmapped(
UserCoord(0.0.into()),
UserCoord(50.0.into()),
UserCoord(100.0.into()),
)
.default_idx,
1
);
}
}
67 changes: 57 additions & 10 deletions glyphs-reader/src/font.rs
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ pub struct Font {
pub glyph_to_codepoints: BTreeMap<String, BTreeSet<u32>>,
// tag => (user:design) tuples
pub axis_mappings: RawUserToDesignMapping,
pub virtual_masters: Vec<BTreeMap<String, OrderedFloat<f64>>>,
pub features: Vec<FeatureSnippet>,
pub names: BTreeMap<String, String>,
pub instances: Vec<Instance>,
Expand Down Expand Up @@ -273,12 +274,19 @@ struct RawFont {
pub custom_parameters: CustomParameters,
}

// we use a vec of tuples instead of a map because there can be multiple
// values for the same name (e.g. 'Virtual Master')
#[derive(Clone, Default, Debug, PartialEq, Eq, Hash)]
pub(crate) struct CustomParameters(BTreeMap<String, CustomParameterValue>);
pub(crate) struct CustomParameters(Vec<(String, CustomParameterValue)>);

impl CustomParameters {
/// Get the first parameter with the given name, or `None` if not found.
fn get(&self, name: &str) -> Option<&CustomParameterValue> {
self.0.iter().find_map(|(n, v)| (n == name).then_some(v))
}

fn int(&self, name: &str) -> Option<i64> {
let Some(CustomParameterValue::Int(i)) = self.0.get(name) else {
let Some(CustomParameterValue::Int(i)) = self.get(name) else {
return None;
};
Some(*i)
Expand All @@ -289,40 +297,51 @@ impl CustomParameters {
}

fn string(&self, name: &str) -> Option<&str> {
let Some(CustomParameterValue::String(str)) = self.0.get(name) else {
let Some(CustomParameterValue::String(str)) = self.get(name) else {
return None;
};
Some(str)
}

fn axes(&self) -> Option<&Vec<Axis>> {
let Some(CustomParameterValue::Axes(axes)) = self.0.get("Axes") else {
let Some(CustomParameterValue::Axes(axes)) = self.get("Axes") else {
return None;
};
Some(axes)
}

fn axis_mappings(&self) -> Option<&Vec<AxisMapping>> {
let Some(CustomParameterValue::AxesMappings(mappings)) = self.0.get("Axis Mappings") else {
let Some(CustomParameterValue::AxesMappings(mappings)) = self.get("Axis Mappings") else {
return None;
};
Some(mappings)
}

fn axis_locations(&self) -> Option<&Vec<AxisLocation>> {
let Some(CustomParameterValue::AxisLocations(locations)) = self.0.get("Axis Location")
else {
let Some(CustomParameterValue::AxisLocations(locations)) = self.get("Axis Location") else {
return None;
};
Some(locations)
}

fn glyph_order(&self) -> Option<&Vec<String>> {
let Some(CustomParameterValue::GlyphOrder(names)) = self.0.get("glyphOrder") else {
let Some(CustomParameterValue::GlyphOrder(names)) = self.get("glyphOrder") else {
return None;
};
Some(names)
}

fn virtual_masters(&self) -> impl Iterator<Item = &Vec<AxisLocation>> {
self.0.iter().filter_map(|(name, value)| {
if name == "Virtual Master" {
let CustomParameterValue::VirtualMaster(locations) = value else {
panic!("Virtual Master parameter has wrong type!");
};
return Some(locations);
}
None
})
}
}

#[derive(Clone, Debug, PartialEq, Eq, Hash)]
Expand All @@ -333,13 +352,14 @@ enum CustomParameterValue {
AxesMappings(Vec<AxisMapping>),
AxisLocations(Vec<AxisLocation>),
GlyphOrder(Vec<String>),
VirtualMaster(Vec<AxisLocation>),
}

/// Hand-parse these because they take multiple shapes
impl FromPlist for CustomParameters {
fn parse(tokenizer: &mut Tokenizer<'_>) -> Result<Self, crate::plist::Error> {
use crate::plist::Error;
let mut params = BTreeMap::new();
let mut params = Vec::new();

tokenizer.eat(b'(')?;

Expand Down Expand Up @@ -415,6 +435,13 @@ impl FromPlist for CustomParameters {
value =
Some(CustomParameterValue::AxisLocations(tokenizer.parse()?));
}
_ if name == Some(String::from("Virtual Master")) => {
let Token::OpenParen = peek else {
return Err(Error::UnexpectedChar('('));
};
value =
Some(CustomParameterValue::VirtualMaster(tokenizer.parse()?));
}
_ => tokenizer.skip_rec()?,
}
}
Expand All @@ -424,7 +451,7 @@ impl FromPlist for CustomParameters {
}

if let (Some(name), Some(value)) = (name, value) {
params.insert(name, value);
params.push((name, value));
}

tokenizer.eat(b'}')?;
Expand Down Expand Up @@ -1407,6 +1434,10 @@ impl RawAxisUserToDesignMap {
pub fn iter(&self) -> impl Iterator<Item = &(OrderedFloat<f32>, OrderedFloat<f32>)> {
self.0.iter()
}

pub fn is_identity(&self) -> bool {
self.0.iter().all(|(u, d)| u == d)
}
}

impl RawUserToDesignMapping {
Expand Down Expand Up @@ -1860,6 +1891,21 @@ impl TryFrom<RawFont> for Font {
})
.collect();

let virtual_masters = from
.custom_parameters
.virtual_masters()
.map(|vm| {
vm.iter()
.map(
|AxisLocation {
axis_name,
location,
}| (axis_name.clone(), *location),
)
.collect()
})
.collect();

Ok(Font {
units_per_em,
use_typo_metrics,
Expand All @@ -1871,6 +1917,7 @@ impl TryFrom<RawFont> for Font {
glyph_order,
glyph_to_codepoints,
axis_mappings,
virtual_masters,
features,
names,
instances,
Expand Down
29 changes: 4 additions & 25 deletions glyphs2fontir/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -86,6 +86,7 @@ impl GlyphsIrSource {
glyph_order: Default::default(),
glyph_to_codepoints: Default::default(),
axis_mappings: font.axis_mappings.clone(),
virtual_masters: Default::default(),
features: Default::default(),
names: Default::default(),
instances: font.instances.clone(),
Expand Down Expand Up @@ -114,6 +115,7 @@ impl GlyphsIrSource {
glyph_order: Default::default(),
glyph_to_codepoints: Default::default(),
axis_mappings: Default::default(),
virtual_masters: Default::default(),
features: Default::default(),
names: Default::default(),
instances: font.instances.clone(),
Expand Down Expand Up @@ -804,20 +806,16 @@ impl Work<Context, WorkId, WorkError> for GlyphIrWork {
}
}

// It's helpful if glyphs are defined at min, default, and max (some of which may be cooincident)
// It's helpful if glyphs are defined at default
for axis in axes.iter() {
let min = axis.min.to_normalized(&axis.converter);
let max = axis.max.to_normalized(&axis.converter);
let default = axis.max.to_normalized(&axis.converter);
let default = axis.default.to_normalized(&axis.converter);
let Some(positions) = axis_positions.get(&axis.tag) else {
return Err(WorkError::NoAxisPosition(
self.glyph_name.clone(),
axis.name.clone(),
));
};
check_pos(&self.glyph_name, positions, axis, &min)?;
check_pos(&self.glyph_name, positions, axis, &default)?;
check_pos(&self.glyph_name, positions, axis, &max)?;
}

context.anchors.set(ir_anchors.try_into()?);
Expand Down Expand Up @@ -1206,25 +1204,6 @@ mod tests {
assert_eq!(expected_locations, actual_locations);
}

#[test]
fn glyph_must_define_min() {
let glyph_name = GlyphName::from("min-undefined");
let (source, context) = build_static_metadata(glyphs2_dir().join("MinUndef.glyphs"));
let result = build_glyphs(&source, &context, &[&glyph_name]);
assert!(result.is_err());
let Err(WorkError::GlyphUndefAtNormalizedPosition {
glyph_name,
axis,
pos,
}) = result
else {
panic!("Wrong error");
};
assert_eq!("min-undefined", glyph_name.as_str());
assert_eq!(Tag::new(b"wght"), axis);
assert_eq!(NormalizedCoord::new(-1.0), pos);
}

#[test]
fn read_axis_location() {
let (_, context) = build_static_metadata(glyphs3_dir().join("WghtVar_AxisLocation.glyphs"));
Expand Down
Loading

0 comments on commit 10d79dc

Please sign in to comment.