From 1985197fb582fe4f7cdf2d296f79cf1c2e5152c8 Mon Sep 17 00:00:00 2001
From: rsheeter <rsheeter@google.com>
Date: Fri, 11 Oct 2024 16:34:53 -0700
Subject: [PATCH] Expose and fix bug when counting limits of repeatedly
 referenced components

---
 fontbe/src/metrics_and_limits.rs           | 10 +--
 fontc/src/lib.rs                           | 19 +++++-
 resources/testdata/glyphs3/PixelRef.glyphs | 76 ++++++++++++++++++++++
 3 files changed, 100 insertions(+), 5 deletions(-)
 create mode 100644 resources/testdata/glyphs3/PixelRef.glyphs

diff --git a/fontbe/src/metrics_and_limits.rs b/fontbe/src/metrics_and_limits.rs
index d95cdf07..a0403969 100644
--- a/fontbe/src/metrics_and_limits.rs
+++ b/fontbe/src/metrics_and_limits.rs
@@ -1,9 +1,10 @@
-//! Generates the [hmtx](https://learn.microsoft.com/en-us/typography/opentype/spec/hmtx) and
-//! [hhea](https://learn.microsoft.com/en-us/typography/opentype/spec/hhea) tables
+//! Generates the [hmtx](https://learn.microsoft.com/en-us/typography/opentype/spec/hmtx),
+//! [hhea](https://learn.microsoft.com/en-us/typography/opentype/spec/hhea), and
+//! [maxp](https://learn.microsoft.com/en-us/typography/opentype/spec/maxp) tables
 
 use std::{
     cmp::{max, min},
-    collections::{HashMap, HashSet},
+    collections::HashMap,
 };
 
 use fontdrasil::orchestration::{Access, AccessBuilder, Work};
@@ -51,7 +52,7 @@ struct FontLimits {
 struct GlyphInfo {
     /// For simple glyphs always present. For composites, set by [`FontLimits::update_composite_limits`]
     limits: Option<GlyphLimits>,
-    components: Option<HashSet<GlyphId16>>,
+    components: Option<Vec<GlyphId16>>,
 }
 
 impl GlyphInfo {
@@ -138,6 +139,7 @@ impl FontLimits {
         self.glyph_info.insert(id, glyph_info);
     }
 
+    // FontTools maxp <https://github.com/fonttools/fonttools/blob/e8146a6d0725d398cfa110cba683946ee762f8e2/Lib/fontTools/ttLib/tables/_m_a_x_p.py#L53>
     fn update_composite_limits(&mut self) -> GlyphLimits {
         let mut pending = self
             .glyph_info
diff --git a/fontc/src/lib.rs b/fontc/src/lib.rs
index 7030216a..1905121f 100644
--- a/fontc/src/lib.rs
+++ b/fontc/src/lib.rs
@@ -1916,7 +1916,7 @@ mod tests {
 
     #[test]
     fn maxp_with_shallow_components() {
-        assert_maxp_composite("glyphs2/Component.glyphs", 1, 4, 1);
+        assert_maxp_composite("glyphs2/Component.glyphs", 1, 8, 2);
     }
 
     #[test]
@@ -3155,4 +3155,21 @@ mod tests {
             "Should have no font specific names, got {font_specific_names:?}"
         );
     }
+
+    // <https://github.com/googlefonts/fontc/issues/1022>
+    // We used to mistakenly only count repeat use of the same component once
+    #[test]
+    fn duplicate_components_count_in_maxp() {
+        let compile = TestCompile::compile_source("glyphs3/PixelRef.glyphs");
+        let font = compile.font();
+        let maxp = font.maxp().unwrap();
+        // 3 uses of the "pixel" component which has 4 points
+        assert_eq!(
+            (12, 3),
+            (
+                maxp.max_composite_points().unwrap(),
+                maxp.max_composite_contours().unwrap()
+            )
+        );
+    }
 }
diff --git a/resources/testdata/glyphs3/PixelRef.glyphs b/resources/testdata/glyphs3/PixelRef.glyphs
new file mode 100644
index 00000000..10c78de5
--- /dev/null
+++ b/resources/testdata/glyphs3/PixelRef.glyphs
@@ -0,0 +1,76 @@
+{
+.formatVersion = 3;
+familyName = WghtVar;
+fontMaster = (
+{
+id = m01;
+name = Regular;
+}
+);
+glyphs = (
+{
+glyphname = space;
+layers = (
+{
+layerId = m01;
+width = 200;
+}
+);
+unicode = 32;
+},
+{
+glyphname = pixel;
+layers = (
+{
+layerId = m01;
+shapes = (
+{
+closed = 1;
+nodes = (
+(30,0,l),
+(0,0,l),
+(0,30,l),
+(30,30,l)
+);
+}
+);
+width = 30;
+}
+);
+},
+{
+glyphname = hyphen;
+layers = (
+{
+layerId = m01;
+shapes = (
+{
+alignment = -1;
+pos = (60,420);
+ref = pixel;
+},
+{
+alignment = -1;
+pos = (90,420);
+ref = pixel;
+},
+{
+alignment = -1;
+pos = (120,420);
+ref = pixel;
+}
+);
+width = 570;
+}
+);
+unicode = 45;
+}
+);
+
+settings = {
+gridLength = 30;
+};
+unitsPerEm = 1230;
+versionMajor = 1;
+versionMinor = 0;
+}