Canvas arcTo Rendering Incorrectly in Firefox - Works in Safari

RESOLVED FIXED

Status

()

Core
Canvas: 2D
RESOLVED FIXED
9 years ago
6 years ago

People

(Reporter: William Warby, Assigned: Dirk Schulze)

Tracking

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(3 attachments, 6 obsolete attachments)

(Reporter)

Description

9 years ago
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

9 years ago
Component: General → Layout: Canvas
Product: Firefox → Core
QA Contact: general → layout.canvas
(Assignee)

Comment 1

9 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

9 years ago
Created attachment 354627 [details] [diff] [review]
arcTo-code

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

9 years ago
Created attachment 354628 [details]
a drawing to the implementation

This is a drawing to the implementation. I hope it helps to understand the code.
(Assignee)

Comment 4

9 years ago
Created attachment 354691 [details] [diff] [review]
arcTo-code

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

9 years ago
Created attachment 354694 [details] [diff] [review]
arcTo-code

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

9 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

9 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

9 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 9

8 years ago
Any updates on this? Is this really still not fixed 3 months later?

Comment 10

8 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

8 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

8 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

8 years ago
Created attachment 367258 [details] [diff] [review]
arcTo-code unified

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

8 years ago
ah sorry. The other way around :-P ... uploading a new version in a bit.
(Assignee)

Comment 16

8 years ago
Created attachment 367259 [details] [diff] [review]
arcTo-code unified (right direction)

The right direction this time. sorry again.
Attachment #367258 - Attachment is obsolete: true
Attachment #367259 - Flags: review?
Attachment #367258 - Flags: review?

Comment 17

8 years ago
Any news on this bug? Will it be coming out in the next FF update?
Attachment #367259 - Flags: review? → review?(jmuizelaar)
Created attachment 368056 [details]
Testcase

This is the test case that the reporter attached. Gotta figure out some way to test this in an automated way.
Status: UNCONFIRMED → NEW
Ever confirmed: true
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

8 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.
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

8 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 :-)
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

8 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
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

8 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

8 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.
(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

8 years ago
arcto2.png is the way it should look like. And that the old code passes the test is luck.

Comment 30

8 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

8 years ago
Created attachment 368533 [details] [diff] [review]
arcTo-code fix for all platforms

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

8 years ago
Created attachment 368591 [details] [diff] [review]
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.
Attachment #368533 - Attachment is obsolete: true
Attachment #368591 - Flags: review?
Attachment #368533 - Flags: review?
Attachment #368591 - Flags: review? → review?(jmuizelaar)
(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

8 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?
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

8 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

8 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.
Attachment #368591 - Flags: review?(jmuizelaar) → review+
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

8 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.
http://hg.mozilla.org/mozilla-central/rev/b116b49459f8
Status: NEW → RESOLVED
Last Resolved: 8 years ago
Resolution: --- → FIXED
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 → ---
(jeff backed this out in http://hg.mozilla.org/mozilla-central/rev/ac55c0c1c34a )
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

8 years ago
What is the difference between this two test boxes?
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

8 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.
This should be fixed by the landing of bug 499161
Relanded as:
http://hg.mozilla.org/mozilla-central/rev/e97819582eed
Status: REOPENED → RESOLVED
Last Resolved: 8 years ago8 years ago
Resolution: --- → FIXED

Updated

8 years ago
Duplicate of this bug: 512217

Comment 50

7 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?
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.
Assignee: nobody → vbs85
You need to log in before you can comment on or make changes to this bug.