Skip to content

Commit

Permalink
Fix a couple otfautohint bugs found by testing with Momochidori (#1751)
Browse files Browse the repository at this point in the history 
Add a flag to make the overlap mapping looser
  • Loading branch information
@skef
skef authored Jul 11, 2024
1 parent e8d9fa3 commit ed6287d
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 22 deletions.
10 changes: 10 additions & 0 deletions python/afdko/otfautohint/__main__.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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:
Expand Down Expand Up @@ -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',
Expand Down Expand Up @@ -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:
Expand Down
1 change: 1 addition & 0 deletions python/afdko/otfautohint/autohint.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
39 changes: 22 additions & 17 deletions python/afdko/otfautohint/glyphData.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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):
Expand Down Expand Up @@ -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:
Expand Down Expand Up @@ -1264,19 +1266,21 @@ 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
else:
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:
Expand All @@ -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)
Expand All @@ -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:
Expand All @@ -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:
Expand All @@ -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:
Expand Down
9 changes: 4 additions & 5 deletions python/afdko/otfautohint/hinter.py
View file
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down Expand Up @@ -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
Expand Down Expand Up @@ -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()):
Expand Down

0 comments on commit ed6287d

Please sign in to comment.