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

CJK metrics #159

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

CJK metrics #159

wants to merge 6 commits into from

Conversation

aaronbell
Copy link

Updated the recommendation regarding CJK metrics reflective of our discussion on the matter.

Let me know if you feel more information / justification is required!

@NightFurySL2001
Copy link

Also if the em-box information in the OS/2.sTypo* of the original spec is to be retained in BASE table, then the horizontal idtp must be required. ideo will store the original OS/2.sTypoDescender and idtp will store the original OS/2.sTypoAscender.

@NightFurySL2001
Copy link

NightFurySL2001 commented Mar 14, 2025

Given the high expectations from major softwares that sTypo* values align with the em-box, it would be better for divergent metrics proposed here to include the vhea/vmtx tables to prevent vertical spacing issues such as in google/fonts#8694.

@aaronbell
Copy link
Author

Thanks for the comments! I'll take a look to update the documentation with your feedback :)

@aaronbell
Copy link
Author

Also if the em-box information in the OS/2.sTypo* of the original spec is to be retained in BASE table, then the horizontal idtp must be required. ideo will store the original OS/2.sTypoDescender and idtp will store the original OS/2.sTypoAscender.

I don't think it is absolutely necessary to include idtp in all fonts, since this value can be calculated fairly easily (assuming standard design practices). If nothing else, it is likely ideo + UPM. That said, in fonts that have unexpected metrics, then definitely should include it.

Updating based on feedback from NightFury and to clarify the situation around the vmtx / vhea tables.
@NightFurySL2001
Copy link

one other thing is i think "em-box" is the better convention here in text rather than "emBox"; it's what used in the Microsoft OpenType Spec docs.

changing naming
adding further information about the vhea / vmtx table and how to set them correctly.
@m4rc1e m4rc1e self-requested a review March 19, 2025 14:20
Copy link
Contributor

@m4rc1e m4rc1e left a comment

Choose a reason for hiding this comment

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

Excellent work! Happy for this to be merged. We can always improve it if needed but imo it's much better than the original implementation.

@NightFurySL2001
Copy link

NightFurySL2001 commented Mar 19, 2025

A few more things:

  1. Might want to include how ideo em top and bottom comes from as starter of the current industry practice. The new wordings does not mention anything about it. May reference Update CJK vertical metrics requirements #135 for description.
  2. I would argue icfb and icft tags should be optional too in the BASE table. Only ideo and romn should be mandated.
  3. UFOs may define vmtx values through individual .glif files through advance height="" and lib.public.verticalOrigin. Might want to include that as part of vmtx description.
  4. hhea values probably should be harmonised in the same family like what was required in the original GF spec and Latin vertical metrics. See Update CJK vertical metrics requirements #135 for more details.

@aaronbell
Copy link
Author

aaronbell commented Mar 19, 2025

Might want to include how ideo em top and bottom comes from as starter of the current industry practice.

Hm. Initially, I thought that it would make sense to include something like this, but I think that it feels a bit like telling people how to set their cap height or x-height values. I don't think that GF has an opinion about where the em-box is placed in the font space, just that the metrics are set in a certain way. Wouldn't most type designers have a sense about how to do this, or can reference other guidance? I don't know if it is a QA issue, essentially.

I would argue icfb and icft tags should be optional too in the BASE table. Only ideo and romn should be mandated.

I've included them based on what Adobe puts into their fonts, and since the BASE table is primarily for their scenarios, I think they are worth including.

UFOs may define vmtx values through individual .glif files through advance height="" and lib.public.verticalOrigin. Might want to include that as part of vmtx description.

Generally the .glif files are generated automatically by the font tool. Rather than suggesting folks modify .glif files directly, it would be better to pursue tool developers (FL / Glyphs / FCP) to ensure that they're generating these values properly.

hhea values probably should be harmonised in the same family like what was required in the original GF spec and Latin vertical metrics.

Isn't that what I've already stated by indicating that the hhea values should be set to align with the sTypo values?

@NightFurySL2001
Copy link

NightFurySL2001 commented Mar 19, 2025

Generally the .glif files are generated automatically by the font tool. Rather than suggesting folks modify .glif files directly, it would be better to pursue tool developers (FL / Glyphs / FCP) to ensure that they're generating these values properly.

The main 3editors have varying support. FontLab straight up doesn't support vhea/vmtx and does not plan to support CJK anytime soon. FontCreator sets the advance height per glyph like the advance width. From what is observed here, Glyphs uses the AFDKO table syntax (?) for overrides.

Isn't that what I've already stated by indicating that the hhea values should be set to align with the sTypo values?

I mean as in a multi-weight family such as Source Han, but given the old hhea reference to the highest glyph, maybe it should be the usWin values for the new metrics here that should be harmonised across the typeface family.

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.

3 participants