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

Couldn't merge the fonts (GPOS.table.FeatureList.FeatureCount) #894

Closed
rsms opened this issue Jun 12, 2022 · 32 comments
Closed

Couldn't merge the fonts (GPOS.table.FeatureList.FeatureCount) #894

rsms opened this issue Jun 12, 2022 · 32 comments

Comments

@rsms
Copy link

rsms commented Jun 12, 2022

While building a relatively small and simple variable font without features, I'm getting an error "Couldn't merge the fonts, because some values were different, but should have been the same." The UFOs are generated from a .glyphs file by glyphsLib. If I export the font inside Glyphs it works just fine (and the resulting ttf is good.)

However, if I remove all glyphs except /.undef /a and /A the build succeeds. So I'm guessing this is an issue related to anchor data in *.ufo/glyphs/*.glif files (which I guess is used to construct mark tables.)

I'm included a copy of the source files to be used for debugging and repro:
src.zip

Example:

$ fontmake -o variable -m Intra.designspace --output-path Intra.var.ttf
# ... (regular output truncated)
INFO:fontTools.varLib:Merging OpenType Layout tables
fontmake: Error: In 'build/ufo/Intra.designspace': Generating fonts from Designspace failed:

Couldn't merge the fonts, because some values were different, but should have
been the same. This happened while performing the following operation:
GPOS.table.FeatureList.FeatureCount

The problem is likely to be in RSMS Intra Thin Regular:

Incompatible features between masters.
Expected: mark, mkmk.
Got: mark, mkmk.

Versions:

  • fontmake --version # = 3.3.0
  • python --version # = Python 3.9.12
@rsms
Copy link
Author

rsms commented Jun 12, 2022

Trying to bisect my way to the culprit, I find that if I remove the following glyphs from *.ufo/glyphs/contents.plist fontmake succeeds. /s is used as a component and has three anchors, but interestingly /f is really simple: not used as a component and has only two anchors (top and bottom.)

  • f
  • s
  • sacute
  • scaron
  • scedilla
  • schwa
  • scircumflex
  • scommaaccent

@rsms
Copy link
Author

rsms commented Jun 12, 2022

The three /f .glif files are identical with the only exception of path coordinates: (I removed the duplicate metrics keys of the "Black" master, which had no effect on build success.)

Screen Shot 2022-06-12 at 12 22 00

@anthrotype
Copy link
Member

thanks for the report and for the reproducer.
the error message is a bit misleading, it complains the FeatureCount is different across masters, but then it reports that the expected and the "got" features are the same which is confusing.
So the wrong error message is a bug in fonttools that needs fixing.
In the case of your particular font, I attached a debugger and I can see there's indeed one master that has ["kern", "mark", "mkmk"], with the additional "kern" feature, whereas the rest only has ["mark", "mkmk"].
The Regular UFO contains a kerning.plist file whereas the other two master UFOs don't, so I think that's why this happens.

@anthrotype
Copy link
Member

I fixed the confusing error message in fonttools/fonttools@8d99250

it now looks like below when building your Intra.designspace with fontmake:

Couldn't merge the fonts, because some values were different, but should have
been the same. This happened while performing the following operation:
GPOS.table.FeatureList.FeatureCount

The problem is likely to be in RSMS Intra Thin:
Expected to see .FeatureCount==3, instead saw 2

Incompatible features between masters.
Expected: kern, mark, mkmk.
Got: mark, mkmk.

@rsms
Copy link
Author

rsms commented Jun 13, 2022

In the case of your particular font, I attached a debugger and I can see there's indeed one master that has ["kern", "mark", "mkmk"], with the additional "kern" feature, whereas the rest only has ["mark", "mkmk"].

Ah! Thank you.
Added a kerning pair to each master and now fontmake succeeds

@rsms rsms closed this as completed Jun 13, 2022
@emmamarichal
Copy link

Hi @rsms, @anthrotype
I have exactly the same problem, but I didn't quite understand how you managed to solve it?
Because I did a test by keeping only one letter, and trying to remove a pair of kerning, but the font still exports well...
I can't find the origin of the bug 🤔 I have 3 masters and almost 300 pairs for each, do you know a way to check them quickly?

