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

Add parenleft and parenright with smoother curves #452

Merged
merged 2 commits into from
Oct 20, 2018

Conversation

vl4dimir
Copy link
Contributor

@vl4dimir vl4dimir commented Sep 4, 2018

Submitting PR as result of discussions – source-foundry/alt-hack#41 and #433


@chrissimpkins edits below:

Test fonts at commit eeb942b

macOS / Linux download with these links and install

ttf-eeb942b43.zip
web-eeb942b43.zip

Windows users can install with the Hack test font installer at this release:

https://github.com/source-foundry/Hack-Test-Win-Installer/releases/tag/v1.2.105

The Windows installer for these dev builds installs the fonts under the family name "Hack Test" so that you can see side-by-side comparisons between the upstream default and test builds.

@chrissimpkins
Copy link
Member

Vladimir, can I ask you to update these glif files to UFO v3 format? We are moving to v3 as of the v4.000 release of Hack and the dev branch that you are based on here is v3.

http://unifiedfontobject.org/versions/ufo3/glyphs/glif/

Copy link
Member

@chrissimpkins chrissimpkins left a comment

Choose a reason for hiding this comment

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

Please update all proposed glif files to UFO v3.

@@ -1,15 +1,15 @@
<?xml version="1.0" encoding="UTF-8"?>
<glyph name="parenleft" format="2">
<glyph name="parenleft" format="1">
Copy link
Member

Choose a reason for hiding this comment

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

For each of the glif files, format will be "2" for UFO v3. See link to UFO v3 documentation in the main thread.

@chrissimpkins
Copy link
Member

You can perform the update with the PyPI package defcon. See docs for save() for more information. Will just need to specify 3 as the ufo version number argument when you save your files and it should perform the update for you. Please don't commit any files other than the .glif in your PR here though! thx

https://ts-defcon.readthedocs.io/en/latest/objects/font.html#defcon.Font.save

@chrissimpkins
Copy link
Member

Are these updates something that you will be able to get to Vladimir?

@vl4dimir
Copy link
Contributor Author

@chrissimpkins I'm on it. Sorry for the delay.

@vl4dimir
Copy link
Contributor Author

@chrissimpkins So, defcon is a Python library. Is there a standalone tool I can run in order to fix this?

Or maybe I can just manually change the 2 to 3? There's no structural differences between the old files and the ones in this PR.

@chrissimpkins
Copy link
Member

chrissimpkins commented Sep 20, 2018

Here are the changes in the new *.glif format from the UFO specification:

The version number was increased to 2.

Contour
A new identifier attribute was added.

Component
A new identifier attribute was added.

A new rule explaining how components relate to layers was added.

Point
A new identifier attribute was added.

Removed references to Super Bezier.
Image

A new image element was added.

Guideline
A new guideline element was added.

Anchor
A new anchor element was added.

Lib
Added a mechanism for standardizing lib keys.

Added a standard place for glyph mark color storage.

I will pull together a script that you can use to do this. I used defcon to make the transition here, but did not save the script... Mistake :)

@vl4dimir
Copy link
Contributor Author

@chrissimpkins Sure, but my changes neither make use of some older v2 functionality, nor do they remove some v3 functionality which was previously present in these files. So IMO I can just manually change the version numbers using a text editor if that's OK with you.

@chrissimpkins
Copy link
Member

Give this a try:

https://github.com/source-foundry/ufo2to3

@chrissimpkins
Copy link
Member

chrissimpkins commented Sep 20, 2018

So IMO I can just manually change the version numbers using a text editor if that's OK with you.

Can you confirm that the files that include manual adjustments of the version number in the glif files build with our existing UFO 3 source?

Edit: i.e., the source in the dev branch

@chrissimpkins
Copy link
Member

chrissimpkins commented Sep 20, 2018

And you can ignore our current CI fails in this PR. I am working on a conversion to Py 3.7 interpreter testing and having some issues with this conversion. Hope to have this resolved this week.

