kosbarts / Commissioner

Commissioner is a variable and static sans typeface.
SIL Open Font License 1.1
196 stars 10 forks source link

Build with gftools builder #47

Closed RosaWagner closed 1 year ago

RosaWagner commented 2 years ago

Hi @kosbarts! I changed the build system to use gftools builder since it is super clean and avoid using multiple patches and post-process scripts. I am not sure if you would be happy with it cause I had to save the source as Glyphs 3. Maybe check if the intermediate layers are still working, that is the only issue that might arise with conversion to glyphs3.

Cheers Rosa

kosbarts commented 2 years ago

Hey @RosaWagner. That looks great. Thanks. I will need some time to check it, I have lots on my table these days. I just want to make sure the automated STAT table is the same as the manually built and check the G3 files.

K.

RosaWagner commented 2 years ago

I added FLAR and VOLM in the STAT, but only for the default values (0), and I corrected the axis order for following GF requirements. In any case you can modify that in the config.yml. No rush on our side, the axis registration for this font is planned in the batch 3, and we are only finishing batch 1.

m4rc1e commented 1 year ago

@kosbarts Have you managed to review this PR yet?

kosbarts commented 1 year ago

Hi @m4rc1e. Sorry I haven't been able to do it yet. It got lost under other more pressing stuff.

davelab6 commented 1 year ago

We are getting ready to publish this family so your attention this week would be invaluable :) We finished batch 3 axis registration so there's a few small changes to the axes metadata needed too

kosbarts commented 1 year ago

@davelab6 we are on holidays in India showing Neil around to Pria's family. I had to put up a fight to find this one hour to check the file. :)

@RosaWagner @m4rc1e The G3 file looks good, interpolations seems to be all fine, and the build works. However when I checked the STAT table on Samsa it doesn't seem to work well - at least not as good as the previous build..

If you are happy with this STAT then I can go ahead and merge. I do however prefer the previous version.

m4rc1e commented 1 year ago

Thanks @kosbarts. The new STAT looks good to me. It complies with our recent spec updates so we should merge. The existing STAT lacks AxisValues for the flair and volume axes which we need.

kosbarts commented 1 year ago

@m4rc1e when I say existing I mean the STAT from the Commissioner[FLAR,VOLM,slnt,wght].ttf font in my original repo.

It has format 4 AxisValues for both Flair and Volume axes as well as ranges instead of fixed values. I don't know if ranges are not in your recent spec. You can compare the two vf's on Samsa and see the difference.

m4rc1e commented 1 year ago

We don't support ranges, only fixed.

It has format 4 AxisValues for both Flair and Volume axes

Rosalie probably thought it was important to allow users to select styles which are Flared or Loud.

kosbarts commented 1 year ago

I think we got lost somehow in the messages and I am not sure it makes sense continuing this. It's been quite some time that I had a look at the repo. From what I can remember my build had Loud and Flare named instances.

I am not sure what changes Rosalie made. The build I am supposed to merge now doesn't have Loud and Flare named instances (as far as I can see in Rosalie's repo).

Check one last time and then I will merge.

m4rc1e commented 1 year ago

We only allow the following fvar instances:

Roman Italic
Thin Thin Italic
Light Light Italic
Regular Italic
Medium Medium Italic
SemiBold SemiBold Italic
Bold Bold Italic
ExtraBold ExtraBold Italic
Black Black Italic

https://googlefonts.github.io/gf-guide/variable.html#fvar-instances.

So it does make sense that she's included these as STAT AxisValues instead.

kosbarts commented 1 year ago

OK. Now I see.

m4rc1e commented 1 year ago

Perfect. Thank you for the merge!