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.

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

L4 fixes #3179

Merged
merged 11 commits into from
Jul 11, 2023
83 changes: 71 additions & 12 deletions Lib/fontTools/varLib/instancer/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -204,8 +204,8 @@
default = None
if n == 2:
minimum, maximum = v
elif n == 3:
minimum, default, maximum = v
elif n >= 3:
return cls(*v)
else:
raise ValueError(f"expected sequence of 2 or 3; got {n}: {v!r}")
return cls(minimum, default, maximum)
Expand Down Expand Up @@ -234,12 +234,14 @@


@dataclasses.dataclass(frozen=True, order=True, repr=False)
class NormalizedAxisTriple(AxisTriple):
behdad marked this conversation as resolved.
Show resolved Hide resolved
class NormalizedAxisTripleAndDistances(AxisTriple):
"""A triple of (min, default, max) normalized axis values."""
behdad marked this conversation as resolved.
Show resolved Hide resolved

minimum: float
default: float
maximum: float
distanceNegative: Optional[float] = 1
distancePositive: Optional[float] = 1

def __post_init__(self):
if self.default is None:
Expand All @@ -250,6 +252,50 @@
f"minimum={self.minimum:g}, default={self.default:g}, maximum={self.maximum:g})"
)

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):
behdad marked this conversation as resolved.
Show resolved Hide resolved
lower, default, upper, distanceNegative, distancePositive = self
assert lower <= default <= upper

if not extrapolate:
v = max(lower, min(upper, v))

if v == default:
return 0

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

# default >= 0 and v != default

if v > default:
return (v - default) / (upper - default)

# v < default

if lower >= 0:
return (v - default) / (default - lower)
behdad marked this conversation as resolved.
Show resolved Hide resolved

# lower < 0 and v < default
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see that your example make sense but I'm still not 100% clear why this situation in which default >=0 and v < default and lower < 0 needs to take into account the positive/negative distances, whereas the other cases do not. Is it because in this particular case the value v falls in a segment that may span both the negative and positive sides and thus the ratio between these two matters?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it because in this particular case the value v falls in a segment that may span both the negative and positive sides and thus the ratio between these two matters?

Correct.


totalDistance = distanceNegative * -lower + distancePositive * default

if v >= 0:
vDistance = (default - v) * distancePositive
else:
vDistance = -v * distanceNegative + distancePositive * default

if totalDistance == 0:
# This happens
behdad marked this conversation as resolved.
Show resolved Hide resolved
if default == 0:
return -v / lower
return 0 # Shouldn't happen

Check warning on line 295 in Lib/fontTools/varLib/instancer/__init__.py

View check run for this annotation

Codecov / codecov/patch

Lib/fontTools/varLib/instancer/__init__.py#L295

Added line #L295 was not covered by tests
behdad marked this conversation as resolved.
Show resolved Hide resolved

return -vDistance / totalDistance


class _BaseAxisLimits(Mapping[str, AxisTriple]):
def __getitem__(self, key: str) -> AxisTriple:
Expand Down Expand Up @@ -334,8 +380,13 @@
normalizedLimits = {}

for axis_tag, triple in axes.items():
distanceNegative = triple[1] - triple[0]
distancePositive = triple[2] - triple[1]

if self[axis_tag] is None:
normalizedLimits[axis_tag] = NormalizedAxisTriple(0, 0, 0)
normalizedLimits[axis_tag] = NormalizedAxisTripleAndDistances(

Check warning on line 387 in Lib/fontTools/varLib/instancer/__init__.py

View check run for this annotation

Codecov / codecov/patch

Lib/fontTools/varLib/instancer/__init__.py#L387

Added line #L387 was not covered by tests
0, 0, 0, distanceNegative, distancePositive
)
continue

minV, defaultV, maxV = self[axis_tag]
Expand All @@ -344,8 +395,10 @@
defaultV = triple[1]

avarMapping = avarSegments.get(axis_tag, None)
normalizedLimits[axis_tag] = NormalizedAxisTriple(
*(normalize(v, triple, avarMapping) for v in (minV, defaultV, maxV))
normalizedLimits[axis_tag] = NormalizedAxisTripleAndDistances(
*(normalize(v, triple, avarMapping) for v in (minV, defaultV, maxV)),
distanceNegative,
distancePositive,
)

return NormalizedAxisLimits(normalizedLimits)
Expand All @@ -358,7 +411,7 @@
self._data = data = {}
for k, v in dict(*args, **kwargs).items():
try:
triple = NormalizedAxisTriple.expand(v)
triple = NormalizedAxisTripleAndDistances.expand(v)
except ValueError as e:
raise ValueError(f"Invalid axis limits for {k!r}: {v!r}") from e
data[k] = triple
Expand Down Expand Up @@ -442,7 +495,7 @@


def changeTupleVariationAxisLimit(var, axisTag, axisLimit):
assert isinstance(axisLimit, NormalizedAxisTriple)
assert isinstance(axisLimit, NormalizedAxisTripleAndDistances)

# Skip when current axis is missing (i.e. doesn't participate),
lower, peak, upper = var.axes.get(axisTag, (-1, 0, 1))
Expand Down Expand Up @@ -505,7 +558,7 @@
"Instancing accross VarComposite axes with variation is not supported."
)
limits = axisLimits[tag]
loc = normalizeValue(loc, limits)
loc = limits.normalizeValue(loc, extrapolate=False)
newLocation[tag] = loc
component.location = newLocation

