Skip to content

Commit

Permalink
Merge pull request #431 from googlefonts/rn
Browse files Browse the repository at this point in the history
Rename static metadata fields for clarity
  • Loading branch information
rsheeter authored Sep 6, 2023
2 parents 0eab689 + a586b11 commit f07a2df
Show file tree
Hide file tree
Showing 11 changed files with 42 additions and 42 deletions.
4 changes: 2 additions & 2 deletions fontbe/src/avar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -75,14 +75,14 @@ impl Work<Context, AnyWorkId, Error> for AvarWork {
fn exec(&self, context: &Context) -> Result<(), Error> {
let static_metadata = context.ir.static_metadata.get();
// Guard clause: don't produce avar for a static font
if static_metadata.variable_axes.is_empty() {
if static_metadata.axes.is_empty() {
debug!("Skip avar; this is not a variable font");
return Ok(());
}
context.avar.set_unconditionally(
Avar::new(
static_metadata
.variable_axes
.axes
.iter()
.map(to_segment_map)
.filter(|sm| !sm.axis_value_maps.is_empty())
Expand Down
12 changes: 4 additions & 8 deletions fontbe/src/features.rs
Original file line number Diff line number Diff line change
Expand Up @@ -102,7 +102,7 @@ impl<'a> FeaVariationInfo<'a> {
fn new(static_metadata: &'a StaticMetadata) -> FeaVariationInfo<'a> {
FeaVariationInfo {
fea_rs_axes: static_metadata
.variable_axes
.axes
.iter()
.enumerate()
.map(|(i, a)| {
Expand All @@ -120,11 +120,7 @@ impl<'a> FeaVariationInfo<'a> {
)
})
.collect(),
axes: static_metadata
.variable_axes
.iter()
.map(|a| (a.tag, a))
.collect(),
axes: static_metadata.axes.iter().map(|a| (a.tag, a)).collect(),
static_metadata,
}
}
Expand Down Expand Up @@ -262,8 +258,8 @@ impl<'a> VariationInfo for FeaVariationInfo<'a> {
for (region, value) in deltas.iter().filter(|(r, _)| !r.is_default()) {
// https://learn.microsoft.com/en-us/typography/opentype/spec/otvarcommonformats#variation-regions
// Array of region axis coordinates records, in the order of axes given in the 'fvar' table.
let mut region_axes = Vec::with_capacity(self.static_metadata.variable_axes.len());
for axis in self.static_metadata.variable_axes.iter() {
let mut region_axes = Vec::with_capacity(self.static_metadata.axes.len());
for axis in self.static_metadata.axes.iter() {
let Some(tent) = region.get(&axis.tag) else {
return Err(Box::new(MissingTentError::new(axis.tag)));
};
Expand Down
2 changes: 1 addition & 1 deletion fontbe/src/font.rs
Original file line number Diff line number Diff line change
Expand Up @@ -136,7 +136,7 @@ impl Work<Context, AnyWorkId, Error> for FontWork {
let mut builder = FontBuilder::default();

// A fancier implementation would mmap the files. We basic.
let is_static = context.ir.static_metadata.get().variable_axes.is_empty();
let is_static = context.ir.static_metadata.get().axes.is_empty();
for (work_id, tag, table_type) in TABLES_TO_MERGE {
if is_static && matches!(table_type, TableType::Variable) {
debug!("Skip {tag} because this is a static font");
Expand Down
6 changes: 3 additions & 3 deletions fontbe/src/fvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,7 @@ pub fn create_fvar_work() -> Box<BeWork> {

fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {
// Guard clause: don't produce fvar for a static font
if static_metadata.variable_axes.is_empty() {
if static_metadata.axes.is_empty() {
trace!("Skip fvar; this is not a variable font");
return None;
}
Expand All @@ -39,7 +39,7 @@ fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {

let axes_and_instances = AxisInstanceArrays::new(
static_metadata
.variable_axes
.axes
.iter()
.map(|ir_axis| {
let mut var = VariationAxisRecord {
Expand All @@ -62,7 +62,7 @@ fn generate_fvar(static_metadata: &StaticMetadata) -> Option<Fvar> {
.map(|ni| InstanceRecord {
subfamily_name_id: *reverse_names.get(&ni.name).unwrap(),
coordinates: static_metadata
.variable_axes
.axes
.iter()
.map(|axis| {
let loc = ni
Expand Down
2 changes: 1 addition & 1 deletion fontbe/src/glyphs.rs
Original file line number Diff line number Diff line change
Expand Up @@ -398,7 +398,7 @@ impl Work<Context, AnyWorkId, Error> for GlyphWork {
)?
} else {
let locations: HashSet<_> = ir_glyph.sources().keys().cloned().collect();
let sub_model = VariationModel::new(locations, static_metadata.variable_axes.clone())
let sub_model = VariationModel::new(locations, static_metadata.axes.clone())
.map_err(|e| Error::VariationModelError(self.glyph_name.clone(), e))?;
compute_deltas(
&self.glyph_name,
Expand Down
6 changes: 1 addition & 5 deletions fontbe/src/gvar.rs
Original file line number Diff line number Diff line change
Expand Up @@ -56,11 +56,7 @@ impl Work<Context, AnyWorkId, Error> for GvarWork {
fn exec(&self, context: &Context) -> Result<(), Error> {
// We built the gvar fragments alongside glyphs, now we need to glue them together into a gvar table
let static_metadata = context.ir.static_metadata.get();
let axis_order: Vec<_> = static_metadata
.variable_axes
.iter()
.map(|a| a.tag)
.collect();
let axis_order: Vec<_> = static_metadata.axes.iter().map(|a| a.tag).collect();
let glyph_order = context.ir.glyph_order.get();

let variations: Vec<_> = make_variations(&glyph_order, |glyph_name| {
Expand Down
4 changes: 2 additions & 2 deletions fontbe/src/stat.rs
Original file line number Diff line number Diff line change
Expand Up @@ -37,7 +37,7 @@ impl Work<Context, AnyWorkId, Error> for StatWork {
let static_metadata = context.ir.static_metadata.get();

// Guard clause: don't produce fvar for a static font
if static_metadata.variable_axes.is_empty() {
if static_metadata.axes.is_empty() {
trace!("Skip stat; this is not a variable font");
return Ok(());
}
Expand All @@ -54,7 +54,7 @@ impl Work<Context, AnyWorkId, Error> for StatWork {
context.stat.set_unconditionally(
Stat {
design_axes: static_metadata
.variable_axes
.axes
.iter()
.enumerate()
.map(|(idx, a)| AxisRecord {
Expand Down
10 changes: 6 additions & 4 deletions fontir/src/ir.rs
Original file line number Diff line number Diff line change
Expand Up @@ -41,12 +41,14 @@ pub struct StaticMetadata {
pub units_per_em: u16,

/// Every axis used by the font being compiled, including point axes.
pub axes: Vec<Axis>,
///
/// This is relatively rarely what you want.
pub all_source_axes: Vec<Axis>,

/// Every variable (non-point) axis used by the font being compiled.
///
/// If empty this is a static font.
pub variable_axes: Vec<Axis>,
pub axes: Vec<Axis>,

/// Named locations in variation space
pub named_instances: Vec<NamedInstance>,
Expand Down Expand Up @@ -267,8 +269,8 @@ impl StaticMetadata {
Ok(StaticMetadata {
units_per_em,
names: key_to_name,
axes,
variable_axes,
all_source_axes: axes,
axes: variable_axes,
named_instances,
variation_model,
axes_default,
Expand Down
2 changes: 1 addition & 1 deletion fontir/src/serde.rs
Original file line number Diff line number Diff line change
Expand Up @@ -80,7 +80,7 @@ impl From<StaticMetadata> for StaticMetadataSerdeRepr {
let glyph_locations = from.variation_model.locations().cloned().collect();
StaticMetadataSerdeRepr {
units_per_em: from.units_per_em,
axes: from.axes,
axes: from.all_source_axes,
named_instances: from.named_instances,
glyph_locations,
names: from.names,
Expand Down
20 changes: 10 additions & 10 deletions glyphs2fontir/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -579,11 +579,7 @@ impl Work<Context, WorkId, WorkError> for KerningWork {
let font_info = self.font_info.as_ref();
let font = &font_info.font;

let variable_axes: HashSet<_> = static_metadata
.variable_axes
.iter()
.map(|a| a.tag)
.collect();
let variable_axes: HashSet<_> = static_metadata.axes.iter().map(|a| a.tag).collect();
let master_positions: HashMap<_, _> = font
.masters
.iter()
Expand Down Expand Up @@ -693,7 +689,7 @@ impl Work<Context, WorkId, WorkError> for GlyphIrWork {
let font = &font_info.font;

let static_metadata = context.static_metadata.get();
let axes = &static_metadata.axes;
let axes = &static_metadata.all_source_axes;

let glyph = font
.glyphs
Expand Down Expand Up @@ -899,7 +895,7 @@ mod tests {
context
.static_metadata
.get()
.axes
.all_source_axes
.iter()
.map(|a| a.tag)
.collect::<Vec<_>>()
Expand Down Expand Up @@ -992,7 +988,7 @@ mod tests {
),
},
],
static_metadata.axes
static_metadata.all_source_axes
);
}

Expand Down Expand Up @@ -1081,7 +1077,11 @@ mod tests {
build_glyphs(&source, &context, &[&glyph_name]).unwrap(); // we dont' care about geometry

let static_metadata = context.static_metadata.get();
let axes = static_metadata.axes.iter().map(|a| (a.tag, a)).collect();
let axes = static_metadata
.all_source_axes
.iter()
.map(|a| (a.tag, a))
.collect();

let mut expected_locations = HashSet::new();
for (opsz, wght) in &[
Expand Down Expand Up @@ -1164,7 +1164,7 @@ mod tests {
#[test]
fn read_axis_location() {
let (_, context) = build_static_metadata(glyphs3_dir().join("WghtVar_AxisLocation.glyphs"));
let wght = &context.static_metadata.get().axes;
let wght = &context.static_metadata.get().all_source_axes;
assert_eq!(1, wght.len());
let wght = &wght[0];

Expand Down
16 changes: 11 additions & 5 deletions ufo2fontir/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -815,7 +815,8 @@ impl Work<Context, WorkId, WorkError> for GlobalMetricsWork {

let designspace_dir = self.designspace_file.parent().unwrap();
let font_infos = font_infos(designspace_dir, &self.designspace)?;
let master_locations = master_locations(&static_metadata.axes, &self.designspace.sources);
let master_locations =
master_locations(&static_metadata.all_source_axes, &self.designspace.sources);
let Some((_, default_master)) = default_master(&self.designspace) else {
return Err(WorkError::NoDefaultMaster(self.designspace_file.clone()));
};
Expand Down Expand Up @@ -1011,7 +1012,8 @@ impl Work<Context, WorkId, WorkError> for KerningWork {
let designspace_dir = self.designspace_file.parent().unwrap();
let glyph_order = context.glyph_order.get();
let static_metadata = context.static_metadata.get();
let master_locations = master_locations(&static_metadata.axes, &self.designspace.sources);
let master_locations =
master_locations(&static_metadata.all_source_axes, &self.designspace.sources);

let mut kerning = Kerning::default();

Expand Down Expand Up @@ -1161,7 +1163,11 @@ impl Work<Context, WorkId, WorkError> for GlyphIrWork {
let static_metadata = context.static_metadata.get();

// Migrate glif_files into internal coordinates
let axes_by_name = static_metadata.axes.iter().map(|a| (a.tag, a)).collect();
let axes_by_name = static_metadata
.all_source_axes
.iter()
.map(|a| (a.tag, a))
.collect();
let mut glif_files = HashMap::new();
for (path, design_locations) in self.glif_files.iter() {
let normalized_locations: Vec<NormalizedLocation> = design_locations
Expand Down Expand Up @@ -1588,7 +1594,7 @@ mod tests {
fn glyph_locations() {
let (_, context) = build_static_metadata("wght_var.designspace");
let static_metadata = &context.static_metadata.get();
let wght = static_metadata.variable_axes.first().unwrap();
let wght = static_metadata.axes.first().unwrap();

assert_eq!(
vec![
Expand All @@ -1608,7 +1614,7 @@ mod tests {
fn no_metrics_for_glyph_only_sources() {
let (_, context) = build_global_metrics("wght_var.designspace");
let static_metadata = &context.static_metadata.get();
let wght = static_metadata.variable_axes.first().unwrap();
let wght = static_metadata.axes.first().unwrap();
let mut metric_locations = context
.global_metrics
.get()
Expand Down

0 comments on commit f07a2df

Please sign in to comment.