fontmake.errors.FontmakeError: In 'Fontexemple.glyphs' -> 'master_ufo/Fontexemple.designspace': Generating fonts from Designspace failed: 

Couldn't merge the fonts, because some values were different, but should have
been the same. This happened while performing the following operation:
GPOS.table.FeatureList.FeatureCount

The problem is likely to be in Fontexemple:
Expected to see .FeatureCount==5, instead saw 4

Incompatible features between masters.
Expected: kern, kern, kern, mark, mkmk.
Got: kern, kern, mark, mkmk.

@anthrotype anthrotype reopened this Sep 30, 2022
@anthrotype
Copy link
Member

@emmamarichal which version of ufo2ft have you got installed? Try pip show ufo2ft.

The issue seems to be that some masters ends up with an additional kern feature record compared to the default master (5 instead of expected 4 as the error message says) and varLib.merge expect all master GPOS tables to have the same number of feature records.

We need to debug why this happens. Do you have any manually written kern feature code in any of the master's features.fea? I suppose not, because you're using Glyphs app and the features are font-wide global.

We can try to export the masters to UFO with fontmake Fontexample.glyphs -o ufo, then run the ufo2ft kernFeatureWriter on each one and get the resulting generated kern feature and compare the three of them and verify that they align and are interpolation compatible.

$ python -m ufo2ft.featureWriters KernFeatureWriter --output master_ufo/Fontexample-Regular_modified.ufo master_ufo/Fontexample-Regular.ufo
$ python -m ufo2ft.featureWriters KernFeatureWriter --output master_ufo/Fontexample-Bold_modified.ufo master_ufo/Fontexample-Bold.ufo
# ...

Then diff the respective features.fea files of the thus modified UFOs and see if you spot anything that signals that one font would end up with GPOS table with 5 feature records instead of 4...

@emmamarichal
Copy link

I use 2.28.0 version.
No I haven't, I'm using Glyphs indeed, and I can't see any kern features in the file.

Thank you, I'll try that and tell you what I get!

@emmamarichal
Copy link

@anthrotype I understood the whole process you explain, but I never used ufo2ft, my question will sound stupid, but how to run ufo2ft kernFeatureWriter on ufos? I didn't find a --help that explains how to use this
I tried several way to write the command but without any success haha

@anthrotype
Copy link
Member

no worries!

Don't the commands I wrote in my earlier comment (#894 (comment)) work?

One can run one or more feature writers on a UFO from the console by running the command python -m ufo2ft.featureWriters --help

@anthrotype
Copy link
Member

oh looks like I messed up the order of the arguments in the commands above, you're right. The first argument is the path to the input UFO, then any following arguments are the names of feature writer classes.

$ python -m ufo2ft.featureWriters --output master_ufo/Fontexample-Regular_modified.ufo master_ufo/Fontexample-Regular.ufo KernFeatureWriter
$ python -m ufo2ft.featureWriters --output master_ufo/Fontexample-Bold_modified.ufo master_ufo/Fontexample-Bold.ufo  KernFeatureWriter
# ...

@anthrotype
Copy link
Member

I used --output option to avoid modifying the input UFO in-place but instead create a new modified one with a different filename, you can omit that if you prefer to apply the modifications to the input UFO. Note that command should preferably only be used for testing/debugging purposes.

@emmamarichal
Copy link

Maybe I missed something? I'm not very good using the command line haha

