551034 - intermittent Windows reftest failure in 385569-1a.html/385569-1b.html

Status: RESOLVED FIXED

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

Description

[:Gavin Sharp [email: [e-mail]]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268085619.1268087032.672.gz&fulltext=1#err0

REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/moz2_slave/mozilla-central-win32-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/385569-1a.html |

REFTEST TEST-UNEXPECTED-FAIL | file:///e:/builds/moz2_slave/mozilla-central-win32-opt-unittest-reftest/build/reftest/tests/layout/reftests/bugs/385569-1b.html |


Looks like a subtle anti-aliasing issue, maybe?

Comment 1

[Jonathan Kew [:jfkthame]]

(In reply to comment #0)
> Looks like a subtle anti-aliasing issue, maybe?

Yes, it is. A few antialiasing pixels at the top of the curve of the 'f' sometimes get clipped, but it apparently depends on details of timing/repainting. (Perhaps something about the timing of the window refresh in relation to the downloadable font initialization.)

Comment 2

[Jonathan Kew [:jfkthame]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268141244.1268143626.3969.gz

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268131049.1268132391.26097.gz

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268124316.1268127174.9698.gz

Comment 3

[Jonathan Kew [:jfkthame]]

Attachment: add reftest-wait class to ensure onload script gets a chance to run

We're seeing occasional failures on tinderbox with this test, but I haven't been able to reproduce them locally. I tried adding padding to the testcases but that did not stop the failures.

To try and stabilize the behavior, this patch adds reftest-wait to the testcases to ensure that the onload scripts have had a chance to run. I don't know if this will necessarily fix the tinderbox failures, but it seems like it can't hurt, anyhow. Are there other tweaks that might also help?

Comment 4

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

I don't think this patch will change behavior at all. Reftests already wait for 'load' to finish.

Comment 5

[Phil Ringnalda (:philor)]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268151013.1268152605.1805.gz
WINNT 5.2 mozilla-central opt test reftest on 2010/03/09 08:10:13
s: mw32-ix-slave04

Comment 6

[Jonathan Kew [:jfkthame]]

Attachment: another attempt to fix orangeness

Ok, I've been looking at the failures more closely and I think I understand what's happening. The first thing to realize is that on the tinderboxes, the Windows system is running an older Uniscribe that doesn't apply ligatures in Latin script, so this test isn't really working as designed anyhow. (The original bug was reported on Mac OS.)

This also means that in the testcases where the "i" is added dynamically, we don't have to redraw the "f", so the two glyphs get painted separately. And the "f" has 6 antialiasing pixels along the top that fall outside the bounding rect that cairo gets (from GDI) for the glyph. Therefore, these pixels fail to paint; cairo's drawing (when the destination is canvas) is clipped tightly to what it believes to be the glyph's bbox. However, in the reference, we paint the "fi" as a single string, and the "i" is taller; therefore the bbox is higher, and the AA pixels from the "f" don't get clipped.

So this patch aims to resolve this by adding an initial "i" to the text, before the "f". This should get painted along with the "f" in all cases, and should therefore ensure that we get an equally high clipping rect in both the testcases and the reference.

Comment 7

[cmtalbert]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268176281.1268180518.20524.gz&fulltext=1
WINNT 5.2 mozilla-central debug test reftest at 2010/03/09 14:24:00
s: mw32-ix-slave08

Comment 8

[Jonathan Kew [:jfkthame]]

http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268222872.1268224126.5744.gz
http://tinderbox.mozilla.org/showlog.cgi?log=Firefox/1268221490.1268224148.5804.gz

Comment 9

[Jonathan Kew [:jfkthame]]

Landed http://hg.mozilla.org/mozilla-central/rev/b354554ea352
I believe this should stop the failures; please re-open if it happens again.