Preserve ordering of glyph alternates when round-tripping through TTX

Also fixes a bug where glyph alternates in MTI feature files were
wrongly sorted by glyph name. After this change, the output is using
the same ordering as in the input MTI feature file.

Fixes https://github.com/fonttools/fonttools/issues/833.
This commit is contained in:
Sascha Brawer 2017-02-11 16:48:27 +01:00
parent a9201196a9
commit 706858646a
4 changed files with 74 additions and 72 deletions

View File

@ -159,6 +159,8 @@ class Builder(object):
alternates.setdefault(glyph, set()).update(alts) alternates.setdefault(glyph, set()).update(alts)
single = {glyph: list(repl)[0] for glyph, repl in alternates.items() single = {glyph: list(repl)[0] for glyph, repl in alternates.items()
if len(repl) == 1} if len(repl) == 1}
# TODO: Figure out the glyph alternate ordering used by makeotf.
# https://github.com/fonttools/fonttools/issues/836
multi = {glyph: sorted(repl, key=self.font.getGlyphID) multi = {glyph: sorted(repl, key=self.font.getGlyphID)
for glyph, repl in alternates.items() for glyph, repl in alternates.items()
if len(repl) > 1} if len(repl) > 1}

View File

@ -536,7 +536,7 @@ class AlternateSubst(FormatSwitchingBaseTable):
cov.glyphs = [ item[1] for item in items] cov.glyphs = [ item[1] for item in items]
alternates = [] alternates = []
setList = [ item[-1] for item in items] setList = [ item[-1] for item in items]
for set in setList: for set in setList:
alts = AlternateSet() alts = AlternateSet()
alts.Alternate = set alts.Alternate = set
alternates.append(alts) alternates.append(alts)
@ -553,7 +553,7 @@ class AlternateSubst(FormatSwitchingBaseTable):
for glyphName, alternates in items: for glyphName, alternates in items:
xmlWriter.begintag("AlternateSet", glyph=glyphName) xmlWriter.begintag("AlternateSet", glyph=glyphName)
xmlWriter.newline() xmlWriter.newline()
for alt in sorted(alternates): for alt in alternates:
xmlWriter.simpletag("Alternate", glyph=alt) xmlWriter.simpletag("Alternate", glyph=alt)
xmlWriter.newline() xmlWriter.newline()
xmlWriter.endtag("AlternateSet") xmlWriter.endtag("AlternateSet")

View File

