Closed
Bug 471281
Opened 16 years ago
Closed 15 years ago
Canvas arcTo Rendering Incorrectly in Firefox - Works in Safari
Categories
(Core :: Graphics: Canvas2D, defect)
Tracking
()
RESOLVED
FIXED
People
(Reporter: tag+bugzilla, Assigned: krit)
References
Details
Attachments
(3 files, 6 obsolete files)
User-Agent: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 Build Identifier: Mozilla/5.0 (Windows; U; Windows NT 5.1; en-GB; rv:1.9.0.5) Gecko/2008120122 Firefox/3.0.5 In trying to build a simple rectangle with rounded corners I think I have found a bug with the arcTo method of the canvas API in Firefox. The sample code I have provided works fine in Safari and as far as I can tell, my code is correct (hard to say for certain - documentation of the canvas API is rarer than hens' teeth). In Firefox, the corners are inexplicably shot off in a direction I didn't ask for. Reproducible: Always Steps to Reproduce: 1.Create the following HTML page: <!DOCTYPE html PUBLIC "-//W3C//DTD XHTML 1.0 Strict//EN" "http://www.w3.org/TR/xhtml1/DTD/xhtml1-strict.dtd"> <html xmlns="http://www.w3.org/1999/xhtml"> <head> <title>Bug Test Case</title> </head> <body> <canvas id="foo" width="300" height="300"></canvas> <script> var ctx = document.getElementById('foo').getContext('2d'); ctx.beginPath(); ctx.moveTo(120, 100); ctx.lineTo(180, 100); ctx.arcTo(200, 100, 200, 120, 20); ctx.lineTo(200, 180); ctx.arcTo(200, 200, 180, 200, 20); ctx.lineTo(120, 200); ctx.arcTo(100, 200, 100, 180, 20); ctx.lineTo(100, 120); ctx.arcTo(100, 100, 120, 100, 20); ctx.fill(); </script> </body> </html> 2. View the page in Firefox, then in Safari Actual Results: A square with rounded corners Expected Results: A square with two rounded corners and two corners that are pushed way out from the edges of the square.
Updated•16 years ago
|
Component: General → Layout: Canvas
Product: Firefox → Core
QA Contact: general → layout.canvas
Assignee | ||
Comment 1•16 years ago
|
||
yes, FireFox doesn't follow the specification here. The author seems to missunderstand the specification. eg x2, y2 should never be added to the path:
> mThebes->LineTo(gfxPoint(x2, y2));
There are two tangents. One from currentPoint to x1,y1 and one from (x1,y1) to (x2,y2). The points where the circle touches the tangents, are the points the arc is drawn to. The point on the tangent (x1,y1) to (x2,y2), where the circle touches the tangent, should be the last point on the path. The circle is only discribed by the radius, that means the points, where the circle touches the tangents depends on the radius of the circle and the angle between the two tangents.
Assignee | ||
Comment 2•16 years ago
|
||
I haven't compiled the code yet. But the code base itself works and should give an idea how to implement arcTo.
Assignee | ||
Comment 3•16 years ago
|
||
This is a drawing to the implementation. I hope it helps to understand the code.
Assignee | ||
Comment 4•16 years ago
|
||
This implementation matches the specification of HTML5 and arcTo (http://www.whatwg.org/specs/web-apps/current-work/#dom-context-2d-arcto). I tested it with http://philip.html5.org/tests/canvas/suite/tests/index.2d.path.arcTo.html (the tests included in the test-folder), http://canvex.lazyilluminati.com/misc/arcto.html and my own tests.
Attachment #354627 -
Attachment is obsolete: true
Attachment #354691 -
Flags: review?
Assignee | ||
Comment 5•16 years ago
|
||
sorry, forgot a check if angle1 should be greater or lower pi. The test above works now too.
Attachment #354691 -
Attachment is obsolete: true
Attachment #354694 -
Flags: review?
Attachment #354691 -
Flags: review?
Reporter | ||
Comment 6•16 years ago
|
||
Schulze, Delighted to see how quickly you picked this up and sorted it out - such is the wonderfulness of open source projects (can you imagine how long Microsoft would have taken, if ever, to sort out such a bug in IE?) Unfortunately my maths skills are nowhere near up to the level needed to resolve an issue like this otherwise I would have had a go myself. Is this fix likely to make it into Firefox 3.0.6?
Assignee | ||
Comment 7•16 years ago
|
||
(In reply to comment #6) > Unfortunately my maths skills are nowhere near up to the level needed to > resolve an issue like this otherwise I would have had a go myself. Is this fix > likely to make it into Firefox 3.0.6? Do you ask me if it is ready for Firefox 3.0.6? It's my first patch for Firefox, but isn 3.0.x only for security issues? Otherwise I'll explain the code to everyone, who want's to review it. There are only basic skills in vector analysis needed.
Flags: wanted1.9.1?
Reporter | ||
Comment 8•16 years ago
|
||
> isn't 3.0.x only for security issues?
I have no idea - I'm a web developer, not a web browser developer ;) I reported this because it was causing me a problem as a user of the browser. I'll take your word on the level of skills in vector analysis needed because I only just about know what a vector is!
Anyway, I hope it does make it into the 3.0.x branch of Firefox - the sooner it's fixed, the sooner I can start using it. I see the status of this bug remains "UNCONFIRMED" - how does it get confirmed?
Comment 10•16 years ago
|
||
(In reply to comment #2) > Created an attachment (id=354627) [details] > arcTo-code > > I haven't compiled the code yet. But the code base itself works and should give > an idea how to implement arcTo. This bug is still broken, so it seems. Is there anything you can do.. or anything I can do to get this fixed? Looking forward to hearing from you. Al
Assignee | ||
Comment 11•16 years ago
|
||
(In reply to comment #10) > This bug is still broken, so it seems. Is there anything you can do.. or > anything I can do to get this fixed? Looking forward to hearing from you. > > Al Well, I uploaded a patch and set the review tag. There is nothing else I can do. We just need a reviewer. I wrote it above, this is my first patch for Firefox. I don't know any reviewer who can look at this patch.
I can review this -- can you reupload as a unified diff though? (diff -u) If not I can probably convert the context diff, but unified diffs are usually the norm for patches. Thanks!
Comment 13•16 years ago
|
||
Thanks Vlad! Getting this fixed would make FireFox's ArcTo work with all the Canvas tutorials out there. At the moment people are getting confused about what ArcTo is "meant" to do, thinking that FireFox is doing it correctly.
Assignee | ||
Comment 14•16 years ago
|
||
Ok, this is the unified version of the same patch.
Attachment #354694 -
Attachment is obsolete: true
Attachment #367258 -
Flags: review?
Attachment #354694 -
Flags: review?
Assignee | ||
Comment 15•16 years ago
|
||
ah sorry. The other way around :-P ... uploading a new version in a bit.
Assignee | ||
Comment 16•16 years ago
|
||
The right direction this time. sorry again.
Attachment #367258 -
Attachment is obsolete: true
Attachment #367259 -
Flags: review?
Attachment #367258 -
Flags: review?
Comment 17•16 years ago
|
||
Any news on this bug? Will it be coming out in the next FF update?
Updated•16 years ago
|
Attachment #367259 -
Flags: review? → review?(jmuizelaar)
Comment 18•16 years ago
|
||
This is the test case that the reporter attached. Gotta figure out some way to test this in an automated way.
Updated•16 years ago
|
Status: UNCONFIRMED → NEW
Ever confirmed: true
Comment 19•16 years ago
|
||
It looks like we already have a bunch of arcto tests that are currently todo'd. that I'm hoping this will fix. The seem to test a bunch of things so this testcase probably isn't needed.
Assignee | ||
Comment 20•16 years ago
|
||
(In reply to comment #19) > It looks like we already have a bunch of arcto tests that are currently todo'd. > that I'm hoping this will fix. The seem to test a bunch of things so this > testcase probably isn't needed. Where are this testcases? Can i test them and are there result images, how it should look like? I found some examples in Web as well as some stress tests. The patch worked for all of them. But I didn't find examples of Mozilla.
Comment 21•16 years ago
|
||
The tests in the mozilla tree are basically a copy of these: http://philip.html5.org/tests/canvas/suite/tests/index.2d.path.arcTo.html
Assignee | ||
Comment 22•16 years ago
|
||
(In reply to comment #21) > The tests in the mozilla tree are basically a copy of these: > http://philip.html5.org/tests/canvas/suite/tests/index.2d.path.arcTo.html ah ok, I tested them :-)
Comment 23•16 years ago
|
||
Does http://philip.html5.org/tests/canvas/suite/tests/2d.path.arcTo.transformation.html pass for you? When running the mochitests on try server I get: 15960 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 0,0 is 63,192,0,255; expected 0,255,0,255 +/- 0 15961 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 50,0 is 255,0,0,255; expected 0,255,0,255 +/- 0 15962 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 99,0 is 255,0,0,255; expected 0,255,0,255 +/- 0 15964 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 50,25 is 63,192,0,255; expected 0,255,0,255 +/- 0 15965 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 99,25 is 255,0,0,255; expected 0,255,0,255 +/- 0 15968 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.transformation.html | pixel 99,49 is 191,64,0,255; expected 0,255,0,255 +/- 0
Assignee | ||
Comment 24•16 years ago
|
||
I tested it with trunk, and this test did not fail. Failing tests on Philip's test suite are: 2d.path.arcTo.collinear.2 2d.path.arcTo.collinear.3 fail, but the spec will change here. It wants to draw infinite lines. 2d.path.arcTo.nonfinite fails with "Aborted with exception: An invalid or illegal string was specified". The spec wants us to return silently here. 2d.path.arcTo.emptysubpath is not related to arcTo
Comment 25•16 years ago
|
||
2d.path.arcTo.transformation fails with this patch for me on os x. I've not tried it on other platforms. I get the following: http://people.mozilla.com/~jmuizelaar/arcto.png
Assignee | ||
Comment 26•16 years ago
|
||
Maybe other transformation tests fail for you on Mac too? It's a bit strange that it works on linux, but not on Mac. I haven no possibilities to test it on windows. Can it be a bug in cairo? Or different versions of cairo? I followed the instruction for downloading and building firefox. Does the build process take the cairo dev's of my linux system, or the cairo source you get with firefox? I get the following on linux: http://www1.inf.tu-dresden.de/~s7468461/screenshot-on-linux.png
Assignee | ||
Comment 27•16 years ago
|
||
(In reply to comment #25) > 2d.path.arcTo.transformation fails with this patch for me on os x. I've not > tried it on other platforms. > > I get the following: > http://people.mozilla.com/~jmuizelaar/arcto.png Can you please post a screenshot of http://www1.inf.tu-dresden.de/~s7468461/arcto.html ? This makes it easier to track down the problem.
Comment 28•16 years ago
|
||
(In reply to comment #26) > Maybe other transformation tests fail for you on Mac too? It's a bit strange > that it works on linux, but not on Mac. I haven no possibilities to test it on > windows. > Can it be a bug in cairo? Or different versions of cairo? I followed the > instruction for downloading and building firefox. Does the build process take > the cairo dev's of my linux system, or the cairo source you get with firefox? It should use the cairo source you get with firefox unless you spefically enable system-cairo. But it could certainly be a bug in cairo but it looks like it's not specific to your system. The try server's results match what we've seen. It passes on linux and fails on os x and win32. The cairo code paths on OS X and linux are pretty different so I'm less surprised by the mismatch there, but the fact that linux and win32 don't match does surprise me. > I get the following on linux: > http://www1.inf.tu-dresden.de/~s7468461/screenshot-on-linux.png (In reply to comment #27) > Can you please post a screenshot of > http://www1.inf.tu-dresden.de/~s7468461/arcto.html ? This makes it easier to > track down the problem. Here you go: http://people.mozilla.com/~jmuizelaar/arcto2.png and here's how it looks without your patch (which passes the transformation tests): http://people.mozilla.com/~jmuizelaar/arcto2-old.png
Assignee | ||
Comment 29•16 years ago
|
||
arcto2.png is the way it should look like. And that the old code passes the test is luck.
Comment 30•16 years ago
|
||
Glad the bug is finally out of luck! If you guy need me to test here I can. I have PC: Vista, XP, Ubuntu & MAC: Leopard, Tiger & whatever is running on the old G4 (probably tiger again).
Assignee | ||
Comment 31•16 years ago
|
||
This should hopefully work for all platforms now.
Attachment #367259 -
Attachment is obsolete: true
Attachment #368533 -
Flags: review?
Attachment #367259 -
Flags: review?(jmuizelaar)
Assignee | ||
Comment 32•16 years ago
|
||
This code is based on Philip Taylers arcTo emulation, written in Canvas/JavaScript. Both patches, this one as well as the last one should work on all platforms. But this code is cleaner and makes use of arctan instead of arccos and therefor is saver.
Attachment #368533 -
Attachment is obsolete: true
Attachment #368591 -
Flags: review?
Attachment #368533 -
Flags: review?
Updated•16 years ago
|
Attachment #368591 -
Flags: review? → review?(jmuizelaar)
Comment 33•16 years ago
|
||
(In reply to comment #32) > Created an attachment (id=368591) [details] > arcTo-code fix for all platforms > > This code is based on Philip Taylers arcTo emulation, written in > Canvas/JavaScript. Both patches, this one as well as the last one should work > on all platforms. > But this code is cleaner and makes use of arctan instead of arccos and therefor > is saver. It passes the tests that it should for me on OS X. Nice work all.
Comment 34•16 years ago
|
||
I've been testing the .arcto() in the nightly and it still looks broken. How long would it be until these fixes make it through to the Nightly?
Comment 35•15 years ago
|
||
It'll be in the trunk nightlies shortly, I am reliably informed, and then very very likely in Firefox 3.5.1. Apologies for the delay!
Flags: wanted1.9.1?
Assignee | ||
Comment 36•15 years ago
|
||
(In reply to comment #35) > It'll be in the trunk nightlies shortly, I am reliably informed, and then very > very likely in Firefox 3.5.1. Apologies for the delay! No problem. I'm realy happy when this get fixed :-)
Comment 37•15 years ago
|
||
Awesome! In one of the Firefox 35 days projects I mention that this is not fixed yet. Would look cool to have it fixed and display a message to the user that Moz squashed the bug as part of the release. It will also make parsing SVGs in the DOM much quicker when rendering them to the Canvas, as you wont have to recalculate the path to "fake" an arcTo.
Updated•15 years ago
|
Attachment #368591 -
Flags: review?(jmuizelaar) → review+
Comment 38•15 years ago
|
||
Comment on attachment 368591 [details] [diff] [review] arcTo-code fix for all platforms Sorry for dropping the ball here re: review. I remember giving this a second review, but must have never hit submit. Looks good. It would be nice if there were some more explanatory comments about how it worked, but that's not critical.
Comment 39•15 years ago
|
||
I can put together a page with some test cases on it and we can make sure it's hammered down. I can make it so it loads arcTo's from InkScape so we can test it in some different situations really quickly. I think I will have some time this weekend, so I will link to something here soon.
Comment 40•15 years ago
|
||
http://hg.mozilla.org/mozilla-central/rev/b116b49459f8
Status: NEW → RESOLVED
Closed: 15 years ago
Resolution: --- → FIXED
Comment 41•15 years ago
|
||
Was getting failures on mac. No idea why as it passed on try. Error 0 (Unknown error: 0).16263 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.coincide.1.html | pixel 50,1 is 255,0,0,255; expected 0,255,0,255 +/- 0 16264 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.coincide.1.html | pixel 50,25 is 255,0,0,255; expected 0,255,0,255 +/- 0 16265 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.coincide.1.html | pixel 50,48 is 255,0,0,255; expected 0,255,0,255 +/- 0 16271 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.collinear.1.html | pixel 50,25 is 255,0,0,255; expected 0,255,0,255 +/- 0 16311 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.curve1.html | pixel 80,45 is 255,0,0,255; expected 0,255,0,255 +/- 0 16312 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.curve1.html | pixel 80,46 is 255,0,0,255; expected 0,255,0,255 +/- 0 16315 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.curve2.html | pixel 50,25 is 255,0,0,255; expected 0,255,0,255 +/- 0 16318 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.curve2.html | pixel 55,21 is 121,134,0,255; expected 0,255,0,255 +/- 0 16321 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.curve2.html | pixel 72,28 is 4,251,0,255; expected 0,255,0,255 +/- 0 16331 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.end.html | pixel 1,48 is 255,0,0,255; expected 0,255,0,255 +/- 0 16332 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.end.html | pixel 50,25 is 255,0,0,255; expected 0,255,0,255 +/- 0 16334 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.end.html | pixel 98,48 is 255,0,0,255; expected 0,255,0,255 +/- 0 16338 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.start.html | pixel 1,48 is 255,0,0,255; expected 0,255,0,255 +/- 0 16341 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.shape.start.html | pixel 98,48 is 255,0,0,255; expected 0,255,0,255 +/- 0 16355 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.zero.1.html | should not throw exception 16358 ERROR TEST-UNEXPECTED-FAIL | /tests/content/canvas/test/test_2d.path.arcTo.zero.2.html | should not throw exception
Status: RESOLVED → REOPENED
Resolution: FIXED → ---
Comment 42•15 years ago
|
||
(jeff backed this out in http://hg.mozilla.org/mozilla-central/rev/ac55c0c1c34a )
Comment 43•15 years ago
|
||
The test didn't fail on the normal unittest box, only on the "Mochitest" box from the Firefox-Unittest tree. And even on that box it only failed in 4 of 6 runs, as far as I can tell.
Assignee | ||
Comment 44•15 years ago
|
||
What is the difference between this two test boxes?
Comment 45•15 years ago
|
||
The Mochitest-only boxes are new, and might be not ready yet. Three hours ago, the Mac Mochitest box went orange again; this time with TEST-UNEXPECTED-PASS in the arcTo tests: http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1245300543.1245302381.22395.gz How can this be possible? Does it mean that the box tested the wrong build?
Comment 46•15 years ago
|
||
(In reply to comment #45) > The Mochitest-only boxes are new, and might be not ready yet. > Three hours ago, the Mac Mochitest box went orange again; this time with > TEST-UNEXPECTED-PASS in the arcTo tests: > http://tinderbox.mozilla.org/showlog.cgi?log=Firefox-Unittest/1245300543.1245302381.22395.gz > > How can this be possible? Does it mean that the box tested the wrong build? As far as I understand it, the only difference between the mochitest-only box and the main unittest box is that the mochitest-only box tests the same bits from the main "build" box. The unittest box builds its own version of the code using a different mozconfig than the standard "build" box uses. I believe the only difference in those two mozconfigs would be --enable-tests and the flags to enable leak logging. Both are testing optimized builds, so it isn't a opt vs. debug issue. It might be possible that the mochitest only box pulled the wrong build (from the build box) to test. CC'ing catlee.
Comment 47•15 years ago
|
||
This should be fixed by the landing of bug 499161
Comment 48•15 years ago
|
||
Relanded as: http://hg.mozilla.org/mozilla-central/rev/e97819582eed
Status: REOPENED → RESOLVED
Closed: 15 years ago → 15 years ago
Resolution: --- → FIXED
Comment 50•14 years ago
|
||
This still does not seem to work for this example: http://canvex.lazyilluminati.com/misc/arcto.html It appears that Opera is the only vendor for which the arcTo() working as expected across the board. Perhaps someone from SVG could verify whether the example above is getting the expected results?
![]() |
||
Comment 51•14 years ago
|
||
Alistair, why is SVG relevant here? Note that SVG specifies arcs via endpoints, while canvas specifies them via control points. It looks like Opera is doing endpoint-specified arcs in canvas, which is wrong.
Updated•13 years ago
|
Assignee: nobody → vbs85
You need to log in
before you can comment on or make changes to this bug.
Description
•