python -m ufo2ft.featureWriters --output master_ufo/FrankRuhlLibre-Medium_modified.ufo master_ufo/FrankRuhlLibre-Medium.ufo KernFeatureWriter 
Traceback (most recent call last):
  File "/opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 197, in _run_module_as_main
    return _run_code(code, main_globals, None,
  File "/opt/homebrew/Cellar/python@3.9/3.9.10/Frameworks/Python.framework/Versions/3.9/lib/python3.9/runpy.py", line 87, in _run_code
    exec(code, run_globals)
  File "/Users/marichalemma/Google/env/lib/python3.9/site-packages/ufo2ft/featureWriters/__main__.py", line 35, in <module>
    ufo = loader(args.ufo)
TypeError: __init__() takes 1 positional argument but 2 were given

anthrotype added a commit to googlefonts/ufo2ft that referenced this issue Sep 30, 2022
@anthrotype
Copy link
Member

oops you're right, there's a bug sorry! LOL

@anthrotype
Copy link
Member

@emmamarichal please try again after installing ufo2ft from that fix-feature-writers-main git branch:

pip install -e git+https://github.com/googlefonts/ufo2ft.git@fix-feature-writers-main#egg=ufo2ft

@emmamarichal
Copy link

No problem, thanks a lot for your help!
I'll take a look next week :)

@emmamarichal
Copy link

emmamarichal commented Oct 7, 2022

The command worked! thanks 🙏

One of the lookup in the Black master is missing. Do you know what parameter in Glyphs can change this? I tried to add it manually, but it doesn't work.
Also I tried to export only from Glyphs and it works

Black

Capture d’écran 2022-10-06 à 17 26 37

Regular

Capture d’écran 2022-10-06 à 17 26 55

@anthrotype
Copy link
Member

yes, it looks like kern_dflt is missing from the Black master

Do you know what parameter in Glyphs can change this?

It's ufo2ft's kernFeatureWriter that is producing the kern feature from the kerning data in the master UFO's kerning.plist, so it's not something you can control from Glyphs.app.

What does the kern_dflt lookup contain in the Regular master? Could you maybe remove those kerning pairs from Regular as well, so it's aligned with the Black? Or conversely add some to the Black so it's aligned with Regular?
By removing or adding kerning pairs, I don't mean you go and edit the kern feature directly, but actually modify the kerning data in the font editor.

@emmamarichal
Copy link

Hi @anthrotype !
Thanks, it works now :)
I used New Tab with Kerning missing in masters script from mekkablue, and added some pairs.

@guidoferreyra
Copy link

I just got the following on a font that was able to compile using an older version of fontmake.

Expected to see .FeatureCount==4, instead saw 5

Incompatible features between masters.
Expected: kern, kern, mark, mkmk.
Got: kern, kern, kern, mark, mkmk.

I tried building using --feature-writer KernFeatureWriter but same result.

I wonder how can I debug this on a font with 16 masters 😬 Any help it’s appreaciated

@madig
Copy link
Collaborator

madig commented Apr 3, 2023

@guidoferreyra If you're using ufo2ft 2.31.0, you are likely running into the exacerbated problem of missing kerning in at least one of your sources. The new kerning feature writer splits kerning into different lookups by script, which means that if one source has kerning for all scripts, but a fresh new source without full kerning coverage yet will get fewer lookups, and then merging fails. Try inserting empty glyph-to-glyph kerns for each script in your new source, or copy-paste existing kerning pairs over and set them all to zero.

The real solution is either googlefonts/ufo2ft#635 or a change in varLib or ufo2ft to insert empty kerns or lookups, but I still don't understand what varLib does while merging, so that effort has stalled so far.

@guidoferreyra
Copy link

Hi @madig Thanks for you detailed reply I will try what you said.

@emmamarichal
Copy link

cc @m4rc1e @simoncozens
Me again!
This time I have an error a bit different:

Couldn't merge the fonts, because some values were different, but should have
been the same. This happened while performing the following operation:
GPOS.table.FeatureList.FeatureRecord[0].Feature.LookupListIndex[0]

The problem is likely to be in BioRhyme Expanded ExtraBold:
Expected to see [0]==3, instead saw 4

I don't have the part concerning the kern features

Incompatible features between masters.
Expected: kern, kern, mark, mkmk.
Got: kern, kern, kern, mark, mkmk.

I still tried to add kerning just in case, thanks to this script: New Tab with Kerning missing in masters. Now I know I don't have missing pairs anymore.
So I wonder from where this error came. I have some pairs with "0", and some of them have decimal numbers. Maybe that's a factor?
And maybe it doesn't have anything to do with kerning anymore 🤔

If someone has any clue it would be awesome!
Thanks!

@anthrotype
Copy link
Member

some masters end up with more kern lookups than others. the new kern feature writer splits kerning lookups by script, if you have kerning for some scripts but not in others you may end up with master GPOS with different number of kerning lookups. Another source of discrepancy might be the fact that the kern writer may generate distinct lookups (one with and other without lookupflag IgnoreMarks) when it finds kerning pairs that involve glyphs classified as marks in GDEF (e.g. kerning between base vs mark, or mark vs mark). You need to make sure that either all or none of the masters contain such pairs..