@ -9,34 +9,34 @@
<!-- SubTableCount=1 --> <!-- SubTableCount=1 -->
<AlternateSubst index="0" Format="1"> <AlternateSubst index="0" Format="1">
<AlternateSet glyph="eight"> <AlternateSet glyph="eight">
<Alternate glyph="uni2078"/> <Alternate glyph="uniF738"/>
<Alternate glyph="uni2088"/>
<Alternate glyph="uniE0BD"/>
<Alternate glyph="uniE0BE"/>
<Alternate glyph="uniE0BF"/>
<Alternate glyph="uniE0C0"/> <Alternate glyph="uniE0C0"/>
<Alternate glyph="uniE0C1"/> <Alternate glyph="uniE0C1"/>
<Alternate glyph="uniF738"/> <Alternate glyph="uniE0BE"/>
<Alternate glyph="uni2078"/>
<Alternate glyph="uni2088"/>
<Alternate glyph="uniE0BF"/>
<Alternate glyph="uniE0BD"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="five"> <AlternateSet glyph="five">
<Alternate glyph="uni2075"/> <Alternate glyph="uniF735"/>
<Alternate glyph="uni2085"/>
<Alternate glyph="uniE0CA"/>
<Alternate glyph="uniE0CB"/>
<Alternate glyph="uniE0CC"/>
<Alternate glyph="uniE0CD"/> <Alternate glyph="uniE0CD"/>
<Alternate glyph="uniE0CE"/> <Alternate glyph="uniE0CE"/>
<Alternate glyph="uniF735"/> <Alternate glyph="uniE0CB"/>
<Alternate glyph="uni2075"/>
<Alternate glyph="uni2085"/>
<Alternate glyph="uniE0CC"/>
<Alternate glyph="uniE0CA"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="four"> <AlternateSet glyph="four">
<Alternate glyph="uni2074"/> <Alternate glyph="uniF734"/>
<Alternate glyph="uni2084"/>
<Alternate glyph="uniE0D1"/>
<Alternate glyph="uniE0D2"/>
<Alternate glyph="uniE0D3"/>
<Alternate glyph="uniE0D4"/> <Alternate glyph="uniE0D4"/>
<Alternate glyph="uniE0D5"/> <Alternate glyph="uniE0D5"/>
<Alternate glyph="uniF734"/> <Alternate glyph="uniE0D2"/>
<Alternate glyph="uni2074"/>
<Alternate glyph="uni2084"/>
<Alternate glyph="uniE0D3"/>
<Alternate glyph="uniE0D1"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="guilsinglleft"> <AlternateSet glyph="guilsinglleft">
<Alternate glyph="uniE0DB"/> <Alternate glyph="uniE0DB"/>
@ -47,77 +47,77 @@
<Alternate glyph="uniE0DE"/> <Alternate glyph="uniE0DE"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="nine"> <AlternateSet glyph="nine">
<Alternate glyph="uni2079"/> <Alternate glyph="uniF739"/>
<Alternate glyph="uni2089"/>
<Alternate glyph="uniE0E9"/>
<Alternate glyph="uniE0EA"/>
<Alternate glyph="uniE0EB"/>
<Alternate glyph="uniE0EC"/> <Alternate glyph="uniE0EC"/>
<Alternate glyph="uniE0ED"/> <Alternate glyph="uniE0ED"/>
<Alternate glyph="uniF739"/> <Alternate glyph="uniE0EA"/>
<Alternate glyph="uni2079"/>
<Alternate glyph="uni2089"/>
<Alternate glyph="uniE0EB"/>
<Alternate glyph="uniE0E9"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="one"> <AlternateSet glyph="one">
<Alternate glyph="uni00B9"/> <Alternate glyph="uniF731"/>
<Alternate glyph="uni2081"/>
<Alternate glyph="uniE0F0"/>
<Alternate glyph="uniE0F1"/>
<Alternate glyph="uniE0F2"/>
<Alternate glyph="uniE0F3"/> <Alternate glyph="uniE0F3"/>
<Alternate glyph="uniE0F4"/> <Alternate glyph="uniE0F4"/>
<Alternate glyph="uniE0F1"/>
<Alternate glyph="uni00B9"/>
<Alternate glyph="uni2081"/>
<Alternate glyph="uniE0F2"/>
<Alternate glyph="uniE0F0"/>
<Alternate glyph="uniE0F8"/> <Alternate glyph="uniE0F8"/>
<Alternate glyph="uniF731"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="seven"> <AlternateSet glyph="seven">
<Alternate glyph="uni2077"/> <Alternate glyph="uniF737"/>
<Alternate glyph="uni2087"/>
<Alternate glyph="uniE119"/>
<Alternate glyph="uniE11A"/>
<Alternate glyph="uniE11B"/>
<Alternate glyph="uniE11C"/> <Alternate glyph="uniE11C"/>
<Alternate glyph="uniE11D"/> <Alternate glyph="uniE11D"/>
<Alternate glyph="uniF737"/> <Alternate glyph="uniE11A"/>
<Alternate glyph="uni2077"/>
<Alternate glyph="uni2087"/>
<Alternate glyph="uniE11B"/>
<Alternate glyph="uniE119"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="six"> <AlternateSet glyph="six">
<Alternate glyph="uni2076"/> <Alternate glyph="uniF736"/>
<Alternate glyph="uni2086"/>
<Alternate glyph="uniE11E"/>
<Alternate glyph="uniE11F"/>
<Alternate glyph="uniE120"/>
<Alternate glyph="uniE121"/> <Alternate glyph="uniE121"/>
<Alternate glyph="uniE122"/> <Alternate glyph="uniE122"/>
<Alternate glyph="uniF736"/> <Alternate glyph="uniE11F"/>
<Alternate glyph="uni2076"/>
<Alternate glyph="uni2086"/>
<Alternate glyph="uniE120"/>
<Alternate glyph="uniE11E"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="three"> <AlternateSet glyph="three">
<Alternate glyph="uni00B3"/> <Alternate glyph="uniF733"/>
<Alternate glyph="uni2083"/>
<Alternate glyph="uniE128"/>
<Alternate glyph="uniE129"/>
<Alternate glyph="uniE12A"/>
<Alternate glyph="uniE12B"/> <Alternate glyph="uniE12B"/>
<Alternate glyph="uniE12C"/> <Alternate glyph="uniE12C"/>
<Alternate glyph="uniF733"/> <Alternate glyph="uniE129"/>
<Alternate glyph="uni00B3"/>
<Alternate glyph="uni2083"/>
<Alternate glyph="uniE12A"/>
<Alternate glyph="uniE128"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="two"> <AlternateSet glyph="two">
<Alternate glyph="uni00B2"/> <Alternate glyph="uniF732"/>
<Alternate glyph="uni2082"/>
<Alternate glyph="uniE0F9"/>
<Alternate glyph="uniE130"/>
<Alternate glyph="uniE131"/>
<Alternate glyph="uniE132"/>
<Alternate glyph="uniE133"/> <Alternate glyph="uniE133"/>
<Alternate glyph="uniE134"/> <Alternate glyph="uniE134"/>
<Alternate glyph="uniF732"/> <Alternate glyph="uniE131"/>
<Alternate glyph="uni00B2"/>
<Alternate glyph="uni2082"/>
<Alternate glyph="uniE132"/>
<Alternate glyph="uniE130"/>
<Alternate glyph="uniE0F9"/>
</AlternateSet> </AlternateSet>
<AlternateSet glyph="zero"> <AlternateSet glyph="zero">
<Alternate glyph="uni2070"/> <Alternate glyph="uniF730"/>
<Alternate glyph="uni2080"/>
<Alternate glyph="uniE139"/>
<Alternate glyph="uniE13A"/>
<Alternate glyph="uniE13B"/>
<Alternate glyph="uniE13C"/>
<Alternate glyph="uniE13D"/> <Alternate glyph="uniE13D"/>
<Alternate glyph="uniE13E"/> <Alternate glyph="uniE13E"/>
<Alternate glyph="uniF730"/> <Alternate glyph="uniE13A"/>
<Alternate glyph="uni2070"/>
<Alternate glyph="uni2080"/>
<Alternate glyph="uniE13B"/>
<Alternate glyph="uniE139"/>
<Alternate glyph="uniE13C"/>
</AlternateSet> </AlternateSet>
</AlternateSubst> </AlternateSubst>
</Lookup> </Lookup>

