[glyphs-reader] duplicate GSMetric with 'filter' attribute leading to diffs in OS/2 and MVAR

[anthrotype]

in Glyhps3, global font metrics can define an optional 'filter' to make them apply only to particular classes of glyphs (e.g. the x-height of the small caps glyphs). For setting OS/2 or MVAR metrics, which is font-wide by definition, these should be ignored (i'm not even sure what they are used for within Glyphs.app itself, maybe hinting?).
However they way we collect these metrics into a BTreeMap keyed by metric name, makes is such that the last metric with a given name wins and overwrite the ones that preceed it.

fontc/glyphs-reader/src/font.rs

Lines 2526 to 2534 in 51f1bd8

	metric_values: m
	.metric_values
	.into_iter()
	.enumerate()
	.filter_map(|(idx, value)| {
	metric_names.get(&idx).map(|name| (name.clone(), value))
	})
	.filter(|(_, metric)| !metric.is_empty())
	.collect(),

E.g. take this Etcetera-Type-Co/Epilogue

python resources/scripts/ttx_diff.py 'https://github.com/Etcetera-Type-Co/Epilogue#sources/Epilogue.glyphs' --compare gftools --config ~/.fontc_crater_cache/Etcetera-Type-Co/Epilogue/sources/config.yaml

you'll notice the OS/2 and MVAR tables differ from fontmake's. Some masters end up with a different strikeout offset and xheight, because there are GSMetrics with a filter that selects only the x-height of small caps glyphs, which are defined after the default x-height and thus prevails.

In glyphsLib, only the first metric with a given name is actually used, and these filter attributes are completely unused.

So it's tempting to simply do this:

diff --git a/glyphs-reader/src/font.rs b/glyphs-reader/src/font.rs
index f39386de..789248e0 100644
--- a/glyphs-reader/src/font.rs
+++ b/glyphs-reader/src/font.rs
@@ -2525,7 +2525,12 @@ impl TryFrom<RawFont> for Font {
                             metric_names.get(&idx).map(|name| (name.clone(), value))
                         })
                         .filter(|(_, metric)| !metric.is_empty())
-                        .collect(),
+                        .fold(BTreeMap::new(), |mut acc, (name, value)| {
+                            if !acc.contains_key(&name) {
+                                acc.insert(name, value);
+                            }
+                            acc
+                        }),
                     number_values: from
                         .numbers
                         .iter()

and then this particular Epilogue.glyphs becomes 'output is identical' in ttx_diff

[anthrotype]

for reference, search

https://github.com/schriftgestalt/GlyphsSDK/blob/Glyphs3/GlyphsFileFormat/GlyphsFileFormatv3.md

metric: object – (GSMetric)
    filter: string – The filter of the metric limiting the scope of the metric to a subset of glyphs.

but again, I don't think fontc need to care about this for OS/2 or MVAR it can only take the generic unfiltered metrics.

[cmyr]

Given that our goal right now is matching the behaviour of fontmake, doing what glyphslib does makes sense to me?

Also for reference the best way to only add something to a map if it is missing is with the entry API, like:

let mut my_map = BTreeMap::new();
my_map.entry("my_key").or_insert(5);

the point being that this only has to do a single map lookup. 🤷

[anthrotype]

good point, thanks! I'll do that