581081 - REFTEST TEST-UNEXPECTED-FAIL | file:///c:/talos-slave/mozilla-central-win7-opt-u-reftest-d2d/build/reftest/tests/layout/reftests/text/zwnj-01.xhtml |

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Comment 1

[Joe Drew (not getting mail)]

Jonathan, can you take a brief look at this and see if it's up your alley?

Comment 2

[Bas Schouten (:bas.schouten)]

(In reply to comment #1)
> Jonathan, can you take a brief look at this and see if it's up your alley?

Adding jfkthame to the CC list.

Comment 3

[Jonathan Kew [:jfkthame]]

I tried logging the cairo_show_glyphs calls made during the rendering of the testcase and reference. These logs are from a simplified example that has only the last line of text in each of the files:

cairo_show_glyphs(cr=6C069A68)
mat=(1.000000 0.000000 0.000000 1.000000 0.000000 0.000000)
surface=041CF898, type=7, font-opts-hash=0l, offsets=0.000000,0.000000
antialias=0, op=2, tol=0.100000
glyphs: 941@(8.000000,70.000000), 909@(25.583333,70.000000), 944@(33.033333,70.000000), 979@(50.616667,70.000000), 909@(60.233333,70.000000), 3@(67.683333,70.000000), 993@(67.683333,70.000000), 942@(79.833333,70.000000), 999@(97.416667,70.000000)

cairo_show_glyphs(cr=6C069F18)
mat=(1.000000 0.000000 0.000000 1.000000 0.000000 0.000000)
surface=041CFAA0, type=7, font-opts-hash=0l, offsets=0.000000,0.000000
antialias=0, op=2, tol=0.100000
glyphs: 941@(8.000000,70.000000), 909@(25.583333,70.000000), 944@(33.033333,70.000000), 979@(50.616667,70.000000), 909@(60.233333,70.000000), 3@(67.683333,70.000000), 993@(67.683333,70.000000), 942@(79.833333,70.000000), 999@(97.416667,70.000000)

cairo_show_glyphs(cr=6C069F18)
mat=(1.000000 0.000000 0.000000 1.000000 0.000000 0.000000)
surface=041CFAA0, type=7, font-opts-hash=0l, offsets=0.000000,0.000000
antialias=0, op=2, tol=0.100000
glyphs: 941@(8.000000,70.000000), 909@(25.583333,70.000000), 944@(33.033333,70.000000), 979@(50.616667,70.000000), 909@(60.233333,70.000000)

cairo_show_glyphs(cr=6C069F18)
mat=(1.000000 0.000000 0.000000 1.000000 0.000000 0.000000)
surface=041CFAA0, type=7, font-opts-hash=0l, offsets=0.000000,0.000000
antialias=0, op=2, tol=0.100000
glyphs: 993@(67.683333,70.000000), 942@(79.833333,70.000000), 999@(97.416667,70.000000)

This shows that the testcase and reference are drawing the exact same glyphs at the same positions. The differences in subpixel rendering seem to be an artifact of the fact that the testcase paints all the glyphs with a single cairo_show_glyphs call, whereas the reference paints them in two separate operations.

Comment 4

[Jonathan Kew [:jfkthame]]

One interesting thing shown by this log is that the testcase apparently gets rendered twice, with identical cairo_show_glyphs calls but a different cairo_t context and surface. The reference only seems to be rendered once, using the second of those cairo contexts & surfaces. I don't know why this is; something about how the reftest harness works, maybe? Could we eliminate the duplicate rendering of the testcase and thus speed up reftest? cc'ing roc & dbaron for any thoughts on this.

Comment 5

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

Not sure, but the callstacks would give a strong clue.

Comment 6

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

Both the test and the reference should be rendered through the NonWhiteToBlack SVG filter; I'm not sure why they should be different, though.  (You're sure you weren't looking at one of the *.html files rather than one of the *.xhtml files?)

Comment 7

[Jonathan Kew [:jfkthame]]

(In reply to comment #6)
> Both the test and the reference should be rendered through the NonWhiteToBlack
> SVG filter; I'm not sure why they should be different, though.  (You're sure
> you weren't looking at one of the *.html files rather than one of the *.xhtml
> files?)

Yes, very sure. With regard to the NonWhiteToBlack filter, it appears to actually be performing a colors-to-grayscale filtering, it is NOT resulting in purely black/white output. (This seems to be true across multiple platforms; see bug 578840 comment #5, for example.)

Comment 8

[Jonathan Kew [:jfkthame]]

Attachment: logging cairo_show_glyphs calls during zwnj-01.xhtml reftest, with callstacks

(In reply to comment #5)
> Not sure, but the callstacks would give a strong clue.

The attachment shows callstacks for each of the cairo_show_glyphs calls during a zwnj-01.xhtml reftest run (last line of text only). The reference seems to be getting re-rendered here, giving us 6 calls instead of the 4 seen earlier; I guess that's probably a result of pausing to capture the stacks, leading to an extra repaint of the window before termination.

Comment 9

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

(In reply to comment #7)
> (In reply to comment #6)
> > Both the test and the reference should be rendered through the NonWhiteToBlack
> > SVG filter; I'm not sure why they should be different, though.  (You're sure
> > you weren't looking at one of the *.html files rather than one of the *.xhtml
> > files?)
> 
> Yes, very sure. With regard to the NonWhiteToBlack filter, it appears to
> actually be performing a colors-to-grayscale filtering, it is NOT resulting in
> purely black/white output. (This seems to be true across multiple platforms;
> see bug 578840 comment #5, for example.)

Hmmm.  I think what's wrong there is actually that the test needs a background color inside the filter, since the NonWhiteToBlack filter doesn't touch the alpha channel.

Comment 10

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

Attachment: possible patch

I haven't tested this, but it seems likely to work.

This adds a foreground and background color to the HTML in zwnj-01.  zwnj-02 (which is the test I originally wrote filters.svg for) was already correct.

I also added a foreground color to the filter-* tests in reftest-sanity (even though the foreground color is unused in that test) in case somebody copies from it, and added comments to filters.svg pointing out the issue.

Comment 11

[Jonathan Kew [:jfkthame]]

Comment on attachment 461574 [details] [diff] [review]
possible patch

Thanks for looking into this. Sounds reasonable, and this seems to resolve the test failure for me locally, at least.

Comment 12

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

http://hg.mozilla.org/mozilla-central/rev/55105cdc45c9