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 version 5 + method to convert from v5 to v4 + method to build STAT table and instance names #2436

Merged
merged 4 commits into from
Apr 14, 2022

Conversation

belluzj
Copy link
Contributor

@belluzj belluzj commented Oct 29, 2021

This is a continuation of the extension to the designspace format initially proposed here and discussed at the last UFO meeting in July 2020.

  • [designspaceLib] Increment format version to 5.0
    • Add discrete axes, variable fonts, STAT information, either design- or user-space location on instances
      • Add fontTools.designspaceLib.split module to split a designspace into sub-spaces that interpolate and that represent the variable fonts listed in the document
    • Make instance names optional and allow computing them from STAT data instead
      • Add fontTools.designspaceLib.statNames module
    • Allow instances to have the same location as a previously defined STAT label
    • Deprecate some attributes
      • SourceDescriptor: copyLib, copyInfo, copyGroups, copyFeatures
      • InstanceDescriptor: kerning, info; glyphs: use rules or sparse sources
      • both: location: use the more explicit designLocation
      • Note: all are soft deprecations and existing code should keep working
    • Update documentation for Python methods and the XML format
  • [varLib] Add build_many to build several variable fonts from a single designspace document
    • Add fontTools.varLib.stat module to build STAT tables from a designspace document

@justvanrossum
Copy link
Collaborator

Hi Jany,
Thank you so much for this work! I've done a rough scan of the changes and in general I tend to agree with your changes.

With regard to your question:

Related to the above: is it beneficial to have a default value on discrete axes?

I think so, yes: for example in an Upright/Italic setup, I consider "Upright" the default, and "Italic" the optional "special" one.

@justvanrossum
Copy link
Collaborator

justvanrossum commented Oct 29, 2021

Btw. I'm not fully understanding the problem with using fontTools.otlLib.builder.buildStatTable instead of statmake. Yes, the statmake dependency should be removed, but why would a lib key be necessary? Coult varLib.build be changed to satisfy the needs? Does fontTools.otlLib.builder.buildStatTable need improvements?

@belluzj
Copy link
Contributor Author

belluzj commented Oct 29, 2021

We (Nikolaus and I) used statmake to get to a working POC fast because we're used to it and wanted to use it as a target format for the conversion script. But I don't think there's any problem with the fontTools STAT utilities, we just haven't looked into them properly yet. Statmake will certainly not be needed when building a VF directly from DSv5.

@justvanrossum
Copy link
Collaborator

Slightly tangential: I'm often somewhat confused by the fact that instances are defined by designspace coordinates, and not userspace coordinates. The latter intuitively makes more sense to me. I wonder if other people feel the same way, and if yes, should we consider doing something about it? For instance by adding a "uservalue" attribute to the <dimension> element, to be mutually exclusive with "xvalue", or by adding some attribute to the <location> to specify in which space the dimensions are given.

@belluzj
Copy link
Contributor Author

belluzj commented Oct 29, 2021

I agree on instances, it's listed in the open questions :) happy to implement your suggestion next week

@justvanrossum
Copy link
Collaborator

Urg! See how thoroughly I reviewed your work :) I mean, sorry :/ Yes, let's try to address this in DS5. I'm by no means 100% convinced of either of my suggestions, so if you have different ideas: let's hear it.

@behdad
Copy link
Member

behdad commented Oct 29, 2021

LGTM in general as well.

And yes, I agree instances are better specified in user-space.

In fact, that has always been my preference. C.f also adobe-type-tools/afdko#1350

In DS5 please let's make it possible to specify all coordinates in both userspace and designspace.

@LettError
Copy link
Collaborator

Ooh that looks promising. I know my responsibility is fading here but if it is possible I'd like to make some notes over the weekend?

@benkiel
Copy link
Collaborator

benkiel commented Oct 30, 2021

One thing in reading this that I really like is the Variable font; my understanding is that this would allow one to specify in the Designspace a larger variable font, and then subsets; for instance a variable font with an optical size and weight axis: this would allow generating a variable font with both of those axes, and then a variable font with just optical size at a particular weight value. Is that a correct reading, or am I off base?

@belluzj
Copy link
Contributor Author

belluzj commented Oct 30, 2021

@LettError that would be great, thanks in advance!

@benkiel yes absolutely, check the Aktiv Grotesk example file:

<variable-fonts>
<variable-font name="AktivGroteskVF_WghtWdthItal">
<axis-subsets>
<axis-subset name="Weight"/>
<axis-subset name="Width"/>
<axis-subset name="Italic"/>
</axis-subsets>
</variable-font>
<variable-font name="AktivGroteskVF_WghtWdth">
<axis-subsets>
<axis-subset name="Weight"/>
<axis-subset name="Width"/>
</axis-subsets>
</variable-font>
<variable-font name="AktivGroteskVF_Wght">
<axis-subsets>
<axis-subset name="Weight"/>
</axis-subsets>
</variable-font>
<variable-font name="AktivGroteskVF_Italics_WghtWdth">
<axis-subsets>
<axis-subset name="Weight"/>
<axis-subset name="Width"/>
<axis-subset name="Italic" uservalue="1"/>
</axis-subsets>
</variable-font>
<variable-font name="AktivGroteskVF_Italics_Wght">
<axis-subsets>
<axis-subset name="Weight"/>
<axis-subset name="Italic" uservalue="1"/>
</axis-subsets>
</variable-font>
</variable-fonts>

@behdad
Copy link
Member

behdad commented Oct 30, 2021

 <axis-subsets> 
   <axis-subset name="Weight"/> 
   <axis-subset name="Width"/> 
   <axis-subset name="Italic" uservalue="1"/> 
 </axis-subsets>

Can axes limit the range? I also don't think the name axis-subset is needed. Why not just axis with min/default/max, possibly omitted, etc?

@LettError
Copy link
Collaborator

LettError commented Oct 31, 2021

  • userspace values in instances: I see it can be useful. But if possible it would be good to allow designspace coords as well, but never at the same time.
  • During the design of a system, sources come first, instances right after. Axis mapping is likely to come in quite late in the process. Which means instances with userspace locations might shift. This is not an argument against userspace coords, just to say that they don't always exist.
  • DS5 would still allow anisotropic locations for instances, right? xvalue="1" yvalue="2" I know such locations are not possible in Variable Fonts - they are useful in design.
  • Discrete axes: good idea. I don't understand the question on defaults - but I can't think of a need for a default on a discrete axis.
  • Deprecated: <copyLib, -Info, -Group, -Features> elements on <source> elements - ok. But is there a way to record that a UFO source should only contribute glyph geometry? This is necessary for partial sources. It might also be useful to be able to indicate that a UFO source if for kerning only. I have not built such a thing, but maybe someone else has.
  • Deprecated: <glyphs>, <kerning>, <info> elements on <instance> elements - ok.
  • No comment on the STAT related things - if this is what you need. :)

@madig
Copy link
Contributor

madig commented Nov 1, 2021

If you'd like to look at a few things this should make possible, see https://github.com/daltonmaag/fonttools/tree/impl-designspace-v5/Tests/designspaceLib/data. I sketched out various use-cases:

@madig
Copy link
Contributor

madig commented Nov 1, 2021

@LettError I was wondering the other day,

DS5 would still allow anisotropic locations for instances, right? xvalue="1" yvalue="2" I know such locations are not possible in Variable Fonts - they are useful in design.

Are anisotropic locations allowed anywhere else? How should they to be evaulated in rules?

@LettError
Copy link
Collaborator

@LettError I was wondering the other day,

DS5 would still allow anisotropic locations for instances, right? xvalue="1" yvalue="2" I know such locations are not possible in Variable Fonts - they are useful in design.

Are anisotropic locations allowed anywhere else? How should they to be evaulated in rules?

Type designers use designspaces to construct systems as part of the design process and anisotropic have been a very useful part of that. I don't expect any kind of support in a variable font pipeline. But is should be possible to store the data.

@belluzj
Copy link
Contributor Author

belluzj commented Nov 1, 2021

