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

Inherit repo from https://bitbucket.org/m4rc1e/zillaslab #5

Merged
merged 5 commits into from
Jul 5, 2017

Conversation

m4rc1e
Copy link
Contributor

@m4rc1e m4rc1e commented May 22, 2017

Myself, @davelab6, Peter Bilak and Yuliya Gorlovetsky have discussed hosting this project on Google Fonts. In order to make this happen, I've had to master the family. The following changes have occured:

  • Fonts get built using fontmake with a build script.
  • .fea files use relative paths. This will solve Missing files? #3
  • Repository structure compliant with Google Fonts structure. Looks similar to this

This is a pretty hefty pr so please check things through. I thought I'd send the pr now so we can all sync up. I/We still need to add .woff conversion to the build chain. When we're all happy, I'll delete my gitlab repo. The benefit of this approach is anyone can build the project using open source tools.

Just a side note, the binaries should also be stored as tagged releases :-)

Family mastering was undertaken by Google Fonts.
@alexgibson
Copy link
Member

I/We still need to add .woff conversion to the build chain.

Can we please make sure both .woff and .woff2 are included? I will look at this PR later, thanks.

@alexgibson
Copy link
Member

It looks like this branch is missing requirements.txt, so I can't install dependencies to test building the font.

Copy link
Member

@alexgibson alexgibson left a comment

Choose a reason for hiding this comment

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

Looks like we're missing the requirements file? Once I can verify that installing/compiling from source works we can merge this. Thanks

@alexgibson
Copy link
Member

I seem to get an error when compiling from source

(env)agibson-10162:zilla-slab agibson$ cd sources
(env)agibson-10162:sources agibson$ sh build.sh
INFO:fontmake.font_project:Building TTFs
INFO:fontmake.font_project:Decomposing glyphs for ZillaSlab-Bold
INFO:fontmake.font_project:Removing overlaps for ZillaSlab-Bold
INFO:fontmake.font_project:Converting curves for ZillaSlab-Bold
INFO:cu2qu.ufo:New spline lengths:
1: 1219
2: 3856
3: 231
4: 38
5: 2

INFO:fontmake.font_project:Saving TTF for ZillaSlab-Bold
Traceback (most recent call last):
  File "/Users/agibson/Sites/zilla-slab/env/bin/fontmake", line 11, in <module>
    sys.exit(main())
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontmake/__main__.py", line 195, in main
    ufo_paths, is_instance=args.pop('masters_as_instances'), **args)
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontmake/font_project.py", line 481, in run_from_ufos
    **kwargs)
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontmake/font_project.py", line 193, in build_ttfs
    self.save_otfs(ufos, ttf=True, **kwargs)
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontTools/misc/loggingTools.py", line 372, in wrapper
    return func(*args, **kwds)
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontmake/font_project.py", line 320, in save_otfs
    ttfautohint(otf_path, hinted_otf_path, args=autohint)
  File "/Users/agibson/Sites/zilla-slab/env/lib/python2.7/site-packages/fontmake/ttfautohint.py", line 32, in ttfautohint
    return subprocess.call(arg_list + args.split() + file_args)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 522, in call
    return Popen(*popenargs, **kwargs).wait()
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 710, in __init__
    errread, errwrite)
  File "/System/Library/Frameworks/Python.framework/Versions/2.7/lib/python2.7/subprocess.py", line 1335, in _execute_child
    raise child_exception
OSError: [Errno 2] No such file or directory

@m4rc1e
Copy link
Contributor Author

m4rc1e commented May 24, 2017

@alexgibson Good spot. The chain runs the fonts through ttfautohint. You're getting this because you don't have it installed on your system. Try brew install ttfautohint.

I'll flag this as an issue on fontmake. A better exception should be raised for this.

@alexgibson
Copy link
Member

You're getting this because you don't have it installed on your system. Try brew install ttfautohint.

I'll give this a go. Can we please add this to the readme?

@m4rc1e
Copy link
Contributor Author

m4rc1e commented May 24, 2017

@alexgibson Once you've built the fonts, it may be best to get Peter Bilak to review the hints in Windows as well. He may prefer to have no autohinting. If he prefers the autohinting, we'll add installing ttfautohint to the readme.