try compiling each individual master UFOs and passing --debug-feature-file=path/to/some/file.fea and inspect the generated features.

@emmamarichal
Copy link

Great, thank you, I'll try that!

@madig
Copy link
Collaborator

madig commented Jul 7, 2023

Alternatively, run the script from https://gist.github.com/madig/76567a9650de639bbff51ce010783790 on the offending Designspace. It will fill in the kerning. Don't commit the changes though 😬

@m4rc1e
Copy link
Contributor

m4rc1e commented Jul 12, 2023

If you're using GlyphsApp, here's a terrible script to pad the kerning, https://gist.github.com/m4rc1e/999669c69fe8e7c16997bbdd21ed89af. Copy and paste it into your macro window.

I've padded the missing kerns with -1 because 0 doesn't seem to work (cba to look into this).

cc @emmamarichal

@rsms
Copy link
Author

rsms commented Aug 30, 2023

After upgrading to fontmake 3.6 from 3.5 my build fails with a similar message as discussed here. If I downgrade fontmake to 3.5, it works as expected.

$ fontmake -o variable \
  -m build/ufo/Inter-Roman.var.designspace \
  --output-path build/fonts/var/.Inter-Roman.var.ttf \
  --verbose WARNING \
  --overlaps-backend pathops --flatten-components --no-autohint --production-names
fontmake: Error: In 'build/ufo/Inter-Roman.var.designspace': Generating fonts from Designspace failed:

Couldn't merge the fonts, because a list of objects had inconsistent lengths.
This happened while performing the following operation:
GDEF.table.MarkGlyphSetsDef.Coverage[0].glyphs

The problem is likely to be in Inter Black:
Expected to see .glyphs==1, instead saw 2

If I run fontmake with --verbose INFO I get thousands of messages like these:

INFO:ufo2ft.featureWriters.kernFeatureWriter:Skipping kerning pair <('Enghecyrillic', 'Gamma', 'Ge-cy', 'Ghestrokecyrillic', 'T', 'Tau', 'Tbar', 'Tcaron', 'Tcircumflexbelow', 'Tcommaaccent', 'Tdotaccent', 'Tdotbelow', 'Te-cy', 'Tedescendercyrillic', 'Thook', 'Thook.base', 'Tlinebelow', 'Trthook', 'afii10050', 'afii10052', 'uni021A', 'uni023E', 'uni04F6', 'uni04FA') ('ibreve', 'idieresis', 'idieresisacute', 'iinvertedbreve', 'imacron', 'itilde', 'yi-cy') 186> with mixed script (Grek, Latn)

There are no warnings. Here's a complete transcript: https://gist.github.com/rsms/1685cb4e86982851e39261e075dd66d1

Reproduction:

git clone https://github.com/rsms/inter.git
cd inter
git checkout --detach 11c9e05d7f1642e818634976f4db03fa6e612455
patch -p1 <<PATCH
diff --git a/Pipfile b/Pipfile
index 2a3e3a96d..62f85699d 100644
--- a/Pipfile
+++ b/Pipfile
@@ -7,2 +7 @@ name = "pypi"
-ufo2ft = "==2.30.0"
-fontmake = "==3.5.*"
+fontmake = "==3.6.*"
PATCH
make DEBUG=1 var

@anthrotype
Copy link
Member

@rsms I suspect this might have been caused by the following change in ufo2ft kernFeatureWriter googlefonts/ufo2ft#720. Note I'm not 100% sure as I haven't tested my assumption but you may wish to try revert that PR and see if it fixes your issue.
Also see further discussion at googlefonts/ufo2ft#721 and a possible way to remedy this situation in googlefonts/ufo2ft#722 (comment)

/cc @simoncozens

@simoncozens
Copy link
Contributor

Yeah, this is the cause of the failure. The simple solution is to zero the width of the commabelowcomb in all masters. Currently they're treated as zero-width marks in some masters but not in others.

@anthrotype
Copy link
Member

filed separate issue #1036

@simoncozens
Copy link
Contributor

Here's a script which should help further debug merger issues: https://gist.github.com/simoncozens/35fc0f75993673ae38b1f1179c7e35f6

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

7 participants