474819 - vertical deltas (baseline offset) handled incorrectly by ATSUI font backend

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jonathan Kew [:jfkthame]]

Attachment: test file using the Nastaliq Navees R "hacked Urdu" font

When ATSUI returns non-zero baseline offsets for some of the glyphs in a text run, the gfxAtsuiFont backend applies these in the wrong direction, so that glyphs are lowered instead of raised and vice versa.

AAT fonts that use baseline shifts are rare (I don't know of any in wide circulation), which is presumably why this has not been noted previously; the code has never been tested.

A font that does illustrate the issue is Nastaliq Navees R, an old Urdu font developed for QuickDraw GX, available from <http://scripts.sil.org/Nastaliq_Download>. This version of the font was encoded with the Urdu letters at MacRoman codepoints, as a hack for old 8-bit systems without Arabic support, but can still be used to show the issue.

The sample file attached includes a few test character sequences that involve baseline offsets when rendered with Nastaliq Navees R. Viewing this file with TextEdit.app shows the expected result. Viewing with Minefield shows the character sequences badly broken due to the inversion of vertical deltas.

Comment 1

[Jonathan Kew [:jfkthame]]

Attachment: the navees.html file as displayed by TextEdit (correct)

Comment 2

[Jonathan Kew [:jfkthame]]

Attachment: the navees.html file as displayed by Minefield (broken)

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: patch to fix the sign error with ATSUI vertical offsets

With this patch, the test file using the Nastaliq font renders correctly.

Comment 4

[Jonathan Kew [:jfkthame]]

Attachment: the navees.html file as displayed by Minefield with patch 358232 (correct)

Comment 5

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Is there any way to reftest this?

Comment 6

[Jonathan Kew [:jfkthame]]

I can't think of a way to do so except by creating custom AAT fonts (e.g, one font that has vertical positioning, and a second version that has the glyphs themselves shifted within the em-square so that the result should be the same). But given how obscure the situation is, that's a fair bit of work (there are no tools to create such a test font, I'd have to build the cross-stream kern table by hand), so I'm reluctant to put this at the top of my list. I'd rather solve some of the rendering glitches that people actually see, like clipped ClearType glyphs.

(I'd guess that this has never shown up in real-life usage. I only dug out that old font and tried it because I was trying to create a scenario comparable to the vertical-offset bug we were seeing on Windows. But AFAIK there are no current AAT fonts in circulation that actually work like this.)

Comment 7

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Fair enough.

Comment 8

[Jonathan Kew [:jfkthame]]

It'd be nice to go ahead and get this landed, so it doesn't get forgotten or start to rot.... does it need any further review (sr?) or can we mark it checkin-needed on the basis of your r+?

Comment 9

[John Daggett (:jtd)]

Seems like we could write a reftest that overlaid the Urdu with glyphs from Ahem, then compared against a blank window.  If the veritcal offsets extend the wrong way they would show below the overlaid Ahem glyphs.  Hacky, but at least we'd have something to capture this and you wouldn't need to create a special font.

Comment 10

[Jonathan Kew [:jfkthame]]

I guess something like that could work.... but would require the Navees font to be available, and strictly speaking we shouldn't put that in our tree as it's not free.

Comment 11

[John Daggett (:jtd)]

Yeah, we need an openly licensed font for this sort of test.  Are there other available openly available AAT fonts for Urdu that exhibit this problem?  My guess is that they are somewhat rare.

Comment 12

[Matthew Gregan [:kinetik]]

Pushed 7122bd06f018.

Comment 13

[Robert O'Callahan (:roc) (email my personal email if necessary)]

Should we take this on 1.9.1?

Comment 14

[Jonathan Kew [:jfkthame]]

For the sake of correctness (and in view of how low-risk the actual change looks), I think we should.

On the other hand, it seems unlikely that it will affect anyone in real-world usage, so practically speaking it's not really important.

Comment 15

[Mike Beltzner [:beltzner, not reading bugmail]]

Comment on attachment 358232 [details] [diff] [review]
patch to fix the sign error with ATSUI vertical offsets

a191=beltzner

Comment 16

[Dão Gottwald [:dao]]

http://hg.mozilla.org/releases/mozilla-1.9.1/rev/f0b94ce853e8