Bug 38537 – [Qt] Failed at http://philip.html5.org/tests/canvas/suite/tests/2d.path.arc.angle.3.html

• Component: New Bugs
• Status: RESOLVED FIXED

[qi]

also failed at :
http://philip.html5.org/tests/canvas/suite/tests/2d.path.arc.angle.5.html
http://philip.html5.org/tests/canvas/suite/tests/2d.path.arc.empty.html

[qi]

Created attachment 55035 [details]
patch

For path.arc, there are some special case need to handle:
1> If the anticlockwise argument is false and endAngle-startAngle is equal to or greater than 2π, or, if the
anticlockwise argument is true and startAngle-endAngle is equal to or greater than 2π, then the arc is the whole
circumference of this circle.
   Otherwise, the arc is the path along the circumference of this circle from the start point to the end point, going
anti-clockwise if the anticlockwise argument is true, and clockwise otherwise. Since the points are on the circle, as
opposed to being simply angles from zero, the arc can never cover an angle greater than 2π radians.

   Currently, our span can be bigger than 2π, this is wrong.

2> If the context has any
subpaths, then the method must add a straight line from the last point in the subpath to the start point of the arc.
   
   Currently, we always add a line, didn't check if has any subpaths.
3> When span is 0, QPainterPath still draw something, the behavior is different with required.

[WebKit Review Bot]

Attachment 55035 [details] did not pass style-queue:

Failed to run "['WebKitTools/Scripts/check-webkit-style', '--no-squash']" exit_code: 1
WebCore/platform/graphics/qt/PathQt.cpp:302:  Boolean expressions that span multiple lines should have their operators on the left side of the line instead of the right side.  [whitespace/operators] [4]
Total errors found: 1 in 2 files


If any of these errors are false positives, please file a bug against check-webkit-style.

[qi]

Created attachment 55036 [details]
patch

fix style issue

[Darin Adler]

Comment on attachment 55036 [details]
patch

Patches that fix bugs need to include regression tests for the WebKit regression test system.

[Laszlo Gombos]

*** Bug 38526 has been marked as a duplicate of this bug. ***

[Eric Seidel (no email)]

Comment on attachment 55036 [details]
patch

r- for lack of test.

[Eric Seidel (no email)]

Seems we should import philip's tests into LayoutTests.  I'm cetain we already have a bugs.webkit.org bug about doing just that.

[qi]

(In reply to comment #7)
> Seems we should import philip's tests into LayoutTests.  I'm cetain we already
> have a bugs.webkit.org bug about doing just that.

Eric, What's your means about "I'm cetain we already have a bugs.webkit.org bug about doing just that."? Is that Laszlo talked Bug 38526? 

Actually in this patch I try to fix 3 issues. Just because they are all in one function, so I make one patch. Bug 38526 is one of the 3 issues. 

Actually, I think Bug 38526 fix is not correct. 

Based on HTML5 spec:

The arc(x, y, radius, startAngle, endAngle, anticlockwise) method draws an arc. If the context has any
subpaths, then the method must add a straight line from the last point in the subpath to the start point of the arc.

Basically, if there are some elements before the arc, we need add a line from that point to the arc start point, otherwise we don’t.

Currently, we always add a line to the arc, even there is no element before the arc, by default it will add a line from (0,0) to the arc, that's wrong. See my change.

I want to know what I need to do. This patch is not need any more? or I need to create a layout test for it? or else..

[qi]

Created attachment 57668 [details]
patch3

[qi]

Since Bug 38526 is fixed, I changed my patch to only fix the following issue:
1> If the anticlockwise argument is false and endAngle-startAngle is equal to or greater than 2π, or, if the
anticlockwise argument is true and startAngle-endAngle is equal to or greater than 2π, then the arc is the whole
circumference of this circle.
   Otherwise, the arc is the path along the circumference of this circle from the start point to the end point, going
anti-clockwise if the anticlockwise argument is true, and clockwise otherwise. Since the points are on the circle, as
opposed to being simply angles from zero, the arc can never cover an angle greater than 2π radians.

   Currently, our span can be bigger than 2π, this is wrong.


This patch will fix canvas/philp/tests/2d.path.arc.angle.3.html and canvas/philp/tests/2d.path.arc.angle.5.html

[qi]

Created attachment 57767 [details]
patch4

fix comments issue.

[Laszlo Gombos]

Comment on attachment 57767 [details]
patch4

looks good to me, r+.

[WebKit Commit Bot]

Comment on attachment 57767 [details]
patch4

Clearing flags on attachment: 57767

Committed r60663: <http://trac.webkit.org/changeset/60663>

[WebKit Commit Bot]

All reviewed patches have been landed.  Closing bug.