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.
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."
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
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
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.
Created attachment 99671 [details] patch The 5 layout tests that were failed don't use Canvas arc api correctly. I fixed those.
Created attachment 99673 [details] patch
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
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
Created attachment 99691 [details] patch
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.
Thank you for good advice. I did send email about it to Philip Taylor.
(In reply to comment #11) > Thank you for good advice. > I did send email about it to Philip Taylor. any updates?
(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.
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?
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).