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

Broken rendering of AAT font Devanagari Sangam MN #2531

Closed
jfkthame opened this issue Jul 3, 2020 · 3 comments · Fixed by #2539
Closed

Broken rendering of AAT font Devanagari Sangam MN #2531

jfkthame opened this issue Jul 3, 2020 · 3 comments · Fixed by #2539

Comments

@jfkthame
Copy link
Collaborator

jfkthame commented Jul 3, 2020

See https://bugzilla.mozilla.org/show_bug.cgi?id=1650414.
The Devanagari Sangam MN font mis-renders on macOS 10.15 (at least).

Testcase:

$ hb-shape --font-file Devanagari\ Sangam\ MN.ttc --unicodes U+092D,U+0942 --shaper coretext
[dn_bha=0+1043|dn_uu_matra.mrk=0@0,11+296]

$ hb-shape --font-file Devanagari\ Sangam\ MN.ttc --unicodes U+092D,U+0942 --shaper ot
[dn_bha=0+1339|dn_uu_matra.mrk=0@-296,1409+0]

Note the large y-offset applied to the dn_uu_matra.mrk with shaper=ot.

@jfkthame
Copy link
Collaborator Author

jfkthame commented Jul 3, 2020

I'm beginning to suspect this may be the result of a discrepancy between the spec for the kerx table and what is actually implemented in the Devanagari Sangam MN font and in Core Text.

Specifically, regarding the Format 4 Kerning Subtable, action type 1, the spec says of Anchor Point Actions:

Type Name Description
uint16 markAnchorPoint Anchor point in marked glyph.
uint16 currAnchorPoint Anchor point in current glyph.

but this does not appear to match the usage in Devanagari Sangam MN. Tracing execution in harfbuzz for the sequence <U+092D,U+0942>, I'm seeing it retrieving markAnchorPoint=0 for the "marked" glyph dn_bha, and currAnchorPoint=1 for the "current" glyph dn_uu_matra.mrk; but that makes no sense, as dn_uu_matra.mrk only has a single anchor (0) defined in the ankr table, while dn_bha does have two anchors, and its anchor (1) is the appropriate one for a "below" mark.

So if I reverse the harfbuzz code such that it reads the first uint16 here as currAnchorPoint and the second uint16 as markAnchorPoint, that fixes the rendering of this example.

@nedley @litherum Could you possibly check on this and confirm whether my understanding is correct, and whether the spec is wrong or the font & implementation are both broken (in a matching way)? I don't know how widely used the ankr table is at this point; maybe the spec should simply be adjusted to match the reality of the implementation (despite how that will result in a mismatch with other action types where mark precedes curr)... or does Core Text somehow detect this broken font and change its interpretation to match? Are there existing fonts that in fact depend on interpreting this as per the current spec?

@jfkthame
Copy link
Collaborator Author

jfkthame commented Jul 3, 2020

(The other possibility, of course, is that harfbuzz is mis-reading the table in some way; if that's the case, we just need to fix it.)

@jfkthame
Copy link
Collaborator Author

jfkthame commented Jul 4, 2020

@nedley @litherum Never mind about the above -- I believe I have located the bug in harfbuzz that's causing it to mis-read the table here.

Patch coming.

jfkthame added a commit that referenced this issue Jul 5, 2020
…ormat4 (#2539)

* [aat] Correct array indexing when looking up actions in KerxSubTableFormat4.

- For action_type 0 and 1, there are 2 values per action record; for action_type 2, there are 4. So we need to account for these factors when indexing into the ankrData array.

Fixes #2531.
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 a pull request may close this issue.

1 participant