View File

@ -308,13 +308,13 @@ class AlternateSubstTest(unittest.TestCase):
rawTable = { rawTable = {
"Coverage": makeCoverage(["G", "Z"]), "Coverage": makeCoverage(["G", "Z"]),
"AlternateSet": [ "AlternateSet": [
self.makeAlternateSet("G.alt1 G.alt2"), self.makeAlternateSet("G.alt2 G.alt1"),
self.makeAlternateSet("Z.fina") self.makeAlternateSet("Z.fina")
] ]
} }
table.postRead(rawTable, self.font) table.postRead(rawTable, self.font)
self.assertEqual(table.alternates, { self.assertEqual(table.alternates, {
"G": ["G.alt1", "G.alt2"], "G": ["G.alt2", "G.alt1"],
"Z": ["Z.fina"] "Z": ["Z.fina"]
}) })
@ -325,25 +325,25 @@ class AlternateSubstTest(unittest.TestCase):
def test_preWrite_format1(self): def test_preWrite_format1(self):
table = otTables.AlternateSubst() table = otTables.AlternateSubst()
table.alternates = {"G": ["G.alt1", "G.alt2"], "Z": ["Z.fina"]} table.alternates = {"G": ["G.alt2", "G.alt1"], "Z": ["Z.fina"]}
rawTable = table.preWrite(self.font) rawTable = table.preWrite(self.font)
self.assertEqual(table.Format, 1) self.assertEqual(table.Format, 1)
self.assertEqual(rawTable["Coverage"].glyphs, ["G", "Z"]) self.assertEqual(rawTable["Coverage"].glyphs, ["G", "Z"])
[g, z] = rawTable["AlternateSet"] [g, z] = rawTable["AlternateSet"]
self.assertIsInstance(g, otTables.AlternateSet) self.assertIsInstance(g, otTables.AlternateSet)
self.assertEqual(g.Alternate, ["G.alt1", "G.alt2"]) self.assertEqual(g.Alternate, ["G.alt2", "G.alt1"])
self.assertIsInstance(z, otTables.AlternateSet) self.assertIsInstance(z, otTables.AlternateSet)
self.assertEqual(z.Alternate, ["Z.fina"]) self.assertEqual(z.Alternate, ["Z.fina"])
def test_toXML2(self): def test_toXML2(self):
writer = XMLWriter(StringIO()) writer = XMLWriter(StringIO())
table = otTables.AlternateSubst() table = otTables.AlternateSubst()
table.alternates = {"G": ["G.alt1", "G.alt2"], "Z": ["Z.fina"]} table.alternates = {"G": ["G.alt2", "G.alt1"], "Z": ["Z.fina"]}
table.toXML2(writer, self.font) table.toXML2(writer, self.font)
self.assertEqual(writer.file.getvalue().splitlines()[1:], [ self.assertEqual(writer.file.getvalue().splitlines()[1:], [
'<AlternateSet glyph="G">', '<AlternateSet glyph="G">',
' <Alternate glyph="G.alt1"/>',
' <Alternate glyph="G.alt2"/>', ' <Alternate glyph="G.alt2"/>',
' <Alternate glyph="G.alt1"/>',
'</AlternateSet>', '</AlternateSet>',
'<AlternateSet glyph="Z">', '<AlternateSet glyph="Z">',
' <Alternate glyph="Z.fina"/>', ' <Alternate glyph="Z.fina"/>',
@ -354,15 +354,15 @@ class AlternateSubstTest(unittest.TestCase):
table = otTables.AlternateSubst() table = otTables.AlternateSubst()
for name, attrs, content in parseXML( for name, attrs, content in parseXML(
'<AlternateSet glyph="G">' '<AlternateSet glyph="G">'
' <Alternate glyph="G.alt1"/>'
' <Alternate glyph="G.alt2"/>' ' <Alternate glyph="G.alt2"/>'
' <Alternate glyph="G.alt1"/>'
'</AlternateSet>' '</AlternateSet>'
'<AlternateSet glyph="Z">' '<AlternateSet glyph="Z">'
' <Alternate glyph="Z.fina"/>' ' <Alternate glyph="Z.fina"/>'
'</AlternateSet>'): '</AlternateSet>'):
table.fromXML(name, attrs, content, self.font) table.fromXML(name, attrs, content, self.font)
self.assertEqual(table.alternates, { self.assertEqual(table.alternates, {
"G": ["G.alt1", "G.alt2"], "G": ["G.alt2", "G.alt1"],
"Z": ["Z.fina"] "Z": ["Z.fina"]
}) })