769115 - SVG marker orientation wrong on curved paths

Status: RESOLVED FIXED

(Core :: SVG, defect)

Description

[dirk bergstrom]

Attachment: File demonstrating the problem

User Agent: Mozilla/5.0 (X11; Ubuntu; Linux x86_64; rv:13.0) Gecko/20100101 Firefox/13.0.1
Build ID: 20120615112143

Steps to reproduce:

When rendering a curved SVG path with an orient="auto" marker at the start and/or end, the marker is not properly oriented.  See the attached test case.


Actual results:

The wide black arrows on the ends of the arc point roughly perpendicular to the line.

The arrows point in different directions depending on how the line is drawn, and in many cases they point in the opposite direction that they should.


Expected results:

The arrows should point "along" the line, with the arrow on the right pointing down and the one on the left pointing up.  View the testcase in Chrome to see the expected/proper behavior.

Comment 1

[dirk bergstrom]

This is very similar to bug 288165, but I didn't want to reopen a seven year old bug.

Comment 2

[Loic]

There is a regression indeed. Left arrow is up and right arrow is down in FF12-. Broken in FF13+ (Nightly too).

Mozregression range:

m-c
good=2012-02-20
bad=2012-02-21
http://hg.mozilla.org/mozilla-central/pushloghtml?fromchange=561771f01881&tochange=be559203ece8

m-i
good=2012-02-19
bad=2012-02-20
http://hg.mozilla.org/integration/mozilla-inbound/pushloghtml?fromchange=fc2b5bfc02a2&tochange=4ad3d672bf46

Suspected bug:
766fdf473acd	Cameron McCormack — Bug 728661 - Remove CalcVectorAngle and use AngleOfVector instead. r=jwatt

Comment 3

[Cameron McCormack (:heycam)]

Thanks for the regression range, I'll look into it.

Comment 4

[Cameron McCormack (:heycam)]

Attachment: patch

The previous fix was bogus was completely bogus -- AngleOfVector computes the angle between the vector and the x axis, so for computing theta we didn't need to use the (1,0) vector and for delta we can just pass the second point to AngleOfVector and get the difference between that and theta.

Comment 5

[Cameron McCormack (:heycam)]

Comment on attachment 637398 [details] [diff] [review]
patch

*cough double bogosity*

Comment 6

[boris]

I experienced the same problem on firefox 13.0.1 and 14.0 beta. firefox 12.0 has no problem.

The problem is 
 marker arrow is added to arch with refX and refY, but the arrow position is not put properly.


Boris
[e-mail]

Comment 7

[Cameron McCormack (:heycam)]

Comment on attachment 637398 [details] [diff] [review]
patch

I think jwatt is away at the moment.

Comment 8

[Jonathan Watt [:jwatt]]

Comment on attachment 637398 [details] [diff] [review]
patch

r=jwatt, but fix the bug number mentioned in the test and ref to 769115.

We should land this on branches too.

Comment 9

[Cameron McCormack (:heycam)]

https://hg.mozilla.org/integration/mozilla-inbound/rev/a8b4c3e86584

Comment 10

[Alex Keybl [:akeybl]]

Not tracking for Firefox 14 as this is not a new regression in FF14, and doesn't sound particularly critical from the user perspective. If this patch carries near-zero risk, please nominate for Beta approval on top of Aurora approval.

Comment 11

[Ryan VanderMeulen [:RyanVM]]

https://hg.mozilla.org/mozilla-central/rev/a8b4c3e86584

Comment 12

[boris]

Attachment: Network topology traffic with arch and marker

The bug - marker on arch was fixed in the http://ftp.mozilla.org/pub/mozilla.org/firefox/tinderbox-builds/mozilla-inbound-win32/1341747570/firefox-16.0a1.en-US.win32.zip

It has been tested with NetFlow Auditor on several cases.

Thanks for so fast respond.

[e-mail]

Comment 13

[Jonathan Watt [:jwatt]]

Dirk, thanks for the bug report, and for the nice small testcase.

Boris, you're welcome, and thanks for testing the patch.

By the way, the SVG team could really do with some help catching problems like this _before_ they reach the release builds. We don't seem to have enough people testing SVG before that stage right now. The Beta builds are very stable (get virtually no changes before release), so if you'd be willing to use Beta or Aurora builds for your day-to-day browsing and report any regressions that you notice, that would be a great help. If you're interested, you can find Beta/Aurora builds here:

http://www.mozilla.org/en-US/firefox/channel/

Comment 14

[Jonathan Watt [:jwatt]]

(In reply to Alex Keybl [:akeybl] from comment #10)
> If this patch carries near-zero risk, please nominate for Beta approval on top
> of Aurora approval.

It does indeed have near zero risk. Pretty much all this patch can do is change the orientation of markers, and if that were to be wrong for some reason, well we'd just swap one type of orientation breakage for another. I think it's worth taking this on beta, so nominating.

Comment 15

[boris]

Jonathan,

We are happy to do some test for you. Our product only uses a small part of SVG. It would be useful to understand your release/development cycle so we can align some of our development. Alternatively you can install an eval of our software (NetFlow Auditor) in your network so that you can test SVG with our netflow visualization feature at any stage of your development.

Comment 16

[Jonathan Watt [:jwatt]]

Pushed https://hg.mozilla.org/releases/mozilla-aurora/rev/57da580cdad0

Awaiting beta approval before pushing to beta.

Comment 17

[Jonathan Watt [:jwatt]]

Boris: https://wiki.mozilla.org/Releases provides some info on upcoming releases. Basically I'm suggesting that you use one of the upcoming releases instead of the release version of Firefox when developing your product. That way you'll be able to report in advance if we break something that affects you, and hopefully we can fix it before it affects your customers. We don't have the time or resources to run everyone's applications for them I'm afraid.

Comment 18

[boris]

No problem, we will try it. Thanks.

Comment 19

[Jonathan Watt [:jwatt]]

Awesome, thank you!

Comment 20

[Lukas Blakk [:lsblakk] use ?needinfo]

Comment on attachment 637398 [details] [diff] [review]
patch

Thanks for getting this into Aurora, it's too late to land this on Beta now though so we'll have to let this ride another cycle.

Comment 22

[Paul Silaghi, QA [:pauly]]

Verified fixed on FF 15b3 on Win 7, Ubuntu 12.04 and Mac OS X 10.6 using the STR in comment 0.