Sync with 4.1

[LaurenzV]

Status	Commit message	HB	Comments
🟢	[USE] Treat visible viramas like dependent vowels	Link
⚪️	Make load_num_glyphs_from_loca conditional on HB_NO_BORING_EXPANSION	Link
⚪️	[baseline] Use ot-metrics fallback API	Link
⚪️	[metrics] Simplify x-height fallback	Link
⚪️	[layout] Whitespace	Link
⚪️	[baseline] Fix HB_NO_METRICS build	Link
⚪️	[subset] bug fix in prune_langsys	Link
🟢	restores unintended addition in 43be5ba	Link
⚪️	[coretext] Remove dead code	Link
⚪️	Add log base 2 versions of constants.	Link
⚪️	Use bit shifting instead of multiplying and dividing.	Link
⚪️	Fix typo.	Link
⚪️	Add option to insert a sorted arrays of values to sets.	Link
⚪️	Remove null checks.	Link
⚪️	Move fn	Link
⚪️	[set] Fix-up previous commits	Link
🟢	[indic] Clear syllables before presentation features	Link
🟢	[indic] Test clearing syllables earlier	Link
⚪️	[set] Fix documentation	Link
⚪️	[buffer] Fix out-buffer under memory-alloc failure	Link
⚪️	[cmap] In collect_unicodes() of format 12/13	Link
🟢	[ot-font] Only use vmtx side-bearing if table exists	Link	ported and test cases pass, but maybe check if there is a better way to do it
⚪️	[hmtx] Change default advance for horizontal direction to upem/2 again	Link
🟢	[ot-font] Use ascent+descent for fallback vertical advance	Link	same
🟢	[ot-font] Vertically center glyph in vertical writing fallback	Link	same
🟢	Update tests for recent changes	Link
⚪️	Merge pull request #3497 from harfbuzz/vertical-origin	Link
⚪️	[buffer] Whitespace	Link
⚪️	Remove old TODO file	Link
⚪️	4.1	Link

[LaurenzV]

That one was a bit of a struggle once again due to diverging code bases. :( I tried my best, and the new test cases pass for now. But once you're done with what you want to do I will try to look into whether we can make that part of the codebase closer to what harfbuzz actually does. But for now it should be fine, and you can keep working on the [reorg] commits that are up next, if you feel like doing so.

[LaurenzV]

Also https://github.com/RazrFalcon/ttf-parser/pull/143 needs to be merged first, though.

[LaurenzV]

Actually, I don't think we have to change to returning a boolean for glyph_extents. We can just return the default instead in the new case. If you want, I can change it back. But if you think it's better to make it look more similar to harfbuzz, I'll leave it the way it is.

[RazrFalcon]

Can you point me to the glyph_extents change in the original code? Because the if !glyf_table.has_contours(glyph) { check is very weird. Why it doesn't check CFF as well?

Otherwise looks good.

[LaurenzV]

It wasn't part of this change, it already existed but seems like it wasn't ported (otherwise the tests wouldn't have failed):

https://github.com/harfbuzz/harfbuzz/blob/c36844d6d923bfc765f841fde10d6f505ff297fd/src/hb-ot-glyf-table.hh#L892-L897

That's the one for CFF1 I think:

https://github.com/harfbuzz/harfbuzz/blob/c36844d6d923bfc765f841fde10d6f505ff297fd/src/hb-ot-cff1-table.cc#L409-L443

[RazrFalcon]

Hmm... then your glyf check is incorrect. What you check is if the glyf table has a data block for the selected glyph. But what harfbuzz checks is if this data block has zero contours. Which is the same as Face::glyph_bounding_box().is_none()

[LaurenzV]

So should I do this:

if self.ttfp_face.tables().glyf.is_some() {
    if self.ttfp_face.glyph_bounding_box(glyph).is_none() {
        // Empty glyph; zero extents.
        return true;
    }
}

let Some(bbox) = self.ttfp_face.glyph_bounding_box(glyph) else {
    return false;
};

or just:

let Some(bbox) = self.ttfp_face.glyph_bounding_box(glyph) else {
    return true;
};

[RazrFalcon]

Hard to say. I'm not really sure what harfbuzz is doing here. A concept of an "empty" glyph is very abstract in TrueType. To my understanding, harfbuzz returns false only on parsing error. But ttf-parser never returns an error during glyph outlining. From the docs:

Face::outline_glyph Returns None when glyph has no outline or on error.

To mimic harfbuzz here we need to heavily update ttf-parser API, which I would rather do later. So for now, let's do:

let bbox = self.ttfp_face.glyph_bounding_box(glyph);

if self.ttfp_face.tables().glyf.is_some() && bbox.is_none() {
    // Empty glyph; zero extents.
    return true;
}

let Some(bbox) = bbox else {
    return false;
};

Hopefully this would be enough for now.

[LaurenzV]

Done.

[LaurenzV]

Yeah it's tricky... Maybe we should somehow change ttf-parser to also allow access the low-level stuff, or maybe it should be a separate crate and ttf-parser is then just an abstraction layer on top of it. Not sure... But one step at a time.

[RazrFalcon]

Yep, a lot of ideas, not a lot of time.