Skip to content

Commit

Permalink
Filter out glyphs that don't exist in the default master in a .design…
Browse files Browse the repository at this point in the history
…space
  • Loading branch information
rsheeter committed Dec 12, 2022
1 parent e46de68 commit 0622a24
Show file tree
Hide file tree
Showing 3 changed files with 49 additions and 8 deletions.
13 changes: 13 additions & 0 deletions resources/testdata/WghtVar-Bold.ufo/glyphs/bonus_bar.glif
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
<?xml version='1.0' encoding='UTF-8'?>
<glyph name="bonus_bar" format="2">
<advance width="551"/>
<unicode hex="007F"/>
<outline>
<contour>
<point x="222" y="-227" type="line"/>
<point x="329" y="-227" type="line"/>
<point x="329" y="757" type="line"/>
<point x="222" y="757" type="line"/>
</contour>
</outline>
</glyph>
3 changes: 3 additions & 0 deletions resources/testdata/WghtVar-Bold.ufo/glyphs/contents.plist
Original file line number Diff line number Diff line change
Expand Up @@ -6,5 +6,8 @@
<string>bar.glif</string>
<key>plus</key>
<string>plus.glif</string>
<!-- should be eliminated as it does not exist in default master -->
<key>bonus_bar</key>
<string>bonus_bar.glif</string>
</dict>
</plist>
41 changes: 33 additions & 8 deletions ufo2fontir/src/source.rs
Original file line number Diff line number Diff line change
Expand Up @@ -140,14 +140,14 @@ impl DesignSpaceIrSource {
) -> Result<StateSet, Error> {
let mut font_info = StateSet::new();
font_info.track_file(&self.designspace_file)?;
let default_master = default_master(designspace)
let (default_master_idx, _) = default_master(designspace)
.ok_or_else(|| Error::NoDefaultMaster(self.designspace_file.clone()))?;

for source in designspace.sources.iter() {
for (idx, source) in designspace.sources.iter().enumerate() {
let ufo_dir = self.designspace_dir.join(&source.filename);
for filename in ["fontinfo.plist", "layercontents.plist", "lib.plist"] {
// Only track lib.plist for the default master
if filename == "lib.plist" && source != default_master {
if filename == "lib.plist" && idx != default_master_idx {
continue;
}

Expand Down Expand Up @@ -192,7 +192,20 @@ impl Source for DesignSpaceIrSource {
let mut glif_locations: HashMap<PathBuf, Vec<DesignSpaceLocation>> = HashMap::new();
let mut glyph_names = HashSet::new();

for source in designspace.sources.iter() {
let Some((default_master_idx, default_master)) = default_master(&designspace) else {
return Err(Error::NoDefaultMaster(self.designspace_file.clone()));
};
let mut sources_default_first = vec![default_master];
sources_default_first.extend(
designspace
.sources
.iter()
.enumerate()
.filter(|(idx, _)| *idx != default_master_idx)
.map(|(_, s)| s),
);

for (idx, source) in sources_default_first.iter().enumerate() {
// Track files within each UFO
// The UFO dir *must* exist since we were able to find fontinfo in it earlier
let ufo_dir = self.designspace_dir.join(&source.filename);
Expand All @@ -203,6 +216,10 @@ impl Source for DesignSpaceIrSource {
if !glif_file.exists() {
return Err(Error::FileExpected(glif_file));
}
if idx > 0 && !glyph_names.contains(&glyph_name) {
warn!("The glyph name '{}' exists in {} but not in the default master and will be ignored", glyph_name, source.filename);
continue;
}
glyph_names.insert(glyph_name.clone());
let glif_file = glif_file.clone();
glyphs
Expand Down Expand Up @@ -302,7 +319,7 @@ struct StaticMetadataWork {
glyph_names: Arc<HashSet<String>>,
}

fn default_master(designspace: &DesignSpaceDocument) -> Option<&designspace::Source> {
fn default_master(designspace: &DesignSpaceDocument) -> Option<(usize, &designspace::Source)> {
// The master at the default location on all axes
// TODO: fix me; ref https://github.com/googlefonts/fontmake-rs/issues/22
warn!("Using an incorrect algorithm to determine the default master");
Expand All @@ -314,7 +331,8 @@ fn default_master(designspace: &DesignSpaceDocument) -> Option<&designspace::Sou
designspace
.sources
.iter()
.find(|source| to_ir_location(&source.location) == default_location)
.enumerate()
.find(|(_, source)| to_ir_location(&source.location) == default_location)
}

fn load_lib_plist(ufo_dir: &Path) -> Result<plist::Dictionary, WorkError> {
Expand All @@ -337,7 +355,7 @@ fn glyph_order(
// The UFO at the default master *may* elect to specify a glyph order
// That glyph order *may* deign to overlap with the actual glyph set
let mut glyph_order = Vec::new();
if let Some(source) = default_master(designspace) {
if let Some((_, source)) = default_master(designspace) {
let lib_plist = load_lib_plist(&designspace_dir.join(&source.filename))?;
if let Some(plist::Value::Array(ufo_order)) = lib_plist.get("public.glyphOrder") {
let mut pending_add: HashSet<_> = glyph_names.iter().map(|s| s.as_str()).collect();
Expand Down Expand Up @@ -549,6 +567,13 @@ mod tests {
assert_eq!(expected_glif_files, work.glif_files);
}

#[test]
pub fn only_glyphs_present_in_default() {
let (_, inputs) = test_source();
// bonus_bar is not present in the default master; should discard
assert!(!inputs.glyphs.contains_key("bonus_bar"));
}

#[test]
pub fn find_default_master() {
let (source, _) = test_source();
Expand All @@ -557,7 +582,7 @@ mod tests {
expected.insert("Weight".to_string(), 400_f32.into());
assert_eq!(
expected,
to_ir_location(&default_master(&ds).unwrap().location)
to_ir_location(&default_master(&ds).unwrap().1.location)
);
}

Expand Down

0 comments on commit 0622a24

Please sign in to comment.