1711947 - Cherry-pick harfbuzz shaping fix for Apple Color Emoji from https://github.com/harfbuzz/harfbuzz/pull/2968

Status: RESOLVED FIXED

(Core :: Layout: Text and Fonts, defect, P3)

Description

[Jonathan Kew [:jfkthame]]

This will be part of the next harfbuzz release, whenever that happens, but as it affects rendering of [a few] real-world emoji sequences in the Apple emoji font, it would be nice to fix ASAP.

The fix triggered a reftest failure in 421955-1.html, but this is in fact because it improves the rendering: the stray macron glyph in Times Roman no longer gets shifted left of the origin, which (given that there's no preceding character for it to overprint) was not really desired anyhow. But now its rendered pixels come right up against the following (all-white) span in the reference, and a couple of faint antialiasing pixels get erased.

I think we can fix this by adding a bit of letter-spacing to the test/reference, so that the painted pixels of the glyph (including any antialiasing) don't get touched by what follows.

Try run to confirm: https://treeherder.mozilla.org/#/jobs?repo=try&revision=e3f8d634beaa4e4e5bcc13b302e57a5f487651da

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: Bug 1711947 - Update glyph properties during AAT substitutions so that mark zeroing works as expected. r=ryanvm

Cherry-pick of the following upstream commits:
[aat] Update glyph properties from GDEF if available when doing a replacement.
[aat] If shaping via morx, don't adjust mark positioning when zeroing widths.

https://github.com/harfbuzz/harfbuzz/pull/2968

Comment 2

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/d3e99acd6510
Update glyph properties during AAT substitutions so that mark zeroing works as expected. r=RyanVM

Comment 3

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/d3e99acd6510

Comment 4

[Jonathan Kew [:jfkthame]]

Comment on attachment 9222701 [details]
Bug 1711947 - Update glyph properties during AAT substitutions so that mark zeroing works as expected. r=ryanvm

Beta/Release Uplift Approval Request

• User impact if declined: Combining emoji sequences such as 👩🏻‍🤝‍👨🏿 <1F469,1F3FB,200D,1F91D,200D,1F468,1F3FF> (which should appear as a version of 👫 with specific skin-tone modifiers) render with spurious spacing on macOS.
• Is this code covered by automated tests?: Yes
• Has the fix been verified in Nightly?: Yes
• Needs manual test from QE?: No
• If yes, steps to reproduce:
• List of other uplifts needed: None
• Risk to taking this patch: Low
• Why is the change risky/not risky? (and alternatives if risky): Cherry-picking specific fix for AAT font shaping, already tested upstream.
• String changes made/needed:

Comment 5

[Pascal Chevrel:pascalc]

Comment on attachment 9222701 [details]
Bug 1711947 - Update glyph properties during AAT substitutions so that mark zeroing works as expected. r=ryanvm

P3/S3 and it just landed in nightly, we build our last beta in a few hours so it seems that the risk/benefit ratio is too high to me at this point of the cycle. I think we should let it ride the 90 train, thanks.

Comment 6

[Jonathan Kew [:jfkthame]]

Fair enough, thanks for taking a look. It can ride 90.