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

Preparing for Google Fonts #6

Open
wants to merge 15 commits into
base: main
Choose a base branch
from
Open

Conversation

aaronbell
Copy link

@aaronbell aaronbell commented Mar 3, 2025

Hello!

I've been working on preparing UoqMunThenKhung for onboarding to GF and wanted to provide you with a PR of the changes I made, in case you'd like to merge them back upstream, or have any questions / comments.

Note, all source changes have been made in the .fcp source with version 15.0.0.2978. Hopefully this means it'll be easy for you to work on in the future.

Here's a list of the changes made:

  • Added a build system similar to the one @hfhchan created for Cactus Serif
  • Implement production names in the output font
  • Delete softhyphen / softhyphen.rotat
  • Delete unreachable glyph (_10457)
  • Rename to Regular & adjust weightClass to 400
  • Correct version number on export
  • Add BASE table
  • Removed overlapping and broken paths in:
    uni0032.half
    uni7B25
    uni5352
    uni5B58
    uni6D1B
    uni58BA
    uni5E57
    uni695D
    uni78C6
    uni8E1F
    uni7147
    uni0032.half.rotat
  • Updated vertical metrics

On that last item, we recently have had a long discussion regarding how vertical metrics should be set for C/J font families, and finally decided to proceed with implementing a new approach on them. Please feel free to read that thread if you're interested in the background!

For UoqMunThenKhung, I have set the following metrics:

Metric Value
sTypoAscent 941
sTypoDescent -187
hhea.Ascent 941
hhea.Descent -187
winAscent 949
winDescent 213
useTypoMetrics Enabled

If you have a preference for these to be set differently, let's talk! I'm happy to discuss.

FontBakery FAILS

At present there are still 3 Fontbakery FAILS:

  • googlefonts/glyph_coverage
    This check needs to be updated as the font contains GF's kernal glyph coverage.
  • googlefonts/cjk_vertical_metrics
    The new vertical metrics are not yet reflected in fontbakery.
  • googlefonts/family_name_compliance
    "UoqMunThenKhung" is a camel case name, so I have submitted a PR to the whitelist file to bypass this error.

Changelist
- implemented production names
- Deleted softhyphen
- Deleted unreachable glyph (_10457)
- Renamed to Regular & adajusted weightClass
- Adjusted vertical metrics to follow new GF recommendations
- Updated version string.
- Set Vertical metrics
- Implement production names
- Delete soft hyphen
- Delete unreachable? (_10457)
- Rename to Regular & adjust weightClass to 400
- Remove soft hyphen (00AD)
- Correct version number
- modified build shell script to ensure working properly
- added requirements file
- disabled webfont generation and added version number in config
- Corrected encoding ID bug in ufo_process file and added code to remove localized NID 16/17 which confuse things.
- Cleaned up metadata in .fcp file (removing localized names from certain parts which appeared to cause problems).
Added comments to the UFO processing script, added BASE table support. Also resolved overlapping and broken outlines in some glyphs.

Not sure why it is fully replacing the UFOs, but here we are.
The ufo is no longer included in the repository, but instead the ufoz. So there's now an extract script (and a "compress.sh" to create the .ufoz before uploading) that converts before building.

I've also updated the pre-processing file to add NID entries for localized NID1, and Latin NID16 to keep things similar across languages.
Seems some of the modifications I made in the .fcp file didn't make it to the UFO. So I have re-exported them.

Also changed the approach on the opentype name records
@aaronbell
Copy link
Author

In reviewing the comments on Cactus, I recalled that there was a request to use .ufoz instead of .ufo. So I've updated the build system here to reflect that. I've also added a compress.sh file to compress the exported .ufo file in preparation for uploading.

Aside from that, I changed my approach on the name table entries per FB feedback.

Had a wrong variable in my name table modifications. Now will include other localized name table entries, in addition to adding NID1, and stripping 16/17.
- FCP doesn't properly export the USE_TYPO_METRICS flag, so I need to re-add it.
- Also updated source to get localized NID4
@NightFurySL2001
Copy link
Collaborator

