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

Implement a simple API for fetching opentype metrics #1432

Open
wants to merge 1 commit into
base: master
from

Conversation

Projects
None yet
4 participants
@ebraminio
Copy link
Member

commented Nov 30, 2018

Continuing #1396
Related to #1337

Show resolved Hide resolved src/hb-ot-metrics.h Outdated
Show resolved Hide resolved src/hb-ot-var-mvar-table.hh Outdated

@ebraminio ebraminio force-pushed the metrics branch from 6f55afe to 87feebc Nov 30, 2018

Show resolved Hide resolved src/hb-ot-metrics.h Outdated
Show resolved Hide resolved src/hb-ot-metrics.cc Outdated
Show resolved Hide resolved src/hb-ot-metrics.cc Outdated
Show resolved Hide resolved src/hb-ot-metrics.cc Outdated
@behdad

This comment has been minimized.

Copy link
Member

commented Nov 30, 2018

So previously I said we should get ascender/descender/linegap form hmtx/vmtx accelerators. Now I think it should be the other way around: those should get it from this API. Which means, this API should implement the logic there. The logic, for horizontal ascender/descender is that if face->table.OS2->fsSelection & USE_TYPO_METRICS then OS/2 values are used, otherwise hhea values.

Now, MVAR specifically refers to OS2 values, because there's wording somewhere else in OpenType 1.8 that says variable fonts should always have USE_TYPO_METRICS bit one. We can ignore that wording and continue the above logic. So, that logic needs to be implemented here if both tables are present.

Show resolved Hide resolved src/hb-ot-metrics.cc Outdated
Show resolved Hide resolved src/hb-ot-metrics.cc Outdated
Show resolved Hide resolved src/hb-ot-hmtx-table.hh Outdated

@ebraminio ebraminio force-pushed the metrics branch from 73c8c01 to fba54ee Nov 30, 2018

Show resolved Hide resolved src/hb-ot-hmtx-table.hh Outdated
Show resolved Hide resolved src/hb-ot-hmtx-table.hh Outdated
@jfkthame

This comment has been minimized.

Copy link
Collaborator

commented Nov 30, 2018

@ebraminio

This comment has been minimized.

Copy link
Member Author

commented Nov 30, 2018

Big majority of them have OS/2 but right there are apparently some that don't (comparing number of maxp and os/2 occurrence)

10.12.6 https://gist.github.com/ebraminio/7627d0e18989a25b16c55701118fc069

10.13.6 https://gist.github.com/ebraminio/d432e831b3f7ebe30245dde5775e1c7e

@ebraminio ebraminio force-pushed the metrics branch 2 times, most recently from 3bf1c80 to 2a79b08 Nov 30, 2018

Show resolved Hide resolved src/hb-ot-metrics.cc Outdated

@ebraminio ebraminio force-pushed the metrics branch 3 times, most recently from a504d2f to e558bd3 Dec 1, 2018

@ebraminio ebraminio force-pushed the metrics branch 3 times, most recently from c911178 to eaa2eb5 Dec 1, 2018

ebraminio added a commit to ebraminio/harfbuzz that referenced this pull request Dec 5, 2018

ebraminio added a commit that referenced this pull request Dec 5, 2018

[gasp] Implement the table parsing
May or may not be used in #1432

@ebraminio ebraminio force-pushed the metrics branch 2 times, most recently from e41d945 to 4a71397 Dec 5, 2018

@ebraminio ebraminio force-pushed the metrics branch 2 times, most recently from ce35b52 to 4505f5d Dec 5, 2018

@ebraminio ebraminio force-pushed the metrics branch 4 times, most recently from f7a5afe to e2bfcb1 Dec 5, 2018

@ebraminio ebraminio force-pushed the metrics branch 3 times, most recently from 9ce1dba to 69f0a4b Dec 8, 2018

@ebraminio ebraminio force-pushed the metrics branch 2 times, most recently from 36e5299 to 4fb78b2 Dec 16, 2018

@behdad

This comment has been minimized.

Copy link
Member

commented Apr 29, 2019

What's the status here? Let's finish and get this in.

@behdad

This comment has been minimized.

Copy link
Member

commented May 24, 2019

Can you rework this? We like to use it in Pango.

cc @matthiasclasen

@ebraminio

This comment has been minimized.

Copy link
Member Author

commented May 24, 2019

Sure, I'd love to

@Gillibald Gillibald referenced this pull request May 24, 2019

Draft

GlyphRun support #2387

0 of 3 tasks complete

gnomesysadmins pushed a commit to GNOME/pango that referenced this pull request May 24, 2019

Matthias Clasen
Replace get_face_metrics with harfbuzz
This is incomplete; harfbuzz does not have
underline and strikethrough metrics currently.

See harfbuzz/harfbuzz#1432

gnomesysadmins pushed a commit to GNOME/pango that referenced this pull request May 25, 2019

Matthias Clasen
fc: Replace get_face_metrics with harfbuzz
This is incomplete; harfbuzz does not have
underline and strikethrough metrics currently.

See harfbuzz/harfbuzz#1432
@ebraminio

This comment has been minimized.

Copy link
Member Author

commented May 26, 2019

Can you rework this? We like to use it in Pango.

Ok. Still not in ideal state for working on codes but this feels OK I guess? I mean what feels bad about this?

@behdad

This comment has been minimized.

Copy link
Member

commented Jun 17, 2019

This looks very good. A couple things:

  • What's the justification for separate -internal.cc file and the regular .cc file?
  • Let's add hb_ot_metrics_get_variation that doesn't do any scaling. I have a feeling that we will be encoding non-geometric values in there at some point, if it's not already... Anyway, that one should probably return a float.
  • I see that hb_ot_metrics_get_x/y_variation also return float. Probably a good idea, though technically should be hb_position_t. Not sure :(.
  • I have to think through the ascent/descent etc changes. I'll do that when merging.

@ebraminio ebraminio force-pushed the metrics branch from 4fb78b2 to eb41aa7 Jun 18, 2019

@ebraminio

This comment has been minimized.

Copy link
Member Author

commented Jun 18, 2019

What's the justification for separate -internal.cc file and the regular .cc file?

You wanted to reuse the logic but we don't want to offer the API in hb-subset, which uses the table thus the API, so I had to separate it to use in the subset.

Let's add hb_ot_metrics_get_variation that doesn't do any scaling. I have a feeling that we will be encoding non-geometric values in there at some point, if it's not already... Anyway, that one should probably return a float.

Done, I think

I see that hb_ot_metrics_get_x/y_variation also return float. Probably a good idea, though technically should be hb_position_t. Not sure :(.

I turned it into hb_position_t as the scale already removed its precision but your concern is understandable.

@ebraminio ebraminio force-pushed the metrics branch 2 times, most recently from f5e55ca to 9bfe6d8 Jun 18, 2019

@ebraminio ebraminio force-pushed the metrics branch from 9bfe6d8 to 5455a36 Jun 18, 2019

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.