RESOLVED FIXED 105997
Add 'float FloatPoint::slopeAngleRadians()' 
https://bugs.webkit.org/show_bug.cgi?id=105997
Summary Add 'float FloatPoint::slopeAngleRadians()'
Steve Block
Reported 2013-01-02 22:03:21 PST
Add 'float FloatPoint::theta()'
Attachments
Patch (5.12 KB, patch)
2013-01-02 22:04 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Patch (5.27 KB, patch)
2013-01-03 19:42 PST, Steve Block 		no flags
DetailsFormatted DiffDiff
Show Obsolete (1) View All Add attachment proposed patch, testcase, etc.
Steve Block
Comment 1 2013-01-02 22:04:50 PST
Created attachment 181149 [details] Patch
Steve Block
Comment 2 2013-01-02 22:15:32 PST
There are several places where we calculate the angle between a FloatPoint/Size and the x/horizontal axis, so it seems like a helper method would be useful. FloatPoint seems the more natural location, and it goes well with the existing 'length()' method. Open to suggestions for naming.
WebKit Review Bot
Comment 3 2013-01-02 23:38:18 PST
Comment on attachment 181149 [details] Patch Attachment 181149 [details] did not pass chromium-ews (chromium-xvfb): Output: http://queues.webkit.org/results/15628806 New failing tests: fast/frames/removal-before-attach-crash.html
Stephen Chenney
Comment 4 2013-01-03 06:25:59 PST
Comment on attachment 181149 [details] Patch I fine with the change, but not real happy about the name. I see how you came to choose it (radial coordinates) but the cognitive load to figure out what it means seems high. The SVG spec refers to this as the "slope" of a curve, so I think I would prefer "slopeAngleRadians".
Steve Block
Comment 5 2013-01-03 19:42:51 PST
Created attachment 181266 [details] Patch
Steve Block
Comment 6 2013-01-03 19:43:43 PST
Renamed to slopeAngleRadians
Stephen White
Comment 7 2013-01-04 07:41:43 PST
Comment on attachment 181266 [details] Patch View in context: https://bugs.webkit.org/attachment.cgi?id=181266&action=review OK. r=me > Source/WebCore/rendering/svg/SVGMarkerData.h:88 > + double inslope = rad2deg(inslopeChange.slopeAngleRadians()); There is a slight change of precision here (calling atan2f()/rad2deg() on the floats directly instead of promoting the floats to doubles, and calling atan2()/rad2deg() (double flavour), then downcasting to float). Luckily, it doesn't seem to have an impact on pixel results.
WebKit Review Bot
Comment 8 2013-01-04 08:15:29 PST
Comment on attachment 181266 [details] Patch Clearing flags on attachment: 181266 Committed r138800: <http://trac.webkit.org/changeset/138800>
WebKit Review Bot
Comment 9 2013-01-04 08:15:33 PST
All reviewed patches have been landed. Closing bug.
Dirk Schulze
Comment 10 2013-01-04 09:32:24 PST
I don't understand why it needs to go into FloatPoint for such a rare usage? (just two file IIRC).
Note You need to log in before you can comment on or make changes to this bug.