I must apologise for the amount of extra work this is involving. I was given a set of .ufo files which needed mastering. My task was to get this project onto Google Fonts only, hence why I've been rather sloppy. I shouldn't be devoting so much time to this. I think Mozilla needs to factor this into account and we may have underestimated the amount of time needed to get this production worthy.

cc @davelab6

@alexgibson
Copy link
Member

I must apologise for the amount of extra work this is involving. I was given a set of .ufo files which needed mastering. My task was to get this project onto Google Fonts only, hence why I've been rather sloppy. I shouldn't be devoting so much time to this. I think Mozilla needs to factor this into account and we may have underestimated the amount of time needed to get this production worthy.

Thanks for the update. This is all news to me as well (all I was asked to do was push some files to a GitHub repo). I'll try and make sure Yulia gets this feedback.

@alexgibson
Copy link
Member

Ok, I can successfully generate the fonts from source after installing ttfautohint and running the build script, nice work! 🌮

Once you've built the fonts, it may be best to get Peter Bilak to review the hints in Windows as well. He may prefer to have no autohinting. If he prefers the autohinting, we'll add installing ttfautohint to the readme.

I'll email Yulia and see if she can help get Peter to answer this question.

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Jun 26, 2017

@alexgibson I've now added .woff and .woff2 generation to the pipeline.

cc @davelab6

@alexgibson
Copy link
Member

alexgibson commented Jul 4, 2017

@m4rc1e - thanks for the updates. I ran things locally and it generated the fonts as expected. A couple of small things and then I think we can merge this:

1.) Can we please include a compiled version of the fonts in this repo? This repo has existed for long enough now that people within the org are using it, and they may not want to have to go through compiling the fonts, or use the google hosted versions. I think just being able to grab them here is very convenient, so it's not something I think we should lose.

2.) Any reason why the license file was renamed to OFL.txt? Does GitHub recognize this convention as a license file?

3.) Once this merges, would you like me to tag a release?

Thanks

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Jul 5, 2017

Hey Alex,

Thanks for reviewing this.

Can we please include a compiled version of the fonts in this repo?

I suggest we keep the binaries as a tagged release. Roboto follows this approach. I don't recommend storing binaries in git repos. They're hard to diff and people often forget to update them. By having them as a tagged release which only one individual can make keeps the repo much cleaner.

2.) Any reason why the license file was renamed to OFL.txt? Does GitHub recognize this convention as a license file?

At GF, if a project uses the SIL Open Font License (this one does), we name the license OFL.txt. I guess we do this because our repo is a collection of many different families which contain differing license types. I am more than happy to revert this.

3.) Once this merges, would you like me to tag a release?
Yes please. Hopefully this will address 1.

@alexgibson
Copy link
Member

I suggest we keep the binaries as a tagged release. Roboto follows this approach. I don't recommend storing binaries in git repos. They're hard to diff and people often forget to update them. By having them as a tagged release which only one individual can make keeps the repo much cleaner.

Ok, this makes sense agree.

At GF, if a project uses the SIL Open Font License (this one does), we name the license OFL.txt. I guess we do this because our repo is a collection of many different families which contain differing license types. I am more than happy to revert this.

If you could stick to the previous naming convention that would be great, thanks.

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Jul 5, 2017

Done

Thanks for your feedback.

Copy link
Member

@alexgibson alexgibson left a comment

Choose a reason for hiding this comment

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

Thanks @m4rc1e - I can appreciate the extra mile you've gone here. Nice work! 💯

@alexgibson alexgibson merged commit b4786b7 into mozilla:master Jul 5, 2017
@adrientetar
Copy link

I've now added .woff and .woff2 generation to the pipeline.

Why isn't that done with fontTools? The current steps aren't very portable either...

@m4rc1e
Copy link
Contributor Author

m4rc1e commented Aug 10, 2017

@adrientetar the tap bramstein/webfonttools allows for .eot font generation as well. I only included .woff and .woff2 because they had these before, 36d43f9.

The current steps aren't very portable either...

This was bolted together quickly

@davelab6
Copy link

davelab6 commented Aug 11, 2017 via email

@m4rc1e m4rc1e deleted the gf-mastering branch August 14, 2017 09:34
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.

4 participants