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

Problems going from UFOs to .glyphs using v3 file format #754

Open
DavidRaymond opened this issue Dec 1, 2021 · 38 comments
Open

Problems going from UFOs to .glyphs using v3 file format #754

DavidRaymond opened this issue Dec 1, 2021 · 38 comments

Comments

@DavidRaymond
Copy link

DavidRaymond commented Dec 1, 2021

Using v6.0.0 I've been looking at Glyphs v3 file format support.

I see that ufo2glyphs does not currently have an option to output in v3 file format. I created an edited version to test with by adding font.format_version = 3 prior to the font.save section and successfully created a v3 format .glyphs file from a design space file. Is that the correct approach to use to create v3 format files?

Round-tripping using that and then glyphs2ufo produced many differences. Some examples are:

  • postscriptBlueValues, postscriptStemSnapH and postscriptStemSnapV removed from fontinfo.plist
  • styleName in fontinfo.plist changed to Regular for all styles
  • com.schriftgestaltung.weight in lib.plist changed to Regular for all styles
  • public.verticalOrigin added to the lib of all .glif files

Note with the postscript keys, this data does end up in a .glyphs file if it is produced by saving from the Glyphs App - for example blue values end up in Alignment Zones.

@anthrotype
Copy link
Member

thanks for reporting this. Yes, we should add an option to set format_version to 3 for ufo2glyphs.
Regarding the difference you see when roundtripping, at least the first three look like bugs which we should fix; the last one about verticalOrigin I'm not sure. What value do you get? Did the original ufo glyphs contain public.verticalOrigin in their lib?

@DavidRaymond
Copy link
Author

The original .glifs did not have public.verticalOrigin. It was added with a value of 2100 - which is the same value as ascender in fontinfo.

@DavidRaymond
Copy link
Author

DavidRaymond commented Dec 6, 2021

I've done some further testing on this, and discovered that the addition of verticalOrigin is not connected with v3 format support - it happens round-tripping using the standard ufo2glyphs then glyphs2ufo as well.

@DavidRaymond
Copy link
Author

To reproduce the other changes I'd reported:

  • Clone https://github.com/silnrsi/font-andika-mtihani
  • Round-trip the designspace files in source/ using standard ufo2glyphs and glyphs2ufo and save the resulting UFOs
  • Edit ufo2glyphs in cli.py and add font.format_version = 3 after font.disablesAutomaticAlignment = options.enable_automatic_alignment
  • Repeat the round-trip and compare the new UFOs with those saved above

Note that there will be other changes to com.schriftgestaltung... values in addition to those mentioned in my original report. Those reported were just examples.

@DavidRaymond
Copy link
Author

Any news on this? We would like to be able to start using the v3 format.

@jvgaultney
Copy link
Contributor

Any progress on this? The v3 support added in 6.0.0 is pretty much useless without these basic fixes.

@arrowtype
Copy link

arrowtype commented Sep 12, 2022

Just adding in my comment that I’m currently really missing support for converting GlyphsApp “Alignment Zones” into postscriptBlueValues and postscriptOtherBlues and “Stems” into postscriptStemSnapV and postscriptStemSnapH (plus whatever else might relevant here).

@simoncozens
Copy link
Collaborator

The v3 support added in 6.0.0 is pretty much useless without these basic fixes.

This is a rather dramatic statement. Files in Glyphs 3 format can now be compiled using fontmake, and they couldn't before, so I am not sure I would call it "useless".

I'm seeing a similar issue with master names going missing, so I shall take a look at this.

@jvgaultney
Copy link
Contributor

Thanks for looking into the reported issues. Loss of data when going from UFO > Glyphs3 does make it useless (at least to us). As we keep our sources in UFO we need to be able to roundtrip without data loss - or at least without losing data we can't reconstruct. It would also be good for ufo2glyphs to optionally set the format_version to 3.

@kontur
Copy link
Contributor

kontur commented Oct 4, 2022

