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

Use font-provided metadata to determine glyph height #154

Open
kkaefer opened this issue Oct 24, 2013 · 11 comments
Open

Use font-provided metadata to determine glyph height #154

kkaefer opened this issue Oct 24, 2013 · 11 comments
Assignees

Comments

@kkaefer
Copy link
Member

kkaefer commented Oct 24, 2013

We currently hardcode the glyph height so that we place the line centrally on the line. We should someone deduce the offset from the font metrics (or the shaped text bbox?)

@kkaefer
Copy link
Member Author

kkaefer commented Feb 20, 2014

This needs to happen when shaping the text, and we need to include that information into the shaping result.

@mikemorris
Copy link
Contributor

@kkaefer @ansis The glyph ascender is now added to glyph_info structs at https://github.com/mapbox/node-fontnik/blob/master/src/mapnik/text/face.cpp#L84-85 and used to offset glyph.top at https://github.com/mapbox/node-fontnik/blob/master/src/fontnik/glyphs.cpp#L87 - should this offset be removed in favor of shipping the ascender value in the protobuf?

@ansis
Copy link
Contributor

ansis commented Jun 17, 2014

I think so, but lets see what @kkaefer says. This would be a breaking change, right?

@mikemorris
Copy link
Contributor

Yea, it would unfortunately be a breaking change @ansis.

@kkaefer
Copy link
Member Author

kkaefer commented Jun 18, 2014

We could do this without a breaking change; just add the ascender property as well. When reading the metrics and we don't have an ascender, we use the current algorithm, if we do have the ascender, we use to to position the font.

@mikemorris
Copy link
Contributor

Opened a pull request enabling this with a few questions at mapbox/node-fontnik#50 @kkaefer @ansis

@mikemorris
Copy link
Contributor

Could this be causing mapbox/mapbox-gl-style-spec#61 @nickidlugash?

@mourner mourner added this to the future milestone Jun 24, 2014
@jfirebaugh jfirebaugh removed this from the future milestone Jun 15, 2015
@mourner mourner removed the placement label Oct 27, 2015
@lucaswoj lucaswoj changed the title Do not hardcode label glyph height Use font-provided metadata to determine glyph height Jul 28, 2016
@mikemorris mikemorris removed their assignment Oct 19, 2016
@lbud
Copy link
Contributor

lbud commented Feb 1, 2017

Inadvertently closed due to some 🌳 history magic — reopening.

@lbud lbud reopened this Feb 1, 2017
@lucaswoj
Copy link
Contributor

Closing as stale. I haven't seen this become a practical concern yet.

@kkaefer
Copy link
Member Author

kkaefer commented Feb 21, 2017

Assuming that this will be subsumed by @ChrisLoer's push on fonts + harfbuzz.

@lucaswoj
Copy link
Contributor

Ah! Good point. We can keep this open to track this aspect of our text shaping work.

@lucaswoj lucaswoj reopened this Feb 21, 2017
@ChrisLoer ChrisLoer self-assigned this Feb 27, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

8 participants