[feaLib] Merge SinglePos chain targets

Fixes https://github.com/fonttools/fonttools/issues/514.
This commit is contained in:
Sascha Brawer 2017-02-17 11:21:11 +01:00
parent fa69c64466
commit a2e7d96cf4
4 changed files with 189 additions and 5 deletions

View File

@ -933,16 +933,31 @@ class Builder(object):
for glyph in glyphs: for glyph in glyphs:
lookup.add_pos(location, glyph, value) lookup.add_pos(location, glyph, value)
def find_chainable_SinglePos_(self, lookups, glyphs, value):
"""Helper for add_single_pos_chained_()"""
for look in lookups:
if all(look.can_add(glyph, value) for glyph in glyphs):
return look
return None
def add_single_pos_chained_(self, location, prefix, suffix, pos): def add_single_pos_chained_(self, location, prefix, suffix, pos):
# https://github.com/fonttools/fonttools/issues/514
chain = self.get_lookup_(location, ChainContextPosBuilder) chain = self.get_lookup_(location, ChainContextPosBuilder)
sub = self.get_chained_lookup_(location, SinglePosBuilder) targets = []
for _, _, _, lookups in chain.rules:
for lookup in lookups:
if isinstance(lookup, SinglePosBuilder):
targets.append(lookup)
subs = [] subs = []
for glyphs, value in pos: for glyphs, value in pos:
if value is None: if value is None:
subs.append(None) subs.append(None)
continue continue
if not set(glyphs).isdisjoint(sub.mapping.keys()): otValue, _ = makeOpenTypeValueRecord(value, pairPosContext=False)
sub = self.find_chainable_SinglePos_(targets, glyphs, otValue)
if sub is None:
sub = self.get_chained_lookup_(location, SinglePosBuilder) sub = self.get_chained_lookup_(location, SinglePosBuilder)
targets.append(sub)
for glyph in glyphs: for glyph in glyphs:
sub.add_pos(location, glyph, value) sub.add_pos(location, glyph, value)
subs.append(sub) subs.append(sub)
@ -1471,8 +1486,7 @@ class SinglePosBuilder(LookupBuilder):
def add_pos(self, location, glyph, valueRecord): def add_pos(self, location, glyph, valueRecord):
otValueRecord, _ = makeOpenTypeValueRecord( otValueRecord, _ = makeOpenTypeValueRecord(
valueRecord, pairPosContext=False) valueRecord, pairPosContext=False)
curValue = self.mapping.get(glyph) if not self.can_add(glyph, otValueRecord):
if curValue is not None and curValue != otValueRecord:
otherLoc = self.locations[glyph] otherLoc = self.locations[glyph]
raise FeatureLibError( raise FeatureLibError(
'Already defined different position for glyph "%s" at %s:%d:%d' 'Already defined different position for glyph "%s" at %s:%d:%d'
@ -1482,6 +1496,11 @@ class SinglePosBuilder(LookupBuilder):
self.mapping[glyph] = otValueRecord self.mapping[glyph] = otValueRecord
self.locations[glyph] = location self.locations[glyph] = location
def can_add(self, glyph, value):
assert isinstance(value, otl.ValueRecord)
curValue = self.mapping.get(glyph)
return curValue is None or curValue == value
def equals(self, other): def equals(self, other):
return (LookupBuilder.equals(self, other) and return (LookupBuilder.equals(self, other) and
self.mapping == other.mapping) self.mapping == other.mapping)

View File

@ -63,7 +63,7 @@ class BuilderTest(unittest.TestCase):
spec9a spec9b spec9c1 spec9c2 spec9c3 spec9d spec9e spec9f spec9g spec9a spec9b spec9c1 spec9c2 spec9c3 spec9d spec9e spec9f spec9g
spec10 spec10
bug453 bug457 bug463 bug501 bug502 bug504 bug505 bug506 bug509 bug453 bug457 bug463 bug501 bug502 bug504 bug505 bug506 bug509
bug512 bug568 bug633 bug512 bug514 bug568 bug633
name size size2 multiple_feature_blocks omitted_GlyphClassDef name size size2 multiple_feature_blocks omitted_GlyphClassDef
ZeroValue_SinglePos ZeroValue_SinglePos
ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical ZeroValue_PairPos_horizontal ZeroValue_PairPos_vertical

View File

@ -0,0 +1,11 @@
# The many chain targets for this feature should get combined into
# two separate SinglePos lookups: {A:-40, B:-40, C:-40} and {A:-111}.
# https://github.com/fonttools/fonttools/issues/514
#
# makeotf produces {A:-40, B:-40, C:-40} and {A:-111, B:-40} which
# is redundant. https://github.com/adobe-type-tools/afdko/issues/169
feature test {
pos X [A-B]' -40 B' -40 A' -40 Y;
pos X A' -111 Y;
pos X B' -40 A' -111 [A-C]' -40 Y;
} test;

View File

@ -0,0 +1,154 @@
<?xml version="1.0" encoding="UTF-8"?>
<ttFont sfntVersion="true" ttLibVersion="3.7">
<GPOS>
<Version value="0x00010000"/>
<ScriptList>
<!-- ScriptCount=1 -->
<ScriptRecord index="0">
<ScriptTag value="DFLT"/>
<Script>
<DefaultLangSys>
<ReqFeatureIndex value="65535"/>
<!-- FeatureCount=1 -->
<FeatureIndex index="0" value="0"/>
</DefaultLangSys>
<!-- LangSysCount=0 -->
</Script>
</ScriptRecord>
</ScriptList>
<FeatureList>
<!-- FeatureCount=1 -->
<FeatureRecord index="0">
<FeatureTag value="test"/>
<Feature>
<!-- LookupCount=1 -->
<LookupListIndex index="0" value="0"/>
</Feature>
</FeatureRecord>
</FeatureList>
<LookupList>
<!-- LookupCount=3 -->
<Lookup index="0">
<LookupType value="8"/>
<LookupFlag value="0"/>
<!-- SubTableCount=3 -->
<ChainContextPos index="0" Format="3">
<!-- BacktrackGlyphCount=1 -->
<BacktrackCoverage index="0">
<Glyph value="X"/>
</BacktrackCoverage>
<!-- InputGlyphCount=3 -->
<InputCoverage index="0">
<Glyph value="A"/>
<Glyph value="B"/>
</InputCoverage>
<InputCoverage index="1">
<Glyph value="B"/>
</InputCoverage>
<InputCoverage index="2">
<Glyph value="A"/>
</InputCoverage>
<!-- LookAheadGlyphCount=1 -->
<LookAheadCoverage index="0">
<Glyph value="Y"/>
</LookAheadCoverage>
<!-- PosCount=3 -->
<PosLookupRecord index="0">
<SequenceIndex value="0"/>
<LookupListIndex value="1"/>
</PosLookupRecord>
<PosLookupRecord index="1">
<SequenceIndex value="1"/>
<LookupListIndex value="1"/>
</PosLookupRecord>
<PosLookupRecord index="2">
<SequenceIndex value="2"/>
<LookupListIndex value="1"/>
</PosLookupRecord>
</ChainContextPos>
<ChainContextPos index="1" Format="3">
<!-- BacktrackGlyphCount=1 -->
<BacktrackCoverage index="0">
<Glyph value="X"/>
</BacktrackCoverage>
<!-- InputGlyphCount=1 -->
<InputCoverage index="0">
<Glyph value="A"/>
</InputCoverage>
<!-- LookAheadGlyphCount=1 -->
<LookAheadCoverage index="0">
<Glyph value="Y"/>
</LookAheadCoverage>
<!-- PosCount=1 -->
<PosLookupRecord index="0">
<SequenceIndex value="0"/>
<LookupListIndex value="2"/>
</PosLookupRecord>
</ChainContextPos>
<ChainContextPos index="2" Format="3">
<!-- BacktrackGlyphCount=1 -->
<BacktrackCoverage index="0">
<Glyph value="X"/>
</BacktrackCoverage>
<!-- InputGlyphCount=3 -->
<InputCoverage index="0">
<Glyph value="B"/>
</InputCoverage>
<InputCoverage index="1">
<Glyph value="A"/>
</InputCoverage>
<InputCoverage index="2">
<Glyph value="A"/>
<Glyph value="B"/>
<Glyph value="C"/>
</InputCoverage>
<!-- LookAheadGlyphCount=1 -->
<LookAheadCoverage index="0">
<Glyph value="Y"/>
</LookAheadCoverage>
<!-- PosCount=3 -->
<PosLookupRecord index="0">
<SequenceIndex value="0"/>
<LookupListIndex value="1"/>
</PosLookupRecord>
<PosLookupRecord index="1">
<SequenceIndex value="1"/>
<LookupListIndex value="2"/>
</PosLookupRecord>
<PosLookupRecord index="2">
<SequenceIndex value="2"/>
<LookupListIndex value="1"/>
</PosLookupRecord>
</ChainContextPos>
</Lookup>
<Lookup index="1">
<LookupType value="1"/>
<LookupFlag value="0"/>
<!-- SubTableCount=1 -->
<SinglePos index="0" Format="1">
<Coverage>
<Glyph value="A"/>
<Glyph value="B"/>
<Glyph value="C"/>
</Coverage>
<ValueFormat value="4"/>
<Value XAdvance="-40"/>
</SinglePos>
</Lookup>
<Lookup index="2">
<LookupType value="1"/>
<LookupFlag value="0"/>
<!-- SubTableCount=1 -->
<SinglePos index="0" Format="1">
<Coverage>
<Glyph value="A"/>
</Coverage>
<ValueFormat value="4"/>
<Value XAdvance="-111"/>
</SinglePos>
</Lookup>
</LookupList>
</GPOS>
</ttFont>