617231 - mixedChartype-02 reftest failures with HarfBuzz

Status: RESOLVED FIXED

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

Description

[Karl Tomlinson (:karlt)]

Attachment: rev3 fedora reftest failures

With the patches at
https://hg.mozilla.org/users/ktomlinson_mozilla.com/bug597147/file/8594856ff4a0
the mixedChartype-02 tests involving "direction:ltr;
unicode-bidi:bidi-override" are failing apparently due to differences in
shaping.

From experiments on my local system with Times New Roman 184812, Nazli 131662,
and KacstArt 65536 (which looks very similar to tryserver results), it seems
this may have been passing previously because no shaping was being performed.

Comment 1

[Karl Tomlinson (:karlt)]

fc-match -v "Geeza Pro,serif:lang=ar" on a rev3 fedora machine gives:
Pattern has 31 elts (size 32)
	family: "KacstArt"(s)
	familylang: "en"(s)
	style: "Medium"(s)
	stylelang: "en"(s)
	fullname: "KacstArt"(s)
	fullnamelang: "en"(s)
	slant: 0(i)(s)
	weight: 100(i)(s)
	width: 100(i)(s)
	size: 12(f)(s)
	pixelsize: 12.5(f)(s)
	foundry: "unknown"(s)
	antialias: FcFalse(w)
	hintstyle: 3(i)(s)
	hinting: FcTrue(s)
	verticallayout: FcFalse(s)
	autohint: FcFalse(s)
	globaladvance: FcTrue(s)
	file: "/usr/share/fonts/kacst/KacstArt.ttf"(s)
	index: 0(i)(s)
	outline: FcTrue(s)
	scalable: FcTrue(s)
	dpi: 75(f)(s)
	scale: 1(f)(s)
	charset: 
	0000: 00000000 ffffffff ffffffff ffffffff 00000000 ffbfffff 00800001 9a90cf85
	0001: 00000000 00020000 000c0000 01000003 00040000 00000000 00000000 00000000
	0002: 00000000 00000000 00000000 00000000 00000000 00000000 000000c0 00000000
	0006: 88001000 07fffffe 0007ffff 000027ff 00000000 00000000 00000000 00000000
	0020: 7718f000 06010044 00000000 00000000 00000000 00000000 00000000 00000000
	0021: 00000000 00000004 00000000 00000000 00000000 00000000 00000000 00000000
	00e8: 01000000 007c3fff 00000000 00000000 00000000 00000000 00000000 00000000
	00fc: 00000000 00000000 c0000000 00000007 00000000 00000000 00000000 001c0000
	00fd: 00000000 c0000000 00000000 00000000 00000000 00000000 00000000 00040000
	00fe: 00000000 00000000 00000000 ffd70000 ffffffff ffffffff ffffffff 9fffffff
(s)
	lang: ar|fj|ho|ia|ie|io|nr|om|so|ss|st|sw|ts|ug|uz|xh|zu|kj|kwm|ms|ng|rn|rw|sn|za(s)
	fontversion: 65536(i)(s)
	capability: "otlayout:arab otlayout:latn"(s)
	fontformat: "TrueType"(s)
	embeddedbitmap: FcTrue(s)
	decorative: FcFalse(s)

Comment 2

[Karl Tomlinson (:karlt)]

Attachment: pass shapshot

This is a tryserver pass with gfx.font_rendering.harfbuzz.level = 1.
It looks like the reference with level = 2, as if some shaping is happening.
(This is not what I'm seeing on my local system.)

Comment 3

[Simon Montagu :smontagu]

We may be able to fix this by using pre-shaped Arabic characters, like in bug 616281.

Comment 4

[Karl Tomlinson (:karlt)]

I don't know what this text is meant to look like, but I got the impression from the results that it was shaping consistently with Pango, but not with HarfBuzz.

Is there a (pending) regression here?  Or was the Pango shaping wrong anyway?

Comment 5

[Simon Montagu :smontagu]

The screenshot in attachment 495783 [details] is what it's meant to look like. Is that with Pango?

Comment 6

[Karl Tomlinson (:karlt)]

(In reply to comment #5)
> The screenshot in attachment 495783 [details] is what it's meant to look like. > Is that with Pango?

Yes.  That's with Pango.

Sorry, I wasn't clear.  The patches mentioned in comment 0, leading to the reftest failures, switch to (in-tree) HarfBuzz.

Comment 7

[Jonathan Kew [:jfkthame]]

So the reference rendering is correct, and the pango rendering of the LTR-override test matches that, but with harfbuzz, we're getting the glyphs shaped as though they were in the opposite order - without regard for the fact that they'll be laid out "backwards" from their natural order, in other words.

Looks like this must be a harfbuzz issue (unless perhaps we're failing to pass in the correct direction); I'll look into it a bit. It's a sufficiently obscure use-case that even if we land the harfbuzz patches with this regression, I wouldn't feel too bad about it.... and then we could go ahead and fix it for all platforms. (It doesn't actually pass anywhere else at present, either, except with AAT fonts on OS X.)

Comment 8

[Jonathan Kew [:jfkthame]]

Confirmed that this is a harfbuzz issue, and reported upstream. We could patch it locally for now, but I'd prefer to see if we get a quick response from Behdad.

I don't think this should block us landing the Linux harfbuzz patches, even if we have to mark this test as failing for the time being - it's a pretty low-importance issue, and fails on other platforms (and other browsers, IIRC) anyway.

Comment 9

[Karl Tomlinson (:karlt)]

Behdad has a fix here, thanks!
http://cgit.freedesktop.org/harfbuzz/commit/?id=ee8aaf976a6eb42be49b63b4c51c7a0a338e0298

Pushed to try
http://hg.mozilla.org/try/rev/5f0cf0c94932

(In reply to comment #2)
> (This is not what I'm seeing on my local system.)

I was getting no shaping on my system because I had pango-graphite installed and that was overriding Pango's arabic shaper.

Comment 10

[Karl Tomlinson (:karlt)]

Attachment: Fix arabic shaping of LTR text

Would you be happy to cherry pick this patch, Jonathan?

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 495992 [details] [diff] [review]
Fix arabic shaping of LTR text

Yes, we can cherry-pick this. (We might want to take another complete harfbuzz refresh before FF4.0, depending what updates happen. But for now this is fine.)

Comment 12

[Karl Tomlinson (:karlt)]

Comment on attachment 495992 [details] [diff] [review]
Fix arabic shaping of LTR text

a2.0?
Need this to land bug 569770 with arabic shaping enabled and not regress reftests.

Comment 14

[Karl Tomlinson (:karlt)]

http://hg.mozilla.org/mozilla-central/rev/0bd4cd152565