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

Designspace axis definitions have reversed userspace (input) and designspace (output) coordinates #745

Closed
jpt opened this issue Nov 6, 2021 · 15 comments

Comments

@jpt
Copy link

jpt commented Nov 6, 2021

Given the following example Axis Mappings custom parameter:

{
customParameters = (
{
name = "Axis Mappings";
value = {
wght = {
22 = 100;
50 = 400;
72 = 700;
};
};
}
);
}

I get this output:

    <axis tag="wght" name="Weight" minimum="22" maximum="72" default="22">
      <map input="22" output="100"/>
      <map input="50" output="400"/>
      <map input="72" output="700"/>
    </axis>

Expected output:

    <axis tag="wght" name="Weight" minimum="100" maximum="700" default="100">
      <map input="100" output="22"/>
      <map input="400" output="50"/>
      <map input="700" output="72"/>
    </axis>

The dimension and xvalues elsewhere in the designspace file correctly point to the internal coordinates.

I wonder if this could be caused by the masters themselves also having remapped axis locations? For example on the master with 72 as the internal coordinate, the Axis Location custom parameter is defined as such:

{
customParameters = (
{
name = "Axis Location";
value = (
{
Axis = Weight;
Location = 700;
}
);
}
);
}
@jpt jpt changed the title Designspace axis definitions have reversed userspace (input) and designspace (output) Designspace axis definitions have reversed userspace (input) and designspace (output) coordinates Nov 6, 2021
@jpt
Copy link
Author

jpt commented Nov 6, 2021

I've attached a v2 Glyphs file that can reproduce the problem. Note that this fails entirely if saved as v3 (see #746)

TestFontv2.glyphs.zip

@anthrotype
Copy link
Member

@jpt thanks for filing this issue!

I just stumbled on this while tinkering with Glyphs.app, and I wonder how could this have gone unnoticed for so long. Apologies for that.

Ever since we added support for this "Axis Mappings" custom parameter, we have been interpreting it as a mapping from user-space to internal design-space coordinates, see:

mapping = {float(k): v for k, v in custom_mapping[axis.tag].items()}
regularDesignLoc = axis_def.get_design_loc(regular_master)
reverse_mapping = {dl: ul for ul, dl in sorted(mapping.items())}
regularUserLoc = piecewiseLinearMap(regularDesignLoc, reverse_mapping)

The key part is ul, dl in sorted(mapping.items()), where "ul" presumably means "user-space location" and "dl" means "design-space location" thus => {external: internal}.

However... Glyphs.app interprets it the other way around, as a map from {internal: external} coordinates.
This is also confirmed in this blog post https://glyphsapp.com/learn/creating-a-variable-font at paragraph "Option 2: Axis Mappings parameter" ("left" means "internal", "right" means "external"... also see the labels on the horizontal and vertical axes)

Screen Shot 2022-12-19 at 12 39 06


I think this is a complete disaster. I am pretty certain that there are a lot of .glyphs-based font projects that were meant to be built using fontmake that define Axis Mappings as {external: internal}, as there are similarly lots of others that do the opposite but meant to be exported directly from Glyphs.app...

There's no way to be certain what the intention of the font developer actually was, and it is too late to change either Glyphs.app or glyphsLib at this point..
So I believe this Axis Mappings parameter should probably be deprecated and avoided as much as possible..
In its place one could use Axis Location parameters defined on masters as well as instances.

One limitation of Axis Location vs Axis Mappings is that the latter could in theory define additional mappings that don't directly correspond to an existing master or instance (just like an avar can do), wheres Axis Location is tied to the existing masters and instances.

@anthrotype
Copy link
Member

/cc @schriftgestalt

@khaledhosny
Copy link
Collaborator

I think we should fix glyphsLib and let font developers deal with the fallout (I’m certain I have a few fonts that fell victim to this). A bug is a bug and the fact that some projects worked around it does not change much.

@anthrotype
Copy link
Member

A bug is a bug

IIRC, glyphsLib implemented support for the "Axis Mappings" custom parameter even before Georg did in Glyphs.app, with #618
I would argue that the way we implemented it as mapping from user (or external) coordinates to design (internal) ones was deliberate and intentional, and not a bug. We were following the way these axis mappings had already been defined in the designspace spec, which in turn were modelled after the way the opentype avar table maps from fvar's user-space coordinates to normalized internal coordinates.

@anthrotype
Copy link
Member

digging into this a bit more, before @m4rc1e worked on implementing support for Axis Mappings in glyphsLib we had discussions with @schriftgestalt about how to best define the new custom parameter (Marc even wrote a doc about this).
Already back then, there were disagreements/confusions as to whether it should map from user-external to design-internal coordinates or viceversa, see

#568 (comment)

but it seemed we had finally settled on {external: internal} .. until now I realised Glyphs.app actually implemented it the other way around.

@khaledhosny
Copy link
Collaborator

I still think it is a bug if it doesn't match user expectation i.e. what Glyphs UI shows. I was certainly confused by this more than once and re-edited the source to match what glyphsLib is doing, and it is going to confuse more people in the future.

anthrotype added a commit that referenced this issue Dec 19, 2022
…way around...

Fixes #745 (by breaking everyone's font)
@rsheeter
Copy link
Contributor

To my understanding we have a bunch of existing, working, builds that presumably rely on this being "backwards." Having them all build wrong on the next fontmake update strikes me as breaking user programs in the sense described in https://lkml.org/lkml/2012/12/23/75.

How might we avoid nuking users from orbit and in time reach a place where our compiler and Glyphs agree on this mapping?

Perhaps a good start would be to track down some basic data:

  • How many .glyphs sources do we have?
  • How many of them use Axis Mappings?

anthrotype added a commit to anthrotype/google_fonts_sources that referenced this issue Dec 20, 2022
anthrotype added a commit to anthrotype/google_fonts_sources that referenced this issue Dec 20, 2022
@rsheeter
Copy link
Contributor

@anthrotype's data in rsheeter/google_fonts_sources#3 makes it look pretty plausible to make fontmake figure out which direction the mapping goes. That might allow us to support Glyphs intended mapping direction without breaking existing sources because we'd detect the reversal, warn, and then compile the font correctly.

@schriftgestalt
Copy link
Collaborator

How might we avoid nuking users from orbit and in time reach a place where our compiler and Glyphs agree on this mapping?

If I need to update some data structures when opening files, I check the build number from .appVersion and switch the mapping for all files that are older than the current version.

OR

I could change it in Glyphs (the upcoming 3.2 update could allow for a change like that). We are discouraging the usage of "Axis Mapping" for some time now (in favor of the "Axis Location" parameters. So the fallout should be rather small.

@anthrotype
Copy link
Member

I could change it in Glyphs...

actually that would be most welcome, if you could implement that in a backward compatible way by looking at the .appVersion 👍

@schriftgestalt
Copy link
Collaborator

done in version 3.2 3168

@anthrotype
Copy link
Member

Thank you Georg!

@madig
Copy link
Collaborator

madig commented Apr 11, 2023

Random thought: should we drop Axis Mappings and always just write Axis Locations?

@khaledhosny
Copy link
Collaborator

This is fixed on GlyphsApp side, no need to change anything here.

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

No branches or pull requests

6 participants