From 9c0c3a1375e9d9b6d9df4f43e942ed20672a4e3f Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <clupo@google.com>
Date: Thu, 12 Dec 2019 12:29:08 +0000
Subject: [PATCH 1/2] subset_test: Add (failing) test to repro #1777

the 'rvrn' feature is being incorrectly dropped because the feature indexes
change if some other feature occuring before 'rvrn' is dropped, like
in this test case.
---
 Tests/subset/subset_test.py | 40 +++++++++++++++++++++++++++++++++++++
 1 file changed, 40 insertions(+)

diff --git a/Tests/subset/subset_test.py b/Tests/subset/subset_test.py
index 42d302151..3d8c27ecf 100644
--- a/Tests/subset/subset_test.py
+++ b/Tests/subset/subset_test.py
@@ -1,5 +1,7 @@
+import io
 from fontTools.misc.py23 import *
 from fontTools import subset
+from fontTools.fontBuilder import FontBuilder
 from fontTools.ttLib import TTFont, newTable
 from fontTools.misc.loggingTools import CapturingLogHandler
 import difflib
@@ -728,5 +730,43 @@ class SubsetTest(unittest.TestCase):
         self.assertEqual(ttf.flavor, None)
 
 
+def test_subset_feature_variations():
+    fb = FontBuilder(unitsPerEm=100)
+    fb.setupGlyphOrder([".notdef", "f", "f_f", "dollar", "dollar.rvrn"])
+    fb.setupCharacterMap({ord("f"): "f", ord("$"): "dollar"})
+    fb.setupNameTable({"familyName": "TestFeatureVars", "styleName": "Regular"})
+    fb.setupPost()
+    fb.setupFvar(axes=[("wght", 100, 400, 900, "Weight")], instances=[])
+    fb.addOpenTypeFeatures("""\
+        feature dlig {
+            sub f f by f_f;
+        } dlig;
+    """)
+    fb.addFeatureVariations(
+        [([{"wght": (0.20886, 1.0)}], {"dollar": "dollar.rvrn"})],
+        featureTag="rvrn"
+    )
+    buf = io.BytesIO()
+    fb.save(buf)
+    buf.seek(0)
+
+    font = TTFont(buf)
+
+    options = subset.Options()
+    subsetter = subset.Subsetter(options)
+    subsetter.populate(unicodes=[ord("f"), ord("$")])
+    subsetter.subset(font)
+
+    featureTags = {
+        r.FeatureTag for r in font["GSUB"].table.FeatureList.FeatureRecord
+    }
+    # 'dlig' is discretionary so it is dropped by default
+    assert "dlig" not in featureTags
+    assert "f_f" not in font.getGlyphOrder()
+    # 'rvrn' is required so it is kept by default
+    assert "rvrn" in featureTags
+    assert "dollar.rvrn" in font.getGlyphOrder()
+
+
 if __name__ == "__main__":
     sys.exit(unittest.main())

From d68a59a2f255fbcb58c484213fb7c49ab498ffb8 Mon Sep 17 00:00:00 2001
From: Cosimo Lupo <clupo@google.com>
Date: Thu, 12 Dec 2019 12:38:31 +0000
Subject: [PATCH 2/2] subset: remap FeatureVariations
 SubstitutionRecord.FeatureIndex

Fixes #1777
---
 Lib/fontTools/subset/__init__.py | 3 +++
 1 file changed, 3 insertions(+)

diff --git a/Lib/fontTools/subset/__init__.py b/Lib/fontTools/subset/__init__.py
index 581c3948a..7205c347e 100644
--- a/Lib/fontTools/subset/__init__.py
+++ b/Lib/fontTools/subset/__init__.py
@@ -1307,6 +1307,9 @@ def subset_features(self, feature_indices):
 	self.ensureDecompiled()
 	self.SubstitutionRecord = [r for r in self.SubstitutionRecord
 				     if r.FeatureIndex in feature_indices]
+	# remap feature indices
+	for r in self.SubstitutionRecord:
+		r.FeatureIndex = feature_indices.index(r.FeatureIndex)
 	self.SubstitutionCount = len(self.SubstitutionRecord)
 	return bool(self.SubstitutionCount)