From 42bef176a33dc7321eb70f5d49c90317e59ad0e5 Mon Sep 17 00:00:00 2001 From: Behdad Esfahbod Date: Mon, 26 Mar 2018 20:31:06 -0700 Subject: [PATCH] [varLib] Tweak support-resolution algorithm Improve varLib model algorithm. This, basically means any varfont built that had an unusual master configuration will change when rebuilt. Here's a good test: a two-axis with 8 masters at unusual locations: 2-----------------5----------3 | | | 7 | | | | 6 | | | | | | | 0-----------4----------------1 Previously, the reach of master 3 (Black Extended) would have started from A=.4, ie, the A position of master 4. It now correctly starts from 0. Same thing with masters after it. Ie, master 5 gets a span on the A axis from 0 to 1, whereas before it was getting from .4 to 1. Previously, the on-axis masters always cut the space. They don't anymore. That's more consistent, and fixes the main issue Erik showed at TYPO Labs 2017. Same issue was also causing the reach of master 3 to be limited, which I think is the issue being discussed in the linked issue. Both should be fixed. It's hard to describe exactly what happened before / after. Best to read the actual support values: Before: Sorted locations: $ ./fonttools varLib.models 0,0 0,1 1,0 1,1 .4,0 .6,1 .5,.5 .7,.7 [{}, {'A': 0.4}, {'A': 1.0}, {'B': 1.0}, {'A': 1.0, 'B': 1.0}, {'A': 0.6, 'B': 1.0}, {'A': 0.5, 'B': 0.5}, {'A': 0.7, 'B': 0.7}] Supports: [{}, {'A': (0.0, 0.4, 1.0)}, {'A': (0.4, 1.0, 1.0)}, {'B': (0.0, 1.0, 1.0)}, {'A': (0.4, 1.0, 1.0), 'B': (0.0, 1.0, 1.0)}, {'A': (0.4, 0.6, 1.0), 'B': (0.0, 1.0, 1.0)}, {'A': (0.4, 0.5, 1.0), 'B': (0.0, 0.5, 1.0)}, {'A': (0.5, 0.7, 1.0), 'B': (0.5, 0.7, 1.0)}] After: $ ./fonttools varLib.models 0,0 0,1 1,0 1,1 .4,0 .6,1 .5,.5 .7,.7 Sorted locations: [{}, {'A': 0.4}, {'A': 1.0}, {'B': 1.0}, {'A': 1.0, 'B': 1.0}, {'A': 0.6, 'B': 1.0}, {'A': 0.5, 'B': 0.5}, {'A': 0.7, 'B': 0.7}] Supports: [{}, {'A': (0, 0.4, 1.0)}, {'A': (0.4, 1.0, 1.0)}, {'B': (0, 1.0, 1.0)}, {'A': (0, 1.0, 1.0), 'B': (0, 1.0, 1.0)}, {'A': (0, 0.6, 1.0), 'B': (0, 1.0, 1.0)}, {'A': (0, 0.5, 1.0), 'B': (0, 0.5, 1.0)}, {'A': (0.5, 0.7, 1.0), 'B': (0.5, 0.7, 1.0)}] TODO: We should add this as a test case. There's another improvement I want to make, but that's separate. --- Lib/fontTools/varLib/models.py | 11 +++++++---- Tests/varLib/models_test.py | 4 ++-- 2 files changed, 9 insertions(+), 6 deletions(-) diff --git a/Lib/fontTools/varLib/models.py b/Lib/fontTools/varLib/models.py index 8e058f05b..89369c483 100644 --- a/Lib/fontTools/varLib/models.py +++ b/Lib/fontTools/varLib/models.py @@ -152,12 +152,12 @@ class VariationModel(object): {0: 1.0}, {0: 1.0}, {0: 1.0, 4: 1.0, 5: 1.0}, - {0: 1.0, 3: 0.75, 4: 0.25, 5: 1.0, 6: 0.25}, + {0: 1.0, 3: 0.75, 4: 0.25, 5: 1.0, 6: 0.6666666666666666}, {0: 1.0, 3: 0.75, 4: 0.25, 5: 0.6666666666666667, - 6: 0.16666666666666669, + 6: 0.4444444444444445, 7: 0.6666666666666667}] """ @@ -233,7 +233,10 @@ class VariationModel(object): if not axis in loc: continue locV = loc[axis] - box[axis] = (self.lowerBound(locV, values), locV, self.upperBound(locV, values)) + if locV > 0: + box[axis] = (0, locV, max({locV}|values)) + else: + box[axis] = (min({locV}|values), locV, 0) locAxes = set(loc.keys()) # Walk over previous masters now @@ -244,7 +247,7 @@ class VariationModel(object): # If it's NOT in the current box, it does not participate relevant = True for axis, (lower,_,upper) in box.items(): - if axis in m and not (lower < m[axis] < upper): + if axis not in m or not (lower < m[axis] < upper): relevant = False break if not relevant: diff --git a/Tests/varLib/models_test.py b/Tests/varLib/models_test.py index 25f1ef0b8..c1b7afa77 100644 --- a/Tests/varLib/models_test.py +++ b/Tests/varLib/models_test.py @@ -68,10 +68,10 @@ def test_VariationModel(): {0: 1.0}, {0: 1.0}, {0: 1.0, 4: 1.0, 5: 1.0}, - {0: 1.0, 3: 0.75, 4: 0.25, 5: 1.0, 6: 0.25}, + {0: 1.0, 3: 0.75, 4: 0.25, 5: 1.0, 6: 0.6666666666666666}, {0: 1.0, 3: 0.75, 4: 0.25, 5: 0.6666666666666667, - 6: 0.16666666666666669, + 6: 0.4444444444444445, 7: 0.6666666666666667}]