Expand Down Expand Up @@ -925,15 +978,21 @@
mappedMax = floatToFixedToFloat(
piecewiseLinearMap(axisRange.maximum, mapping), 14
)
mappedAxisLimit = NormalizedAxisTripleAndDistances(
mappedMin,
mappedDef,
mappedMax,
axisRange.distanceNegative,
axisRange.distancePositive,
)
newMapping = {}
for fromCoord, toCoord in mapping.items():

if fromCoord < axisRange.minimum or fromCoord > axisRange.maximum:
continue
fromCoord = normalizeValue(fromCoord, axisRange)
fromCoord = axisRange.normalizeValue(fromCoord)

assert mappedMin <= toCoord <= mappedMax
toCoord = normalizeValue(toCoord, (mappedMin, mappedDef, mappedMax))
toCoord = mappedAxisLimit.normalizeValue(toCoord)

fromCoord = floatToFixedToFloat(fromCoord, 14)
toCoord = floatToFixedToFloat(toCoord, 14)
Expand Down
9 changes: 5 additions & 4 deletions Lib/fontTools/varLib/instancer/featureVars.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
from fontTools.ttLib.tables import otTables as ot
from fontTools.varLib.models import normalizeValue
from copy import deepcopy
import logging

Expand Down Expand Up @@ -41,7 +40,9 @@ def _limitFeatureVariationConditionRange(condition, axisLimit):
# condition invalid or out of range
return

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


