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

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
9 years ago

People

(Reporter: jrmuizel, Assigned: jfkthame)

Tracking

(Blocks 1 bug)

Trunk
x86
Windows 7
Points:
---
Dependency tree / graph

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(2 attachments)

No description provided.
Jonathan, can you take a brief look at this and see if it's up your alley?
Assignee: nobody → jfkthame
(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.
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.
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.
Not sure, but the callstacks would give a strong clue.
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?)
(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.)
(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.
(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.
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.
Attachment #461574 - Flags: review?(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.
Attachment #461574 - Flags: review?(jfkthame) → review+
http://hg.mozilla.org/mozilla-central/rev/55105cdc45c9
Status: NEW → RESOLVED
Closed: 9 years ago
Resolution: --- → FIXED
You need to log in before you can comment on or make changes to this bug.