[otlLib] make ClassDefBuilder class order match varLib.mergers

see https://github.com/fonttools/fonttools/blob/c3d876/Lib/fontTools/misc/classifyTools.py#L77

i.e. we want (large classes first, then lexicographic order on the glyphs); previously otlLib was sorting by the _reverse_ of (small classes first, then glyphs lexicographic order) -- effectively comparing the reverse of the glyph sets of classes of the same size.

Fixes https://github.com/fonttools/fonttools/issues/3321

note the ttx dump of previously built fonts may change but there won't be any functional changes.
This commit is contained in:
Cosimo Lupo 2023-11-02 16:32:15 +00:00
parent 2d9b80acd1
commit 2c07619344
6 changed files with 32 additions and 32 deletions

View File

@ -2674,7 +2674,7 @@ class ClassDefBuilder(object):
# class form a contiguous range, the encoding is actually quite # class form a contiguous range, the encoding is actually quite
# compact, whereas a non-contiguous set might need a lot of bytes # compact, whereas a non-contiguous set might need a lot of bytes
# in the output file. We don't get this right with the key below. # in the output file. We don't get this right with the key below.
result = sorted(self.classes_, key=lambda s: (len(s), s), reverse=True) result = sorted(self.classes_, key=lambda s: (-len(s), s))
if not self.useClass0_: if not self.useClass0_:
result.insert(0, frozenset()) result.insert(0, frozenset())
return result return result

View File

@ -93,14 +93,14 @@
<ValueFormat1 value="4"/> <ValueFormat1 value="4"/>
<ValueFormat2 value="0"/> <ValueFormat2 value="0"/>
<ClassDef1> <ClassDef1>
<ClassDef glyph="b" class="1"/>
<ClassDef glyph="o" class="1"/>
</ClassDef1>
<ClassDef2>
<ClassDef glyph="c" class="2"/>
<ClassDef glyph="d" class="2"/>
<ClassDef glyph="v" class="1"/> <ClassDef glyph="v" class="1"/>
<ClassDef glyph="w" class="1"/> <ClassDef glyph="w" class="1"/>
</ClassDef1>
<ClassDef2>
<ClassDef glyph="c" class="1"/>
<ClassDef glyph="d" class="1"/>
<ClassDef glyph="v" class="2"/>
<ClassDef glyph="w" class="2"/>
</ClassDef2> </ClassDef2>
<!-- Class1Count=2 --> <!-- Class1Count=2 -->
<!-- Class2Count=3 --> <!-- Class2Count=3 -->
@ -112,7 +112,7 @@
<Value1 XAdvance="0"/> <Value1 XAdvance="0"/>
</Class2Record> </Class2Record>
<Class2Record index="2"> <Class2Record index="2">
<Value1 XAdvance="-20"/> <Value1 XAdvance="-10"/>
</Class2Record> </Class2Record>
</Class1Record> </Class1Record>
<Class1Record index="1"> <Class1Record index="1">
@ -120,7 +120,7 @@
<Value1 XAdvance="0"/> <Value1 XAdvance="0"/>
</Class2Record> </Class2Record>
<Class2Record index="1"> <Class2Record index="1">
<Value1 XAdvance="-10"/> <Value1 XAdvance="-20"/>
</Class2Record> </Class2Record>
<Class2Record index="2"> <Class2Record index="2">
<Value1 XAdvance="0"/> <Value1 XAdvance="0"/>

View File

@ -43,10 +43,10 @@
<ClassDef1> <ClassDef1>
</ClassDef1> </ClassDef1>
<ClassDef2> <ClassDef2>
<ClassDef glyph="C" class="2"/> <ClassDef glyph="C" class="1"/>
<ClassDef glyph="O" class="2"/> <ClassDef glyph="O" class="1"/>
<ClassDef glyph="V" class="1"/> <ClassDef glyph="V" class="2"/>
<ClassDef glyph="W" class="1"/> <ClassDef glyph="W" class="2"/>
</ClassDef2> </ClassDef2>
<!-- Class1Count=1 --> <!-- Class1Count=1 -->
<!-- Class2Count=3 --> <!-- Class2Count=3 -->
@ -55,10 +55,10 @@
<Value1 XAdvance="0"/> <Value1 XAdvance="0"/>
</Class2Record> </Class2Record>
<Class2Record index="1"> <Class2Record index="1">
<Value1 XAdvance="0"/> <Value1 XAdvance="-20"/>
</Class2Record> </Class2Record>
<Class2Record index="2"> <Class2Record index="2">
<Value1 XAdvance="-20"/> <Value1 XAdvance="0"/>
</Class2Record> </Class2Record>
</Class1Record> </Class1Record>
</PairPos> </PairPos>

View File