I've reviewed the new system here on the latest branch, the syetem seems good to work with. Since neither me nor the author are expecting to run the system locally, I think a makefile + GitHub actions would better fit the three repos here. A few points on the build system:

  1. Are unreachable glyphs required to be removed? (I haven't see which glyph is that, but it's probably from the upstream font).
  2. The version string update is done through config.yaml version, yes?
  3. I think as all files in UFO are normal text file, will it be easier to just open(encoding="utf-8-sig") then save(encoding="utf-8") to remove the BOM? (Actually its a problem with FontTools FEA parser, fixed in github.com/Allow UTF-8 with BOM for features.fea fonttools/fonttools#3495)
  4. What the encodingID code was fixing is the problem that encodingID was not exported at all in the UFO, which is later fixed in newer FC. Might want to account for the fact that it might just not be there at all.

Adding a build script. Let's see what happens!
Fixing permissions
making sure that the venv is active when building
disabling bits that aren't necessary
Fixing the encodingID bug
@aaronbell
Copy link
Author

@NightFurySL2001 Thanks for the feedback!

I've created the github workflow stuff that run off of the makefile. Figure it would be easier to keep it so just in case y'all want to run it on Windows, you can.

per your questions:

  1. unreachable glyphs
    Not required to remove them. They tend to be a holdover of converting an OTF that follows Adobe's specific glyph order collections and once they are shipped as a TTF, they become inaccessible as there is no unicode value assigned, nor can they be accessed via OT. So all they serve to do is bloat the font.
  1. Yes. Due to the weird way that FCP appears to set version string on its own, I found it necessary to set the version string directly.

  2. That's effectively what I think I did in the processing script—check if it is a BOM, then open as a BOM, convert, and save.

  3. Also reverted the processing script to add the encodingID.

@NightFurySL2001
Copy link
Collaborator

NightFurySL2001 commented Mar 7, 2025

Also reverted the processing script to add the encodingID.

That is back to the other problem where older FC does not add encodingID, but newer FC does so correctly. Is the BASE table added in FCP file or post process?

Technical part so far seems fine for me, I'll call @MoonlitOwen to come in and check the FCP and UFOZ files. I'll do some minor fixes to the GitHub Actions later to commit the built binary file in repo.

@aaronbell
Copy link
Author

That is back to the other problem where older FC does not add encodingID, but newer FC does so correctly.

Unfortunately I don't think there's a way around this, except by updating FCP. Having two encodingID (that are identical) in the UFO doesn't seem to be problematic, though, so I think the current implementation is OK.

Is the BASE table added in FCP file or post process?

It is added in the FCP file prior to building.

Technical part so far seems fine for me, I'll call @MoonlitOwen to come in and check the FCP and UFOZ files. I'll do some minor fixes to the GitHub Actions later to commit the built binary file in repo.

Thanks!

@NightFurySL2001
Copy link
Collaborator

NightFurySL2001 commented Mar 8, 2025

It is added in the FCP file prior to building.

Just want to ask where is the BASE table in the FontCreator UI to validate the changes.

That's effectively what I think I did in the processing script—check if it is a BOM, then open as a BOM, convert, and save.

I think this would do:

with open(file, "r", encoding="utf-8-sig") as f:
  contents = f.read()
with open(file, "w", encoding="utf-8") as f:
  f.write(contents)

@aaronbell
Copy link
Author

aaronbell commented Mar 8, 2025

Just want to ask where is the BASE table in the FontCreator UI to validate the changes.

I didn't see a place to do it directly in the FC UI. There was a binary editor which isn't particularly useful, but I was able to add the table in via the fcp_ufo_process.py script. Feel free to edit it as you'd like!

I think this would do:

It is worth keeping in mind that the files may not be utf-8-BOM (I recall seeing some weren't), so good to evaluate if the file is BOM first, before processing it.

@NightFurySL2001
Copy link
Collaborator

NightFurySL2001 commented Mar 8, 2025

(in Python it's utf-8-sig, incorrect naming above for utf-8-bom) From Python, opening utf-8-sig will only remove (skip) the BOM mark if it is present, otherwise it is treated as standard UTF-8. This is why it is suggested to open with utf-8-sig for greater compatibility (Windows Notepad add BOM too), but save as utf-8 for standardisation in Python community.

Quote from https://docs.python.org/3/howto/unicode.html:

In some areas, it is also convention to use a “BOM” at the start of UTF-8 encoded files; the name is misleading since UTF-8 is not byte-order dependent. The mark simply announces that the file is encoded in UTF-8. For reading such files, use the ‘utf-8-sig’ codec to automatically skip the mark if present.

Quote from https://docs.python.org/3/library/codecs.html#encodings-and-unicode:

On decoding utf-8-sig will skip those three bytes if they appear as the first three bytes in the file

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.

2 participants