From 991e7914e377a3b183cc850fa92647bb64f78c68 Mon Sep 17 00:00:00 2001 From: rsheeter Date: Tue, 12 Nov 2024 08:54:45 -0800 Subject: [PATCH 1/3] Lets not add inconsistent names? --- Lib/fontTools/varLib/__init__.py | 17 +++- Lib/fontTools/varLib/mutator.py | 4 +- Tests/varLib/data/test_results/Build.ttx | 12 +-- Tests/varLib/data/test_results/BuildMain.ttx | 92 ++++--------------- .../varLib/data/test_results/DropOnCurves.ttx | 3 - .../varLib/data/test_results/SingleMaster.ttx | 3 - .../data/test_results/SparseMasters.ttx | 3 - 7 files changed, 39 insertions(+), 95 deletions(-) diff --git a/Lib/fontTools/varLib/__init__.py b/Lib/fontTools/varLib/__init__.py index 36b1851cb..43a16da5a 100644 --- a/Lib/fontTools/varLib/__init__.py +++ b/Lib/fontTools/varLib/__init__.py @@ -85,6 +85,15 @@ def _add_fvar(font, axes, instances: List[InstanceDescriptor]): fvar = newTable("fvar") nameTable = font["name"] + # if there are not currently any mac names don't add them here, that's inconsistent + # https://github.com/fonttools/fonttools/issues/683 + macNames = any(nr.platformID == 1 for nr in getattr(nameTable, "names", ())) + + # we have all the best ways to express mac names + platforms = ((3, 1, 0x409),) + if macNames: + platforms = ((1, 0, 0),) + platforms + for a in axes.values(): axis = Axis() axis.axisTag = Tag(a.tag) @@ -95,7 +104,7 @@ def _add_fvar(font, axes, instances: List[InstanceDescriptor]): a.maximum, ) axis.axisNameID = nameTable.addMultilingualName( - a.labelNames, font, minNameID=256 + a.labelNames, font, minNameID=256, mac=macNames ) axis.flags = int(a.hidden) fvar.axes.append(axis) @@ -121,10 +130,12 @@ def _add_fvar(font, axes, instances: List[InstanceDescriptor]): psname = instance.postScriptFontName inst = NamedInstance() - inst.subfamilyNameID = nameTable.addMultilingualName(localisedStyleName) + inst.subfamilyNameID = nameTable.addMultilingualName( + localisedStyleName, mac=macNames + ) if psname is not None: psname = tostr(psname) - inst.postscriptNameID = nameTable.addName(psname) + inst.postscriptNameID = nameTable.addName(psname, platforms=platforms) inst.coordinates = { axes[k].tag: axes[k].map_backward(v) for k, v in coordinates.items() } diff --git a/Lib/fontTools/varLib/mutator.py b/Lib/fontTools/varLib/mutator.py index 80e46bb24..f9f937902 100644 --- a/Lib/fontTools/varLib/mutator.py +++ b/Lib/fontTools/varLib/mutator.py @@ -408,7 +408,9 @@ def instantiateVariableFont(varfont, location, inplace=False, overlap=True): if set(excludedUnicodeLangIDs) == set(range(len((varfont["ltag"].tags)))): del varfont["ltag"] varfont["name"].names[:] = [ - n for n in varfont["name"].names if n.nameID not in exclude + n + for n in varfont["name"].names + if n.nameID < 256 or n.nameID not in exclude ] if "wght" in location and "OS/2" in varfont: diff --git a/Tests/varLib/data/test_results/Build.ttx b/Tests/varLib/data/test_results/Build.ttx index 144cca5e7..f4fa0ae48 100644 --- a/Tests/varLib/data/test_results/Build.ttx +++ b/Tests/varLib/data/test_results/Build.ttx @@ -161,42 +161,42 @@ - + - + - + - + - + - + diff --git a/Tests/varLib/data/test_results/BuildMain.ttx b/Tests/varLib/data/test_results/BuildMain.ttx index 3a1bcfd37..1c2c1fa8d 100644 --- a/Tests/varLib/data/test_results/BuildMain.ttx +++ b/Tests/varLib/data/test_results/BuildMain.ttx @@ -443,63 +443,6 @@ کنتراست - - Weight - - - Contrast - - - ExtraLight - - - TestFamily-ExtraLight - - - Light - - - TestFamily-Light - - - Regular - - - TestFamily-Regular - - - Semibold - - - TestFamily-Semibold - - - Bold - - - TestFamily-Bold - - - Black - - - TestFamily-Black - - - Black Medium Contrast - - - TestFamily-BlackMediumContrast - - - Black High Contrast - - - TestFamily-BlackHighContrast - - - Kontrast - Kontrast @@ -546,39 +489,36 @@ TestFamily-Light - Regular - - TestFamily-Regular - + Semibold - + TestFamily-Semibold - + Bold - + TestFamily-Bold - + Black - + TestFamily-Black - + Black Medium Contrast - + TestFamily-BlackMediumContrast - + Black High Contrast - + TestFamily-BlackHighContrast @@ -807,42 +747,42 @@ - + - + - + - + - + - + diff --git a/Tests/varLib/data/test_results/DropOnCurves.ttx b/Tests/varLib/data/test_results/DropOnCurves.ttx index 4bfd36ad0..58a830f3c 100644 --- a/Tests/varLib/data/test_results/DropOnCurves.ttx +++ b/Tests/varLib/data/test_results/DropOnCurves.ttx @@ -228,9 +228,6 @@ - - Weight - Test Family diff --git a/Tests/varLib/data/test_results/SingleMaster.ttx b/Tests/varLib/data/test_results/SingleMaster.ttx index 02cfe32bd..47f957743 100644 --- a/Tests/varLib/data/test_results/SingleMaster.ttx +++ b/Tests/varLib/data/test_results/SingleMaster.ttx @@ -68,9 +68,6 @@ - - Weight - Test Family diff --git a/Tests/varLib/data/test_results/SparseMasters.ttx b/Tests/varLib/data/test_results/SparseMasters.ttx index 2871e24fb..a35260029 100644 --- a/Tests/varLib/data/test_results/SparseMasters.ttx +++ b/Tests/varLib/data/test_results/SparseMasters.ttx @@ -236,9 +236,6 @@ - - Weight - Layer Font From d2ce6e075c96d9659548492d8ddfa0d148f8cd15 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Nov 2024 12:13:19 +0100 Subject: [PATCH 2/3] check for mac names in buildVFStatTable for DSv5 sources with STAT data --- Lib/fontTools/varLib/stat.py | 7 +++++++ Tests/varLib/stat_test.py | 37 +++++++++++++++++++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/Lib/fontTools/varLib/stat.py b/Lib/fontTools/varLib/stat.py index 46c9498dc..eacbf580a 100644 --- a/Lib/fontTools/varLib/stat.py +++ b/Lib/fontTools/varLib/stat.py @@ -39,11 +39,18 @@ def buildVFStatTable(ttFont: TTFont, doc: DesignSpaceDocument, vfName: str) -> N region = getVFUserRegion(doc, vf) + # if there are not currently any mac names don't add them here, that's inconsistent + # https://github.com/fonttools/fonttools/issues/683 + macNames = any( + nr.platformID == 1 for nr in getattr(ttFont.get("name"), "names", ()) + ) + return fontTools.otlLib.builder.buildStatTable( ttFont, getStatAxes(doc, region), getStatLocations(doc, region), doc.elidedFallbackName if doc.elidedFallbackName is not None else 2, + macNames=macNames, ) diff --git a/Tests/varLib/stat_test.py b/Tests/varLib/stat_test.py index ce04423a5..f7af071a0 100644 --- a/Tests/varLib/stat_test.py +++ b/Tests/varLib/stat_test.py @@ -3,7 +3,8 @@ from pathlib import Path import pytest from fontTools.designspaceLib import DesignSpaceDocument from fontTools.designspaceLib.split import Range -from fontTools.varLib.stat import getStatAxes, getStatLocations +from fontTools.ttLib import TTFont, newTable +from fontTools.varLib.stat import buildVFStatTable, getStatAxes, getStatLocations @pytest.fixture @@ -189,3 +190,37 @@ def test_getStatLocations(datadir): "name": {"en": "Other"}, }, ] + + +@pytest.mark.parametrize( + "with_mac_names", + [ + pytest.param(True, id="with_mac_names"), + pytest.param(False, id="without_mac_names"), + ], +) +def test_buildVFStatTable(datadir, with_mac_names): + doc = DesignSpaceDocument.fromfile(datadir / "test_v5.designspace") + ttFont = TTFont() + + nameTable = newTable("name") + nameTable.names = [] + ttFont["name"] = nameTable + + if with_mac_names: + # addName adds a name string for both Macintosh and Windows platforms by default + nameTable.addName("Regular") + + buildVFStatTable(ttFont, doc, vfName="Test_WghtWdth") + + assert "STAT" in ttFont + + name_recs = ttFont["name"].names + assert len({nr.nameID for nr in name_recs}) == 15 + + # test that mac names don't get added if there weren't any before + mac_recs = [nr for nr in name_recs if nr.platformID == 1] + if with_mac_names: + assert len(mac_recs) > 1 + else: + assert len(mac_recs) == 0 From b371f237604000fac40122dd150245c5487024c0 Mon Sep 17 00:00:00 2001 From: Cosimo Lupo Date: Thu, 14 Nov 2024 12:51:59 +0100 Subject: [PATCH 3/3] fontBuilder: don't add mac names for fvar and STAT if name table hasn't any --- Lib/fontTools/fontBuilder.py | 27 +++++++++++++++++--- Tests/fontBuilder/data/test_var.ttf.ttx | 33 ------------------------- Tests/fontBuilder/fontBuilder_test.py | 2 +- 3 files changed, 24 insertions(+), 38 deletions(-) diff --git a/Lib/fontTools/fontBuilder.py b/Lib/fontTools/fontBuilder.py index 16b7ee167..d4af38fba 100644 --- a/Lib/fontTools/fontBuilder.py +++ b/Lib/fontTools/fontBuilder.py @@ -918,7 +918,15 @@ class FontBuilder(object): """ from .otlLib.builder import buildStatTable - buildStatTable(self.font, axes, locations, elidedFallbackName) + assert "name" in self.font, "name must to be set up first" + + buildStatTable( + self.font, + axes, + locations, + elidedFallbackName, + macNames=any(nr.platformID == 1 for nr in self.font["name"].names), + ) def buildCmapSubTable(cmapping, format, platformID, platEncID): @@ -938,6 +946,15 @@ def addFvar(font, axes, instances): fvar = newTable("fvar") nameTable = font["name"] + # if there are not currently any mac names don't add them here, that's inconsistent + # https://github.com/fonttools/fonttools/issues/683 + macNames = any(nr.platformID == 1 for nr in getattr(nameTable, "names", ())) + + # we have all the best ways to express mac names + platforms = ((3, 1, 0x409),) + if macNames: + platforms = ((1, 0, 0),) + platforms + for axis_def in axes: axis = Axis() @@ -963,7 +980,7 @@ def addFvar(font, axes, instances): if isinstance(name, str): name = dict(en=name) - axis.axisNameID = nameTable.addMultilingualName(name, ttFont=font) + axis.axisNameID = nameTable.addMultilingualName(name, ttFont=font, mac=macNames) fvar.axes.append(axis) for instance in instances: @@ -980,9 +997,11 @@ def addFvar(font, axes, instances): name = dict(en=name) inst = NamedInstance() - inst.subfamilyNameID = nameTable.addMultilingualName(name, ttFont=font) + inst.subfamilyNameID = nameTable.addMultilingualName( + name, ttFont=font, mac=macNames + ) if psname is not None: - inst.postscriptNameID = nameTable.addName(psname) + inst.postscriptNameID = nameTable.addName(psname, platforms=platforms) inst.coordinates = coordinates fvar.instances.append(inst) diff --git a/Tests/fontBuilder/data/test_var.ttf.ttx b/Tests/fontBuilder/data/test_var.ttf.ttx index e6d4d8d54..132e80d73 100644 --- a/Tests/fontBuilder/data/test_var.ttf.ttx +++ b/Tests/fontBuilder/data/test_var.ttf.ttx @@ -177,39 +177,6 @@ - - HelloTestFont - - - TotallyNormal - - - HelloTestFont-TotallyNormal - - - Left - - - Right - - - Up - - - Down - - - Right Up - - - Neutral - - - HalloTestFont - - - TotaalNormaal - HelloTestFont diff --git a/Tests/fontBuilder/fontBuilder_test.py b/Tests/fontBuilder/fontBuilder_test.py index 0cbf5d0ce..0138dcd00 100644 --- a/Tests/fontBuilder/fontBuilder_test.py +++ b/Tests/fontBuilder/fontBuilder_test.py @@ -236,7 +236,7 @@ def test_build_var(tmpdir): fb.setupHorizontalMetrics(metrics) fb.setupHorizontalHeader(ascent=824, descent=200) - fb.setupNameTable(nameStrings) + fb.setupNameTable(nameStrings, mac=False) axes = [ ("LEFT", 0, 0, 100, "Left"),