From ed6287d9a670642da24feb58e5cfb89e24366740 Mon Sep 17 00:00:00 2001 From: skef <6175836+skef@users.noreply.github.com> Date: Thu, 11 Jul 2024 15:00:12 -0700 Subject: [PATCH] Fix a couple otfautohint bugs found by testing with Momochidori (#1751) Add a flag to make the overlap mapping looser --- python/afdko/otfautohint/__main__.py | 10 +++++++ python/afdko/otfautohint/autohint.py | 1 + python/afdko/otfautohint/glyphData.py | 39 +++++++++++++++------------ python/afdko/otfautohint/hinter.py | 9 +++---- 4 files changed, 37 insertions(+), 22 deletions(-) diff --git a/python/afdko/otfautohint/__main__.py b/python/afdko/otfautohint/__main__.py index ee21d3cc1..a29d692f7 100644 --- a/python/afdko/otfautohint/__main__.py +++ b/python/afdko/otfautohint/__main__.py @@ -343,6 +343,7 @@ def __init__(self, pargs): self.writeToDefaultLayer = pargs.write_to_default_layer self.maxSegments = pargs.max_segments self.verbose = pargs.verbose + self.looseOverlapMapping = pargs.loose_overlap_mapping if pargs.force_overlap: self.overlapForcing = True elif pargs.force_no_overlap: @@ -581,6 +582,14 @@ def add_common_options(parser, term, name): 'default when hinting individual, non-variable fonts and not ' 'supplying an overlap list)' ) + overlap_parser.add_argument( + '--loose-overlap-mapping', + action='store_true', + help='Some fonts see high numbers of "Unable to map derived path ' + 'element from ..." warnings when processing overlap. This flag ' + 'loosens the matching heuristics and should lower (but not ' + 'eliminate) the number of those warnings' + ) parser.add_argument( '--log', metavar='PATH', @@ -865,6 +874,7 @@ def __init__(self, pargs): self.report_zones = pargs.alignment_zones self.report_all_stems = pargs.all_stems self.verbose = pargs.verbose + self.looseOverlapMapping = pargs.loose_overlap_mapping if pargs.force_overlap: self.overlapForcing = True elif pargs.force_no_overlap: diff --git a/python/afdko/otfautohint/autohint.py b/python/afdko/otfautohint/autohint.py index 286da23e1..9fad62f7e 100644 --- a/python/afdko/otfautohint/autohint.py +++ b/python/afdko/otfautohint/autohint.py @@ -37,6 +37,7 @@ def __init__(self): self.excludeGlyphList = False self.overlapList = [] self.overlapForcing = None + self.looseOverlapMapping = False self.hintAll = False self.readHints = True self.allowChanges = False diff --git a/python/afdko/otfautohint/glyphData.py b/python/afdko/otfautohint/glyphData.py index 9df302d7a..476b27e53 100644 --- a/python/afdko/otfautohint/glyphData.py +++ b/python/afdko/otfautohint/glyphData.py @@ -515,6 +515,7 @@ class pathElement: """ assocMatchFactor = 93 tSlop = .005 + middleMult = 2 def __init__(self, *args, is_close=False, masks=None, flex=False, position=None): @@ -625,12 +626,13 @@ def cubicParameters(self): """Returns the fontTools cubic parameters for this pathElement""" return calcCubicParameters(self.s, self.cs, self.ce, self.e) - def getAssocFactor(self): + def getAssocFactor(self, loose=False): if self.is_line: l = sqrt(self.s.distsq(self.e)) else: l = approximateCubicArcLength(self.s, self.cs, self.ce, self.e) - return l / self.assocMatchFactor + return l / (self.assocMatchFactor / 2 + if loose else self.assocMatchFactor) def containsPoint(self, p, factor, returnT=False): if self.is_line: @@ -1264,11 +1266,12 @@ def __iter__(self): # utility - def checkAssocPoint(self, segs, spe, ope, sp, op, mapEnd, factor=None): + def checkAssocPoint(self, segs, spe, ope, sp, op, mapEnd, loose, + factor=None): if factor is None: - factor = ope.getAssocFactor() + factor = ope.getAssocFactor(loose) if sp == op or ope.containsPoint(sp, factor): - if not ope.containsPoint(spe.atT(.5), factor): + if not ope.containsPoint(spe.atT(.5), factor * ope.middleMult): return False spe.association = ope return True @@ -1276,7 +1279,8 @@ def checkAssocPoint(self, segs, spe, ope, sp, op, mapEnd, factor=None): does, t = spe.containsPoint(op, factor, True) if does and spe.tSlop < t < 1 - spe.tSlop: midMapped = (1 - (1 - t) / 2) if mapEnd else t / 2 - if not ope.containsPoint(spe.atT(midMapped), factor): + if not ope.containsPoint(spe.atT(midMapped), + factor * ope.middleMult): return False following = spe.splitAt(t) if mapEnd: @@ -1293,7 +1297,7 @@ def checkAssocPoint(self, segs, spe, ope, sp, op, mapEnd, factor=None): return True return False - def associatePath(self, orig): + def associatePath(self, orig, loose=False): peMap = defaultdict(list) for oc in orig: peMap[tuple(oc.e.round(1))].append(oc) @@ -1309,13 +1313,14 @@ def associatePath(self, orig): oepel = peMap.get(tuple(c.e.round(1)), []) if oepel: for oepe in oepel: - if self.checkAssocPoint(segs, c, oepe, c.s, oepe.s, True): + if self.checkAssocPoint(segs, c, oepe, c.s, oepe.s, True, + loose): done = True break else: oepen = orig.nextInSubpath(oepe) if self.checkAssocPoint(segs, c, oepen, c.s, oepen.e, - True): + True, loose): done = True break if done: @@ -1325,24 +1330,24 @@ def associatePath(self, orig): for ospe in ospel: ospen = orig.nextInSubpath(ospe) if self.checkAssocPoint(segs, c, ospen, c.e, ospen.e, - False): + False, loose): done = True break elif self.checkAssocPoint(segs, c, ospe, c.e, ospe.s, - False): + False, loose): done = True break if done: continue cBounds = c.getBounds() for oc in orig: - factor = oc.getAssocFactor() + factor = oc.getAssocFactor(loose) if cBounds.intersects(oc.getBounds(), factor): if (oc.containsPoint(c.s, factor) and (self.checkAssocPoint(segs, c, oc, c.e, oc.e, - False, factor) or + False, loose, factor) or self.checkAssocPoint(segs, c, oc, c.e, oc.s, - False, factor))): + False, loose, factor))): done = True break if done: @@ -1351,13 +1356,13 @@ def associatePath(self, orig): # was very close to but not quite identical to another start point. # Try again from the other end. for oc in orig: - factor = oc.getAssocFactor() + factor = oc.getAssocFactor(loose) if cBounds.intersects(oc.getBounds(), factor): if (oc.containsPoint(c.e, factor) and (self.checkAssocPoint(segs, c, oc, c.s, oc.s, - True, factor) or + True, loose, factor) or self.checkAssocPoint(segs, c, oc, c.s, oc.e, - True, factor))): + True, loose, factor))): done = True break if done: diff --git a/python/afdko/otfautohint/hinter.py b/python/afdko/otfautohint/hinter.py index a21565387..740ad6846 100644 --- a/python/afdko/otfautohint/hinter.py +++ b/python/afdko/otfautohint/hinter.py @@ -667,8 +667,7 @@ def extremaSegment(self, pe, extp, extt, isMn): of the segment are within ExtremaDist of pe """ a, b, c, d = pe.cubicParameters() - loc = round(extp.o) + (-self.ExtremaDist - if isMn else self.ExtremaDist) + loc = extp.o + (-self.ExtremaDist if isMn else self.ExtremaDist) horiz = not self.isV() # When finding vertical stems solve for x sl = solveCubic(a[horiz], b[horiz], c[horiz], d[horiz] - loc) @@ -776,7 +775,8 @@ def genSegs(self): origGlyph = None self.hs.overlapRemoved = False else: - self.glyph.associatePath(origGlyph) + self.glyph.associatePath(origGlyph, + self.options.looseOverlapMapping) self.prepForSegs() self.Bonus = 0 @@ -2446,11 +2446,10 @@ def compatiblePaths(self, gllist, fddicts): gp = g.subpaths[si] dpl, gpl = len(dp), len(gp) if gpl != dpl: - # XXX decide on warning message for these if (gpl == dpl + 1 and gp[-1].isClose() and not dp[-1].isClose()): for _gi in range(i + 1): - gllist[i].addNullClose(si) + gllist[_gi].addNullClose(si) continue if (dpl == gpl + 1 and dp[-1].isClose() and not gp[-1].isClose()):