[varLib] Shift most (all?) delta-rounding to VarModel

Reduces error.

The main varfont-builder now asks the model to do rounding, and asks
VariationStore to do no rounding, so we don't spend extra times rounding
multiple times (specially with the heavy otRound).

I *think* I got it all and right...

Fixes https://github.com/fonttools/fonttools/issues/2213
This commit is contained in:
Behdad Esfahbod 2021-03-03 19:37:50 -07:00
parent 77acdbced3
commit 68004b8fec
4 changed files with 24 additions and 29 deletions

View File

@ -19,7 +19,7 @@ Then you can make a variable-font this way:
API *will* change in near future. API *will* change in near future.
""" """
from fontTools.misc.py23 import * from fontTools.misc.py23 import *
from fontTools.misc.fixedTools import otRound from fontTools.misc.roundTools import noRound, otRound
from fontTools.misc.vector import Vector from fontTools.misc.vector import Vector
from fontTools.ttLib import TTFont, newTable from fontTools.ttLib import TTFont, newTable
from fontTools.ttLib.tables._f_v_a_r import Axis, NamedInstance from fontTools.ttLib.tables._f_v_a_r import Axis, NamedInstance
@ -253,7 +253,7 @@ def _add_gvar(font, masterModel, master_ttfs, tolerance=0.5, optimize=True):
# Update gvar # Update gvar
gvar.variations[glyph] = [] gvar.variations[glyph] = []
deltas = model.getDeltas(allCoords) deltas = model.getDeltas(allCoords, round=round) # builtin round calls into GlyphCoordinates.__round__()
supports = model.supports supports = model.supports
assert len(deltas) == len(supports) assert len(deltas) == len(supports)
@ -262,7 +262,7 @@ def _add_gvar(font, masterModel, master_ttfs, tolerance=0.5, optimize=True):
endPts = control.endPts endPts = control.endPts
for i,(delta,support) in enumerate(zip(deltas[1:], supports[1:])): for i,(delta,support) in enumerate(zip(deltas[1:], supports[1:])):
if all(abs(v) <= tolerance for v in delta.array) and not isComposite: if all(v == 0 for v in delta.array) and not isComposite:
continue continue
var = TupleVariation(support, delta) var = TupleVariation(support, delta)
if optimize: if optimize:
@ -304,7 +304,7 @@ def _remove_TTHinting(font):
font["glyf"].removeHinting() font["glyf"].removeHinting()
# TODO: Modify gasp table to deactivate gridfitting for all ranges? # TODO: Modify gasp table to deactivate gridfitting for all ranges?
def _merge_TTHinting(font, masterModel, master_ttfs, tolerance=0.5): def _merge_TTHinting(font, masterModel, master_ttfs):
log.info("Merging TT hinting") log.info("Merging TT hinting")
assert "cvar" not in font assert "cvar" not in font
@ -363,10 +363,9 @@ def _merge_TTHinting(font, masterModel, master_ttfs, tolerance=0.5):
return return
variations = [] variations = []
deltas, supports = masterModel.getDeltasAndSupports(all_cvs) deltas, supports = masterModel.getDeltasAndSupports(all_cvs, round=round) # builtin round calls into Vector.__round__
for i,(delta,support) in enumerate(zip(deltas[1:], supports[1:])): for i,(delta,support) in enumerate(zip(deltas[1:], supports[1:])):
delta = [otRound(d) for d in delta] if all(v == 0 for v in delta):
if all(abs(v) <= tolerance for v in delta):
continue continue
var = TupleVariation(support, delta) var = TupleVariation(support, delta)
variations.append(var) variations.append(var)
@ -441,7 +440,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs,
vOrigDeltasAndSupports = {} vOrigDeltasAndSupports = {}
for glyph in glyphOrder: for glyph in glyphOrder:
vhAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in advMetricses] vhAdvances = [metrics[glyph][0] if glyph in metrics else None for metrics in advMetricses]
vhAdvanceDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vhAdvances) vhAdvanceDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vhAdvances, round=otRound)
singleModel = models.allEqual(id(v[1]) for v in vhAdvanceDeltasAndSupports.values()) singleModel = models.allEqual(id(v[1]) for v in vhAdvanceDeltasAndSupports.values())
@ -453,7 +452,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs,
# glyphs which have a non-default vOrig. # glyphs which have a non-default vOrig.
vOrigs = [metrics[glyph] if glyph in metrics else defaultVOrig vOrigs = [metrics[glyph] if glyph in metrics else defaultVOrig
for metrics, defaultVOrig in vOrigMetricses] for metrics, defaultVOrig in vOrigMetricses]
vOrigDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vOrigs) vOrigDeltasAndSupports[glyph] = masterModel.getDeltasAndSupports(vOrigs, round=otRound)
directStore = None directStore = None
if singleModel: if singleModel:
@ -463,7 +462,7 @@ def _get_advance_metrics(font, masterModel, master_ttfs,
varTupleIndexes = list(range(len(supports))) varTupleIndexes = list(range(len(supports)))
varData = builder.buildVarData(varTupleIndexes, [], optimize=False) varData = builder.buildVarData(varTupleIndexes, [], optimize=False)
for glyphName in glyphOrder: for glyphName in glyphOrder:
varData.addItem(vhAdvanceDeltasAndSupports[glyphName][0]) varData.addItem(vhAdvanceDeltasAndSupports[glyphName][0], round=noRound)
varData.optimize() varData.optimize()
directStore = builder.buildVarStore(varTupleList, [varData]) directStore = builder.buildVarStore(varTupleList, [varData])
@ -473,14 +472,14 @@ def _get_advance_metrics(font, masterModel, master_ttfs,
for glyphName in glyphOrder: for glyphName in glyphOrder:
deltas, supports = vhAdvanceDeltasAndSupports[glyphName] deltas, supports = vhAdvanceDeltasAndSupports[glyphName]
storeBuilder.setSupports(supports) storeBuilder.setSupports(supports)
advMapping[glyphName] = storeBuilder.storeDeltas(deltas) advMapping[glyphName] = storeBuilder.storeDeltas(deltas, round=noRound)
if vOrigMetricses: if vOrigMetricses:
vOrigMap = {} vOrigMap = {}
for glyphName in glyphOrder: for glyphName in glyphOrder:
deltas, supports = vOrigDeltasAndSupports[glyphName] deltas, supports = vOrigDeltasAndSupports[glyphName]
storeBuilder.setSupports(supports) storeBuilder.setSupports(supports)
vOrigMap[glyphName] = storeBuilder.storeDeltas(deltas) vOrigMap[glyphName] = storeBuilder.storeDeltas(deltas, round=noRound)
indirectStore = storeBuilder.finish() indirectStore = storeBuilder.finish()
mapping2 = indirectStore.optimize() mapping2 = indirectStore.optimize()

View File

@ -20,6 +20,7 @@ from fontTools.varLib.models import allEqual
from fontTools.misc.roundTools import roundFunc from fontTools.misc.roundTools import roundFunc
from fontTools.misc.psCharStrings import T2CharString, T2OutlineExtractor from fontTools.misc.psCharStrings import T2CharString, T2OutlineExtractor
from fontTools.pens.t2CharStringPen import T2CharStringPen from fontTools.pens.t2CharStringPen import T2CharStringPen
from functools import partial
from .errors import VarLibCFFDictMergeError, VarLibCFFPointTypeMergeError, VarLibMergeError from .errors import VarLibCFFDictMergeError, VarLibCFFPointTypeMergeError, VarLibMergeError
@ -585,7 +586,7 @@ class CFF2CharStringMergePen(T2CharStringPen):
def getCommands(self): def getCommands(self):
return self._commands return self._commands
def reorder_blend_args(self, commands, get_delta_func, round_func): def reorder_blend_args(self, commands, get_delta_func):
""" """
We first re-order the master coordinate values. We first re-order the master coordinate values.
For a moveto to lineto, the args are now arranged as: For a moveto to lineto, the args are now arranged as:
@ -628,8 +629,6 @@ class CFF2CharStringMergePen(T2CharStringPen):
else: else:
# convert to deltas # convert to deltas
deltas = get_delta_func(coord)[1:] deltas = get_delta_func(coord)[1:]
if round_func:
deltas = [round_func(delta) for delta in deltas]
coord = [coord[0]] + deltas coord = [coord[0]] + deltas
new_coords.append(coord) new_coords.append(coord)
cmd[1] = new_coords cmd[1] = new_coords
@ -640,7 +639,7 @@ class CFF2CharStringMergePen(T2CharStringPen):
self, private=None, globalSubrs=None, self, private=None, globalSubrs=None,
var_model=None, optimize=True): var_model=None, optimize=True):
commands = self._commands commands = self._commands
commands = self.reorder_blend_args(commands, var_model.getDeltas, self.round) commands = self.reorder_blend_args(commands, partial (var_model.getDeltas, round=self.round))
if optimize: if optimize:
commands = specializeCommands( commands = specializeCommands(
commands, generalizeFirst=False, commands, generalizeFirst=False,

View File

@ -1,4 +1,4 @@
from fontTools.misc.fixedTools import otRound from fontTools.misc.roundTools import noRound, otRound
from fontTools.ttLib.tables import otTables as ot from fontTools.ttLib.tables import otTables as ot
from fontTools.varLib.models import supportScalar from fontTools.varLib.models import supportScalar
from fontTools.varLib.builder import (buildVarRegionList, buildVarStore, from fontTools.varLib.builder import (buildVarRegionList, buildVarStore,
@ -83,15 +83,12 @@ class OnlineVarStoreBuilder(object):
def storeMasters(self, master_values): def storeMasters(self, master_values):
deltas = self._model.getDeltas(master_values) deltas = self._model.getDeltas(master_values, round=otRound)
base = otRound(deltas.pop(0)) base = deltas.pop(0)
return base, self.storeDeltas(deltas) return base, self.storeDeltas(deltas, round=noRound)
def storeDeltas(self, deltas): def storeDeltas(self, deltas, round=otRound):
# Pity that this exists here, since VarData_addItem deltas = [round(d) for d in deltas]
# does the same. But to look into our cache, it's
# good to adjust deltas here as well...
deltas = [otRound(d) for d in deltas]
if len(deltas) == len(self._supports) + 1: if len(deltas) == len(self._supports) + 1:
deltas = tuple(deltas[1:]) deltas = tuple(deltas[1:])
else: else:
@ -109,14 +106,14 @@ class OnlineVarStoreBuilder(object):
# Full array. Start new one. # Full array. Start new one.
self._add_VarData() self._add_VarData()
return self.storeDeltas(deltas) return self.storeDeltas(deltas)
self._data.addItem(deltas) self._data.addItem(deltas, round=noRound)
varIdx = (self._outer << 16) + inner varIdx = (self._outer << 16) + inner
self._cache[deltas] = varIdx self._cache[deltas] = varIdx
return varIdx return varIdx
def VarData_addItem(self, deltas): def VarData_addItem(self, deltas, round=otRound):
deltas = [otRound(d) for d in deltas] deltas = [round(d) for d in deltas]
countUs = self.VarRegionCount countUs = self.VarRegionCount
countThem = len(deltas) countThem = len(deltas)

View File

@ -290,7 +290,7 @@
</CharString> </CharString>
<CharString name="cid06449" fdSelectIndex="1"> <CharString name="cid06449" fdSelectIndex="1">
2 vsindex 2 vsindex
-60 30 203 30 -9 9 67 7 -7 14 -14 30 -20 20 80 30 59 30 121 30 18 93 -30 30 -30 108 -23 0 -26 67 2 76 -98 -2 -111 42 0 47 -13 0 -14 13 0 14 -33 0 -37 11 0 13 -11 0 -13 8 0 9 -7 0 -8 53 0 60 -32 0 -36 32 0 36 -52 0 -59 57 1 65 -33 0 -38 53 0 60 -83 -1 -93 54 0 60 -6 -19 -24 33 19 55 -76 -1 -86 76 1 86 -76 -1 -86 59 1 67 26 blend -60 30 203 30 -9 9 67 7 -7 14 -14 30 -20 20 80 30 59 30 121 30 18 93 -30 30 -30 108 -23 0 -26 67 2 76 -98 -2 -111 42 0 47 -13 0 -14 13 0 14 -33 0 -37 11 0 13 -11 0 -13 8 0 9 -7 -1 -8 53 0 60 -32 0 -36 32 0 36 -52 0 -59 57 1 65 -33 0 -38 53 0 60 -83 -1 -93 54 0 60 -6 -19 -24 33 19 55 -76 -1 -86 76 1 86 -76 -1 -86 59 1 67 26 blend
hstemhm hstemhm
77 30 42 30 139 30 23 30 71 10 74 30 15 30 16 30 158 30 28 30 -4 29 -14 0 -16 88 1 99 -82 -1 -92 87 1 98 -130 -1 -146 102 1 114 -73 -1 -82 74 2 84 -112 -2 -126 27 0 30 13 0 15 90 1 101 -126 -1 -142 75 1 84 -68 -1 -76 102 1 115 -144 -1 -162 94 1 105 -79 -1 -88 95 1 106 -81 -1 -91 74 1 83 22 blend 77 30 42 30 139 30 23 30 71 10 74 30 15 30 16 30 158 30 28 30 -4 29 -14 0 -16 88 1 99 -82 -1 -92 87 1 98 -130 -1 -146 102 1 114 -73 -1 -82 74 2 84 -112 -2 -126 27 0 30 13 0 15 90 1 101 -126 -1 -142 75 1 84 -68 -1 -76 102 1 115 -144 -1 -162 94 1 105 -79 -1 -88 95 1 106 -81 -1 -91 74 1 83 22 blend
vstemhm vstemhm