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

Account for linegap in Metrics.Height. #32

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Account for linegap in Metrics.Height. #32

wants to merge 1 commit into from

Conversation

eaburns
Copy link

@eaburns eaburns commented Apr 3, 2016

No description provided.

@googlebot
Copy link

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. I signed it!) and we'll verify. Thanks.


  • If you've already signed a CLA, it's possible we don't have your GitHub username or you're using a different email address. Check your existing CLA data and verify that your email is set on your git commits.
  • If you signed the CLA as a corporation, please let us know the company's name.

@eaburns
Copy link
Author

eaburns commented Apr 3, 2016

Honestly, I'm not sure that the problem is merely linegap. Using the original Height value, my lines were overwriting one another, which can't be explained by line_gap_ alone:

nogap

With this change, they are spaced correctly:

linegap

@@ -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.
Copy link
Contributor

Choose a reason for hiding this comment

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

Drop the "; typically negative".

@nigeltao
Copy link
Contributor

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...

@eaburns
Copy link
Author

eaburns commented Apr 15, 2016

The font in the image is roboto. But I think I was too quick to blame line
gap. As I mentioned, if the problem is just line gap, there shouldn't be
overlap between lines without it. Using scale, the lines overlap one
another.

On Fri, Apr 15, 2016, 03:12 Nigel Tao notifications@github.com wrote:

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...


You are receiving this because you authored the thread.
Reply to this email directly or view it on GitHub
#32 (comment)

@eaburns
Copy link
Author

eaburns commented Apr 15, 2016

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:

The total amount of ascent plus descent plus leading provides a font’s line height.

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:

The values for ascent, descent and lineGap represent the design intentions of the font's creator rather than any computed value, and individual glyphs may well exceed the the limits they represent.

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.

@kirillDanshin
Copy link

@eaburns I can play with it.

fhs added a commit to fhs/duitdraw that referenced this pull request Mar 8, 2019
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
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.

4 participants