googlefonts / glyphsLib

A bridge from Glyphs source files (.glyphs) to UFOs
Apache License 2.0
178 stars 51 forks source link

Hanging intermediate layers off different masters causes a build failure #995

Open simoncozens opened 2 months ago

simoncozens commented 2 months ago

While trying to build Barlow as a variable font, I found that glyph a hangs some of its intermediate layers off the Bold master, like so:

Screenshot 2024-04-05 at 14 53 13

while ae hangs the same intermediate locations off the light master:

Screenshot 2024-04-05 at 14 53 34

Glyphs export is fine with this, but when converting to designspace, glyphsLib converts each one of these to a different source:

    <source filename="Barlow-LightCondensed.ufo" name="Barlow Light Condensed {100, 500}" layer="{100, 500}">
      <location>
        <dimension name="Weight" xvalue="100"/>
        <dimension name="Width" xvalue="500"/>
      </location>
    </source>
    <source filename="Barlow-Light.ufo" name="Barlow Light {100, 500}" layer="{100, 500}">
      <location>
        <dimension name="Weight" xvalue="100"/>
        <dimension name="Width" xvalue="500"/>
      </location>
    </source>

And then it complains that it has two masters with the same location:

  File "/opt/homebrew/lib/python3.11/site-packages/fontmake/font_project.py", line 1302, in _run_from_designspace_interpolatable
    self.build_variable_fonts(
  File "/opt/homebrew/lib/python3.11/site-packages/fontmake/font_project.py", line 450, in build_variable_fonts
    fonts = ufo2ft.compileVariableTTFs(
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/ufo2ft/__init__.py", line 172, in compileVariableTTFs
    return VariableTTFsCompiler(**kwargs).compile_variable(designSpaceDoc)
           ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/ufo2ft/_compilers/baseCompiler.py", line 420, in compile_variable
    vfNameToTTFont = self._merge(designSpaceDoc, excludeVariationTables)
                     ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/ufo2ft/_compilers/interpolatableTTFCompiler.py", line 44, in _merge
    return varLib.build_many(
           ^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/fontTools/varLib/__init__.py", line 1125, in build_many
    vf = build(
         ^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/fontTools/varLib/__init__.py", line 1199, in build
    model = models.VariationModel(normalized_master_locs, axisOrder=axisTags)
            ^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^^
  File "/opt/homebrew/lib/python3.11/site-packages/fontTools/varLib/models.py", line 253, in __init__
    raise VariationModelError("Locations must be unique.")
fontTools.varLib.errors.VariationModelError: Locations must be unique.

We should ignore the putative "master" for an intermediate layer, and should just create a master purely based on the location coordinates.

anthrotype commented 2 months ago

I'm pretty sure I've already seen this. Isn't it the same situation described in https://github.com/googlefonts/glyphsLib/pull/956?

We should ignore the putative "master" for an intermediate layer,

yeah ideally we should. PR most welcome. Maybe you take over the other one /cc @belluzj thanks!

schriftgestalt commented 2 months ago

If someone could put that into a test case, I could fix this in the Glyphs3 branch.

jpt commented 2 months ago

I have a script for this that aligns special layers to one master. I'll just update the Barlow source to do that.

That's probably not the only issue for Barlow VF; for that I'll reply over here