468217 - reftests/font-face/sheet-set-switch-1 fails on mac

Status: RESOLVED WORKSFORME

(Core :: Graphics, defect)

Comment 1

[Karl Tomlinson (:karlt)]

Attachment: reference png (gtk)

Comment 2

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Hmmm.  It works for me, with your patch, though.

Comment 3

[Karl Tomlinson (:karlt)]

Attachment: test png (gtk)

The C appears to be positioned 1 pixel to the left of position of the Y.

(The test passes on my system.)

Comment 4

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I guess it seems most likely to be a text run separation issue, that could probably be fixed by changing the test.

Unlikely to be fixed by the remaining patch I have in my tree (bug 458878).

Comment 5

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

This test has been failing for me and Jonathan Kew on Mac too.

Comment 6

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Er, wait, the images you attached look like sheet-set-switch-1, not sheet-set-base-1.  Which one is failing on the Linux tinderbox?   (And is it the same one that's failing on Mac for roc and Jonathan Kew?)

Comment 7

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

Sorry, I was mistaken --- it's sheet-set-switch-1 that's failing for us on Mac.

Comment 8

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

It's also sheet-set-switch-1 that Karl marked random on GTK2 in the reftest.list.

Comment 9

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

Can we mark sheet-set-switch-1 as random on Mac too, since it fails there for me and Jonathan?

Comment 10

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

I tried running valgrind on this, and I didn't get any warnings (without the patch from bug 476724).

Bug 476724 could be the cause of the randomness on Mac.

Comment 11

[John Daggett (:jtd)]

Traced this down for bug 476724, see:

https://bugzilla.mozilla.org/show_bug.cgi?id=476724#c21

Basically the Mark2B and MarkXMark2Y fonts have differing underline offsets.  The reference would be identical if it simply uses Mark2C, MarkB and the body text 'ACB'.

Comment 12

[John Daggett (:jtd)]

Or another solution would be to explicitly set the metrics in the python script that creates the markXX fonts, then check in corrected versions of these.

Comment 13

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

(In reply to comment #12)
> Or another solution would be to explicitly set the metrics in the python script
> that creates the markXX fonts, then check in corrected versions of these.

I couldn't find an API that let me do this, although changing the vertical extents of the glyphs to match fixed it.

That said, I think we could be testing useful things by having them different, so I'm inclined to scratch markXmark2Y and rewrite the references that use it.

Comment 14

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Attachment: patch

Comment 15

[John Daggett (:jtd)]

Comment on attachment 360821 [details] [diff] [review]
patch

Looks good.

Comment 16

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

http://hg.mozilla.org/mozilla-central/rev/c592bbe68cc7
... but it still fails on Mac (should have tested on my own Mac too; I think I was doing too many things at once last night)...
http://hg.mozilla.org/mozilla-central/rev/97867f9b4e42

Comment 17

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

Worse, it's actually still random on Mac (and I've seen it both ways on my Mac).
http://hg.mozilla.org/mozilla-central/rev/3362518abdc0

Comment 18

[John Daggett (:jtd)]

Pushed reftest change to 1.9.1 branch
http://hg.mozilla.org/releases/mozilla-1.9.1/rev/434c69757f4b

Comment 19

[Brian Crowder]

Is this bug fixed?

Comment 20

[David Baron :dbaron: (⌚️UTC-4, no longer working on Mozilla)]

The test is still marked as random on Mac in the manifest.

Comment 21

[Jonathan Kew [:jfkthame]]

This doesn't seem to be an issue any longer; I tried removing the annotation and running a bunch of test jobs on try, and didn't see any failures.

https://treeherder.mozilla.org/jobs?repo=try&revision=5548dd8bc067ceceb488745770766b0e3b779a6f

So I think we can just remove the random annotation and close this as WFM.

Comment 22

[Jonathan Kew [:jfkthame]]

Attachment: Bug 468217 - Remove reftest annotation for test that no longer seems to fail randomly. r=#layout-reviewers

Comment 23

[Pulsebot]

Pushed by [e-mail]:
https://hg.mozilla.org/integration/autoland/rev/739de847efd4
Remove reftest annotation for test that no longer seems to fail randomly. r=hiro

Comment 24

[Narcis Beleuzu [:NarcisB]]

https://hg.mozilla.org/mozilla-central/rev/739de847efd4

Comment 25

[Jonathan Kew [:jfkthame]]

WORKSFORME is a better resolution than FIXED here, I think, as we don't actually know when the underlying issue was fixed.