Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix a couple otfautohint bugs found by testing with Momochidori #1751

Merged
merged 1 commit into from
Jul 11, 2024
Merged

Fix a couple otfautohint bugs found by testing with Momochidori #1751

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 10 additions & 0 deletions python/afdko/otfautohint/__main__.py
View file
Open in desktop
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
Open in desktop
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
Open in desktop
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
Open in desktop
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
Loading