diff --git a/Lib/fontTools/varLib/instancer/__init__.py b/Lib/fontTools/varLib/instancer/__init__.py
index cde1d39f6..a887e5d38 100644
--- a/Lib/fontTools/varLib/instancer/__init__.py
+++ b/Lib/fontTools/varLib/instancer/__init__.py
@@ -105,6 +105,7 @@ from fontTools.misc.cliTools import makeOutputFileName
from fontTools.varLib.instancer import solver
import collections
import dataclasses
+from contextlib import contextmanager
from copy import deepcopy
from enum import IntEnum
import logging
@@ -694,6 +695,43 @@ def setMvarDeltas(varfont, deltas):
)
+@contextmanager
+def verticalMetricsKeptInSync(varfont):
+ """Ensure hhea vertical metrics stay in sync with OS/2 ones after instancing.
+
+ When applying MVAR deltas to the OS/2 table, if the ascender, descender and
+ line gap change but they were the same as the respective hhea metrics in the
+ original font, this context manager ensures that hhea metrcs also get updated
+ accordingly.
+ The MVAR spec only has tags for the OS/2 metrics, but it is common in fonts
+ to have the hhea metrics be equal to those for compat reasons.
+
+ https://learn.microsoft.com/en-us/typography/opentype/spec/mvar
+ https://googlefonts.github.io/gf-guide/metrics.html#7-hhea-and-typo-metrics-should-be-equal
+ https://github.com/fonttools/fonttools/issues/3297
+ """
+ current_os2_vmetrics = [
+ getattr(varfont["OS/2"], attr)
+ for attr in ("sTypoAscender", "sTypoDescender", "sTypoLineGap")
+ ]
+ metrics_are_synced = current_os2_vmetrics == [
+ getattr(varfont["hhea"], attr) for attr in ("ascender", "descender", "lineGap")
+ ]
+
+ yield metrics_are_synced
+
+ if metrics_are_synced:
+ new_os2_vmetrics = [
+ getattr(varfont["OS/2"], attr)
+ for attr in ("sTypoAscender", "sTypoDescender", "sTypoLineGap")
+ ]
+ if current_os2_vmetrics != new_os2_vmetrics:
+ for attr, value in zip(
+ ("ascender", "descender", "lineGap"), new_os2_vmetrics
+ ):
+ setattr(varfont["hhea"], attr, value)
+
+
def instantiateMVAR(varfont, axisLimits):
log.info("Instantiating MVAR table")
@@ -701,7 +739,9 @@ def instantiateMVAR(varfont, axisLimits):
fvarAxes = varfont["fvar"].axes
varStore = mvar.VarStore
defaultDeltas = instantiateItemVariationStore(varStore, fvarAxes, axisLimits)
- setMvarDeltas(varfont, defaultDeltas)
+
+ with verticalMetricsKeptInSync(varfont):
+ setMvarDeltas(varfont, defaultDeltas)
if varStore.VarRegionList.Region:
varIndexMapping = varStore.optimize()
diff --git a/Tests/varLib/instancer/data/PartialInstancerTest-VF.ttx b/Tests/varLib/instancer/data/PartialInstancerTest-VF.ttx
index 2f1754b05..cee188460 100644
--- a/Tests/varLib/instancer/data/PartialInstancerTest-VF.ttx
+++ b/Tests/varLib/instancer/data/PartialInstancerTest-VF.ttx
@@ -108,9 +108,9 @@
-
+
-
+
@@ -687,6 +687,7 @@
+
@@ -705,6 +706,10 @@
+
+
+
+
diff --git a/Tests/varLib/instancer/instancer_test.py b/Tests/varLib/instancer/instancer_test.py
index 20d9194f6..da6dd9ee0 100644
--- a/Tests/varLib/instancer/instancer_test.py
+++ b/Tests/varLib/instancer/instancer_test.py
@@ -304,39 +304,69 @@ class InstantiateMVARTest(object):
assert len(mvar.VarStore.VarData) == 1
@pytest.mark.parametrize(
- "location, expected",
+ "location, expected, sync_vmetrics",
[
pytest.param(
{"wght": 1.0, "wdth": 0.0},
- {"strs": 100, "undo": -200, "unds": 150},
+ {"strs": 100, "undo": -200, "unds": 150, "hasc": 1100},
+ True,
id="wght=1.0,wdth=0.0",
),
pytest.param(
{"wght": 0.0, "wdth": -1.0},
- {"strs": 20, "undo": -100, "unds": 50},
+ {"strs": 20, "undo": -100, "unds": 50, "hasc": 1000},
+ True,
id="wght=0.0,wdth=-1.0",
),
pytest.param(
{"wght": 0.5, "wdth": -0.5},
- {"strs": 55, "undo": -145, "unds": 95},
+ {"strs": 55, "undo": -145, "unds": 95, "hasc": 1050},
+ True,
id="wght=0.5,wdth=-0.5",
),
pytest.param(
{"wght": 1.0, "wdth": -1.0},
- {"strs": 50, "undo": -180, "unds": 130},
+ {"strs": 50, "undo": -180, "unds": 130, "hasc": 1100},
+ True,
id="wght=0.5,wdth=-0.5",
),
+ pytest.param(
+ {"wght": 1.0, "wdth": 0.0},
+ {"strs": 100, "undo": -200, "unds": 150, "hasc": 1100},
+ False,
+ id="wght=1.0,wdth=0.0,no_sync_vmetrics",
+ ),
],
)
- def test_full_instance(self, varfont, location, expected):
+ def test_full_instance(self, varfont, location, sync_vmetrics, expected):
location = instancer.NormalizedAxisLimits(location)
+ # check vertical metrics are in sync before...
+ if sync_vmetrics:
+ assert varfont["OS/2"].sTypoAscender == varfont["hhea"].ascender
+ assert varfont["OS/2"].sTypoDescender == varfont["hhea"].descender
+ assert varfont["OS/2"].sTypoLineGap == varfont["hhea"].lineGap
+ else:
+ # force them not to be in sync
+ varfont["OS/2"].sTypoDescender -= 100
+ varfont["OS/2"].sTypoLineGap += 200
+
instancer.instantiateMVAR(varfont, location)
for mvar_tag, expected_value in expected.items():
table_tag, item_name = MVAR_ENTRIES[mvar_tag]
assert getattr(varfont[table_tag], item_name) == expected_value
+ # ... as well as after instancing, but only if they were already
+ # https://github.com/fonttools/fonttools/issues/3297
+ if sync_vmetrics:
+ assert varfont["OS/2"].sTypoAscender == varfont["hhea"].ascender
+ assert varfont["OS/2"].sTypoDescender == varfont["hhea"].descender
+ assert varfont["OS/2"].sTypoLineGap == varfont["hhea"].lineGap
+ else:
+ assert varfont["OS/2"].sTypoDescender != varfont["hhea"].descender
+ assert varfont["OS/2"].sTypoLineGap != varfont["hhea"].lineGap
+
assert "MVAR" not in varfont