587316 - [d2d] Fix mochitest canvas mochitest failures

Status: RESOLVED FIXED

(Core :: Graphics, defect)

Description

[Jeff Muizelaar [:jrmuizel]]

30796 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 49,25 of c215 is 49,49,206,255; expected 0,0,255,255 +/- 16
30800 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 149,25 of c215 is 49,49,206,255; expected 0,0,255,255 +/- 16
32213 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c320 is 255,0,0,255; expected 0,255,0,255 +/- 0
32214 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,1 of c320 is 255,0,0,255; expected 0,255,0,255 +/- 0
32215 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,48 of c320 is 255,0,0,255; expected 0,255,0,255 +/- 0
32216 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c320 is 255,0,0,255; expected 0,255,0,255 +/- 0
32217 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c321 is 255,0,0,255; expected 0,255,0,255 +/- 0
32218 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,1 of c321 is 255,0,0,255; expected 0,255,0,255 +/- 0
32219 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,48 of c321 is 255,0,0,255; expected 0,255,0,255 +/- 0
32220 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c321 is 255,0,0,255; expected 0,255,0,255 +/- 0
32225 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c323 is 255,0,0,255; expected 0,255,0,255 +/- 0
32226 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,1 of c323 is 255,0,0,255; expected 0,255,0,255 +/- 0
32227 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,48 of c323 is 255,0,0,255; expected 0,255,0,255 +/- 0
32228 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c323 is 255,0,0,255; expected 0,255,0,255 +/- 0
32229 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c324 is 255,0,0,255; expected 0,255,0,255 +/- 0
32230 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,1 of c324 is 255,0,0,255; expected 0,255,0,255 +/- 0
32231 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,48 of c324 is 255,0,0,255; expected 0,255,0,255 +/- 0
32232 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c324 is 255,0,0,255; expected 0,255,0,255 +/- 0
32233 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c325 is 255,0,0,255; expected 0,255,0,255 +/- 0
32234 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,1 of c325 is 255,0,0,255; expected 0,255,0,255 +/- 0
32235 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 48,48 of c325 is 255,0,0,255; expected 0,255,0,255 +/- 0
32236 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c325 is 255,0,0,255; expected 0,255,0,255 +/- 0
32668 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c438 is 255,0,0,255; expected 0,0,0,0 +/- 0
32675 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c446 is 255,0,0,255; expected 0,255,0,255 +/- 0
32688 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 0,0 of c451 is 16,239,0,255; expected 0,255,0,255 +/- 0
32691 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 0,25 of c451 is 16,239,0,255; expected 0,255,0,255 +/- 0
32694 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 0,49 of c451 is 16,239,0,255; expected 0,255,0,255 +/- 0
32764 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,1 of c477 is 255,0,0,255; expected 0,255,0,255 +/- 0
32765 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 98,1 of c477 is 255,0,0,255; expected 0,255,0,255 +/- 0
32766 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 1,48 of c477 is 255,0,0,255; expected 0,255,0,255 +/- 0
32889 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c521 is 0,0,0,0; expected 0,255,0,255 +/- 2
32890 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c522 is 0,0,0,0; expected 0,255,0,255 +/- 2
33505 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c580 is 255,0,0,255; expected 0,0,0,0 +/- 0
33508 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_canvas.html | pixel 50,25 of c583 is 255,0,0,255; expected 0,0,0,0 +/- 0

Comment 1

[Bas Schouten (:bas.schouten)]

Attachment: Part 1: Fix miter join mode.

Miter joins in canvas always use the miter limit and therefor in D2D should use MITER_OR_BEVEL.

Comment 2

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466027 [details] [diff] [review]
Part 1: Fix miter join mode.

Please add a comment explaining the difference between D2D1_LINE_JOIN_MITER_OR_BEVEL and D2D1_LINE_JOIN_MITER and why we chose D2D1_LINE_JOIN_MITER_OR_BEVEL.

Comment 3

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Do not stroke paths which don't cover any ground

It seems cairo fallback stroke does not stroke a path if its bounds are 0x0. We should do this in D2D as well.

Comment 4

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466035 [details] [diff] [review]
Part 2: Do not stroke paths which don't cover any ground

This is incorrect. Degenerate paths with round caps are stroked in cairo. Also, do you know if GetBounds is O(1) or O(n)?

Comment 5

[Bas Schouten (:bas.schouten)]

Attachment: Part 3: Support custom operators on mask operation

