Bug 38537 - [Qt] Failed at http://philip.html5.org/tests/canvas/suite/tests/2d.path.arc.angle.3.html
Summary: [Qt] Failed at http://philip.html5.org/tests/canvas/suite/tests/2d.path.arc.a...
Status: RESOLVED FIXED
Alias: None
Product: WebKit
Classification: Unclassified
Component: New Bugs (show other bugs)
Version: 528+ (Nightly build)
Hardware: PC Linux
: P3 Normal
Assignee: qi
URL:
Keywords: Qt, QtTriaged
Depends on:
Blocks:
 
Reported: 2010-05-04 11:43 PDT by qi
Modified: 2010-06-04 01:21 PDT (History)
8 users (show)

See Also:


Attachments
patch (2.93 KB, patch)
2010-05-04 12:00 PDT, qi
no flags Details | Formatted Diff | Diff
patch (2.93 KB, patch)
2010-05-04 12:27 PDT, qi
eric: review-
Details | Formatted Diff | Diff
patch3 (3.65 KB, patch)
2010-06-02 11:14 PDT, qi
no flags Details | Formatted Diff | Diff
patch4 (3.67 KB, patch)
2010-06-03 07:56 PDT, qi
no flags Details | Formatted Diff | Diff

Note You need to log in before you can comment on or make changes to this bug.
Comment 1 qi 2010-05-04 12:00:21 PDT
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.
Comment 2 WebKit Review Bot 2010-05-04 12:06:18 PDT
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.
Comment 3 qi 2010-05-04 12:27:30 PDT
Created attachment 55036 [details]
patch

fix style issue
Comment 4 Darin Adler 2010-05-04 12:34:39 PDT
Comment on attachment 55036 [details]
patch

Patches that fix bugs need to include regression tests for the WebKit regression test system.
Comment 5 Laszlo Gombos 2010-05-05 12:34:28 PDT
*** Bug 38526 has been marked as a duplicate of this bug. ***
Comment 6 Eric Seidel (no email) 2010-05-05 21:59:35 PDT
Comment on attachment 55036 [details]
patch

r- for lack of test.
Comment 7 Eric Seidel (no email) 2010-05-05 22:00:10 PDT
Seems we should import philip's tests into LayoutTests.  I'm cetain we already have a bugs.webkit.org bug about doing just that.
Comment 8 qi 2010-05-06 06:31:11 PDT
(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..
Comment 9 qi 2010-06-02 11:14:41 PDT
Created attachment 57668 [details]
patch3
Comment 10 qi 2010-06-02 11:16:54 PDT
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
Comment 11 qi 2010-06-03 07:56:58 PDT
Created attachment 57767 [details]
patch4

fix comments issue.
Comment 12 Laszlo Gombos 2010-06-03 09:01:50 PDT
Comment on attachment 57767 [details]
patch4

looks good to me, r+.
Comment 13 WebKit Commit Bot 2010-06-04 01:21:01 PDT
Comment on attachment 57767 [details]
patch4

Clearing flags on attachment: 57767

Committed r60663: <http://trac.webkit.org/changeset/60663>
Comment 14 WebKit Commit Bot 2010-06-04 01:21:11 PDT
All reviewed patches have been landed.  Closing bug.