RESOLVED CONFIGURATION CHANGED 63516
Some canvas arc tests fail because the arc implementaion does not match the HTML5 canvas specification.
https://bugs.webkit.org/show_bug.cgi?id=63516
Summary Some canvas arc tests fail because the arc implementaion does not match the H...
Dongseong Hwang
Reported 2011-06-28 00:36:37 PDT
Following tests are failed in Chromium port. * LayoutTests/canvas/philip/tests/2d.path.arc.angle.3.html * LayoutTests/canvas/philip/tests/2d.path.arc.angle.5.html If the anticlockwise argument is omitted or false and endAngle-startAngle is less than -2Pi, Chromium draws the whole circumference.
Attachments
patch (3.98 KB, patch)
2011-06-28 00:41 PDT, Dongseong Hwang
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-02 (1.28 MB, application/zip)
2011-06-28 00:58 PDT, WebKit Review Bot
no flags
patch (8.10 KB, patch)
2011-07-05 00:35 PDT, Dongseong Hwang
no flags
patch (12.55 KB, patch)
2011-07-05 00:40 PDT, Dongseong Hwang
webkit.review.bot: commit-queue-
Archive of layout-test-results from ec2-cr-linux-01 (1.39 MB, application/zip)
2011-07-05 02:00 PDT, WebKit Review Bot
no flags
patch (18.57 KB, patch)
2011-07-05 03:21 PDT, Dongseong Hwang
jchaffraix: review-
Dongseong Hwang
Comment 1 2011-06-28 00:41:03 PDT
Created attachment 98872 [details] patch Following tests are failed in Chromium port. * LayoutTests/canvas/philip/tests/2d.path.arc.angle.3.html * LayoutTests/canvas/philip/tests/2d.path.arc.angle.5.html It is not because of skia bug. Other ports pass above test because their graphics backends are familiar with HTML5 Canvas arc specification by chance. This patch makes CanvasRenderingContext2D::arc match following paragraph in HTML5 Canvas spec. http://dev.w3.org/html5/2dcontext/#dom-context-2d-createpattern "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. 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."
WebKit Review Bot
Comment 2 2011-06-28 00:58:16 PDT
Comment on attachment 98872 [details] patch Attachment 98872 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8954269 New failing tests: fast/canvas/canvas-arc-360-winding.html fast/canvas/canvas-strokePath-alpha-shadow.html fast/canvas/canvas-fillPath-alpha-shadow.html fast/canvas/canvas-fillPath-pattern-shadow.html canvas/philip/tests/2d.line.join.round.html fast/canvas/canvas-fillPath-gradient-shadow.html
WebKit Review Bot
Comment 3 2011-06-28 00:58:21 PDT
Created attachment 98876 [details] Archive of layout-test-results from ec2-cr-linux-02 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-02 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Matthew Delaney
Comment 4 2011-06-28 13:19:06 PDT
Comment on attachment 98872 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=98872&action=review > Source/WebCore/html/canvas/CanvasRenderingContext2D.cpp:904 > + ea = fmod(ea , 2 * piFloat); These are just floats here, so I believe you just need fmodf. Same for below.
Dongseong Hwang
Comment 5 2011-07-05 00:35:14 PDT
Created attachment 99671 [details] patch The 5 layout tests that were failed don't use Canvas arc api correctly. I fixed those.
Dongseong Hwang
Comment 6 2011-07-05 00:40:58 PDT
WebKit Review Bot
Comment 7 2011-07-05 02:00:02 PDT
Comment on attachment 99673 [details] patch Attachment 99673 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/8983557 New failing tests: canvas/philip/tests/2d.path.arc.nonempty.html
WebKit Review Bot
Comment 8 2011-07-05 02:00:07 PDT
Created attachment 99684 [details] Archive of layout-test-results from ec2-cr-linux-01 The attached test failures were seen while running run-webkit-tests on the chromium-ews. Bot: ec2-cr-linux-01 Port: Chromium Platform: Linux-2.6.35-28-virtual-x86_64-with-Ubuntu-10.10-maverick
Dongseong Hwang
Comment 9 2011-07-05 03:21:56 PDT
Matthew Delaney
Comment 10 2011-07-06 11:05:39 PDT
The tests in the canvas/philip directory are basically just snapshots of the tests created by Philip Taylor which he has hosted here: http://philip.html5.org/tests/canvas/suite/tests/ IIRC, they've all been submitted to the w3c as well: http://w3c-test.org/html/tests/submission/PhilipTaylor/ So, as a general rule we avoid modifying our local copies of the tests if we find any issues. Instead, if you are 100% positive that a test has issues or is incorrect/outdated then you should contact Philip directly and tell him what's wrong with the test. If he agrees, he'll update the tests, and then we'll grab the latest copies and skip the faulty tests in the meantime. I'm doubtful that Philip had that many errors in his tests as your latest patch shows. So, either we have old versions of those tests and the spec has changed or something (also unlikely) then you might want to take a second look at the validity of your changes to the tests.
Dongseong Hwang
Comment 11 2011-07-07 03:12:21 PDT
Thank you for good advice. I did send email about it to Philip Taylor.
Chang Shu
Comment 12 2012-02-13 05:48:46 PST
(In reply to comment #11) > Thank you for good advice. > I did send email about it to Philip Taylor. any updates?
Dongseong Hwang
Comment 13 2012-02-16 03:14:04 PST
(In reply to comment #12) > (In reply to comment #11) > > Thank you for good advice. > > I did send email about it to Philip Taylor. > > any updates? I'm sorry. Mr Taylor did not response to m, yet.
Dirk Schulze
Comment 14 2013-02-16 00:22:13 PST
Comment on attachment 99691 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=99691&action=review > LayoutTests/ChangeLog:16 > + the arc can never cover an angle greater than 2Ï radians. If the two The pi signe looks strange. Just use pi. > LayoutTests/ChangeLog:18 > + points are the same, or if the radius is zero, then the arc is defined > + as being of zero length in both directions." I don't like this text at all. It basically says: You can not draw a full circle. > LayoutTests/ChangeLog:29 > + Fix 9 canvas layout tests because those are failed after this patch. The layout > + tests use arc api like this, "ctx.arc(x, y, r, 0, Math.PI*2, true);". > + Above code should not draw circle to conform Canvas spec. Above code draw > + nothing. Above code should be changed to following code to draw circle. > + ctx.arc(x, y, r, 0, Math.PI*2, false); > + ctx.arc(x, y, r, Math.PI*2, 0, true); I don't get your point. The necessary quote is "If the two points are the same,..., then the arc is defined as being of zero length in both directions." So it doesn't draw anything, independent of clockwise or anti-clockwise, no?
Julien Chaffraix
Comment 15 2013-02-28 10:39:05 PST
Comment on attachment 99691 [details] patch View in context: https://bugs.webkit.org/attachment.cgi?id=99691&action=review The patch doesn't apply anymore and there were some issues to be sorted out. If this gets cleared out, feel free to resubmit an updated patch. > LayoutTests/ChangeLog:5 > + This ChangeLog doesn't have the "Reviewed by NOBODY (OOPS!)." line so your patch cannot land (our tools will choke on it).
Brent Fulgham
Comment 16 2022-07-18 14:04:20 PDT
WebKit is currently passing all WPT for 2d.path.arc.angle, so I believe this is resolved.
Note You need to log in before you can comment on or make changes to this bug.