From 12d13f0b2628855fb9377c9fcd5fe94e0f7c239f Mon Sep 17 00:00:00 2001
From: Jany Belluz <jany.belluz@daltonmaag.com>
Date: Tue, 18 Oct 2022 17:18:15 +0100
Subject: [PATCH] [designspaceLib] Fix recursion error with linkedUserValue in
 both directions

---
 Lib/fontTools/designspaceLib/statNames.py     | 30 +++++++++++++++----
 Tests/designspaceLib/data/test_v5.designspace |  7 +++++
 Tests/designspaceLib/designspace_v5_test.py   |  9 ++++++
 Tests/designspaceLib/statNames_test.py        | 22 ++++++++++++++
 Tests/varLib/stat_test.py                     | 24 +++++++++++++++
 5 files changed, 86 insertions(+), 6 deletions(-)

diff --git a/Lib/fontTools/designspaceLib/statNames.py b/Lib/fontTools/designspaceLib/statNames.py
index 1b6727030..300ef01dd 100644
--- a/Lib/fontTools/designspaceLib/statNames.py
+++ b/Lib/fontTools/designspaceLib/statNames.py
@@ -48,7 +48,6 @@ class StatNames:
     styleMapStyleName: Optional[RibbiStyle]
 
 
-
 def getStatNames(
     doc: DesignSpaceDocument, userLocation: SimpleLocationDict
 ) -> StatNames:
