From 24264cb7f82df5ce6c8d6e409d4e649a08702e98 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Wed, 1 Apr 2015 18:39:25 -0700 Subject: [PATCH] [subset] Fix chaos handling to be more inclusive If a contextual lookup recurses twice on the same index, that index most be marked chaotic for the second recursion. Also, when a non-1-to-1 recursion happens, only mark glyph locations after current to be chaotic, not everything. I believe this fixes a bug that before we were not inclusive enough. Now we might have introduced more false positives, but we are at least correct. --- Lib/fontTools/subset.py | 30 ++++++++++++++++++------------ 1 file changed, 18 insertions(+), 12 deletions(-) diff --git a/Lib/fontTools/subset.py b/Lib/fontTools/subset.py index 66b8adf1f..1a8b16987 100644 --- a/Lib/fontTools/subset.py +++ b/Lib/fontTools/subset.py @@ -854,12 +854,12 @@ def closure_glyphs(self, s, cur_glyphs): if not all(all(c.Intersect(s.glyphs, cd, k) for k in klist) for cd,klist in zip(ContextData, c.RuleData(r))): continue - chaos = False + chaos = set() for ll in getattr(r, c.LookupRecord): if not ll: continue seqi = ll.SequenceIndex - # XXX Go into chaos mode upon duplicate seqi - if chaos: + if seqi in chaos: + # TODO Can we improve this? pos_glyphs = None else: if seqi == 0: @@ -867,7 +867,9 @@ def closure_glyphs(self, s, cur_glyphs): else: pos_glyphs = frozenset([r.Input[seqi - 1]]) lookup = s.table.LookupList.Lookup[ll.LookupListIndex] - chaos = chaos or lookup.may_have_non_1to1() + chaos.add(seqi) + if lookup.may_have_non_1to1(): + chaos.update(range(seqi, len(r.Input)+2)) recursions.add((lookup, pos_glyphs)) elif self.Format == 2: ClassDef = getattr(self, c.ClassDef) @@ -882,12 +884,12 @@ def closure_glyphs(self, s, cur_glyphs): if not all(all(c.Intersect(s.glyphs, cd, k) for k in klist) for cd,klist in zip(ContextData, c.RuleData(r))): continue - chaos = False + chaos = set() for ll in getattr(r, c.LookupRecord): if not ll: continue seqi = ll.SequenceIndex - # XXX Go into chaos mode upon duplicate seqi - if chaos: + if seqi in chaos: + # TODO Can we improve this? pos_glyphs = None else: if seqi == 0: @@ -895,19 +897,21 @@ def closure_glyphs(self, s, cur_glyphs): else: pos_glyphs = frozenset(ClassDef.intersect_class(s.glyphs, getattr(r, c.Input)[seqi - 1])) lookup = s.table.LookupList.Lookup[ll.LookupListIndex] - chaos = chaos or lookup.may_have_non_1to1() + chaos.add(seqi) + if lookup.may_have_non_1to1(): + chaos.update(range(seqi, len(getattr(r, c.Input))+2)) recursions.add((lookup, pos_glyphs)) elif self.Format == 3: cur_glyphs = frozenset(cur_glyphs) if not all(x.intersect(s.glyphs) for x in c.RuleData(self)): return [] r = self - chaos = False + chaos = set() for ll in getattr(r, c.LookupRecord): if not ll: continue seqi = ll.SequenceIndex - # XXX Go into chaos mode upon duplicate seqi - if chaos: + if seqi in chaos: + # TODO Can we improve this? pos_glyphs = None else: if seqi == 0: @@ -915,7 +919,9 @@ def closure_glyphs(self, s, cur_glyphs): else: pos_glyphs = frozenset(r.InputCoverage[seqi].intersect_glyphs(s.glyphs)) lookup = s.table.LookupList.Lookup[ll.LookupListIndex] - chaos = chaos or lookup.may_have_non_1to1() + chaos.add(seqi) + if lookup.may_have_non_1to1(): + chaos.update(range(seqi, len(r.InputCoverage)+1)) recursions.add((lookup, pos_glyphs)) else: assert 0, "unknown format: %s" % self.Format