Bug 55696 – canvas arc() missing line to start of arc if swing is zero

• Component: Canvas
• Status: RESOLVED FIXED

[Cary Clark]

In https://bugs.webkit.org/show_bug.cgi?id=41420 Andreas Kling quotes
http://www.whatwg.org/specs/web-apps/current-work/#dom-context-2d-stroke
 "Zero-length line segments must be pruned before stroking a path.
Empty subpaths must be ignored."

And implemented this in CanvasRenderingContext2D::arc() by adding:
+    if (sa == ea)
+        return;

However, the context.arc description in the Canvas element spec at
http://dev.w3.org/html5/2dcontext/ reads:

"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. In any case, it must draw the arc between the start
point of the arc and the end point of the arc, and add the start and
end points of the arc to the subpath."

and:

"If the two points are the same, or if the radius is zero, then the
arc is defined as being of zero length in both directions."

This bug http://code.google.com/p/chromium/issues/detail?id=52059
containing this fragment:
   <canvas id="cvs" width="600" height="250" style="border: 1px
dashed black">[No canvas support]</canvas>
   <script>
       co = document.getElementById("cvs").getContext('2d');
       co.beginPath();
           co.moveTo(100,100);
           co.arc(100,100,100,0.785,0.785,0);
           co.lineTo(100,100);
       co.stroke();
   </script>

which shows that this change prevents the line from being drawn from
(100,100) to the arc start.

Opera, Firefox 3 and 4 draw this line, but Chrome and nightly Safari do not.

[Tom Zakrajsek]

Created attachment 105132 [details]
Patch

This patch also has the fix for related bug https://bugs.webkit.org/show_bug.cgi?id=66832.

[WebKit Review Bot]

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

Failed to run "['Tools/Scripts/check-webkit-style', '--diff-files', u'LayoutTests/ChangeLog', u'LayoutTests/canv..." exit_code: 1

Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:830:  Tests for true/false, null/non-null, and zero/non-zero should all be done without equality comparisons.  [readability/comparison_to_zero] [5]
Total errors found: 1 in 6 files


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

[Tom Zakrajsek]

Created attachment 105133 [details]
Patch - fix compare to 0 style error

For the record, I don't like this.  Having to do a boolean check to see if a float is 0 or not, for "style" reasons, doesn't feel right.

[Tom Zakrajsek]

Created attachment 105134 [details]
Patch

[WebKit Review Bot]

Comment on attachment 105133 [details]
Patch - fix compare to 0 style error

Attachment 105133 [details] did not pass chromium-ews (chromium-xvfb):
Output: http://queues.webkit.org/results/9508238

New failing tests:
canvas/philip/tests/2d.path.arc.zeroradius.html
fast/canvas/canvas-arc-zero-lineto.html

[Kent Tamura]

(In reply to comment #4)
> Created an attachment (id=105134) [details]
> Patch

Do you want it is reviewed?  If so, open "Details" page of the attachment, change MIME type, check "Patch" checkbox, and select '?' for review flag.  You had better use "webkit-patch upload" script. It takes care of them.

[Tom Zakrajsek]

Created attachment 105183 [details]
Patch

[Tom Zakrajsek]

Comment on attachment 105183 [details]
Patch

Missed a test failure from previous bot run.

[Tom Zakrajsek]

Created attachment 105242 [details]
Patch

[Andreas Kling]

Comment on attachment 105242 [details]
Patch

View in context: https://bugs.webkit.org/attachment.cgi?id=105242&action=review

> Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:834
> +        FloatPoint pt(x + r * cos(sa), y + r * sin(sa));

Shouldn't that be cosf() and sinf()?

[Tom Zakrajsek]

Created attachment 105382 [details]
Patch

[Andreas Kling]

Comment on attachment 105382 [details]
Patch

LGTM.

[WebKit Review Bot]

Comment on attachment 105382 [details]
Patch

Clearing flags on attachment: 105382

Committed r93982: <http://trac.webkit.org/changeset/93982>

[WebKit Review Bot]

All reviewed patches have been landed.  Closing bug.

[Ryosuke Niwa]

canvas/philip/tests/2d.path.stroke.prune.arc.html started failing on Snow Leopard after this patch:
http://build.webkit.org/results/SnowLeopard%20Intel%20Debug%20(Tests)/r93982%20(1898)/canvas/philip/tests/2d.path.stroke.prune.arc-pretty-diff.html