def _instantiateFeatureVariationRecord(
Expand All @@ -50,9 +51,9 @@ def _instantiateFeatureVariationRecord(
applies = True
shouldKeep = False
newConditions = []
from fontTools.varLib.instancer import NormalizedAxisTriple
from fontTools.varLib.instancer import NormalizedAxisTripleAndDistances

default_triple = NormalizedAxisTriple(-1, 0, +1)
default_triple = NormalizedAxisTripleAndDistances(-1, 0, +1, 0, 0)
behdad marked this conversation as resolved.
Show resolved Hide resolved
for i, condition in enumerate(record.ConditionSet.ConditionTable):
if condition.Format == 1:
axisIdx = condition.AxisIndex
Expand Down
138 changes: 64 additions & 74 deletions Lib/fontTools/varLib/instancer/solver.py
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
from fontTools.varLib.models import supportScalar, normalizeValue
from fontTools.varLib.models import supportScalar
from fontTools.misc.fixedTools import MAX_F2DOT14
from functools import lru_cache

Expand All @@ -12,15 +12,17 @@ def _reverse_negate(v):


def _solve(tent, axisLimit, negative=False):
axisMin, axisDef, axisMax = axisLimit
axisMin, axisDef, axisMax, distanceNegative, distancePositive = axisLimit
lower, peak, upper = tent
behdad marked this conversation as resolved.
Show resolved Hide resolved

# Mirror the problem such that axisDef <= peak
if axisDef > peak:
return [
(scalar, _reverse_negate(t) if t is not None else None)
for scalar, t in _solve(
_reverse_negate(tent), _reverse_negate(axisLimit), not negative
_reverse_negate(tent),
axisLimit.reverse_negate(),
not negative,
)
]
# axisDef <= peak
Expand Down Expand Up @@ -98,9 +100,8 @@ def _solve(tent, axisLimit, negative=False):
# |
# crossing
if gain > outGain:

# Crossing point on the axis.
crossing = peak + ((1 - gain) * (upper - peak) / (1 - outGain))
crossing = peak + (1 - gain) * (upper - peak)

loc = (axisDef, peak, crossing)
scalar = 1
Expand All @@ -116,7 +117,7 @@ def _solve(tent, axisLimit, negative=False):
# the drawing above.
if upper >= axisMax:
loc = (crossing, axisMax, axisMax)
scalar = supportScalar({"tag": axisMax}, {"tag": tent})
scalar = outGain

out.append((scalar - gain, loc))

Expand Down Expand Up @@ -147,84 +148,73 @@ def _solve(tent, axisLimit, negative=False):

# Eternity justify.
loc2 = (upper, axisMax, axisMax)
scalar2 = supportScalar({"tag": axisMax}, {"tag": tent})
scalar2 = 0

out.append((scalar1 - gain, loc1))
out.append((scalar2 - gain, loc2))

# Case 3: Outermost limit still fits within F2Dot14 bounds;
# we keep deltas as is and only scale the axes bounds. Deltas beyond -1.0
# or +1.0 will never be applied as implementations must clamp to that range.
#
# A second tent is needed for cases when gain is positive, though we add it
# unconditionally and it will be dropped because scalar ends up 0.
#
# TODO: See if we can just move upper closer to adjust the slope, instead of
# second tent.
#
# | peak |
# 1.........|............o...|..................
# | /x\ |
# | /xxx\ |
# | /xxxxx\|
# | /xxxxxxx+
# | /xxxxxxxx|\
# 0---|-----|------oxxxxxxxxx|xo---------------1
# axisMin | lower | upper
# | |
# axisDef axisMax
#
elif axisDef + (axisMax - axisDef) * 2 >= upper:

if not negative and axisDef + (axisMax - axisDef) * MAX_F2DOT14 < upper:
# we clamp +2.0 to the max F2Dot14 (~1.99994) for convenience
upper = axisDef + (axisMax - axisDef) * MAX_F2DOT14
assert peak < upper

else:
# Special-case if peak is at axisMax.
if axisMax == peak:
upper = peak

loc1 = (max(axisDef, lower), peak, upper)
scalar1 = 1

loc2 = (peak, upper, upper)
scalar2 = 0

# Don't add a dirac delta!
if axisDef < upper:
out.append((scalar1 - gain, loc1))
if peak < upper:
out.append((scalar2 - gain, loc2))
# Case 3:
# We keep delta as is and only scale the axis upper to achieve
# the desired new tent if feasible.
#
# peak
# 1.....................o....................
# / \_|
# ..................../....+_.........outGain
# / | \
# gain..............+......|..+_.............
# /| | | \
# 0---|-----------o | | | o----------1
# axisMin lower| | | upper
# | | newUpper
# axisDef axisMax
#
newUpper = peak + (1 - gain) * (upper - peak)
assert axisMax <= newUpper # Because outGain >= gain
if newUpper <= axisDef + (axisMax - axisDef) * 2:
upper = newUpper
if not negative and axisDef + (axisMax - axisDef) * MAX_F2DOT14 < upper:
# we clamp +2.0 to the max F2Dot14 (~1.99994) for convenience
upper = axisDef + (axisMax - axisDef) * MAX_F2DOT14
assert peak < upper

loc = (max(axisDef, lower), peak, upper)
scalar = 1

# Case 4: New limit doesn't fit; we need to chop into two tents,
# because the shape of a triangle with part of one side cut off
# cannot be represented as a triangle itself.
#
# | peak |
# 1.........|......o.|...................
# | /x\|
# | |xxy|\_
# | /xxxy| \_
# | |xxxxy| \_
# | /xxxxy| \_
# 0---|-----|-oxxxxxx| o----------1
# axisMin | lower | upper
# | |
# axisDef axisMax
#
else:
out.append((scalar - gain, loc))

loc1 = (max(axisDef, lower), peak, axisMax)
scalar1 = 1
# Case 4: New limit doesn't fit; we need to chop into two tents,
# because the shape of a triangle with part of one side cut off
# cannot be represented as a triangle itself.
#
# | peak |
# 1.........|......o.|....................
# ..........|...../x\|.............outGain
# | |xxy|\_
# | /xxxy| \_
# | |xxxxy| \_
# | /xxxxy| \_
# 0---|-----|-oxxxxxx| o----------1
# axisMin | lower | upper
# | |
# axisDef axisMax
#
else:
loc1 = (max(axisDef, lower), peak, axisMax)
scalar1 = 1

loc2 = (peak, axisMax, axisMax)
scalar2 = supportScalar({"tag": axisMax}, {"tag": tent})
loc2 = (peak, axisMax, axisMax)
scalar2 = outGain

out.append((scalar1 - gain, loc1))
# Don't add a dirac delta!
if peak < axisMax:
out.append((scalar2 - gain, loc2))
out.append((scalar1 - gain, loc1))
# Don't add a dirac delta!
if peak < axisMax:
out.append((scalar2 - gain, loc2))

# Now, the negative side

Expand Down Expand Up @@ -295,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 = axisLimit
axisMin, axisDef, axisMax, distanceNegative, distancePositive = axisLimit
assert -1 <= axisMin <= axisDef <= axisMax <= +1
behdad marked this conversation as resolved.
Show resolved Hide resolved

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

sols = _solve(tent, axisLimit)

n = lambda v: normalizeValue(v, axisLimit, extrapolate=True)
n = lambda v: axisLimit.normalizeValue(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