@@ -89,7 +88,9 @@ def getStatNames(
         # whenever a translation is missing.
         labels = _getAxisLabelsForUserLocation(doc.axes, userLocation)
         if labels:
-            languages = set(language for label in labels for language in label.labelNames)
+            languages = set(
+                language for label in labels for language in label.labelNames
+            )
             languages.add("en")
             for language in languages:
                 styleName = " ".join(
@@ -214,16 +215,33 @@ def _getRibbiStyle(
     axis = axes_by_tag.get("wght")
     if axis is not None:
         for regular_label in axis.axisLabels:
-            if regular_label.linkedUserValue == userLocation[axis.name]:
+            if (
+                regular_label.linkedUserValue == userLocation[axis.name]
+                # In the "recursive" case where both the Regular has
+                # linkedUserValue pointing the Bold, and the Bold has
+                # linkedUserValue pointing to the Regular, only consider the
+                # first case: Regular (e.g. 400) has linkedUserValue pointing to
+                # Bold (e.g. 700, higher than Regular)
+                and regular_label.userValue < regular_label.linkedUserValue
+            ):
                 regularUserLocation[axis.name] = regular_label.userValue
                 bold = True
                 break
 
     axis = axes_by_tag.get("ital") or axes_by_tag.get("slnt")
     if axis is not None:
-        for urpright_label in axis.axisLabels:
-            if urpright_label.linkedUserValue == userLocation[axis.name]:
-                regularUserLocation[axis.name] = urpright_label.userValue
+        for upright_label in axis.axisLabels:
+            if (
+                upright_label.linkedUserValue == userLocation[axis.name]
+                # In the "recursive" case where both the Upright has
+                # linkedUserValue pointing the Italic, and the Italic has
+                # linkedUserValue pointing to the Upright, only consider the
+                # first case: Upright (e.g. ital=0, slant=0) has linkedUserValue
+                # pointing to Italic (e.g ital=1, slant=-12, higher than
+                # Upright in absolute value)
+                and abs(upright_label.userValue) < abs(upright_label.linkedUserValue)
+            ):
+                regularUserLocation[axis.name] = upright_label.userValue
                 italic = True
                 break
 
diff --git a/Tests/designspaceLib/data/test_v5.designspace b/Tests/designspaceLib/data/test_v5.designspace
index d2b3cdaef..498956cb4 100644
--- a/Tests/designspaceLib/data/test_v5.designspace
+++ b/Tests/designspaceLib/data/test_v5.designspace
@@ -21,6 +21,13 @@
         <label uservalue="600" userminimum="450" usermaximum="650" name="Semi Bold"/>
         <label uservalue="700" userminimum="650" usermaximum="850" name="Bold"/>
         <label uservalue="900" userminimum="850" usermaximum="900" name="Black"/>
+        <!--
+          Add "recursive" linked user values, see:
+          https://github.com/fonttools/fonttools/issues/2852
+          https://github.com/fonttools/fonttools/discussions/2790
+        -->
+        <label uservalue="400" name="Regular" elidable="true" linkeduservalue="700"/>
+        <label uservalue="700" name="Bold" linkeduservalue="400"/>
       </labels>
     </axis>
 
diff --git a/Tests/designspaceLib/designspace_v5_test.py b/Tests/designspaceLib/designspace_v5_test.py
index 35ad29b2a..84c927a23 100644
--- a/Tests/designspaceLib/designspace_v5_test.py
+++ b/Tests/designspaceLib/designspace_v5_test.py
@@ -78,6 +78,15 @@ def test_read_v5_document_simple(datadir):
                     AxisLabelDescriptor(
                         name="Black", userMinimum=850, userValue=900, userMaximum=900
                     ),
+                    AxisLabelDescriptor(
+                        name="Regular",
+                        userValue=400,
+                        linkedUserValue=700,
+                        elidable=True,
+                    ),
+                    AxisLabelDescriptor(
+                        name="Bold", userValue=700, linkedUserValue=400
+                    ),
                 ],
             ),
             AxisDescriptor(
diff --git a/Tests/designspaceLib/statNames_test.py b/Tests/designspaceLib/statNames_test.py
index 99d1c7fa8..dd5fb105d 100644
--- a/Tests/designspaceLib/statNames_test.py
+++ b/Tests/designspaceLib/statNames_test.py
@@ -61,6 +61,28 @@ def test_detect_ribbi_aktiv(datadir):
     )
 
 
+def test_detect_ribbi_recursive(datadir):
+    doc = DesignSpaceDocument.fromfile(datadir / "test_v5.designspace")
+
+    assert getStatNames(doc, {"Weight": 700, "Width": 125, "Italic": 1}) == StatNames(
+        familyNames={
+            "en": "MasterFamilyName",
+            "fr": "Montserrat",
+            "ja": "モンセラート",
+        },
+        styleNames={
+            "en": "Wide Bold Italic",
+        },
+        postScriptFontName="MasterFamilyName-WideBoldItalic",
+        styleMapFamilyNames={
+            "en": "MasterFamilyName Wide",
+            "fr": "Montserrat Wide",
+            "ja": "モンセラート Wide",
+        },
+        styleMapStyleName="bold italic",
+    )
+
+
 def test_getStatNames_on_ds4_doesnt_make_up_bad_names(datadir):
     """See this issue on GitHub: https://github.com/googlefonts/ufo2ft/issues/630
 
diff --git a/Tests/varLib/stat_test.py b/Tests/varLib/stat_test.py
index 6def990ec..ce04423a5 100644
--- a/Tests/varLib/stat_test.py
+++ b/Tests/varLib/stat_test.py
@@ -65,6 +65,18 @@ def test_getStatAxes(datadir):
                     "rangeMaxValue": 900.0,
                     "rangeMinValue": 850.0,
                 },
+                {
+                    "flags": 2,
+                    "name": {"en": "Regular"},
+                    "value": 400.0,
+                    "linkedValue": 700.0,
+                },
+                {
+                    "flags": 0,
+                    "name": {"en": "Bold"},
+                    "value": 700.0,
+                    "linkedValue": 400.0,
+                },
             ],
             "name": {"en": "Wéíght", "fa-IR": "قطر"},
             "ordering": 2,
@@ -120,6 +132,18 @@ def test_getStatAxes(datadir):
                     "rangeMaxValue": 850.0,
                     "rangeMinValue": 650.0,
                 },
+                {
+                    "flags": 2,
+                    "name": {"en": "Regular"},
+                    "value": 400.0,
+                    "linkedValue": 700.0,
+                },
+                {
+                    "flags": 0,
+                    "name": {"en": "Bold"},
+                    "value": 700.0,
+                    "linkedValue": 400.0,
+                },
             ],
             "name": {"en": "Wéíght", "fa-IR": "قطر"},
             "ordering": 2,