Bug 16558 - Cairo WebCore Rendering Fails on arc drawing
: Cairo WebCore Rendering Fails on arc drawing
Status: RESOLVED FIXED
Product: WebKit
Classification: Unclassified
Component: WebCore Misc.
: 528+ (Nightly build)
: PC OS X 10.4
: P2 Normal
Assigned To: Nobody
: Cairo
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2007-12-21 10:13 PST by Brent Fulgham
Modified: 2007-12-21 17:27 PST (History)
1 user (show)

See Also:


Attachments
Patch to correct Cairo drawing order (1.68 KB, patch)
2007-12-21 10:25 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Updated based on review of code to use proper 'anticlockwise' label everywhere. (5.72 KB, patch)
2007-12-21 16:19 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Small revision (s/antiClockwise/anticlockwise/) (5.71 KB, patch)
2007-12-21 16:22 PST, Brent Fulgham
no flags Details | Formatted Diff | Diff
Correction for malformed patch. (5.71 KB, patch)
2007-12-21 16:51 PST, Brent Fulgham
alp: review+
Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Brent Fulgham 2007-12-21 10:13:29 PST
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.
Comment 1 Brent Fulgham 2007-12-21 10:25:45 PST
Created attachment 18034 [details]
Patch to correct Cairo drawing order

Please test on Cairo-based system before landing patch.
Comment 2 Alp Toker 2007-12-21 12:13:41 PST
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!
Comment 3 Brent Fulgham 2007-12-21 16:19:25 PST
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'.
Comment 4 Brent Fulgham 2007-12-21 16:22:44 PST
Created attachment 18046 [details]
Small revision (s/antiClockwise/anticlockwise/)
Comment 5 Brent Fulgham 2007-12-21 16:51:17 PST
Created attachment 18048 [details]
Correction for malformed patch.
Comment 6 Alp Toker 2007-12-21 16:58:13 PST
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.
Comment 7 Alp Toker 2007-12-21 17:02:06 PST
Landed in r28944. A follow-up bug should probably be filed on the CG implementation.
Comment 8 Brent Fulgham 2007-12-21 17:27:56 PST
Note that bug #16565 was added to explore the strange PathCG.cpp issue.