@vl4dimir
Copy link
Contributor Author

@chrissimpkins Thanks for that ufo2to3 tool, you're fast! I ran it against my branch, giving it all four variants' .ufo folders as input, but it didn't touch my changes at all:

screen shot 2018-09-20 at 17 30 07

Here's a full diff: https://gist.github.com/vl4dimir/435325359ab56967fc50c27f4ef6ddd7 It seems to be mostly whitespace differences.

I'll try updating the version manually and running the build.

@vl4dimir
Copy link
Contributor Author

Ran make build-with-dependencies with the manually edited glyph versions, it finished without problems.

@chrissimpkins
Copy link
Member

chrissimpkins commented Sep 20, 2018

We had an enormous diff when I ran the tool but I think that you are correct. Most changes appear to be whitespace and some XML attribute order modifications in the glif files along with the update in the glif file spec version. It doesn't appear that there were changes to the data itself in our source.

https://github.com/source-foundry/Hack/pull/441/files?utf8=%E2%9C%93&diff=split&w=1

@chrissimpkins
Copy link
Member

Ran make build-with-dependencies with the manually edited glyph versions, it finished without problems.

Will build a set of fonts so that anyone with an interest can have a look at them in order to comment before we merge. Thanks!

@chrissimpkins
Copy link
Member

No change necessary in BoldItalic parenleft glyph?

@chrissimpkins
Copy link
Member

Added a set of web and desktop font builds at commit eeb942b in the original post. Any/all interested users please download, install (or view in browser), and view the changes. If you have any comments please leave them in this thread.

@chrissimpkins
Copy link
Member

cc @jdw1996 test builds with these changes on your most recent outlines are available for download in the OP. Look forward to your feedback

@chrissimpkins
Copy link
Member

Added a new Windows installer release with these test fonts for any interested Win users. See OP for details.

@chrissimpkins
Copy link
Member

This script is available for anyone who would like to install the fonts under a different family name for A/B comparisons:

https://github.com/chrissimpkins/fontname.py

@chrissimpkins
Copy link
Member

Overall, LGTM. Viewed on Windows too and it looks like we may have a bit of hinting work to do on shapes that will be in close proximity to these glyphs in source. We seem to lose a pixel here and there at different sizes so they appear different heights in some cases

5ertg-image

5szvm-image

56xbc-image

Let's see if we get any other feedback.

@chrissimpkins
Copy link
Member

chrissimpkins commented Sep 21, 2018

Design looks pretty spot on to my eye though when you eliminate the small size / instruction set issue...

sj6kx-image

@chrissimpkins
Copy link
Member

and they render very well on macOS :)

@jdw1996
Copy link
Contributor

jdw1996 commented Sep 22, 2018

Sorry I didn't reply earlier; I've been pretty busy. I just tried the builds from the commit you linked, @chrissimpkins, but it had the old version of the parens (i.e. from before my change). I don't have a chance to build right now, but I should be able to some time this weekend. All the screenshots I've seen so far look good, though. If you think this changeset is good to go, don't let me delay you.

@chrissimpkins
Copy link
Member

Sorry, I may not have been clear about the build location. The builds are linked in the original post of this thread as zip archives. They are not committed to the repository.

@chrissimpkins
Copy link
Member

and this is not delaying anything. Feel free to take your time and have a look. Let's confirm that we are in agreement that this is the proper shape before we commit the work. LGTM but I am interested in other opinions.

@jdw1996
Copy link
Contributor

jdw1996 commented Sep 24, 2018

OK, I just tried out the change and it looks good on my Linux machine, too.

@chrissimpkins
Copy link
Member

Thanks so much for these changes @vl4dimir! And thanks to those who took a look and weighed in. Merging.

@chrissimpkins chrissimpkins merged commit 30bb4a0 into source-foundry:dev Oct 20, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Development

Successfully merging this pull request may close these issues.

3 participants