diff --git a/Lib/fontTools/feaLib/ast.py b/Lib/fontTools/feaLib/ast.py index 763d0d2c6..4672eb089 100644 --- a/Lib/fontTools/feaLib/ast.py +++ b/Lib/fontTools/feaLib/ast.py @@ -1258,9 +1258,15 @@ class MultipleSubstStatement(Statement): """Calls the builder object's ``add_multiple_subst`` callback.""" prefix = [p.glyphSet() for p in self.prefix] suffix = [s.glyphSet() for s in self.suffix] - builder.add_multiple_subst( - self.location, prefix, self.glyph, suffix, self.replacement, self.forceChain - ) + if not self.replacement and hasattr(self.glyph, "glyphSet"): + for glyph in self.glyph.glyphSet(): + builder.add_multiple_subst( + self.location, prefix, glyph, suffix, self.replacement, self.forceChain + ) + else: + builder.add_multiple_subst( + self.location, prefix, self.glyph, suffix, self.replacement, self.forceChain + ) def asFea(self, indent=""): res = "sub " diff --git a/Lib/fontTools/feaLib/parser.py b/Lib/fontTools/feaLib/parser.py index c4bdc55ee..199d9b3bf 100644 --- a/Lib/fontTools/feaLib/parser.py +++ b/Lib/fontTools/feaLib/parser.py @@ -892,16 +892,26 @@ class Parser(object): old, new, old_prefix, old_suffix, forceChain=hasMarks, location=location ) + # Glyph deletion, built as GSUB lookup type 2: Multiple substitution + # with empty replacement. + if is_deletion and len(old) == 1 and num_lookups == 0: + return self.ast.MultipleSubstStatement( + old_prefix, + old[0], + old_suffix, + (), + forceChain=hasMarks, + location=location, + ) + # GSUB lookup type 2: Multiple substitution. # Format: "substitute f_f_i by f f i;" if ( not reverse and len(old) == 1 and len(old[0].glyphSet()) == 1 - and ( - (len(new) > 1 and max([len(n.glyphSet()) for n in new]) == 1) - or len(new) == 0 - ) + and len(new) > 1 + and max([len(n.glyphSet()) for n in new]) == 1 and num_lookups == 0 ): return self.ast.MultipleSubstStatement( diff --git a/Lib/fontTools/ttLib/removeOverlaps.py b/Lib/fontTools/ttLib/removeOverlaps.py index fb5c77ab6..bb6674673 100644 --- a/Lib/fontTools/ttLib/removeOverlaps.py +++ b/Lib/fontTools/ttLib/removeOverlaps.py @@ -5,8 +5,9 @@ Requires https://github.com/fonttools/skia-pathops import itertools import logging -from typing import Iterable, Optional, Mapping +from typing import Callable, Iterable, Optional, Mapping +from fontTools.misc.roundTools import otRound from fontTools.ttLib import ttFont from fontTools.ttLib.tables import _g_l_y_f from fontTools.ttLib.tables import _h_m_t_x @@ -18,6 +19,10 @@ import pathops __all__ = ["removeOverlaps"] +class RemoveOverlapsError(Exception): + pass + + log = logging.getLogger("fontTools.ttLib.removeOverlaps") _TTGlyphMapping = Mapping[str, ttFont._TTGlyph] @@ -76,6 +81,48 @@ def ttfGlyphFromSkPath(path: pathops.Path) -> _g_l_y_f.Glyph: return glyph +def _round_path( + path: pathops.Path, round: Callable[[float], float] = otRound +) -> pathops.Path: + rounded_path = pathops.Path() + for verb, points in path: + rounded_path.add(verb, *((round(p[0]), round(p[1])) for p in points)) + return rounded_path + + +def _simplify(path: pathops.Path, debugGlyphName: str) -> pathops.Path: + # skia-pathops has a bug where it sometimes fails to simplify paths when there + # are float coordinates and control points are very close to one another. + # Rounding coordinates to integers works around the bug. + # Since we are going to round glyf coordinates later on anyway, here it is + # ok(-ish) to also round before simplify. Better than failing the whole process + # for the entire font. + # https://bugs.chromium.org/p/skia/issues/detail?id=11958 + # https://github.com/google/fonts/issues/3365 + # TODO(anthrotype): remove once this Skia bug is fixed + try: + return pathops.simplify(path, clockwise=path.clockwise) + except pathops.PathOpsError: + pass + + path = _round_path(path) + try: + path = pathops.simplify(path, clockwise=path.clockwise) + log.debug( + "skia-pathops failed to simplify '%s' with float coordinates, " + "but succeded using rounded integer coordinates", + debugGlyphName, + ) + return path + except pathops.PathOpsError as e: + path.dump() + raise RemoveOverlapsError( + f"Failed to remove overlaps from glyph {debugGlyphName!r}" + ) from e + + raise AssertionError("Unreachable") + + def removeTTGlyphOverlaps( glyphName: str, glyphSet: _TTGlyphMapping, @@ -93,7 +140,7 @@ def removeTTGlyphOverlaps( path = skPathFromGlyph(glyphName, glyphSet) # remove overlaps - path2 = pathops.simplify(path, clockwise=path.clockwise) + path2 = _simplify(path, glyphName) # replace TTGlyph if simplified path is different (ignoring contour order) if {tuple(c) for c in path.contours} != {tuple(c) for c in path2.contours}: diff --git a/Lib/fontTools/ttLib/tables/_g_l_y_f.py b/Lib/fontTools/ttLib/tables/_g_l_y_f.py index 2447b6881..e3bbddb9c 100644 --- a/Lib/fontTools/ttLib/tables/_g_l_y_f.py +++ b/Lib/fontTools/ttLib/tables/_g_l_y_f.py @@ -25,6 +25,7 @@ import logging import os from fontTools.misc import xmlWriter from fontTools.misc.filenames import userNameToFileName +from fontTools.misc.loggingTools import deprecateFunction log = logging.getLogger(__name__) @@ -412,9 +413,13 @@ class table__g_l_y_f(DefaultTable.DefaultTable): topSideY = defaultVerticalOrigin else: topSideY = verticalAdvanceWidth + glyph = self[glyphName] + glyph.recalcBounds(self) + topSideBearing = otRound(topSideY - glyph.yMax) vMetrics = {glyphName: (verticalAdvanceWidth, topSideBearing)} return vMetrics + @deprecateFunction("use '_getPhantomPoints' instead", category=DeprecationWarning) def getPhantomPoints(self, glyphName, ttFont, defaultVerticalOrigin=None): """Old public name for self._getPhantomPoints(). See: https://github.com/fonttools/fonttools/pull/2266""" @@ -422,6 +427,7 @@ class table__g_l_y_f(DefaultTable.DefaultTable): vMetrics = self._synthesizeVMetrics(glyphName, ttFont, defaultVerticalOrigin) return self._getPhantomPoints(glyphName, hMetrics, vMetrics) + @deprecateFunction("use '_getCoordinatesAndControls' instead", category=DeprecationWarning) def getCoordinatesAndControls(self, glyphName, ttFont, defaultVerticalOrigin=None): """Old public name for self._getCoordinatesAndControls(). See: https://github.com/fonttools/fonttools/pull/2266""" @@ -429,6 +435,7 @@ class table__g_l_y_f(DefaultTable.DefaultTable): vMetrics = self._synthesizeVMetrics(glyphName, ttFont, defaultVerticalOrigin) return self._getCoordinatesAndControls(glyphName, hMetrics, vMetrics) + @deprecateFunction("use '_setCoordinates' instead", category=DeprecationWarning) def setCoordinates(self, glyphName, ttFont): """Old public name for self._setCoordinates(). See: https://github.com/fonttools/fonttools/pull/2266""" diff --git a/Lib/fontTools/varLib/models.py b/Lib/fontTools/varLib/models.py index 1950e6247..54063a90f 100644 --- a/Lib/fontTools/varLib/models.py +++ b/Lib/fontTools/varLib/models.py @@ -1,160 +1,174 @@ """Variation fonts interpolation models.""" -__all__ = ['nonNone', 'allNone', 'allEqual', 'allEqualTo', 'subList', - 'normalizeValue', 'normalizeLocation', - 'supportScalar', - 'VariationModel'] +__all__ = [ + "nonNone", + "allNone", + "allEqual", + "allEqualTo", + "subList", + "normalizeValue", + "normalizeLocation", + "supportScalar", + "VariationModel", +] from fontTools.misc.roundTools import noRound from .errors import VariationModelError def nonNone(lst): - return [l for l in lst if l is not None] + return [l for l in lst if l is not None] + def allNone(lst): - return all(l is None for l in lst) + return all(l is None for l in lst) + def allEqualTo(ref, lst, mapper=None): - if mapper is None: - return all(ref == item for item in lst) - else: - mapped = mapper(ref) - return all(mapped == mapper(item) for item in lst) + if mapper is None: + return all(ref == item for item in lst) + + mapped = mapper(ref) + return all(mapped == mapper(item) for item in lst) + def allEqual(lst, mapper=None): - if not lst: - return True - it = iter(lst) - try: - first = next(it) - except StopIteration: - return True - return allEqualTo(first, it, mapper=mapper) + if not lst: + return True + it = iter(lst) + try: + first = next(it) + except StopIteration: + return True + return allEqualTo(first, it, mapper=mapper) + def subList(truth, lst): - assert len(truth) == len(lst) - return [l for l,t in zip(lst,truth) if t] + assert len(truth) == len(lst) + return [l for l, t in zip(lst, truth) if t] + def normalizeValue(v, triple): - """Normalizes value based on a min/default/max triple. - >>> normalizeValue(400, (100, 400, 900)) - 0.0 - >>> normalizeValue(100, (100, 400, 900)) - -1.0 - >>> normalizeValue(650, (100, 400, 900)) - 0.5 - """ - lower, default, upper = triple - if not (lower <= default <= upper): - raise ValueError( - f"Invalid axis values, must be minimum, default, maximum: " - f"{lower:3.3f}, {default:3.3f}, {upper:3.3f}" - ) - v = max(min(v, upper), lower) - if v == default: - v = 0. - elif v < default: - v = (v - default) / (default - lower) - else: - v = (v - default) / (upper - default) - return v + """Normalizes value based on a min/default/max triple. + >>> normalizeValue(400, (100, 400, 900)) + 0.0 + >>> normalizeValue(100, (100, 400, 900)) + -1.0 + >>> normalizeValue(650, (100, 400, 900)) + 0.5 + """ + lower, default, upper = triple + if not (lower <= default <= upper): + raise ValueError( + f"Invalid axis values, must be minimum, default, maximum: " + f"{lower:3.3f}, {default:3.3f}, {upper:3.3f}" + ) + v = max(min(v, upper), lower) + if v == default: + v = 0.0 + elif v < default: + v = (v - default) / (default - lower) + else: + v = (v - default) / (upper - default) + return v + def normalizeLocation(location, axes): - """Normalizes location based on axis min/default/max values from axes. - >>> axes = {"wght": (100, 400, 900)} - >>> normalizeLocation({"wght": 400}, axes) - {'wght': 0.0} - >>> normalizeLocation({"wght": 100}, axes) - {'wght': -1.0} - >>> normalizeLocation({"wght": 900}, axes) - {'wght': 1.0} - >>> normalizeLocation({"wght": 650}, axes) - {'wght': 0.5} - >>> normalizeLocation({"wght": 1000}, axes) - {'wght': 1.0} - >>> normalizeLocation({"wght": 0}, axes) - {'wght': -1.0} - >>> axes = {"wght": (0, 0, 1000)} - >>> normalizeLocation({"wght": 0}, axes) - {'wght': 0.0} - >>> normalizeLocation({"wght": -1}, axes) - {'wght': 0.0} - >>> normalizeLocation({"wght": 1000}, axes) - {'wght': 1.0} - >>> normalizeLocation({"wght": 500}, axes) - {'wght': 0.5} - >>> normalizeLocation({"wght": 1001}, axes) - {'wght': 1.0} - >>> axes = {"wght": (0, 1000, 1000)} - >>> normalizeLocation({"wght": 0}, axes) - {'wght': -1.0} - >>> normalizeLocation({"wght": -1}, axes) - {'wght': -1.0} - >>> normalizeLocation({"wght": 500}, axes) - {'wght': -0.5} - >>> normalizeLocation({"wght": 1000}, axes) - {'wght': 0.0} - >>> normalizeLocation({"wght": 1001}, axes) - {'wght': 0.0} - """ - out = {} - for tag,triple in axes.items(): - v = location.get(tag, triple[1]) - out[tag] = normalizeValue(v, triple) - return out + """Normalizes location based on axis min/default/max values from axes. + >>> axes = {"wght": (100, 400, 900)} + >>> normalizeLocation({"wght": 400}, axes) + {'wght': 0.0} + >>> normalizeLocation({"wght": 100}, axes) + {'wght': -1.0} + >>> normalizeLocation({"wght": 900}, axes) + {'wght': 1.0} + >>> normalizeLocation({"wght": 650}, axes) + {'wght': 0.5} + >>> normalizeLocation({"wght": 1000}, axes) + {'wght': 1.0} + >>> normalizeLocation({"wght": 0}, axes) + {'wght': -1.0} + >>> axes = {"wght": (0, 0, 1000)} + >>> normalizeLocation({"wght": 0}, axes) + {'wght': 0.0} + >>> normalizeLocation({"wght": -1}, axes) + {'wght': 0.0} + >>> normalizeLocation({"wght": 1000}, axes) + {'wght': 1.0} + >>> normalizeLocation({"wght": 500}, axes) + {'wght': 0.5} + >>> normalizeLocation({"wght": 1001}, axes) + {'wght': 1.0} + >>> axes = {"wght": (0, 1000, 1000)} + >>> normalizeLocation({"wght": 0}, axes) + {'wght': -1.0} + >>> normalizeLocation({"wght": -1}, axes) + {'wght': -1.0} + >>> normalizeLocation({"wght": 500}, axes) + {'wght': -0.5} + >>> normalizeLocation({"wght": 1000}, axes) + {'wght': 0.0} + >>> normalizeLocation({"wght": 1001}, axes) + {'wght': 0.0} + """ + out = {} + for tag, triple in axes.items(): + v = location.get(tag, triple[1]) + out[tag] = normalizeValue(v, triple) + return out + def supportScalar(location, support, ot=True): - """Returns the scalar multiplier at location, for a master - with support. If ot is True, then a peak value of zero - for support of an axis means "axis does not participate". That - is how OpenType Variation Font technology works. - >>> supportScalar({}, {}) - 1.0 - >>> supportScalar({'wght':.2}, {}) - 1.0 - >>> supportScalar({'wght':.2}, {'wght':(0,2,3)}) - 0.1 - >>> supportScalar({'wght':2.5}, {'wght':(0,2,4)}) - 0.75 - >>> supportScalar({'wght':2.5, 'wdth':0}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) - 0.75 - >>> supportScalar({'wght':2.5, 'wdth':.5}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}, ot=False) - 0.375 - >>> supportScalar({'wght':2.5, 'wdth':0}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) - 0.75 - >>> supportScalar({'wght':2.5, 'wdth':.5}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) - 0.75 - """ - scalar = 1. - for axis,(lower,peak,upper) in support.items(): - if ot: - # OpenType-specific case handling - if peak == 0.: - continue - if lower > peak or peak > upper: - continue - if lower < 0. and upper > 0.: - continue - v = location.get(axis, 0.) - else: - assert axis in location - v = location[axis] - if v == peak: - continue - if v <= lower or upper <= v: - scalar = 0. - break - if v < peak: - scalar *= (v - lower) / (peak - lower) - else: # v > peak - scalar *= (v - upper) / (peak - upper) - return scalar + """Returns the scalar multiplier at location, for a master + with support. If ot is True, then a peak value of zero + for support of an axis means "axis does not participate". That + is how OpenType Variation Font technology works. + >>> supportScalar({}, {}) + 1.0 + >>> supportScalar({'wght':.2}, {}) + 1.0 + >>> supportScalar({'wght':.2}, {'wght':(0,2,3)}) + 0.1 + >>> supportScalar({'wght':2.5}, {'wght':(0,2,4)}) + 0.75 + >>> supportScalar({'wght':2.5, 'wdth':0}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) + 0.75 + >>> supportScalar({'wght':2.5, 'wdth':.5}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}, ot=False) + 0.375 + >>> supportScalar({'wght':2.5, 'wdth':0}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) + 0.75 + >>> supportScalar({'wght':2.5, 'wdth':.5}, {'wght':(0,2,4), 'wdth':(-1,0,+1)}) + 0.75 + """ + scalar = 1.0 + for axis, (lower, peak, upper) in support.items(): + if ot: + # OpenType-specific case handling + if peak == 0.0: + continue + if lower > peak or peak > upper: + continue + if lower < 0.0 and upper > 0.0: + continue + v = location.get(axis, 0.0) + else: + assert axis in location + v = location[axis] + if v == peak: + continue + if v <= lower or upper <= v: + scalar = 0.0 + break + if v < peak: + scalar *= (v - lower) / (peak - lower) + else: # v > peak + scalar *= (v - upper) / (peak - upper) + return scalar class VariationModel(object): - """ + """ Locations must be in normalized space. Ie. base master is at origin (0). >>> from pprint import pprint @@ -197,291 +211,319 @@ class VariationModel(object): 7: 0.6666666666666667}] """ - def __init__(self, locations, axisOrder=None): - if len(set(tuple(sorted(l.items())) for l in locations)) != len(locations): - raise VariationModelError("Locations must be unique.") + def __init__(self, locations, axisOrder=None): + if len(set(tuple(sorted(l.items())) for l in locations)) != len(locations): + raise VariationModelError("Locations must be unique.") - self.origLocations = locations - self.axisOrder = axisOrder if axisOrder is not None else [] + self.origLocations = locations + self.axisOrder = axisOrder if axisOrder is not None else [] - locations = [{k:v for k,v in loc.items() if v != 0.} for loc in locations] - keyFunc = self.getMasterLocationsSortKeyFunc(locations, axisOrder=self.axisOrder) - self.locations = sorted(locations, key=keyFunc) + locations = [{k: v for k, v in loc.items() if v != 0.0} for loc in locations] + keyFunc = self.getMasterLocationsSortKeyFunc( + locations, axisOrder=self.axisOrder + ) + self.locations = sorted(locations, key=keyFunc) - # Mapping from user's master order to our master order - self.mapping = [self.locations.index(l) for l in locations] - self.reverseMapping = [locations.index(l) for l in self.locations] + # Mapping from user's master order to our master order + self.mapping = [self.locations.index(l) for l in locations] + self.reverseMapping = [locations.index(l) for l in self.locations] - self._computeMasterSupports(keyFunc.axisPoints) - self._subModels = {} + self._computeMasterSupports() + self._subModels = {} - def getSubModel(self, items): - if None not in items: - return self, items - key = tuple(v is not None for v in items) - subModel = self._subModels.get(key) - if subModel is None: - subModel = VariationModel(subList(key, self.origLocations), self.axisOrder) - self._subModels[key] = subModel - return subModel, subList(key, items) + def getSubModel(self, items): + if None not in items: + return self, items + key = tuple(v is not None for v in items) + subModel = self._subModels.get(key) + if subModel is None: + subModel = VariationModel(subList(key, self.origLocations), self.axisOrder) + self._subModels[key] = subModel + return subModel, subList(key, items) - @staticmethod - def getMasterLocationsSortKeyFunc(locations, axisOrder=[]): - if {} not in locations: - raise VariationModelError("Base master not found.") - axisPoints = {} - for loc in locations: - if len(loc) != 1: - continue - axis = next(iter(loc)) - value = loc[axis] - if axis not in axisPoints: - axisPoints[axis] = {0.} - assert value not in axisPoints[axis], ( - 'Value "%s" in axisPoints["%s"] --> %s' % (value, axis, axisPoints) - ) - axisPoints[axis].add(value) + @staticmethod + def getMasterLocationsSortKeyFunc(locations, axisOrder=[]): + if {} not in locations: + raise VariationModelError("Base master not found.") + axisPoints = {} + for loc in locations: + if len(loc) != 1: + continue + axis = next(iter(loc)) + value = loc[axis] + if axis not in axisPoints: + axisPoints[axis] = {0.0} + assert ( + value not in axisPoints[axis] + ), 'Value "%s" in axisPoints["%s"] --> %s' % (value, axis, axisPoints) + axisPoints[axis].add(value) - def getKey(axisPoints, axisOrder): - def sign(v): - return -1 if v < 0 else +1 if v > 0 else 0 - def key(loc): - rank = len(loc) - onPointAxes = [ - axis for axis, value in loc.items() - if axis in axisPoints - and value in axisPoints[axis] - ] - orderedAxes = [axis for axis in axisOrder if axis in loc] - orderedAxes.extend([axis for axis in sorted(loc.keys()) if axis not in axisOrder]) - return ( - rank, # First, order by increasing rank - -len(onPointAxes), # Next, by decreasing number of onPoint axes - tuple(axisOrder.index(axis) if axis in axisOrder else 0x10000 for axis in orderedAxes), # Next, by known axes - tuple(orderedAxes), # Next, by all axes - tuple(sign(loc[axis]) for axis in orderedAxes), # Next, by signs of axis values - tuple(abs(loc[axis]) for axis in orderedAxes), # Next, by absolute value of axis values - ) - return key + def getKey(axisPoints, axisOrder): + def sign(v): + return -1 if v < 0 else +1 if v > 0 else 0 - ret = getKey(axisPoints, axisOrder) - ret.axisPoints = axisPoints - return ret + def key(loc): + rank = len(loc) + onPointAxes = [ + axis + for axis, value in loc.items() + if axis in axisPoints and value in axisPoints[axis] + ] + orderedAxes = [axis for axis in axisOrder if axis in loc] + orderedAxes.extend( + [axis for axis in sorted(loc.keys()) if axis not in axisOrder] + ) + return ( + rank, # First, order by increasing rank + -len(onPointAxes), # Next, by decreasing number of onPoint axes + tuple( + axisOrder.index(axis) if axis in axisOrder else 0x10000 + for axis in orderedAxes + ), # Next, by known axes + tuple(orderedAxes), # Next, by all axes + tuple( + sign(loc[axis]) for axis in orderedAxes + ), # Next, by signs of axis values + tuple( + abs(loc[axis]) for axis in orderedAxes + ), # Next, by absolute value of axis values + ) - def reorderMasters(self, master_list, mapping): - # For changing the master data order without - # recomputing supports and deltaWeights. - new_list = [master_list[idx] for idx in mapping] - self.origLocations = [self.origLocations[idx] for idx in mapping] - locations = [{k:v for k,v in loc.items() if v != 0.} - for loc in self.origLocations] - self.mapping = [self.locations.index(l) for l in locations] - self.reverseMapping = [locations.index(l) for l in self.locations] - self._subModels = {} - return new_list + return key - def _computeMasterSupports(self, axisPoints): - supports = [] - regions = self._locationsToRegions() - for i,region in enumerate(regions): - locAxes = set(region.keys()) - # Walk over previous masters now - for j,prev_region in enumerate(regions[:i]): - # Master with extra axes do not participte - if not set(prev_region.keys()).issubset(locAxes): - continue - # If it's NOT in the current box, it does not participate - relevant = True - for axis, (lower,peak,upper) in region.items(): - if axis not in prev_region or not (prev_region[axis][1] == peak or lower < prev_region[axis][1] < upper): - relevant = False - break - if not relevant: - continue + ret = getKey(axisPoints, axisOrder) + return ret - # Split the box for new master; split in whatever direction - # that has largest range ratio. - # - # For symmetry, we actually cut across multiple axes - # if they have the largest, equal, ratio. - # https://github.com/fonttools/fonttools/commit/7ee81c8821671157968b097f3e55309a1faa511e#commitcomment-31054804 + def reorderMasters(self, master_list, mapping): + # For changing the master data order without + # recomputing supports and deltaWeights. + new_list = [master_list[idx] for idx in mapping] + self.origLocations = [self.origLocations[idx] for idx in mapping] + locations = [ + {k: v for k, v in loc.items() if v != 0.0} for loc in self.origLocations + ] + self.mapping = [self.locations.index(l) for l in locations] + self.reverseMapping = [locations.index(l) for l in self.locations] + self._subModels = {} + return new_list - bestAxes = {} - bestRatio = -1 - for axis in prev_region.keys(): - val = prev_region[axis][1] - assert axis in region - lower,locV,upper = region[axis] - newLower, newUpper = lower, upper - if val < locV: - newLower = val - ratio = (val - locV) / (lower - locV) - elif locV < val: - newUpper = val - ratio = (val - locV) / (upper - locV) - else: # val == locV - # Can't split box in this direction. - continue - if ratio > bestRatio: - bestAxes = {} - bestRatio = ratio - if ratio == bestRatio: - bestAxes[axis] = (newLower, locV, newUpper) + def _computeMasterSupports(self): + self.supports = [] + regions = self._locationsToRegions() + for i, region in enumerate(regions): + locAxes = set(region.keys()) + # Walk over previous masters now + for prev_region in regions[:i]: + # Master with extra axes do not participte + if not set(prev_region.keys()).issubset(locAxes): + continue + # If it's NOT in the current box, it does not participate + relevant = True + for axis, (lower, peak, upper) in region.items(): + if axis not in prev_region or not ( + prev_region[axis][1] == peak + or lower < prev_region[axis][1] < upper + ): + relevant = False + break + if not relevant: + continue - for axis,triple in bestAxes.items (): - region[axis] = triple - supports.append(region) - self.supports = supports - self._computeDeltaWeights() + # Split the box for new master; split in whatever direction + # that has largest range ratio. + # + # For symmetry, we actually cut across multiple axes + # if they have the largest, equal, ratio. + # https://github.com/fonttools/fonttools/commit/7ee81c8821671157968b097f3e55309a1faa511e#commitcomment-31054804 - def _locationsToRegions(self): - locations = self.locations - # Compute min/max across each axis, use it as total range. - # TODO Take this as input from outside? - minV = {} - maxV = {} - for l in locations: - for k,v in l.items(): - minV[k] = min(v, minV.get(k, v)) - maxV[k] = max(v, maxV.get(k, v)) + bestAxes = {} + bestRatio = -1 + for axis in prev_region.keys(): + val = prev_region[axis][1] + assert axis in region + lower, locV, upper = region[axis] + newLower, newUpper = lower, upper + if val < locV: + newLower = val + ratio = (val - locV) / (lower - locV) + elif locV < val: + newUpper = val + ratio = (val - locV) / (upper - locV) + else: # val == locV + # Can't split box in this direction. + continue + if ratio > bestRatio: + bestAxes = {} + bestRatio = ratio + if ratio == bestRatio: + bestAxes[axis] = (newLower, locV, newUpper) - regions = [] - for i,loc in enumerate(locations): - region = {} - for axis,locV in loc.items(): - if locV > 0: - region[axis] = (0, locV, maxV[axis]) - else: - region[axis] = (minV[axis], locV, 0) - regions.append(region) - return regions + for axis, triple in bestAxes.items(): + region[axis] = triple + self.supports.append(region) + self._computeDeltaWeights() - def _computeDeltaWeights(self): - deltaWeights = [] - for i,loc in enumerate(self.locations): - deltaWeight = {} - # Walk over previous masters now, populate deltaWeight - for j,m in enumerate(self.locations[:i]): - scalar = supportScalar(loc, self.supports[j]) - if scalar: - deltaWeight[j] = scalar - deltaWeights.append(deltaWeight) - self.deltaWeights = deltaWeights + def _locationsToRegions(self): + locations = self.locations + # Compute min/max across each axis, use it as total range. + # TODO Take this as input from outside? + minV = {} + maxV = {} + for l in locations: + for k, v in l.items(): + minV[k] = min(v, minV.get(k, v)) + maxV[k] = max(v, maxV.get(k, v)) - def getDeltas(self, masterValues, *, round=noRound): - assert len(masterValues) == len(self.deltaWeights) - mapping = self.reverseMapping - out = [] - for i,weights in enumerate(self.deltaWeights): - delta = masterValues[mapping[i]] - for j,weight in weights.items(): - if weight == 1: - delta -= out[j] - else: - delta -= out[j] * weight - out.append(round(delta)) - return out + regions = [] + for loc in locations: + region = {} + for axis, locV in loc.items(): + if locV > 0: + region[axis] = (0, locV, maxV[axis]) + else: + region[axis] = (minV[axis], locV, 0) + regions.append(region) + return regions - def getDeltasAndSupports(self, items, *, round=noRound): - model, items = self.getSubModel(items) - return model.getDeltas(items, round=round), model.supports + def _computeDeltaWeights(self): + self.deltaWeights = [] + for i, loc in enumerate(self.locations): + deltaWeight = {} + # Walk over previous masters now, populate deltaWeight + for j, support in enumerate(self.supports[:i]): + scalar = supportScalar(loc, support) + if scalar: + deltaWeight[j] = scalar + self.deltaWeights.append(deltaWeight) - def getScalars(self, loc): - return [supportScalar(loc, support) for support in self.supports] + def getDeltas(self, masterValues, *, round=noRound): + assert len(masterValues) == len(self.deltaWeights) + mapping = self.reverseMapping + out = [] + for i, weights in enumerate(self.deltaWeights): + delta = masterValues[mapping[i]] + for j, weight in weights.items(): + if weight == 1: + delta -= out[j] + else: + delta -= out[j] * weight + out.append(round(delta)) + return out - @staticmethod - def interpolateFromDeltasAndScalars(deltas, scalars): - v = None - assert len(deltas) == len(scalars) - for delta, scalar in zip(deltas, scalars): - if not scalar: continue - contribution = delta * scalar - if v is None: - v = contribution - else: - v += contribution - return v + def getDeltasAndSupports(self, items, *, round=noRound): + model, items = self.getSubModel(items) + return model.getDeltas(items, round=round), model.supports - def interpolateFromDeltas(self, loc, deltas): - scalars = self.getScalars(loc) - return self.interpolateFromDeltasAndScalars(deltas, scalars) + def getScalars(self, loc): + return [supportScalar(loc, support) for support in self.supports] - def interpolateFromMasters(self, loc, masterValues, *, round=noRound): - deltas = self.getDeltas(masterValues, round=round) - return self.interpolateFromDeltas(loc, deltas) + @staticmethod + def interpolateFromDeltasAndScalars(deltas, scalars): + v = None + assert len(deltas) == len(scalars) + for delta, scalar in zip(deltas, scalars): + if not scalar: + continue + contribution = delta * scalar + if v is None: + v = contribution + else: + v += contribution + return v - def interpolateFromMastersAndScalars(self, masterValues, scalars, *, round=noRound): - deltas = self.getDeltas(masterValues, round=round) - return self.interpolateFromDeltasAndScalars(deltas, scalars) + def interpolateFromDeltas(self, loc, deltas): + scalars = self.getScalars(loc) + return self.interpolateFromDeltasAndScalars(deltas, scalars) + + def interpolateFromMasters(self, loc, masterValues, *, round=noRound): + deltas = self.getDeltas(masterValues, round=round) + return self.interpolateFromDeltas(loc, deltas) + + def interpolateFromMastersAndScalars(self, masterValues, scalars, *, round=noRound): + deltas = self.getDeltas(masterValues, round=round) + return self.interpolateFromDeltasAndScalars(deltas, scalars) def piecewiseLinearMap(v, mapping): - keys = mapping.keys() - if not keys: - return v - if v in keys: - return mapping[v] - k = min(keys) - if v < k: - return v + mapping[k] - k - k = max(keys) - if v > k: - return v + mapping[k] - k - # Interpolate - a = max(k for k in keys if k < v) - b = min(k for k in keys if k > v) - va = mapping[a] - vb = mapping[b] - return va + (vb - va) * (v - a) / (b - a) + keys = mapping.keys() + if not keys: + return v + if v in keys: + return mapping[v] + k = min(keys) + if v < k: + return v + mapping[k] - k + k = max(keys) + if v > k: + return v + mapping[k] - k + # Interpolate + a = max(k for k in keys if k < v) + b = min(k for k in keys if k > v) + va = mapping[a] + vb = mapping[b] + return va + (vb - va) * (v - a) / (b - a) def main(args=None): - """Normalize locations on a given designspace""" - from fontTools import configLogger - import argparse + """Normalize locations on a given designspace""" + from fontTools import configLogger + import argparse - parser = argparse.ArgumentParser( - "fonttools varLib.models", - description=main.__doc__, - ) - parser.add_argument('--loglevel', metavar='LEVEL', default="INFO", - help="Logging level (defaults to INFO)") + parser = argparse.ArgumentParser( + "fonttools varLib.models", + description=main.__doc__, + ) + parser.add_argument( + "--loglevel", + metavar="LEVEL", + default="INFO", + help="Logging level (defaults to INFO)", + ) - group = parser.add_mutually_exclusive_group(required=True) - group.add_argument('-d', '--designspace',metavar="DESIGNSPACE",type=str) - group.add_argument('-l', '--locations', metavar='LOCATION', nargs='+', - help="Master locations as comma-separate coordinates. One must be all zeros.") + group = parser.add_mutually_exclusive_group(required=True) + group.add_argument("-d", "--designspace", metavar="DESIGNSPACE", type=str) + group.add_argument( + "-l", + "--locations", + metavar="LOCATION", + nargs="+", + help="Master locations as comma-separate coordinates. One must be all zeros.", + ) - args = parser.parse_args(args) + args = parser.parse_args(args) - configLogger(level=args.loglevel) - from pprint import pprint + configLogger(level=args.loglevel) + from pprint import pprint - if args.designspace: - from fontTools.designspaceLib import DesignSpaceDocument - doc = DesignSpaceDocument() - doc.read(args.designspace) - locs = [s.location for s in doc.sources] - print("Original locations:") - pprint(locs) - doc.normalize() - print("Normalized locations:") - locs = [s.location for s in doc.sources] - pprint(locs) - else: - axes = [chr(c) for c in range(ord('A'), ord('Z')+1)] - locs = [dict(zip(axes, (float(v) for v in s.split(',')))) for s in args.locations] + if args.designspace: + from fontTools.designspaceLib import DesignSpaceDocument + + doc = DesignSpaceDocument() + doc.read(args.designspace) + locs = [s.location for s in doc.sources] + print("Original locations:") + pprint(locs) + doc.normalize() + print("Normalized locations:") + locs = [s.location for s in doc.sources] + pprint(locs) + else: + axes = [chr(c) for c in range(ord("A"), ord("Z") + 1)] + locs = [ + dict(zip(axes, (float(v) for v in s.split(",")))) for s in args.locations + ] + + model = VariationModel(locs) + print("Sorted locations:") + pprint(model.locations) + print("Supports:") + pprint(model.supports) - model = VariationModel(locs) - print("Sorted locations:") - pprint(model.locations) - print("Supports:") - pprint(model.supports) if __name__ == "__main__": - import doctest, sys + import doctest, sys - if len(sys.argv) > 1: - sys.exit(main()) + if len(sys.argv) > 1: + sys.exit(main()) - sys.exit(doctest.testmod().failed) + sys.exit(doctest.testmod().failed) diff --git a/NEWS.rst b/NEWS.rst index f2851f3a5..39cdf760b 100644 --- a/NEWS.rst +++ b/NEWS.rst @@ -1,3 +1,16 @@ +- [removeOverlaps] Retry pathops.simplify after rounding path coordinates to integers + if it fails the first time using floats, to work around a rare and hard to debug + Skia bug (#2288). +- [varLib] Added support for building, reading, writing and optimizing 32-bit + ``ItemVariationStore`` as used in COLRv1 table (#2285). +- [otBase/otConverters] Add array readers/writers for int types (#2285). +- [feaLib] Allow more than one lookahead glyph/class in contextual positioning with + "value at end" (#2293, #2294). +- [COLRv1] Default varIdx should be 0xFFFFFFFF (#2297, #2298). +- [pens] Make RecordingPointPen actually pass on identifiers; replace asserts with + explicit ``PenError`` exception (#2284). +- [mutator] Round lsb for CF2 fonts as well (#2286). + 4.22.1 (released 2021-04-26) ---------------------------- diff --git a/Tests/feaLib/data/delete_glyph.fea b/Tests/feaLib/data/delete_glyph.fea index 36e0f0f9a..ab4b93f11 100644 --- a/Tests/feaLib/data/delete_glyph.fea +++ b/Tests/feaLib/data/delete_glyph.fea @@ -1,3 +1,7 @@ feature test { sub a by NULL; } test; + +feature test { + sub [a b c] by NULL; +} test; diff --git a/Tests/feaLib/data/delete_glyph.ttx b/Tests/feaLib/data/delete_glyph.ttx index 777f6e364..b28259fb6 100644 --- a/Tests/feaLib/data/delete_glyph.ttx +++ b/Tests/feaLib/data/delete_glyph.ttx @@ -22,13 +22,14 @@ - + + - + @@ -37,6 +38,16 @@ + + + + + + + + + + diff --git a/Tests/ttLib/removeOverlaps_test.py b/Tests/ttLib/removeOverlaps_test.py new file mode 100644 index 000000000..1320c9be5 --- /dev/null +++ b/Tests/ttLib/removeOverlaps_test.py @@ -0,0 +1,51 @@ +import logging +import pytest + +pathops = pytest.importorskip("pathops") + +from fontTools.ttLib.removeOverlaps import _simplify, _round_path + + +def test_pathops_simplify_bug_workaround(caplog): + # Paths extracted from Noto Sans Ethiopic instance that fails skia-pathops + # https://github.com/google/fonts/issues/3365 + # https://bugs.chromium.org/p/skia/issues/detail?id=11958 + path = pathops.Path() + path.moveTo(550.461, 0) + path.lineTo(550.461, 366.308) + path.lineTo(713.229, 366.308) + path.lineTo(713.229, 0) + path.close() + path.moveTo(574.46, 0) + path.lineTo(574.46, 276.231) + path.lineTo(737.768, 276.231) + path.quadTo(820.075, 276.231, 859.806, 242.654) + path.quadTo(899.537, 209.077, 899.537, 144.154) + path.quadTo(899.537, 79, 853.46, 39.5) + path.quadTo(807.383, 0, 712.383, 0) + path.close() + + # check that it fails without workaround + with pytest.raises(pathops.PathOpsError): + pathops.simplify(path) + + # check our workaround works (but with a warning) + with caplog.at_level(logging.DEBUG, logger="fontTools.ttLib.removeOverlaps"): + result = _simplify(path, debugGlyphName="a") + + assert "skia-pathops failed to simplify 'a' with float coordinates" in caplog.text + + expected = pathops.Path() + expected.moveTo(550, 0) + expected.lineTo(550, 366) + expected.lineTo(713, 366) + expected.lineTo(713, 276) + expected.lineTo(738, 276) + expected.quadTo(820, 276, 860, 243) + expected.quadTo(900, 209, 900, 144) + expected.quadTo(900, 79, 853, 40) + expected.quadTo(807.242, 0.211, 713, 0.001) + expected.lineTo(713, 0) + expected.close() + + assert expected == _round_path(result, round=lambda v: round(v, 3))