@behdad

  • yes axes can limit range with userminimum="400" usermaximum="700" userdefault="400" (since we're introducing this element we thought useful to prefix the attribute names with user to prevent confusion about user vs design). See example below:

    <variable-font name="TestRB_Wght">
    <!-- As an example, this would be the Roman with a reduced range. -->
    <axis-subsets>
    <axis-subset name="Weight" userminimum="400" usermaximum="700" userdefault="400"/>
    <axis-subset name="Italic" uservalue="0"/>
    </axis-subsets>
    </variable-font>

    The same file has several other examples with annotations. It works as you're suggesting: if you omit the min/max then the whole axis is included. Also, here is what happens to instances when you limit the range of an axis, e.g. range of "axis 1" in the example below:

    image

  • About naming the element just <axis>, I think it might be confusing w.r.t. the top-level <axis> element, because the attributes are not the same. (e.g. uservalue="1" wouldn't make sense for an <axis> but it does for an <axis-subset>). If the concern about confusion is not warranted I'm happy to shorten the name to <axis>.

  • About allowing user space locations wherever possible: yes, I'll look at all the places were it's possible and add that to the changes. Only one of user or design location will be allowed, as Erik suggests.

@LettError

  • userspace values in instances: your suggestion sounds good, we'll add the option to use uservalue="..." to specify locations in user coordinates. About anisotropy, we are leaving both xvalue and yvalue untouched. We'll add a check that for each <dimension> element, only one coordinate system is used to specify the value.

    • Do you think that user coordinates values need to handle anisotropy? I would think not, because user coordinates is concept for VFs, which don't handle anisotropy anyway? Our current understanding is that design space coordinates on instances is the only place where anisotropy makes sense within the designspace file format. Is that right?
    • As for rules, should rules only apply to the xvalue?
  • But is there a way to record that a UFO source should only contribute glyph geometry? This is necessary for partial sources.

    We're planning to change/"generalize" the elements <muteInfo>, <muteKerning>, and <mutedGlyphNames> to also be applicable to varLib and implement support for them in the VF pipeline. Currently they're marked as used only by mutatorMath. This comes from feedback by @arrowtype that sparse sources should be more explicit.

    • If we want to go further about sparse sources, we could also add "allowlist" versions of the attributes above, for example:

      • includedGlyphNames to list glyph names that the source contributes geometry for; provide an empty list for muting all glyphs, omit the element to keep all glyphs.
      • includeFeatures to list features that the source contributes. It could be more general than just muteKerning, e.g. you could contribute kern but not mark and mkmk, which is a common issue. Provide an empty list to not contribute any GPOS. Omit the element to contribute all features.

      What do you think?

Thanks both for the feedback!

@LettError
Copy link
Collaborator

  • Do you think that user coordinates values need to handle anisotropy? I would think not, because user coordinates is concept for VFs, which don't handle anisotropy anyway? Our current understanding is that design space coordinates on instances is the only place where anisotropy makes sense within the designspace file format. Is that right?

Yes. Anisotropy is useful during design. But userspace coordinates imply axis mapping and variable fonts where such coordinates have no meaning. So no anisotropy in userspace.

  • As for rules, should rules only apply to the xvalue?

That seems reasonable.

We're planning to change/"generalize" the elements <muteInfo>, <muteKerning>, and <mutedGlyphNames> to also be applicable to varLib and implement support for them in the VF pipeline. Currently they're marked as used only by mutatorMath. This comes from feedback by @arrowtype that sparse sources should be more explicit.

Ok, good!

  • If we want to go further about sparse sources, we could also add "allowlist" versions of the attributes above, for example:

    • includedGlyphNames to list glyph names that the source contributes geometry for; provide an empty list for muting all glyphs, omit the element to keep all glyphs.
    • includeFeatures to list features that the source contributes. It could be more general than just muteKerning, e.g. you could contribute kern but not mark and mkmk, which is a common issue. Provide an empty list to not contribute any GPOS. Omit the element to contribute all features.

Specifically for sparse kerning interventions there are some scenarios to think through, but I don't know if that needs to be solved in a format description.

Thanks!

@ctrlcctrlv
Copy link
Contributor

Thanks for working on this, many welcome changes making Designspace that much closer to general utility and descriptive of the entire OpenType standard.

I would ask that #2434 please be considered. This is the role of <rule> per your (¿)UML(?) chart:

image

As demonstrated by myself, this is not enough…both GPOS and GSUB can activate via FeatureVariations. Even if you were to keep the current <sub> it still will not work as a Dict because both key and value can be an array; value can be NULL (GSUB type 2, 0 replacement glyphs special case).

Coming down the pipeline is also COLRv1 which allows for varying the transparency of a layer of a glyph according to axis values. This is likely to be expressed in FEA syntax.

Please consider <fea>

〜:mage_man:ピョンピョン

@belluzj
Copy link
Contributor Author

belluzj commented Nov 8, 2021

@ctrlcctrlv thanks for the comment. I feel your proposal is orthogonal to what we want to implement here (it's not tied with STAT data, and is very specific to the <rule> element that so far we were not planning to touch with this MR). May I suggest that your proposal be worked on as part of another merge request rather than be bundled with this one? I have a comment about your suggestion that I'll go make on the issue.

@ctrlcctrlv
Copy link
Contributor

OK :)

Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

thanks Jany and Nikolaus! I left some comments below.
I see a lot of FIXMEs and TODOs. It'd be good to know what's the plan with each of those.
Regarding adding statmake as dependency, I think we should be able to avoid that. It seems like it's currently only being used as a transport mechanism to store the STAT-related data in the DSv4.lib. Maybe the whole convert5to4 should be moved elsewhere.

Lib/fontTools/designspaceLib/__init__.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/__init__.py Show resolved Hide resolved
Lib/fontTools/designspaceLib/__init__.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/__init__.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/__init__.py Outdated Show resolved Hide resolved
Tests/designspaceLib/designspace_v5_test.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/convert5to4.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/convert5to4.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/convert5to4.py Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/convert5to4.py Outdated Show resolved Hide resolved
@belluzj
Copy link
Contributor Author

belluzj commented Nov 15, 2021

@chrissimpkins says: we should show this to font editors and plugin developers to make sure they're aware of the changes and would be happy to implement them or know how to stick to V4.

Maybe the library should keep saving as version 4 for a while or have an option to opt into saving as version 5? Or at least, opening a V4 and saving again it should save as V4.

@chrissimpkins
Copy link
Member

@chrissimpkins says: we should show this to font editors and plugin developers to make sure they're aware of the changes and would be happy to implement them or know how to stick to V4.

Based on recent work with a designer who uses Erik's https://github.com/LettError/designSpaceRoboFontExtension to generate and edit DS files in Robofont. This is simply a backwards compatibility consideration. I recognize that Erik is involved in this thread and I don't think that it is anyone's responsibility to engage developers directly. I don't have a good understanding of the breadth of built-in / 3rd party plugin DS editor tooling out there. It would be useful to make sure that users have a way to get back to a state where their v4 based plugins work when they don't need expanded v5 functionality if any backwards incompatible changes are introduced.

@benkiel
Copy link
Collaborator

benkiel commented Nov 16, 2021

100% this should have backwards compatibility built in; meaning v4 can still be opened, read/used, and saved back as v4. Breaking that will cause a lot of havoc when tools that are built on v4 assumptions cease to work when the library is updated.

I don't know what Glyphs and FontLab do with designspace files, but this should be put in front of them at the minimum and likely others. The very painful lesson from UFO3 is that if it's really hard for tool makers to implement, it won‘t quickly be picked up in tools; and then designers won't use v5.

@anthrotype
Copy link
Member

of couse, no reason adding DS v5 support should be a breaking change

opening a V4 and saving again it should save as V4

yeah, makes sense

@benkiel
Copy link
Collaborator

benkiel commented Nov 16, 2021

I should have been more specific about 'breaking change': I would expect that the API won't change, or at least be made to be backwards compatible so that when this is merged it doesn't then break tools expecting just v4. Seems like everyone agrees on this, just wanted to clearly state it.

@belluzj belluzj force-pushed the impl-designspace-v5 branch 3 times, most recently from 633cedd to cd288d1 Compare February 14, 2022 18:43
@belluzj belluzj force-pushed the impl-designspace-v5 branch 3 times, most recently from 4546e94 to 80b3fd3 Compare March 8, 2022 18:20
@belluzj belluzj force-pushed the impl-designspace-v5 branch 2 times, most recently from 56de3ca to 025ab53 Compare March 15, 2022 18:51
@belluzj belluzj force-pushed the impl-designspace-v5 branch 3 times, most recently from e33bf43 to 63e6d23 Compare March 28, 2022 18:16
Copy link
Member

@anthrotype anthrotype left a comment

Choose a reason for hiding this comment

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

LGTM (with some minor comments below)

Would you mind also writing some brief high-level release notes? Maybe update the first message (i.e. PR decription).

Thanks again Jany for working on this!

Doc/source/designspaceLib/xml.rst Outdated Show resolved Hide resolved
Lib/fontTools/designspaceLib/__init__.py Outdated Show resolved Hide resolved
Lib/fontTools/varLib/__init__.py Outdated Show resolved Hide resolved
Lib/fontTools/varLib/stat.py Outdated Show resolved Hide resolved
@belluzj
Copy link
Contributor Author

belluzj commented Apr 13, 2022

@anthrotype I wrote release notes in the PR description.

@belluzj belluzj merged commit 5b3db49 into fonttools:main Apr 14, 2022
@belluzj belluzj deleted the impl-designspace-v5 branch April 14, 2022 14:17
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.

None yet

9 participants