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

work around MutatorMath "warping" sources/instances locations #552

Merged

Conversation

anthrotype
Copy link
Member

@anthrotype anthrotype commented May 17, 2019

I just made an unexpected discovery..

It turns out that, when a designspace document defines axes map elements... (e.g. these are Noto Sans' axes)

    <axis tag="wght" name="Weight" minimum="100" maximum="900" default="400">
      <map input="100" output="26"/>
      <map input="200" output="39"/>
      <map input="300" output="58"/>
      <map input="400" output="90"/>
      <map input="500" output="108"/>
      <map input="600" output="128"/>
      <map input="700" output="151"/>
      <map input="800" output="169"/>
      <map input="900" output="190"/>
    </axis>
    <axis tag="wdth" name="Width" minimum="62.500000" maximum="100" default="100">
      <map input="62.500000" output="70"/>
      <map input="75" output="79"/>
      <map input="87.500000" output="89"/>
      <map input="100" output="100"/>
    </axis>

... MutatorMath will "bend" the masters and instances location coordinates using the provided mappings.

Now, this is actually wrong -- at least from the point of view of what we've always assumed to be the case in fontTools.varLib when creating variable fonts: which is that the DesignSpace masters/instances locations are specified in what we called internal or design-space coordinate system (i.e. just arbirary numbers that are convenient while desigining and only used internally for interpolation); this internal, design coordinates as such are considered to be already "bent" or "mapped".

The main reason these axes mappings were introduced was to map from user-space coordinates to internal design-space coordinates, in order to build a non-default (non linear) normalization for the avar table. "User" coordinates are the fvar's axes definitions, which may be restricted to predefined ranges or meanings for registered axes (e.g. "Regular" weight must be 400, "Ultra-Condensed" width 50 % of normal, etc.).

But.. MutatorMath seems to assume that the masters and instances' locations are in user-space coordinates instead of internal design coordinates, and uses the mappings to 'bend' the locations, producing sometimes unexpected interpolations.

I noticed this while comparing a (SemiCondensed) instance of NotoSans-VF produced using fontTools.varLib.mutator with a static instance at the same (nominal, user-space) weight/width location interpolated via MutatorMath. In particular, adding map elements to the width axes (the ones above) highlighted this issue, whereas the difference was not apparent when only weight contained similar mappings.

MutatorMath is no longer maintained and we plan to replace that with something built upon fontTools.varLib (@madig is onto that).

In the meantime I just wanted raise this issue, and maybe propose a temporary work-around (a hack really) with this PR.

Basically, I create a temporary designspace document without the axis maps, and with the min/default/max triplet mapped "forward" from user-space coordinates (the "input") to internal designspace coordinates (the "output"), to prevent MutatorMath from distorting the locations.

As @behdad suggested more than a year ago, I agree the only good solution going forward to avoid this confusion is to explicitly add an attribute to DesignSpace location elements to qualify unambiguously whether a location is meant to be mapped or not (I'll open a separate PR in fontTools to propose an update to the DesignSpace spec).

Related issues:
LettError/designSpaceDocument#16
fonttools/fonttools#1395
googlefonts/glyphsLib#483

@anthrotype
Copy link
Member Author

anthrotype commented May 17, 2019

from mutatorMath.objects.mutator import buildMutator
from mutatorMath.objects.location import Location

# source locations implicitly specified in internal, design location.
# the second item is master's value at given location. I use numbers for simplicity
# but it can be any object that supports addition and multiplication (fontMath objects)
masters = [
    (Location(Width=70), 0),
    (Location(Width=100), 100),
]

axes = {
    'Width': {
        'tag': 'wdth',
        'name': 'Width',
        # min/default/max are always in user-space coordinates
        'minimum': 62.5,
        'default': 100.0,
        'maximum': 100.0,
        # first item (input) is user-space coordinate, second item (output) is internal design coordinate
        'map': [(62.5, 70.0), (75.0, 79.0), (87.5, 89.0), (100.0, 100.0)],
    }
}

_, mut = buildMutator(masters, axes)

# instance locations are also specified in internal design coordinates (at least varLib assumes they are)
instance_location = Location(Width=79)

instance = mut.makeInstance(instance_location)

# the result is 27.642276422764223, but it should have been 30.0
print(instance)

@anthrotype
Copy link
Member Author

If I patch mutatorMath buildMutator function to not use the Bender object like so:

diff --git a/Lib/mutatorMath/objects/mutator.py b/Lib/mutatorMath/objects/mutator.py
index e79f27c..e6e8449 100644
--- a/Lib/mutatorMath/objects/mutator.py
+++ b/Lib/mutatorMath/objects/mutator.py
@@ -15,7 +15,7 @@ _EPSILON = sys.float_info.epsilon
 def noBend(loc): return loc


-def buildMutator(items, axes=None, bias=None):
+def buildMutator(items, axes=None, bias=None, mapped=False):
     """
         Build a mutator with the (location, obj) pairs in items.
         Determine the bias based on the given locations.
@@ -23,7 +23,7 @@ def buildMutator(items, axes=None, bias=None):
     from mutatorMath.objects.bender import Bender
     items = [(Location(loc),obj) for loc, obj in items]
     m = Mutator()
-    if axes is not None:
+    if not mapped and axes is not None:
         bender = Bender(axes)
         m.setBender(bender)
     else:

then, using the same masters and axes from above, I get the expected instance value:

_, mut = buildMutator(masters, axes, mapped=True)
instance_location = Location(Width=79)
instance = mut.makeInstance(instance_location)
print(instance)  # the result now is 30.0

@anthrotype
Copy link
Member Author

/cc @LettError
ufoProcessor (successor to mutatorMath) may also be affected by the same problem.

@miguelsousa
Copy link

@anthrotype glad you caught up to the problem I tried to explain a while ago unsuccessfully. I still think that the "bender" functionality in designspace got hijacked to satisfy the weight/width variation requirements of the OT spec.

@anthrotype
Copy link
Member Author

Curious where did you try explain this? (I may well have missed it or I forgot about it)

@miguelsousa
Copy link

Somewhere in a FontTools thread I believe.

@miguelsousa
Copy link

BTW, this is one of the reasons why our projects have more than one designspace file for each style (e.g. https://github.com/adobe-fonts/source-sans-pro/tree/master/Roman/Masters)

MutatorMath assumes that the masters and instances' locations are in
user-space coordinates -- whereas they actually are in internal design-space
coordinates, and thus they do not need any 'bending'. To work around this we
we create a temporary designspace document without the axis maps, and with the
min/default/max triplet mapped "forward" from user-space coordinates (input)
to internal designspace coordinates (output).

Related issues:
LettError/designSpaceDocument#16
fonttools/fonttools#1395
@anthrotype anthrotype force-pushed the fix-mutatormath-warped-locs branch from 52b1f02 to d9c444e Compare May 28, 2019 12:59
@LettError
Copy link

Maybe the problem is applying the bender to the master locations when building the mutator. If I disable it there the results make more sense.

LettError/MutatorMath@5576f9d

@anthrotype
Copy link
Member Author

anthrotype commented May 30, 2019

Maybe the problem is applying the bender to the master locations when building the mutator. If I disable it there the results make more sense.

@LettError yes, that's one problem. Master's locations are in internal design coordinates, not user-space, thus they are already mapped and require no bending. That patch goes in the right direction.

However, there is still the issue that Mutator.makeInstance method defaults to bend=True and thus InstanceWriter calls makeInstance (when instantiating info, kerning and glyphs) assuming that instances' locations are in user-space coordinates, whereas they are implicitly (at least that's what we have been assuming all along in varLib) in internal design coordinates.
So we also need to change that. We either change the default argument value to bend=False in makeInstance, or allow the caller to override this and request that instance location be not warped.
I'm going to send a PR in MutatorMath that includes your patch plus the fix for the latter.

PS: I know that you no longer using the mutatorMath.ufo module (replaced by ufoProcessor) but fontmake still is and I would like to reduce the amount of breakage by trying to avoid sweeping changes, at least for the time being.

@anthrotype
Copy link
Member Author

@LettError another thing which should be clarified is that the axes minimum, default and maximum are meant to be specified in user coordinates whenever axes map elements are defined, i.e. in the same coordinate space as the input of these axes map elements.
I hope we all agree on that.

@anthrotype anthrotype merged commit a75cae3 into googlefonts:master Jun 3, 2019
@anthrotype anthrotype deleted the fix-mutatormath-warped-locs branch June 3, 2019 14:47
@anthrotype
Copy link
Member Author

going to release a fontmake v1.10.0 that includes this work-around. When MutatorMath fixes the issue we can revert this.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants