-
Notifications
You must be signed in to change notification settings - Fork 183
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
Account for linegap in Metrics.Height. #32
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project. Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed, please reply here (e.g.
|
@@ -186,6 +186,7 @@ type Font struct { | |||
fUnitsPerEm int32 | |||
ascent int32 // In FUnits. | |||
descent int32 // In FUnits; typically negative. | |||
lineGap int32 // In FUnits; typically negative. |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Drop the "; typically negative".
What's the name of that font file with a non-zero line gap. I went for a quick search for one earlier, but didn't find one easily. Still, I'm not sure that replacing (a.scale) with (ascent + descent + lineGap) is the right thing for calculating Height. I suspect that the latter is slightly too small for most fonts, where the lineGap is zero and the glyphs don't go all the way to the edge of the em square... |
The font in the image is roboto. But I think I was too quick to blame line On Fri, Apr 15, 2016, 03:12 Nigel Tao notifications@github.com wrote:
|
I was surprised that you consider ascent+descent+linegap to be the wrong way to compute line height, so I did a bit of searching. According to this page from the apple developers guide, it is the correct approach:
I don't think we should worry about linegap=0 causing colliding lines. According to the truetype reference manual the ascent and descent values from the hhea table are just artistic intent anyway:
I interpret these two facts to mean that, if a font has linegap=0, the creator either accounted for it in the ascent+descent, or they intend some lines to touch. I don't have time to play around with this much more. If you'd prefer, I can close this PR (at best it's name is misleading, if not wrong), and open an issue instead. I can include the images from above and a reference to the font I used. It was Roboto-Regular.ttf, from here https://www.google.com/fonts/specimen/Roboto. |
@eaburns I can play with it. |
The height seems to be only slightly bigger than the Ascent. It's small enough such that characters that take up space below the baseline (e.g. 'g', 'y', 'j', 'p') gets cut off at the bottom. (For reference, see https://developer.apple.com/library/archive/documentation/TextFonts/Conceptual/CocoaTextArchitecture/Art/glyph_metrics_2x.png) This is a freetype bug: golang/freetype#34 There are multiple PRs with a fix that haven't been merged: golang/freetype#32 golang/freetype#62 Plan9port's fontsrv also similarly sums up ascent and descent to compute the height: https://github.com/9fans/plan9port/blob/047fd921744f39a82a86d9370e03f7af511e6e84/src/cmd/fontsrv/x11.c#L80
No description provided.