@ -66,9 +66,9 @@
<ClassDef glyph="z" class="1"/> <ClassDef glyph="z" class="1"/>
</BacktrackClassDef> </BacktrackClassDef>
<InputClassDef> <InputClassDef>
<ClassDef glyph="a" class="3"/> <ClassDef glyph="a" class="1"/>
<ClassDef glyph="d" class="2"/> <ClassDef glyph="d" class="2"/>
<ClassDef glyph="n" class="1"/> <ClassDef glyph="n" class="3"/>
</InputClassDef> </InputClassDef>
<LookAheadClassDef> <LookAheadClassDef>
<ClassDef glyph="a" class="1"/> <ClassDef glyph="a" class="1"/>
@ -103,18 +103,12 @@
<!-- ChainSubClassRuleCount=0 --> <!-- ChainSubClassRuleCount=0 -->
</ChainSubClassSet> </ChainSubClassSet>
<ChainSubClassSet index="1"> <ChainSubClassSet index="1">
<!-- ChainSubClassRuleCount=0 -->
</ChainSubClassSet>
<ChainSubClassSet index="2">
<!-- ChainSubClassRuleCount=0 -->
</ChainSubClassSet>
<ChainSubClassSet index="3">
<!-- ChainSubClassRuleCount=3 --> <!-- ChainSubClassRuleCount=3 -->
<ChainSubClassRule index="0"> <ChainSubClassRule index="0">
<!-- BacktrackGlyphCount=1 --> <!-- BacktrackGlyphCount=1 -->
<Backtrack index="0" value="1"/> <Backtrack index="0" value="1"/>
<!-- InputGlyphCount=3 --> <!-- InputGlyphCount=3 -->
<Input index="0" value="1"/> <Input index="0" value="3"/>
<Input index="1" value="2"/> <Input index="1" value="2"/>
<!-- LookAheadGlyphCount=0 --> <!-- LookAheadGlyphCount=0 -->
<!-- SubstCount=0 --> <!-- SubstCount=0 -->
@ -122,7 +116,7 @@
<ChainSubClassRule index="1"> <ChainSubClassRule index="1">
<!-- BacktrackGlyphCount=0 --> <!-- BacktrackGlyphCount=0 -->
<!-- InputGlyphCount=3 --> <!-- InputGlyphCount=3 -->
<Input index="0" value="1"/> <Input index="0" value="3"/>
<Input index="1" value="2"/> <Input index="1" value="2"/>
<!-- LookAheadGlyphCount=1 --> <!-- LookAheadGlyphCount=1 -->
<LookAhead index="0" value="1"/> <LookAhead index="0" value="1"/>
@ -131,7 +125,7 @@
<ChainSubClassRule index="2"> <ChainSubClassRule index="2">
<!-- BacktrackGlyphCount=0 --> <!-- BacktrackGlyphCount=0 -->
<!-- InputGlyphCount=3 --> <!-- InputGlyphCount=3 -->
<Input index="0" value="1"/> <Input index="0" value="3"/>
<Input index="1" value="2"/> <Input index="1" value="2"/>
<!-- LookAheadGlyphCount=0 --> <!-- LookAheadGlyphCount=0 -->
<!-- SubstCount=1 --> <!-- SubstCount=1 -->
@ -141,6 +135,12 @@
</SubstLookupRecord> </SubstLookupRecord>
</ChainSubClassRule> </ChainSubClassRule>
</ChainSubClassSet> </ChainSubClassSet>
<ChainSubClassSet index="2">
<!-- ChainSubClassRuleCount=0 -->
</ChainSubClassSet>
<ChainSubClassSet index="3">
<!-- ChainSubClassRuleCount=0 -->
</ChainSubClassSet>
</ChainContextSubst> </ChainContextSubst>
</Lookup> </Lookup>
<Lookup index="1"> <Lookup index="1">

View File

@ -73,8 +73,8 @@
<ClassDef1> <ClassDef1>
</ClassDef1> </ClassDef1>
<ClassDef2> <ClassDef2>
<ClassDef glyph="g" class="2"/> <ClassDef glyph="g" class="1"/>
<ClassDef glyph="y" class="1"/> <ClassDef glyph="y" class="2"/>
</ClassDef2> </ClassDef2>
<!-- Class1Count=1 --> <!-- Class1Count=1 -->
<!-- Class2Count=3 --> <!-- Class2Count=3 -->
@ -83,6 +83,9 @@
<Value1 XAdvance="0"/> <Value1 XAdvance="0"/>
</Class2Record> </Class2Record>
<Class2Record index="1"> <Class2Record index="1">
<Value1 XAdvance="-5"/>
</Class2Record>
<Class2Record index="2">
<Value1 XAdvance="0"> <Value1 XAdvance="0">
<XAdvDevice> <XAdvDevice>
<StartSize value="0"/> <StartSize value="0"/>
@ -91,9 +94,6 @@
</XAdvDevice> </XAdvDevice>
</Value1> </Value1>
</Class2Record> </Class2Record>
<Class2Record index="2">
<Value1 XAdvance="-5"/>
</Class2Record>
</Class1Record> </Class1Record>
</PairPos> </PairPos>
</Lookup> </Lookup>

View File

@ -1080,7 +1080,7 @@ class ClassDefBuilderTest(object):
b.add({"e", "f", "g", "h"}) b.add({"e", "f", "g", "h"})
cdef = b.build() cdef = b.build()
assert isinstance(cdef, otTables.ClassDef) assert isinstance(cdef, otTables.ClassDef)
assert cdef.classDefs == {"a": 2, "b": 2, "c": 3, "aa": 1, "bb": 1} assert cdef.classDefs == {"a": 1, "b": 1, "c": 3, "aa": 2, "bb": 2}
def test_build_notUsingClass0(self): def test_build_notUsingClass0(self):
b = builder.ClassDefBuilder(useClass0=False) b = builder.ClassDefBuilder(useClass0=False)