Skip to content

Commit

Permalink
[instancer/L4] Address review comments
Browse files Browse the repository at this point in the history
  • Loading branch information
behdad committed Jul 10, 2023
1 parent 71cca00 commit 5c30a15
Show file tree
Hide file tree
Showing 4 changed files with 37 additions and 18 deletions.
42 changes: 30 additions & 12 deletions Lib/fontTools/varLib/instancer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -234,9 +234,29 @@ def limitRangeAndPopulateDefault(self, fvarTriple) -> "AxisTriple":


@dataclasses.dataclass(frozen=True, order=True, repr=False)
class NormalizedAxisTripleAndDistances(AxisTriple):
class NormalizedAxisTrip(AxisTriple):
"""A triple of (min, default, max) normalized axis values."""

minimum: float
default: float
maximum: float

def __post_init__(self):
if self.default is None:
object.__setattr__(self, "default", max(self.minimum, min(self.maximum, 0)))
if not (-1.0 <= self.minimum <= self.default <= self.maximum <= 1.0):
raise ValueError(
"Normalized axis values not in -1..+1 range; got "
f"minimum={self.minimum:g}, default={self.default:g}, maximum={self.maximum:g})"
)


@dataclasses.dataclass(frozen=True, order=True, repr=False)
class NormalizedAxisTripleAndDistances(AxisTriple):
"""A triple of (min, default, max) normalized axis values,
with distances between min and default, and default and max,
in the *pre-normalized* space."""

minimum: float
default: float
maximum: float
Expand All @@ -256,7 +276,11 @@ def reverse_negate(self):
v = self
return self.__class__(-v[2], -v[1], -v[0], v[4], v[3])

def normalizeValue(self, v, extrapolate=True):
def renormalizeValue(self, v, extrapolate=True):
"""Renormalizes a normalized value v to the range of this axis,
considering the pre-normalized distances as well as the new
axis limits."""

lower, default, upper, distanceNegative, distancePositive = self
assert lower <= default <= upper

Expand All @@ -267,7 +291,7 @@ def normalizeValue(self, v, extrapolate=True):
return 0

if default < 0:
return -self.reverse_negate().normalizeValue(-v, extrapolate=extrapolate)
return -self.reverse_negate().renormalizeValue(-v, extrapolate=extrapolate)

# default >= 0 and v != default

Expand All @@ -288,12 +312,6 @@ def normalizeValue(self, v, extrapolate=True):
else:
vDistance = -v * distanceNegative + distancePositive * default

if totalDistance == 0:
# This happens
if default == 0:
return -v / lower
return 0 # Shouldn't happen

return -vDistance / totalDistance


Expand Down Expand Up @@ -558,7 +576,7 @@ def _instantiateGvarGlyph(
"Instancing accross VarComposite axes with variation is not supported."
)
limits = axisLimits[tag]
loc = limits.normalizeValue(loc, extrapolate=False)
loc = limits.renormalizeValue(loc, extrapolate=False)
newLocation[tag] = loc
component.location = newLocation

Expand Down Expand Up @@ -989,10 +1007,10 @@ def instantiateAvar(varfont, axisLimits):
for fromCoord, toCoord in mapping.items():
if fromCoord < axisRange.minimum or fromCoord > axisRange.maximum:
continue
fromCoord = axisRange.normalizeValue(fromCoord)
fromCoord = axisRange.renormalizeValue(fromCoord)

assert mappedMin <= toCoord <= mappedMax
toCoord = mappedAxisLimit.normalizeValue(toCoord)
toCoord = mappedAxisLimit.renormalizeValue(toCoord)

fromCoord = floatToFixedToFloat(fromCoord, 14)
toCoord = floatToFixedToFloat(toCoord, 14)
Expand Down
4 changes: 2 additions & 2 deletions Lib/fontTools/varLib/instancer/featureVars.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,7 @@ def _limitFeatureVariationConditionRange(condition, axisLimit):
return

return tuple(
axisLimit.normalizeValue(v, extrapolate=False) for v in (minValue, maxValue)
axisLimit.renormalizeValue(v, extrapolate=False) for v in (minValue, maxValue)
)


Expand All @@ -53,7 +53,7 @@ def _instantiateFeatureVariationRecord(
newConditions = []
from fontTools.varLib.instancer import NormalizedAxisTripleAndDistances

default_triple = NormalizedAxisTripleAndDistances(-1, 0, +1, 0, 0)
default_triple = NormalizedAxisTripleAndDistances(-1, 0, +1)
for i, condition in enumerate(record.ConditionSet.ConditionTable):
if condition.Format == 1:
axisIdx = condition.AxisIndex
Expand Down
6 changes: 3 additions & 3 deletions Lib/fontTools/varLib/instancer/solver.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ def _reverse_negate(v):


def _solve(tent, axisLimit, negative=False):
axisMin, axisDef, axisMax, distanceNegative, distancePositive = axisLimit
axisMin, axisDef, axisMax, _distanceNegative, _distancePositive = axisLimit
lower, peak, upper = tent

# Mirror the problem such that axisDef <= peak
Expand Down Expand Up @@ -285,7 +285,7 @@ def rebaseTent(tent, axisLimit):
If tent value is None, that is a special deltaset that should
be always-enabled (called "gain")."""

axisMin, axisDef, axisMax, distanceNegative, distancePositive = axisLimit
axisMin, axisDef, axisMax, _distanceNegative, _distancePositive = axisLimit
assert -1 <= axisMin <= axisDef <= axisMax <= +1

lower, peak, upper = tent
Expand All @@ -295,7 +295,7 @@ def rebaseTent(tent, axisLimit):

sols = _solve(tent, axisLimit)

n = lambda v: axisLimit.normalizeValue(v)
n = lambda v: axisLimit.renormalizeValue(v)
sols = [
(scalar, (n(v[0]), n(v[1]), n(v[2])) if v is not None else None)
for scalar, v in sols
Expand Down
3 changes: 2 additions & 1 deletion Tests/varLib/instancer/instancer_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -1950,7 +1950,7 @@ def check_limit_single_var_axis_range(self, var, axisTag, axisRange, expected):
],
)
def test_positive_var(self, var, axisTag, newMax, expected):
axisRange = instancer.NormalizedAxisTripleAndDistances(0, 0, newMax, 1, 1)
axisRange = instancer.NormalizedAxisTripleAndDistances(0, 0, newMax)
self.check_limit_single_var_axis_range(var, axisTag, axisRange, expected)

@pytest.mark.parametrize(
Expand Down Expand Up @@ -2094,6 +2094,7 @@ def test_parseLimits_invalid(limits):
@pytest.mark.parametrize(
"limits, expected",
[
# 300, 500 come from the font having 100,400,900 fvar axis limits.
({"wght": (100, 400)}, {"wght": (-1.0, 0, 0, 300, 500)}),
({"wght": (100, 400, 400)}, {"wght": (-1.0, 0, 0, 300, 500)}),
({"wght": (100, 300, 400)}, {"wght": (-1.0, -0.5, 0, 300, 500)}),
Expand Down

0 comments on commit 5c30a15

Please sign in to comment.