Was trying to understand this today (also needing those postscriptBlueValues), but I cannot tell how or where all these _parse.. methods are getting called, e.g. https://github.com/googlefonts/glyphsLib/blob/10f1b23c207193a6c47c9e86900d3c6dcce04d8b/Lib/glyphsLib/classes.py#L1563 isn't called as such anywhere directly in the codebase, but that GSFont / GSMaster used in the Builder is getting parsed with the default empty alignmentZones... I just don't understand where the v2 vs v3 flip is made — presumably the _serialize_to_plist above it is just for ufo > glyphs?

@anthrotype
Copy link
Member

cannot tell how or where all these _parse.. methods are getting called

maybe here?

def _parse_dict_into_object(self, res, d):
for name in d.keys():
sane_name = name.replace(".", "__")
if hasattr(res, f"_parse_{sane_name}_dict"):
getattr(res, f"_parse_{sane_name}_dict")(self, d[name])

@kontur
Copy link
Contributor

kontur commented Oct 5, 2022

Okay, thanks @anthrotype for the pointer. I'm slowly starting to arrive at the problem (😅).

So the v3 spec defines:

Both the alignmentZones and ascender, capheight etc. metric values have been replaced by the metricValues list of dictionaries. What used to be alignmentZones is now a set of over (overshoot) properties attached to metrics. This metricsValues list is indexed in parallel with the toplevel metrics list.

GSFontMaster::_parse_alignmentZones_dict is "version 2" only; metricsValues get parsed here but either need to additionally populate GSFontMaster.alignmentZones similar to the "version 2", or builder/blue_values::to_ufo_blue_values needs to do an "either or" based on version. I got the later working in a hacky kind of way, but the former would be preferable, no? Where would be a spot to hook in extracting the alignmentZones from the parsed metrics? And then the reverse... And then next the stems are missing still (at least for my own immediate issue at hand, which is the same as @arrowtype points out).

@simoncozens
Copy link
Collaborator

simoncozens commented Oct 5, 2022

Having .alignmentZones be a (computed) property which reads metricsValues is the right way to do this. We should store things as v3 format internally to the object but provide accessors to get at information in a v2 way.

kontur added a commit to kontur/glyphsLib that referenced this issue Oct 5, 2022
@kontur
Copy link
Contributor

kontur commented Oct 5, 2022

Tried to get the alignment Zones to work Glyphs > UFO direction. The _alignmentZonesParsed is a bit clumsy for tracking if the attribute has been parsed directly (file format v2) or if the getter should parse the zones from the metrics — any better way of doing this?

Also, this test breaks because of the condition in the getter, I suppose...

Can one somehow "hook into" those object parsers, sort of like a callback after a particular attribute (e.g. metricsValues) has been parsed, to perform the alignmentZones calculation once, and not in a getter?

@simoncozens
Copy link
Collaborator

Can one somehow "hook into" those object parsers, sort of like a callback after a particular attribute (e.g. metricsValues) has been parsed, to perform the alignmentZones calculation once, and not in a getter?

Sure. That's what the "converter" field on add_parsers does:

GSGlyph._add_parsers(
    [
        ...
        {
            "plist_name": "lastChange",
            "object_name": "lastChange",
            "converter": parse_datetime,
        },

"Read lastChange from the plist, call parse_datetime on it and store it into lastChange in the object."

@kontur
Copy link
Contributor

kontur commented Oct 9, 2022

@simoncozens ah yes, that's better. I'm still using the _alignmentZones and a getter, thought. I'm having trouble with this bit from the file format spec:

What used to be alignmentZones is now a set of over (overshoot) properties attached to metrics. This metricsValues list is indexed in parallel with the toplevel metrics list.

So inside the converter I'd need to know the top level metrics values and the current metricValue name or index being parsed to drop irrelevant ones (e.g. italic angle) — now I'm just filtering against duplicates in the setter, which is not the right thing. Also trying to do this inside the setter the metrics would not be parsed (what determines the order here anyway?), nor is this viable if an arbitrary list of GSAlignmentZones is passed in.

Will ponder some more. Happy to idea from people more familiar with the code base.

kontur added a commit to kontur/glyphsLib that referenced this issue Oct 17, 2022
kontur added a commit to kontur/glyphsLib that referenced this issue Oct 17, 2022
@guidoferreyra
Copy link

Hi @anthrotype Do you think @kontur change can be integrated? It will be great to be able to psauothint fonts after using glyphsLib.

anthrotype added a commit that referenced this issue Feb 13, 2023
Implements part of #754 via getter/setters for proxying the file format 3 attributes
@DavidRaymond
Copy link
Author

I've just revisited this in case any changes have helped, but I'm still seeing the originally reported problems testing with glyphsLib 6.2.3.dev3+g6372da0c.
I would be really useful to be able to use Glyphs v3 file format...

@jvgaultney
Copy link
Contributor

At a minimum, getting the proper style names and settings from the designspace into the Glyphs 3 file would be a great help. That's an immediate showstopper for us, whereas we can work around some of the other issues. What would be the best way to get this moving?

@simoncozens
Copy link
Collaborator

simoncozens commented May 16, 2023

What would be the best way to get this moving?

Send a PR? Yes, I'm being flippant, but it's an open source project, and not a particularly pleasant codebase to work with, which means things get fixed when either the developers need it or when someone who does need it contributes a fix. I mean, there are some things I'll work on for the craic; glyphsLib is not one of them. I only use glyphsLib for Glyphs-to-UFO conversion, so I avoid touching the other side of the code.

I'm kind of surprised that DaMa (who are the main users and maintainers of UFO-to-Glyphs) haven't hit this problem already. Maybe they're all still Glyphs 2? I don't know. @madig, what do you think?

@jvgaultney
Copy link
Contributor

It's certainly reasonable to expect the users (us) to do the work and send pull requests. In this case we've tried to take a look at how we'd fix it, and can't seem to figure it out. Part of that is our unfamiliarity with the intricacies of how glyphsLib works internally and the significantly different family structure model of the Glyphs3 format, whose concept of masters and layers differs from Glyphs2.

If DaMa is still using Glyphs2 format (like us) then it may be wise for them to switch to Glyphs3. I don't think we can reasonably expect Georg to maintain Glyphs2 format support forever, and supporting Glyphs3 leaves the door open to future improvements (like storing data for useful v3 things in UFO data libs).

@belluzj
Copy link
Collaborator

belluzj commented May 17, 2023

Hello, we're still on Glyphs.app 2. We're planning to move to 3 but it's a slow move to not disrupt anything in our workflows.

@schriftgestalt
Copy link
Collaborator

the significantly different family structure model of the Glyphs3 format, whose concept of masters and layers differs from Glyphs2.

The biggest differences:

  • The way font info is stored (in properties, not in individual keys and custom parameters)
  • Vertical metrics (all are connected for all masters and can have names and filters attached).
  • Defining brace/bracket/color layers is done by a proper data structure instead using some special formats in the layer names.
  • using better data structures here and there

@madig
Copy link
Collaborator

madig commented May 17, 2023

What Jany said. I have started writing a custom ufo2glyphs2ufo tool internally (in 🦀, based on Raph's interp-toy::glyphstool parser) because I loathe touching glyphsLib and I want the pain to stop and we have a limited amount of things we blazingly fast care about. We may release it as OSS if the stars align.

It was actually fairly easy to write the direct conversion, all you need to do is take dictionaries laid out one way and transform them to a different layout, with some stealing from glyphsLib. If you don't want to wait for us and don't care to store .glyphs files in version control, you may be able to bang together a u2g2u that meets your needs in a low thousands of lines, maybe directly using openstep-plist to read and write dicts.

@simoncozens
Copy link
Collaborator

My other advice would be "Move to glyphspackage for storage and rewrite whatever tools you have that work on UFO to work using glyphsLib instead". Other font editors are just as good at reading Glyphs files as reading UFOs these days, and you avoid the several disadvantages of UFO format.

@madig
Copy link
Collaborator

madig commented May 18, 2023

The downside then is that you still have to deal with UFO↔️Glyphs if you want to use the fontmake stack, because ufo2ft works on DS+UFOs natively, only :)

@simoncozens
Copy link
Collaborator

Well, if all your tools are Glyphs-focused, you only need to care about Glyphs to UFO for compilation. Or rather, you don't need to care about it at all; you can just ignore UFO altogether and treat it as fontmake's internal representation.

@jvgaultney
Copy link
Contributor

Some reasons why we use UFO are that it is open, well-documented, nicely human-editable, and long-term stable. Where is the Glyphs 3.2 file format spec? And is it complete and accurate? We'd be very hesitant to rewrite all our UFO tools to work on Glyphs files via glyphsLib. That would only make us more dependent on and vulnerable to glyphsLib changes, and we've been bitten too many times by glyphsLib. At the moment we only depend on glyphsLib to roundtrip to Glyphs. If glyphsLib disappears we can always switch to using FontLab or Robofont or whatever comes next - or depend on Glyphs' direct UFO import/export. If we switch to Glyphs format for our canonical source we become dependent on both Glyphs and glyphsLib. Given how much I hear the people who have contributed to glyphsLib complain about how much they hate working on it - and how much behind-the-scenes work seems to be happening in Rust - I hesitate to increase our dependence.

@schriftgestalt
Copy link
Collaborator

I’m currently working on this. You can have a look at the current state in the Glyphs3 branch. Feedback is welcome.

@DavidRaymond
Copy link
Author

DavidRaymond commented Nov 14, 2023

Thanks for working on this. Should I be able to retest to ufo > glyphs > ufo roundtrip?

I've just pulled the latest code from the branch so I'm running 6.2.6.dev125+g096839f6.

gylphs2ufo is failing with:

Traceback (most recent call last):
File "/home/david/src/pysilfont/venv/bin/glyphs2ufo", line 8, in <module>
  sys.exit(_glyphs2ufo_entry_point())
File "/home/david/src/glyphsLib/Lib/glyphsLib/cli.py", line 288, in _glyphs2ufo_entry_point
  return main(args)
File "/home/david/src/glyphsLib/Lib/glyphsLib/cli.py", line 246, in main
  return options.func(options)
File "/home/david/src/glyphsLib/Lib/glyphsLib/cli.py", line 265, in glyphs2ufo
  glyphsLib.build_masters(
File "/home/david/src/glyphsLib/Lib/glyphsLib/__init__.py", line 113, in build_masters
  font = GSFont(filename)
File "/home/david/src/glyphsLib/Lib/glyphsLib/classes.py", line 5504, in __init__
  load(path, self)
File "/home/david/src/glyphsLib/Lib/glyphsLib/parser.py", line 162, in load
  font.post_read()
File "/home/david/src/glyphsLib/Lib/glyphsLib/classes.py", line 5601, in post_read
  master.post_read()
File "/home/david/src/glyphsLib/Lib/glyphsLib/classes.py", line 2100, in post_read
  metricValue.metric = fontMetric

@schriftgestalt
Copy link
Collaborator

Please test with the Glyphs 3 branch. you might need to manually download glyphsLib to use it.

@DavidRaymond
Copy link
Author

I thought I was on the Glyphs3 branch - 096839f seems to be the latest commit on there.

$ glyphs2ufo --version
glyphsLib 6.2.6.dev125+g096839f6

@schriftgestalt
Copy link
Collaborator

schriftgestalt commented Nov 14, 2023

I just saw that I didn't push all my commits.

And you send me the file and the script/command for testing?

@DavidRaymond
Copy link
Author

My testing routine, on a clone of https://github.com/silnrsi/font-andika-mtihani is:

  • Round-trip the designspace files in source/ using standard ufo2glyphs and glyphs2ufo and save the resulting UFOs
  • Edit ufo2glyphs in cli.py and add font.format_version = 3 after font.disablesAutomaticAlignment = options.enable_automatic_alignment
  • Repeat the round-trip and compare the new UFOs with those saved above

So I had run
$ ufo2glyphs source/AndikaMtihaniItalic.designspace
then
$ glyphs2ufo source/AndikaMtihaniItalic.glyphs
and the latter failed.

@schriftgestalt
Copy link
Collaborator

Can you try with the latest state of the Glyphs3 branch? There are a bunch of differences but most of them are actual a good thing. In the .glyphs file are many duplicate custom parameters on the font/masters that are not to be there (e.g. metrics relates parameters on the font or names on the master).

For testing, I have used those commands:

ufo2glyphs --format-version 2 AndikaMtihaniItalic.designspace
$ glyphs2ufo --no-propagate-anchors /FULL/PATH/TO/font-andika-mtihani/source/AndikaMtihaniItalic.glyphs

There are some small issues, still: e.g. Where to store instances related info (styleMapStyleName, weightClass …)? Should that go to the source.ufo/fontinfo or to instance attributes/lib? the later makes more sense for me as this can handle the case when there are more instances than sources. I’m working on that.

@DavidRaymond
Copy link
Author

I've run your test commands and they work OK, but if I try to test with --format-version 3 I still get an error:

(glvenv) david@DWR-laptop:~/src/font-andika-mtihani$ ufo2glyphs --format-version 3 source/AndikaMtihaniItalic.designspace
(glvenv) david@DWR-laptop:~/src/font-andika-mtihani$ glyphs2ufo --no-propagate-anchors source/AndikaMtihaniItalic.glyphs 
Traceback (most recent call last):
  File "/home/david/src2/glyphsLib/glvenv/bin/glyphs2ufo", line 8, in <module>
    sys.exit(_glyphs2ufo_entry_point())
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 297, in _glyphs2ufo_entry_point
    return main(args)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 255, in main
    return options.func(options)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 274, in glyphs2ufo
    glyphsLib.build_masters(
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/__init__.py", line 113, in build_masters
    font = GSFont(filename)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/classes.py", line 5581, in __init__
    load(path, self)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/parser.py", line 162, in load
    font.post_read()
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/classes.py", line 5678, in post_read
    master.post_read()
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/classes.py", line 2139, in post_read
    metricValue.metric = fontMetric
AttributeError: 'str' object has no attribute 'metric'

I also noticed that if I run glyphs2ufo on the original v2 .glyphs file in the repo it also fails:

(glvenv) david@DWR-laptop:~/src/font-andika-mtihani$ glyphs2ufo --no-propagate-anchors source/AndikaMtihaniItalic.glyphs 
Traceback (most recent call last):
  File "/home/david/src2/glyphsLib/glvenv/bin/glyphs2ufo", line 8, in <module>
    sys.exit(_glyphs2ufo_entry_point())
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 297, in _glyphs2ufo_entry_point
    return main(args)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 255, in main
    return options.func(options)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/cli.py", line 274, in glyphs2ufo
    glyphsLib.build_masters(
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/__init__.py", line 123, in build_masters
    designspace = to_designspace(
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/builder/__init__.py", line 130, in to_designspace
    return builder.designspace
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/builder/builders.py", line 344, in designspace
    list(self.masters)  # Make sure that the UFOs are built
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/builder/builders.py", line 210, in masters
    self.to_ufo_font_attributes(self.family_name)  # .font
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/builder/font.py", line 50, in to_ufo_font_attributes
    to_ufo_metadata(master, ufo)
  File "/home/david/src2/glyphsLib/glvenv/lib/python3.10/site-packages/glyphsLib/builder/font.py", line 132, in to_ufo_metadata
    ufo_key = PROPERTIES_FIELDS[infoValue.key]
KeyError: 'styleMapFamilyNames'

@DavidRaymond
Copy link
Author

Any progress on this? I'd be happy to do more testing if useful.

@schriftgestalt
Copy link
Collaborator

There was a lot other stuff in the way. I’m about to continue to work on this.

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

No branches or pull requests

10 participants