Last Comment Bug 769115 - SVG marker orientation wrong on curved paths
: SVG marker orientation wrong on curved paths
Status: RESOLVED FIXED
: regression, testcase
Product: Core
Classification: Components
Component: SVG (show other bugs)
: Trunk
: All All
: -- normal (vote)
: mozilla16
Assigned To: Cameron McCormack (:heycam)
:
Mentors:
: 773266 (view as bug list)
Depends on:
Blocks: 728661
  Show dependency treegraph
 
Reported: 2012-06-27 17:34 PDT by dirk bergstrom
Modified: 2012-08-03 05:19 PDT (History)
10 users (show)
See Also:
Crash Signature:
(edit)
QA Whiteboard:
Iteration: ---
Points: ---
Has Regression Range: ---
Has STR: ---
affected
-
affected
+
verified
+
fixed


Attachments
File demonstrating the problem (328 bytes, image/svg+xml)
2012-06-27 17:34 PDT, dirk bergstrom
no flags Details
patch (8.85 KB, patch)
2012-06-28 00:54 PDT, Cameron McCormack (:heycam)
jwatt: review+
lukasblakk+bugs: approval‑mozilla‑beta-
Details | Diff | Splinter Review
Network topology traffic with arch and marker (277.54 KB, image/png)
2012-07-08 20:02 PDT, boris
no flags Details

Description dirk bergstrom 2012-06-27 17:34:18 PDT
Created attachment 637325 [details]
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 2012-06-27 17:37:03 PDT
This is very similar to bug 288165, but I didn't want to reopen a seven year old bug.
Comment 2 Loic 2012-06-27 18:14:58 PDT
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) 2012-06-27 18:24:19 PDT
Thanks for the regression range, I'll look into it.
Comment 4 Cameron McCormack (:heycam) 2012-06-28 00:54:12 PDT
Created attachment 637398 [details] [diff] [review]
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) 2012-06-28 00:54:56 PDT
Comment on attachment 637398 [details] [diff] [review]
patch

*cough double bogosity*
Comment 6 boris 2012-07-03 23:29:58 PDT
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
boris@netflowauditor.com
Comment 7 Cameron McCormack (:heycam) 2012-07-03 23:50:28 PDT
Comment on attachment 637398 [details] [diff] [review]
patch

I think jwatt is away at the moment.
Comment 8 Jonathan Watt [:jwatt] (catching up after vacation) 2012-07-08 04:06:32 PDT
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) 2012-07-08 04:40:07 PDT
https://hg.mozilla.org/integration/mozilla-inbound/rev/a8b4c3e86584
Comment 10 Alex Keybl [:akeybl] 2012-07-08 09:08:35 PDT
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] 2012-07-08 10:50:50 PDT
https://hg.mozilla.org/mozilla-central/rev/a8b4c3e86584
Comment 12 boris 2012-07-08 20:02:50 PDT
Created attachment 640124 [details]
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.

boris@netflowauditor.com
Comment 13 Jonathan Watt [:jwatt] (catching up after vacation) 2012-07-09 14:50:00 PDT
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] (catching up after vacation) 2012-07-09 16:01:46 PDT
(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 2012-07-09 18:33:02 PDT
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] (catching up after vacation) 2012-07-10 04:23:29 PDT
Pushed https://hg.mozilla.org/releases/mozilla-aurora/rev/57da580cdad0

Awaiting beta approval before pushing to beta.
Comment 17 Jonathan Watt [:jwatt] (catching up after vacation) 2012-07-10 05:20:10 PDT
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 2012-07-10 05:56:15 PDT
No problem, we will try it. Thanks.
Comment 19 Jonathan Watt [:jwatt] (catching up after vacation) 2012-07-10 05:58:46 PDT
Awesome, thank you!
Comment 20 Lukas Blakk [:lsblakk] use ?needinfo 2012-07-10 10:45:51 PDT
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 21 Robert Longson 2012-07-20 10:53:04 PDT
*** Bug 773266 has been marked as a duplicate of this bug. ***
Comment 22 Paul Silaghi, QA [:pauly] 2012-08-03 05:19:24 PDT
Verified fixed on FF 15b3 on Win 7, Ubuntu 12.04 and Mac OS X 10.6 using the STR in comment 0.

Note You need to log in before you can comment on or make changes to this bug.