[d2d] Fix mochitest canvas mochitest failures

RESOLVED FIXED

Status

()

defect
RESOLVED FIXED
9 years ago
8 years ago

People

(Reporter: jrmuizel, Assigned: bas.schouten)

Tracking

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

Firefox Tracking Flags

(blocking2.0 beta4+)

Details

Attachments

(7 attachments, 4 obsolete attachments)

831 bytes, patch
jrmuizel
: review+
Details | Diff | Splinter Review
3.26 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
2.57 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
785 bytes, text/html
Details
2.86 KB, patch
jrmuizel
: review+
vlad
: review+
Details | Diff | Splinter Review
2.33 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
2.58 KB, patch
jrmuizel
: review+
Details | Diff | Splinter Review
(Reporter)

Description

9 years ago
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
(Reporter)

Updated

9 years ago
Blocks: d2d
(Assignee)

Comment 1

9 years ago
Miter joins in canvas always use the miter limit and therefor in D2D should use MITER_OR_BEVEL.
Assignee: nobody → bas.schouten
Status: NEW → ASSIGNED
Attachment #466027 - Flags: review?(jmuizelaar)
(Reporter)

Comment 2

9 years ago
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.
Attachment #466027 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 3

9 years ago
It seems cairo fallback stroke does not stroke a path if its bounds are 0x0. We should do this in D2D as well.
Attachment #466035 - Flags: review?(jmuizelaar)
(Reporter)

Comment 4

9 years ago
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)?
Attachment #466035 - Flags: review?(jmuizelaar) → review-
(Assignee)

Comment 6

9 years ago
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.
Attachment #466035 - Attachment is obsolete: true
Attachment #466044 - Flags: review?(jmuizelaar)
(Assignee)

Comment 7

9 years ago
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.
Attachment #466045 - Flags: review?(jmuizelaar)
(Reporter)

Comment 8

9 years ago
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.
(Reporter)

Comment 9

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

This looks pretty straightforward.
Attachment #466037 - Flags: review?(jmuizelaar) → review+
(Reporter)

Updated

9 years ago
Attachment #466045 - Flags: review?(jmuizelaar) → review+
(Assignee)

Comment 10

9 years ago
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.
Attachment #466044 - Attachment is obsolete: true
Attachment #466088 - Flags: review?(jmuizelaar)
Attachment #466044 - Flags: review?(jmuizelaar)
(Assignee)

Comment 12

9 years ago
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.
Attachment #466090 - Flags: review?(jmuizelaar)
(Assignee)

Comment 13

9 years ago
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.
Attachment #466091 - Flags: review?(jmuizelaar)
(Reporter)

Comment 14

9 years ago
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.
Attachment #466090 - Flags: review?(jmuizelaar) → review-
(Reporter)

Updated

9 years ago
Attachment #466091 - Flags: review?(jmuizelaar) → review+
blocking2.0: --- → beta4+
(Assignee)

Comment 15

9 years ago
See bug 587554.
Attachment #466088 - Attachment is obsolete: true
Attachment #466211 - Flags: review?(jmuizelaar)
Attachment #466088 - Flags: review?(jmuizelaar)
(Assignee)

Comment 16

9 years ago
Disabling this test because of known D2D bug which causes an inaccuracy in rasterization here.
Attachment #466090 - Attachment is obsolete: true
(Assignee)

Updated

9 years ago
Attachment #466213 - Flags: review?(jmuizelaar)
(Reporter)

Updated

9 years ago
Attachment #466211 - Flags: review?(jmuizelaar) → review+
(Reporter)

Comment 17

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

The d2d detection probably belongs in a function
Attachment #466213 - Flags: review?(jmuizelaar) → review+

Updated

8 years ago
Depends on: 620216
You need to log in before you can comment on or make changes to this bug.