Skip to content

Conversation

Hoolean
Copy link
Contributor

@Hoolean Hoolean commented Apr 5, 2025

Implements #668

This PR begins drafting support for vertically typeset fonts in fontc, parsing glyph heights, vertical origins, and associated global metrics, and building vhea, vmtx, VVAR, and vertical gvar phantom points.

The PR is currently sufficient to build identical tables for several fonts on crater, but I am aware that there are blockers to resolve, documentation and tests to add, and drafted changes to iterate into more sensible abstractions before the PR can be considered for merging, and so I am opening a draft now to hash this out collaboratively :)

Status

Noto Serif Bengali's vertical tables are building identically, and at least Noto Sans Symbols, Noto Serif Ethiopic and Noto Serif Tamil's are building identically aside from a difference in fontmake/fontc's serialisation of empty HVAR/VVAR tables (see below).

Seeking feedback

Interpolated metrics are required for default heights and vertical origins

If a glyph does not provide an explicit advance height or vertical origin, these should fallback to ascender - descender and ascender respectively. This depends on being able to access values for these global metrics at all positions in the font where there is glyph data, which would require interpolated metrics in the case of e.g. sparse instances.

The PR currently fallsback to the default metrics and, while I did not encounter any diffs from this when testing with the top fonts on crater, this is necessary for fully-accurate VVAR tables and gvar phantom points.

Shall I spin out a separate issue for this to unblock a fix here?

Abstracting across hmtx and vmtx

I am happy with how the abstraction for sharing between HVAR and VVAR has turned out, but unhappy with my draft for hmtx and vmtx thus far - in particular some gnarly mutation in the final build - and have not had time to revisit it yet.

I also think that the PR's reuse of fonttool's approach of deriving side bearings from advance, side bearing, and bounds-advance may introduce some under and overflow issues post-oxidisation where fontc was previously more robust... any ideas on what the cleanest approach would be here?

(I've allowed edits on the PR in case there is something obvious and it's easier to just dive in 🤜 🤛)

Difference in fontmake/fontc's building of empty HVAR/VVAR

☑️ Update: see below, will be resolved by fonttools/fonttools#3797

fontmake encodes an empty VVAR table with indirect mapping, whereas fontc encodes with direct mapping. fontc's approach is slightly smaller, and so there is either an unwanted semantic difference, or a bug causing sub-optimal serialisation in fontTools.

This can be observed for empty HVAR tables too; crater is reporting the same issue but in HVAR for Doto's designspace.

My understanding is that empty HVAR and VVAR tables are semantically meaningful and important to preserve, as they allow the shaper to skip interpolating and interpreting vertical phantom values only to find that no variation in height is required.

Shall I spin out a separate issue for this too?


Thanks for your attention 🚀

Note: this is a personal contribution independent of my employer, and so I've submitted from a fork under my personal profile and email to make this distinction

@Hoolean
Copy link
Contributor Author

Hoolean commented Apr 6, 2025

Update: the differing empty HVAR/VVAR is caused by fonttools needing to apply location pruning for direct mappings too, which will be resolved by fonttools/fonttools#3797. After this is merged, at least four Noto fonts will have identical vertical tables.

@Hoolean
Copy link
Contributor Author

Hoolean commented Apr 6, 2025

(getting a bit off-topic for this PR but the empty HVAR diff was more widespread than I realised - after the fontTools PR is merged then 14 new cases on crater will be identical 🎉 mostly monospace fonts)

Hoolean added 3 commits April 6, 2025 18:54
This refactor moves the height and vertical origin default calculations
closer to where those defaults are described, and avoids an awkward
import from an individual fontbe work file.
@Hoolean
Copy link
Contributor Author

Hoolean commented Apr 6, 2025

A thought on interpolated metrics: we can drop the requirement for these if:

This would trim a lot of location plumbing at the compilation stage too.

@rsheeter
Copy link
Contributor

rsheeter commented Apr 7, 2025

Any chance this could be split into smaller PRs? - it appears to be trending towards being quite large and thus hard to review

@Hoolean
Copy link
Contributor Author

Hoolean commented Apr 9, 2025

Definitely possible, definitely sensible - it's end-to-end and there's a lot of interdependency but there are still big slices I can make.

I'll probably start by pulling out the frontend parsing into a separate PR, and then vhea/vmtx and VVAR can follow separately.

@Hoolean
Copy link
Contributor Author

Hoolean commented Apr 9, 2025

(The fix for differing empty HVAR/VVAR needed to land in Python land and has now been merged at fonttools/fonttools#3797, so that will arrive on its own when there's a version bump and we update our requirements)

@Hoolean
Copy link
Contributor Author

Hoolean commented May 25, 2025

Closing as obsolete in favour of the smaller PRs this was entirely refactored into:

@Hoolean Hoolean closed this May 25, 2025
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.

2 participants