Comment 6

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Do not stroke paths which don't cover any ground v2

You're right. Fixed it. I'm not sure how fast GetBounds() is, I guess it all depends on if they cache it during geometry creation.

Comment 7

[Bas Schouten (:bas.schouten)]

Attachment: Part 4: Support CAIRO_EXTEND_NONE for D2D source surfaces

This deals with the final Canvas test failure. We should probably look where we do this without need and change those places to not use EXTEND_NONE but something that's easier for the graphics card to deal with.

Comment 8

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466044 [details] [diff] [review]
Part 2: Do not stroke paths which don't cover any ground v2

I think this is saner. You may want to align it like:
      bounds.left == bounds.right &&
      bounds.top  == bounds.bottom &&
      style->line_cap != CAIRO_LINE_CAP_ROUND

to make it easier to read. You might also want to change the comment to something like:
"D2D will stroke degenerate paths with caps that aren't round, however we want to ignore them"

I'm also worried that stroking degenerate paths with multiple rectangle will not have degenerate bounds. In fact I think the inconsistency that this introduces is worse than always drawing degenerate paths like D2D does. Perhaps it's worth investigating what the canvas spec says about this issue to see how much latitude we have. It's also probably worth testing IE9 to see what it does.

Comment 9

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466037 [details] [diff] [review]
Part 3: Support custom operators on mask operation

This looks pretty straightforward.

Comment 10

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Stroke paths which are degenerate only when linecap is round

So, cairo on GDI exposes some very creative behavior here. If lineCap is round, it will stroke anything. If lineCap is anything else, it will not, regardless of lineJoin. If lineCap is round, it will draw a closed shape according to its lineJoin directive, regardless of the fact the shape has no caps.

This patch mimics all that behavior.

There's one behavioral strangeness which it does not account for. If the line cap is set to butt, and the join to miter. Then the lineWidth between the joins seems to adjust to the miter angle on the joints giving a hollow shape even with a large linewidth. An example is included. I'm not sure how to mimic this behavior as I cannot find a logical ground for it.

Comment 11

[Bas Schouten (:bas.schouten)]

Attachment: Example of strange rendering behavior

Comment 12

[Bas Schouten (:bas.schouten)]

Attachment: Part 5: Round transformation matrix values

I'm not entirely sure if I'm happy with this patch. It passes all the tests though. The reason one of the canvas tests fails is because its transforming a rect 180 degrees and then translating it over the square of the test, expecting it to be filled.

The matrix we get into cairo however, has inaccuracies of about 10^-6 in the transformation matrix, this causes the rectangle to be slightly skewed. As we already know from our experience with geometries, D2D can mistake 10^-6th of coverage for 1/16th of coverage. This makes the edge of that canvas testcase become 1/16th red. By rounding the matrix to 5 decimals we avoid this problem. An alternate approach however, is to disable the test.

Comment 13

[Bas Schouten (:bas.schouten)]

Attachment: Part 6: Disable failing gradient test for D2D

Because of bug 582236 there's no complete blue or complete yellow at the hard stop gradient edges in the gradient interpolate test. Since we've already concluded we can't do anything about this and can live with it, we should probably disable the test.

Comment 14

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466090 [details] [diff] [review]
Part 5: Round transformation matrix values

I don't really think this is a good idea. Decreasing the precision to pass a test suggests that the test is being too strict.

Comment 15

[Bas Schouten (:bas.schouten)]

Attachment: Part 2: Disable degenerate stroking tests

See bug 587554.

Comment 16

[Bas Schouten (:bas.schouten)]

Attachment: Part 5: Disable transformed rect test

Disabling this test because of known D2D bug which causes an inaccuracy in rasterization here.

Comment 17

[Jeff Muizelaar [:jrmuizel]]

Comment on attachment 466213 [details] [diff] [review]
Part 5: Disable transformed rect test

The d2d detection probably belongs in a function

Comment 18

[Bas Schouten (:bas.schouten)]

http://hg.mozilla.org/mozilla-central/rev/9cb9cb9dd8fa
http://hg.mozilla.org/mozilla-central/rev/ea6bbdbf1490
http://hg.mozilla.org/mozilla-central/rev/1d7c15818f66
http://hg.mozilla.org/mozilla-central/rev/055a407bb683
http://hg.mozilla.org/mozilla-central/rev/6aa55fbbabc0
http://hg.mozilla.org/mozilla-central/rev/0fb6352dc09c