Closed Bug 999943 Opened 5 years ago Closed 5 years ago
Point At Length returns the wrong point for some path segments
The attached test includes an SVG path. We call getPointAtLength at regular intervals along the path length and plot the returned point. The path created by the returned points deviates significantly from the drawn path. If I change the first path command from: M17.635,27.137c0,0-17.466,36.073-3.425,45.662... to, say: M17.635,27.137c0,1-17.466,36.073-3.425,45.662... The problem is resolved.
Regressed by 4f086025350f Bug 930577 Convert much of the SVG code for calculating path lengths and position at an offset along a path to Moz2D
Bas, could you have a look to this bug? Thanks
I'll fix this today.
Assignee: nobody → bas
Status: NEW → ASSIGNED
We need min to be the lowest point in the approximation range and max the highest. I accidentally forgot an abs call here which made this not always be true. This fixes that problem.
Attachment #8428440 - Flags: review?(jwatt)
Comment on attachment 8428440 [details] [diff] [review] Use abs to make sure min and max don't get swapped. r=me, but maybe also consider storing |abs(aTolerance / (cp41.x - cp41.y))| in a variable unless you're absolutely sure all compilers will optimize it away so the division doesn't happen twice. It probably makes it clearer anyway. Also please note in a comment why we use abs(). And I presume you'll add a test to the Moz2D suite. :)
Attachment #8428440 - Flags: review?(jwatt) → review+
We've already shipped this in 28/29 so I'm untracking for upcoming versions. At this point we are too late in the FF30 cycle to take this on so a low risk uplift will have to be considered for later versions when ready.
https://hg.mozilla.org/integration/mozilla-inbound/rev/0acfd585d47c https://hg.mozilla.org/integration/mozilla-inbound/rev/b520d877a92e Fwiw, this is extremely low risk to uplift.
Status: ASSIGNED → RESOLVED
Closed: 5 years ago
Resolution: --- → FIXED
Target Milestone: --- → mozilla32
(In reply to Bas Schouten (:bas.schouten) from comment #8) > Fwiw, this is extremely low risk to uplift. (Sounds like it might be worth requesting aurora approval, then? (though not beta, given comment 7))
(In reply to Daniel Holbert [:dholbert] from comment #10) > (In reply to Bas Schouten (:bas.schouten) from comment #8) > > Fwiw, this is extremely low risk to uplift. > > (Sounds like it might be worth requesting aurora approval, then? (though not > beta, given comment 7)) I wanted to give it a day or 2 of nightly coverage :).
Verified fixed 32.0a1 (2014-06-02), win 7 x64
(maybe worth going for backport approval now?)
(It's release week, so we'll need to request Beta approval instead of Aurora approval now (or soon).)
Comment on attachment 8428440 [details] [diff] [review] Use abs to make sure min and max don't get swapped. [Approval Request Comment] Bug caused by (feature/regressing bug #): bug 930577 User impact if declined: Incorrect points reported for getPointAtLength Testing completed (on m-c, etc.): nightly and some aurora testing Risk to taking this patch (and alternatives if risky): Low String or IDL/UUID changes made by this patch: None
Attachment #8428440 - Flags: approval-mozilla-beta?
Attachment #8428440 - Flags: approval-mozilla-beta? → approval-mozilla-beta+
Please verify this is fixed in Firefox 31.0b2.
Reproduced the issue on Firefox Nightly from 2014-04-21. Verified as fixed on Win 7 64bit, using Firefox 31 Beta 2: - User Agent: Mozilla/5.0 (Windows NT 6.1; WOW64; rv:31.0) Gecko/20100101 Firefox/31.0 - BuildID: 20140616143923
You need to log in before you can comment on or make changes to this bug.