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

Set openTypeOS2TypoLineGap=0 #256

Closed
wants to merge 2 commits into from
Closed

Conversation

yisibl
Copy link
Contributor

@yisibl yisibl commented Mar 10, 2021

ufo2ft currently calculated value is not equal to 0

Mainly refer to the conclusion of this article:
https://simoncozens.github.io/fonts-and-layout/opentype.html#vertical-metrics-hhea-and-os2:~:text=recommended%20method

To get you started, here is our recommended method (distilled from a discussion on Typedrawers):

  • The sTypoAscender minus the sTypoDescender should equal the unit square. (Usually 1000 units.)

  • sTypoLinegap should be the default linespacing distance for your intended language environment.

  • lineGap should be zero.

  • usWinAscent and usWinDescent should be set so that no clipping occurs. If your font contains glyphs with tall, stacked accents - for instance, the Vietnamese letter LATIN CAPITAL LETTER A WITH BREVE AND HOOK ABOVE (Ẳ) - you will need to ensure that these values can accommodate the highest (and lowest) possible values of your shaped text. They should also be set so that they sum to at least the value of sTypoAscender - sTypoDescender + sTypoLinegap.

  • ascent and descent should be set to the same values as usWinAscent and usWinDescent, remembering that usWinDescent is positive and descent is negative.

  • Bit 7 of fsSelection should be turned on.

@@ -113,6 +113,9 @@ class FontConfig(NamedTuple):
color_format: str = "glyf_colr_1"
upem: int = 1024
width: int = 1024
# TODO: How OpenType Works says:
# The sTypoAscender minus the sTypoDescender should equal the unit square.
# Do we need to adjust the default value here?
ascent: int = 950 # default based on Noto Emoji
descent: int = -250 # default based on Noto Emoji
Copy link
Contributor Author

Choose a reason for hiding this comment

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

@rsheeter PTAL

Copy link
Collaborator

Choose a reason for hiding this comment

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

@anthrotype thoughts? - I'll note Noto does not appear to follow this rule

Copy link
Member

Choose a reason for hiding this comment

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

sTypoAscender minus the sTypoDescender should equal the unit square

not true, that recommendation is bogus. One can set them to whatever makes sense for the design at hand.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

It does not need to be equal the unit square.

I found some introductions about UPM dogma. And the description of the opentype spec:

t is not a general requirement that sTypoAscender - sTypoDescender be equal to unitsPerEm. These values should be set to provide default line spacing appropriate for the primary languages the font is designed to support.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Comment removed.

@anthrotype
Copy link
Member

In general, I agree that we need to provide a way for the user to customise the line gap, which is important when determining the overall line spacing, along with ascender and descender.

What happens now is that we are not setting the line gap explicitly in the UFO, so our font compiler (ufo2ft) uses its own fallback value --- UPEM * 1.2 - ascender + descender, or 0 if negative -- which may not be what the user actually wants.

I agree that openTypeOS2TypoLineGap=0 is a better default value than the current (implicit) one. We should also add a new --linegap CLI flag/config option to set it to something else.

@anthrotype
Copy link
Member

@yisibl if you can hold on for a day or two, I'd like to take care of this myself. But thanks for all the precious issue-filings!

@yisibl
Copy link
Contributor Author

yisibl commented Mar 12, 2021

@anthrotype Do you have any other details to consider? If just add a flag, I can try to do it. I am waiting for the correct lineGap to generate iconfont color fonts.

@anthrotype
Copy link
Member

I'm working on this, please wait thank you.

@yisibl
Copy link
Contributor Author

yisibl commented Mar 12, 2021

Thank you.

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