Bug 16558 – Cairo WebCore Rendering Fails on arc drawing

• Status: RESOLVED
• Resolution: FIXED
• Component: WebCore Misc.

[Brent Fulgham]

The Adobe AIR/Apollo team reports that the Cairo back-end reverses the order of arc drawing, resulting in this test case: "http://developer.mozilla.org/samples/canvas-tutorial/2_2_canvas_moveto.html" drawing an upside-down smiley face.

The attached patch corrects this problem.

Patch needs to be tested on a Cairo-based back-end before landing.

[Brent Fulgham]

Created attachment 18034 [details]
Patch to correct Cairo drawing order

Please test on Cairo-based system before landing patch.

[Alp Toker]

It turns out this fix provides the correct result, but looks like the direction parameter was mis-named in WebKit.

CanvasRenderingContext2D.idl:

        void arc(in float x, in float y, in float radius, in float startAngle, in float endAngle, in boolean clockwise)

HTML5 spec:

        void arc(in float x, in float y, in float radius, in float startAngle, in float endAngle, in boolean anticlockwise);

It seems that based on http://philip.html5.org/tests/canvas/suite/tests/results.html, test 2d.path.arc.angle.1, all browsers implement this parameter as "anticlockwise", so the "clockwise" parameter name in WebKit might be a typo.

If this is right, the patch should be updated to correct the IDL, CanvasRenderingContext2D.cpp, Path.h and also the three Path implementations to fix the variable name.

I'd like to co-review this patch with another implementer of the canvas, either CG or Qt.

Nice work!

[Brent Fulgham]

Created attachment 18045 [details]
Updated based on review of code to use proper 'anticlockwise' label everywhere.

Note that wx/PathWx.cpp did not need to be changed (implementation did not use labels in the declaration, and is a 'not-defined' operation), and qt/PathQt.cpp was already using the label 'anticlockwise'.

[Brent Fulgham]

Created attachment 18046 [details]
Small revision (s/antiClockwise/anticlockwise/)

[Brent Fulgham]

Created attachment 18048 [details]
Correction for malformed patch.

[Alp Toker]

Comment on attachment 18048 [details]
Correction for malformed patch.

r=me without the PathCG.cpp modification (since it doesn't help make the code any clearer). Eric agrees that this is a typo.

[Alp Toker]

Landed in r28944. A follow-up bug should probably be filed on the CG implementation.

[Brent Fulgham]

Note that bug #16565 was added to explore the strange PathCG.cpp issue.